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.
In Session.close, `self.socket.close` could trigger this event handler:
socket.on('close', function () {
if (socket._meteorSession) {
Fiber(function () {
socket._meteorSession.close();
}).run();
}
});
which could trigger a reentrant call to Session.close. The self.inQueue
guard was not sufficient to stop multiple execution, because it was too
low.
Symptoms included:
- The "sessions" server fact would be decremented twice and become
inaccurate (and even negative!)
- Connection.onClose callbacks could be called twice
Fixes#3331.
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
After some consideration, we decided that the extra package list in the README
is not up to date, will never be kept up to date and as such, is actively
unhelpful.
Move the list of packages out of the top section of the `meteor-platform` README.md,
because it doesn't play well with `meteor show`. Leave it in the section below for
people that run into the README in some other context (for example, Atmosphere).
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.
