If a test process does not explicitly call process.exit, pathwatcher
watchers may keep it alive indefinitely (either that, or there's a bug
with the persistent:false option to fs.watchFile).
This accidental immortality can be prevented by explicitly closing all
watchers when we no longer have any interest in file change notifications.
In particular, optimisticReaddir was getting called before relevant dirty
callbacks were firing, and thus failing to notice actual changes on the
file system.
In general, the tools/fs/watch.js code is the one place where we really
need an up-to-date view of the file system. Put another way, if optimistic
functions worked perfectly, we wouldn't need to rely so much on WatchSet
logic, but for now it's a balance of equally important strategies, and we
shouldn't be compromising one by intermingling it with the other.
This will make it easier to use tools like https://yarnpkg.com/ with the
right version of Node, etc.
With this commit, here's all you have to do:
meteor npm install -g yarnpkg
Then test that it works:
meteor yarn info
Note that any commands registered by Meteor itself will not be honored.
This partially reverts commit 96c95629eb.
When running `meteor update`, we call writeReleaseFileAndDevBundleLink
immediately before reading .meteor/release, so there's no time for a file
change notification to invalidate the cached contents of that file.
In the future, we could perhaps call optimisticReadFile.dirty(path) as a
consequence of files.writeFile(path, ...), but that may be tricky.
Judging from the variety and extent of test failures, switching to
chokidar.watch was too drastic a change for this late-beta stage of the
release cycle.
The problem with pathwatcher.watch was that watches don't survive the
deletion of the watched file, because (like fs.watch) it watches files
based on inodes, not paths. This problem can be solved in a relatively
narrow way, by attempting to rewatch the file after any "delete" or
"rename" events.
Especially on Linux, chokidar.watch tends to throw ENOSPC errors when too
many files are being watched.
I removed this logic earlier because I thought chokidar could handle such
failures by itself, but that was wishful thinking.
Healthy competition among fs.watch wrappers appears to have produced a
clear winner: https://www.npmjs.com/package/chokidar
This wrapper is better for Meteor than pathwatcher was, because it can
watch directory trees recursively, and it has no trouble watching
nonexistent file paths, whereas pathwatcher would throw an exception.
As of optimism@0.3.0, the value of `this` within subscribe functions is no
longer the optimistic wrapper function object (instead: null or global, as
if called with no receiver object), so we need to retain a reference to
the optimistic function so that the watcher can call .dirty(...args).
Previously, the Cordova message was failing in CI. It wasn't caught on CircleCI since they have Cordova installed by default so this path was never tested. I found this when I ran this through Semaphore CI who does not have Cordova available by default.
This test fails without meteor/meteor#7884 and passes with it.
`Console.success` is currently called in only one place but it relies on the fact that `pretty` console mode is enabled for it to make any sense. This adds the ability to pass an "ugly" (less pretty?) message to `Console.success` (default: "success") and changes the Cordova dependency checker to use "installed" when showing a met-dependency.
Fixesmeteor/meteor#7883
These functions were both using the `Console._fail` function which is passed a severity of failure as a string (e.g. "warn", "error", etc.) which corresponds to the appropriate `Console` function. Previously, the string was just being called as if it was a function.
Fixesmeteor/meteor#7882
Similar to the treatment given to PackageAPI in my commit
af50b4cc5b.
This clear separation of concerns will be helpful for optimizing
PackageSource#initFromPackageDir.
The options.statOrNull function is not included in the JSON.stringify key,
but it affects the results because the implementation provided by the
ImportScanner relies on scanner._getFile to return fakeFileStat for
make-believe files created by the ImportScanner.
This becomes a problem if PackageSourceBatch#getResolver accidentally uses
a Resolver created initially for the ImportScanner, or vice-versa.
In particular, when a compiler plugin calls addJavaScript with a path
different from the original file path, the ImportScanner generates two
different modules, one of which imports the other. Only one of the files
exists on disk, though, so a special implementation of statOrNull is
necessary to keep the Resolver from having to share implementation details
with the ImportScanner.
When this system fails, users of compiler plugins see errors like this:
https://github.com/meteor/meteor/pull/7668#issuecomment-251976739
It's difficult to keep track of all the factors that could affect the
output of files.realpath, so it doesn't seem feasible to replace this call
with an optimistic function.
Fortunately, it's fine if the paths returned by IsopackCache#getSourceRoot
contain symlinks, and possibly even preferable, since
~/.meteor/packages/<name>/<version>
is more consistent than
~/.meteor/packages/<name>/.<version>.<junk>++<more junk>
though the latter happens to be a symlink to the latter.
Two mistakes: we were maintaining two redundant caches for
findAssignedGlobals, and callings tryToParse before checking globalsCache,
when often globalsCache already had a stored result.
This speeds up both getPrelinkedOutput (because we don't spend any time
generating the identity source map) and toStringWithSourceMap (because we
don't have to combine those source maps).
We still generate line number comments for files without source maps, so
it's relatively easy to find the source line for a stack trace.
An example of an npm package that triggers this bug is the `cli-color`
package, which has a package.json file with a "main" property whose value
is "lib", and a "lib" directory that contains an index.js file.
This is critical because it means Resolver objects can now survive
development server restarts, which means the optimistic Resolver#resolve
method can remember results indefinitely. Thanks to this change, the
Resolver#resolve method has practically disappeared from rebuild
performance profiles.
