The options.transformFilename function is only supposed to transform
target file names, not the names of source files to be copied.
This behavior was broken by c5809a4a1c
between Meteor 1.4.0.1 and 1.4.1. Thank goodness for `git bisect`!
The biggest symptom of this mistake was that `meteor create --package`
no longer created files based on the ~fs-name~.js and ~fs-name~-tests.js
template files.
This probably merits a 1.4.1.4 release in addition to 1.4.2.2.
Set METEOR_DISABLE_FS_FIBERS=false to un-disable files.* fibers if you
suspect this commit has introduced a problematic difference in behavior.
cc @veered
It's a shame that Pathwatcher issues this warning using console.error,
without taking any verbosity options into account:
https://github.com/atom/node-pathwatcher/blob/7ef76e5dfd/src/main.coffee#L53
Fortunately, I believe I've identified the underlying reason why this
happens, which may help resolve the following issue:
https://github.com/atom/node-pathwatcher/issues/98
If all goes well, I'll submit an upstream pull request.
I've also reinstated an old file watching test that I mistakenly removed
when I attempted to switch to chokidar instead of pathwatcher.
This is a bit different from the previous strategy of invalidating
optimistic functions for specific npm package names.
Now, whenever we make changes to the contents of a specific node_modules
directory, or whenever the developer independently modifies an app's
node_modules directory, all optimistic results derived from paths
contained within that node_modules directory will be marked as dirty, and
thus may need to be recomputed.
This strategy prioritizes starting fewer watchers (just one per
node_modules directory) while still allowing npm packages to be added or
removed while the app is running:
https://github.com/meteor/meteor/pull/7668#issuecomment-255120373
The drawback is that changes within subdirectories of node_modules will
not be detected until the server is fully restarted, but that seems like
an acceptable tradeoff, since npm packages change much less often than
application code.
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 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).
I originally added the exception-caching functionality in order to avoid
calling files.stat repeatedly for files known to be missing, but now that
we're using statOrNull, knowledge of missing files (as indicated by
statOrNull returning null) is being properly cached.
The reason it's dangerous to cache exceptions is that (for example) when
an ENOENT exception indicates the file is missing, there will be no more
change events for that file, effectively making the exception permanent,
even if the file comes to exist at a later time.
As a reminder, we use fs.watchFile only as a fallback in environments
where pathwatcher.watch is not available, such as on network or virtual
file systems.
Typically all outstanding watchers will be closed when the development
server restarts, but we can save a lot of effort if they survive the
restart, for the small cost of keeping them open for an extra 30sec.
For the galaxy-server application, this reduces the time spent in
watch.isUpToDate from six seconds to a little over second. Of course we
should also try to call it less often, but making it cheaper helps!
