The logic to figure out which build dirs are temp dirs which need to be
freed and which are not was wrong, since it missed the non-temp
files.pathResolve(files.pathDirname(packagePath),
packageLinkTarget));
Use a more explicit way of tracking this.
(Fixed merge conflict when merging from devel.)
Summary:
According to its contract, mkdir -p returns true if the directory
exists (and creates it if needed) and false if the item exists and isn't
a directory (so we couldn't make one). Because directory creation can
be concurrent, we need to wrap the actual mkdir call in a try/catch to handle
this issue (rather than just checking once).
This issue was always here. Previously, the race was against other apps editing
the same directory (which didn't come up that often). As of 1.0.3, files.js is a lot
more yieldy and this becomes a race condition on Meteor itself.
Test Plan: self-test
Reviewers: glasser
Differential Revision: https://phabricator.meteor.io/D15
This mostly fixes tests:
- removes the 'restarted' check from some tests. We don't need it in those cases
(printing the other banner is enough). We can no longer rely on that executing
after the code in the package (in fact it seems to execute before, and then
get overwritten), and the test still tests what it is intended to (that the new
package code executes).
- minor fixes to essentially syntax errors -- the skeleton now uses double quotes
instead of single quotes, so a regex failed to work, for example. We changed a
version number in one part of the test, but not another.
- fixes selftest.js, sort of, to actually print out what test we are testing. This
is an unfortunate interaction of Console.js changes in 1.0.2 and a progress bar
(that came later). The progress bar erases the message telling you what test is
running when you use a standard terminal. That's awkward, fixed.
Summary:
When running 'meteor show <packageName>' show
Package: <packageName>@<defaultVersion>
(instead of "Package: <packageName>" )
The default version is the version number of the version record
that acts as the source for exports, implies, long description, etc.
It is the local record (in which case, we will show "@local" to be
more clear); if there is no local record, it is the highest semver mainline
record (ie: not a pre-release) and if *that* doesn't exist, it is just
the highest semver record that we have.
Test Plan: self-test show --slow
Reviewers: glasser
Differential Revision: https://phabricator.meteor.io/D8
Implied exports are part of the package's exports, especially for an umbrealla
package like 'meteor-platform' or 'cfs:standard-packages'. However, we can't
tell you the exact exports (ex: "Mongo") without running the constraint solver
(because we don't know what version of the implied package you will end up with).
Showing implies also makes umbrella packages like 'meteor-platform' and
'cfs:standard-packages' more obvious -- the user can tell what is going on with the
package much better.
This change also includes:
- rewrite of the 'meteor-platform' README.md. Don't list the implied packages in this
README.md, since we won't keep (and haven't kept) it up to date reliably. Tell the user
to run 'meteor show' instead. (Also the listing doesn't look good with 'show', but that's
tangential)
- some refactor of commands-packages-query.js. Introduce a base class of PkgDatum which can
store data that needs more processing, such as exports, implies, etc. Get other classes
to inherit from it, and use it to store package dependencies.
Reviewed here: https://phabricator.meteor.io/D5
Specifically, we want to avoid the situation where you run
'meteor show iron:router'
and then spend hours trying to figure out why Meteor can't process
your calls to, for example, BetterRouteController. The answer, of course,
might be that you are several versions behind and your version doesn't have
that export. We do not want to run the constraint solver to figure out what version
you are using; we played around with how to display this data and decided to just
be explicit about it.
My wording is intentionally vague. In reality, we take:
- the local version if one exists OR
- the latest mainline version, which a non-pre-release non-unmigrated(?) version.
- the latest version that we are showing iff there is no mainline version.
- for releases, we take the latest recommended version
This set of rules is much more complicated than the reality. The reality is pretty
intuitive. We don't want to dwell on what these rules are, just get the general idea across.
Some cosmetic changes to the package skeleton:
- Move version to the top
- Move git up and write a comment explaining it. Now the
comment for documentation no longer dominates the stanza!
These are arranges from most to least optional (in some sense).
The documentation is considered the most optional because removing that
line doesn't remove the documentation: the line is mostly there for
an override. I think this makes sense.
- Package: <name>@<version> rather than
Package: <name>
Version: <version>
in the version output
- Remove the summary line from version output.
Move 'Published by' to the bottom as a separate line.
- Move git up.
The impetus behind these changes is to reduce the size of the header on 'meteor show'.
We thought that the long paragraph of "Foo: Bar" type things was too overwhelming.
Some more changes:
- Clean up an extra line that comes up when printing the description sometimes.
- Add 'This package version is built locally from source' to the message about versions
available on the server.
- For releases, process the "non-recommended versions have been hidden message" for the single-hidden-version
case, in the same way that we do for packages.
In 'meteor show', display the list of packages implied by a package/version.
Implied exports are part of the package's exports, especially for an umbrealla
package like 'meteor-platform' or 'cfs:standard-packages'. However, we can't
tell you the exact exports (ex: "Mongo") without running the constraint solver
(because we don't know what version of the implied package you will end up with).
Showing implies also makes umbrella packages like 'meteor-platform' and
'cfs:standard-packages' more obvious -- the user can tell what is going on with the
package much better.
This commit is based on the following design document:
https://mdg.hackpad.com/Creating-and-Updating-Docs-0ZyyDcSZDxp,
and some other stuff from here: https://mdg.hackpad.com/Meteor-Long-Description-wGZ1vIOwVlF
and was code reviewed here: https://github.com/meteor/meteor/pull/3375
It does the following:
- Allow the user to specify package documentation in Package.Describe.
We will take the README.md file by default, to make the transition easier.
Users can specify ‘documentation: null’ to not submit a README.md
- From that documentation, extract the section between the first and second header
to use as the long form description for the package.
- Upload the documentation to the server at publish-time. Allow metadata changes with ‘publish —update’.
- Change the default package skeleton to include the README.md file.
Also, changes the skeleton to have fewer useless placeholders in Package.describe values.
- Fix a minor bug where Git did not show up when running ‘meteor show’ on local packages.
A note on ‘documentation: null’ and blank documentation — we don’t let maintainers upload
blank README.md files, because we want to encourage people to fill them out. (Instead,
we allow a ‘documentation: null’ as an override) This is a UX issue! It is not a technical thing.
There is more discussion and code review in: https://github.com/meteor/meteor/pull/3375
Contains:
- method to aggregate exports for a package in packageSource (exports are per-architecture).
- get this data from packageSource in PackageQuery for ‘meteor show’. Don’t store it in the
local catalog — while it is not a particularly expensive operation, it is still more expensive
than a simple lookup. We really do care about minimizing any sort of computation when we
are initializing packages, since we want the tool to be fast.
- display the data in ‘meteor show’. It makes sense to line wrap this with the ‘Exports:’ label as a
bulletPoint (just look at the test to see an example where this improves user experience). Since we
are doing that, we might as well use that bulletPoint functionality on the other labels as well.
- There is also a test. Run ‘meteor self-test show’ to test, or run ‘meteor show’ on a local package
with exports.
The Troposphere counterpoint to this is: meteor/troposphere#5
This is a thing that I wanted to try -- running 'meteor show' in a
package directory shows you that version's data.
- You might want to run 'meteor show' to get export or dependency
information on a local package, instead of looking through the
package.js file.
- Before publishing your package, or updating its metadata, you might
want to make really sure that its longform description looks good
in 'meteor show'. Hopefully it does! I would want to check.
Running 'meteor show <name>@local' from a package directory feels
slightly janky to me.
- Other commands in the publiction workflow read 'package.js' to figure
out your package name. It feels weird to type it out.
- Many package names don't correspond to the directory name. It is good
to help the user spend less time inspecting package.js files for
obvious information.
This has bothered me a lot during testing, which is not a normal workflow.
I might be somewhat biased here, in a way that normal users would not be.
There is a minor inefficiency around retrieving a local version record twice,
but I think that it is worth it for code simplicity/readability/etc.
Instead of the generic "Some versions of X have been hidden"
message when only showing some versions of a package, use a more
detailed message. For example:
- "Older versions of X have been hidden"
- "Older, pre-release and unmigrated versions of X have been hidden"
- "One older version of X has been hidden."
There is some hand-waving around the logic resolving what to do about,
for example, old pre-releases. Overall, we want to err on the side of
having a clear and obviously consistent user experience:
- any version less than the lowest shown version (ex: 1.0.0-rc.0 vs 1.0.0)
is an 'older' version. Sometimes, that version is also a pre-release. It
is possible that if we were NOT filtering out pre-releases, we would show it.
We still respond that it is ‘older’, because that seems more obviously consistent.
- we report any ‘pre-release’ or ‘unmigrated’ versions in the version interval
that we show. That is, if we are showing ‘1.0.0’ and ‘2.0.0’, and hiding
2.0.0-rc.0 and the un-migrated 1.1.0, we will mention it.
Of course, that interval does depend on what versions we choose to hide. It is
possible to imagine a situation where we don’t hide pre-releases, in which case,
‘1.0.0’ above might not make the cut, and neither would 1.1.0. Luckily, we either
show everything, or hide everything, so this is only theoretical.
The ‘show’ command has been completely rewritten. It has different output
and now does the following:
- Interacts with local package versions. Checks in the local package catalog, and
returns the local versions along with the server versions. When ‘meteor show’ is
run with a specific version request (‘meteor show foo@<version>’), default to
showing the local package version (but show a message that a server version is
available). Running ‘meteor show foo@local’ will always show the local version
(useful for version-less local packages).
- Simplify the interface. Instead of various ‘show-*’ flags, we only have one: show-all.
By default, we only show the top 5 official (non-prerelease) unmigrated versions of a
package (+ local version, if applicable). This can be overridden with ‘show-all’, and we
let the user know that more versions are available. For releases, ‘show-all’ will show
non-recommended releases.
- Display publication time for non-local package versions. This makes it easier to run
‘meteor show <name>’ and see if <name> is actively maintained. For local packages,
we display the root directory (useful for large apps or running with the
LOCAL_PACKAGE_DIRS variable, for example).
- For non-local package versions, show if the version is ‘installed’ (downloaded into the
warehouse). This involved minor changes to tropohouse.js. The idea is that this should
give a pretty good clue whether the version can be added offline.
- Show version dependencies. This should help the user understand, track down and
debug constraint solver failures.
- Do not show version architectures except in —ejson mode.
- Allow an ‘—ejson’ flag to get the output in EJSON format. That should make scripting
easier. (As a bonus, for release versions, the EJSON output acts as a nice template
for the release configuration file.)
The search command now does the following:
- Interacts with local package versions. Specifically, local versions override equivalent
server versions. Also, ‘search’ works on local packages (so, for example,
‘meteor search troposphere’ inside the package server app will give you the troposphere
package).
- Allows an ‘—ejson’ flag to get the outout in EJSON format.
Minor changes to some minor testing infrastructure:
- A new skeleton package, package-for-show. Its versions contain different
values for various metadata, so we can test that metadata comes from
the right version.
- In several places, replace the pattern of copying around
package.js files with using the replace function on a placeholder
string. (Mostly, as applied to package versions).
This is based on these hackpads: https://mdg.hackpad.com/Showing-Package-Metadata-HdGo3Lzx3hR
and https://mdg.hackpad.com/Meteor-Search-Output-1xxEzrAK9YU.
Temp dirs were not getting cleaned up on ctrl-C, including ~100 MB temp
dirs created by test-packages.
Wrapping cleanup handlers in noYieldsAllowed automatically causes sync
versions of fs calls to be used when files.rm_recursive is called.
This fixes a corner case where if you synced the catalog between the
creation of a release and the creation of its tool's build for your
platform, running `meteor --release R` would fail.
Fixes#3317.
This is a breaking change to package-version-parser.
A PackageConstraint used to look like this:
```
{ name: String,
constraintString: String,
constraints: [{version: String|null,
type: String}]}
```
Now it looks like this:
```
{ name: String,
constraintString: String,
vConstraint: {
raw: String,
alternatives: [{versionString: String|null,
type: String}]}}
```
Where (vConstraint instanceof VersionConstraint) and
(vConstraint.raw === constraintString).
This achieves several desirable changes at once.
* `constraint.constraints` for the disjuncts in “1.0.0||2.0.0”
was confusing. `alternatives` is better.
* Having a class for VersionConstraint will come in handy because
we can add methods to it, and we can use it in the constraint
solver to represent the problem statement.
* The names “vConstraint” and “versionString” are a little verbose,
but there really shouldn’t be a lot of code that dives into this
structure, and I really wanted to avoid anyone ever writing:
`constraint.constraint.alternatives[0].version`, and then wondering
what sort of object that was (not a parsed PackageVersion! we could
parse eagerly but that might be slow).
At least, not directly. When the tool wants to parse a string of the
form “package@version,” it now uses utils.parsePackageAtVersion.
This way, if I make a breaking change to PV.parseConstraint, I only have
to change a few call sites.
Mostly I just made parseConstraint clearer and gave the return value a
prototype.
There are new unit tests, and the existing ones have been given much
better names.
PV.parseConstraint also now optionally takes two arguments, a package name and a version constraint. We can eventually propagate this feature to the tool and stop concatenating with “@” so much.
I’m referring to “foo@=1.0.0” as a PackageConstraint and “=1.0.0” as a version constraint. I’ll probably add a VersionConstraint class too.
It parses “=1.0.0” but not “=1.0.0 || 2.0.0”. It shouldn’t be a public thing (and is never used publicly).
There should be only two things: versions and constraints. A value that can legally be “=1.0.0” is not a version, it is a constraint (and yet we still call it versionString in a few places). A value that can be “=1.0.0” but not “1.0.0 || 2.0.0” is not a constraint; it is at best a rare creature called something like a “simple constraint.” We may expand the syntax of constraints again in the future, and we can’t have N things called a “constraint.”
It was only used in one place in resolver.js. If the semantics of constraints are that buildIDs are ignored, that should be implemented when interpreting a constraint, not parsing it. We don’t use buildIDs ourselves anymore, so it’s rather theoretical, but we’ve always stripped them out of constraint strings so it’s not even clear why we allow them at all — ie. why we are willing to parse “=1.0.0+foo” as a valid constraint and then throw away the “+foo”.
Bonus: One less place where we conflate two different options objects (utils.parseConstraint and PV.parseConstraint).
Despite the name “package-version-parser,” PVP didn’t really provide a way to parse a package version. With this change, if you want to get the minor version of “1.2.3_4”, for example, you can just write `PV.parse(“1.2.3_4”).minor`. This simplifies the internals of PVP as well.
When you parse a version, you now get an instance of PackageVersion with a documented set of fields.
Also, make PVP work on the client. When called directly from the tool, semver is loaded the same way as before. When PVP is used as a package, it uses a copy of semver v4.1.0 kept inside the package.
This change is intended to be 100% behavior-preserving.
This commit moves parent pid process from the webapp package to the boot
script. This means that daemonized apps without webapp will also exit
when the runner exits, if run from the runner. (For example, several
self-tests such as 'autoupdate' no longer leak node processes.) This is
controlled via the $METEOR_PARENT_PID environment variable instead of
from command line arguments, in order to make fewer assumptions about
how Meteor apps process arguments.
This also drops the old --keepalive support (which already has stopped
being used by the dev mode runner or any MDG deployment platforms).
Neither --parent-pid nor --keepalive were documented beforehand, and
--keepalive was already deprecated before 1.0.
These flags used to also incidentally trigger printing the LISTENING
line; this is now controlled by $METEOR_PRINT_ON_LISTEN.
Fixes#3315.
Specifically, this means that errors during publish-release
--from-checkout can include the package name, since buildmessage errors
only mention the innermost job name. (It's more important for
publish-release's errors to include the package name than for publish,
since for publish you presumably know what package you're publishing
anyway.)
Fixes#3323.
