From d05560d12f124894731ad843b4769e6ff33a6201 Mon Sep 17 00:00:00 2001 From: Nick Martin Date: Fri, 11 Oct 2013 10:01:29 -0700 Subject: [PATCH 1/5] Don't us fs.watch. Cherry pick and squash 2b8e4a29752 --- tools/watch.js | 87 +++++++++++++++++++++++++++----------------------- 1 file changed, 47 insertions(+), 40 deletions(-) diff --git a/tools/watch.js b/tools/watch.js index ea685773cb..c2007c1f3d 100644 --- a/tools/watch.js +++ b/tools/watch.js @@ -1,6 +1,8 @@ var fs = require("fs"); var path = require("path"); var _ = require('underscore'); +var Future = require('fibers/future'); +var fiberHelpers = require('./fiber-helpers.js'); // Watch for changes to a set of files, and the first time that any of // the files change, call a user-provided callback. (If you want a @@ -53,8 +55,6 @@ var _ = require('underscore'); // nonexistent if they point to something nonexist, etc). Not sure if this is // correct. - - var WatchSet = function () { var self = this; @@ -209,9 +209,10 @@ WatchSet.fromJSON = function (json) { }; var readDirectory = function (options) { + var yielding = !!options._yielding; // Read the directory. try { - var contents = fs.readdirSync(options.absPath); + var contents = readdirSyncOrYield(options.absPath, yielding); } catch (e) { // If the path is not a directory, return null; let other errors through. if (e && (e.code === 'ENOENT' || e.code === 'ENOTDIR')) @@ -226,7 +227,7 @@ var readDirectory = function (options) { // We do stat instead of lstat here, so that we treat symlinks to // directories just like directories themselves. // XXX Does the treatment of symlinks make sense? - var stats = fs.statSync(path.join(options.absPath, entry)); + var stats = statSyncOrYield(path.join(options.absPath, entry), yielding); } catch (e) { if (e && (e.code === 'ENOENT')) { // Disappeared after the readdirSync (or a dangling symlink)? Eh, @@ -274,7 +275,7 @@ var Watcher = function (options) { self.justCheckOnce = !!options._justCheckOnce; self.fileWatches = []; // array of paths - self.directoryWatches = []; // array of watch object + self.directoryWatches = []; // array of interval handles // We track all of the currently active timers so that we can cancel // them at stop() time. This stops the process from hanging at @@ -334,7 +335,7 @@ _.extend(Watcher.prototype, { return true; }, - _fireIfDirectoryChanged: function (info, isDoubleCheck) { + _fireIfDirectoryChanged: function (info, yielding) { var self = this; if (self.stopped) @@ -343,7 +344,8 @@ _.extend(Watcher.prototype, { var newContents = exports.readDirectory({ absPath: info.absPath, include: info.include, - exclude: info.exclude + exclude: info.exclude, + _yielding: yielding }); // If the directory has changed (including being deleted or created). @@ -352,20 +354,6 @@ _.extend(Watcher.prototype, { return true; } - if (!isDoubleCheck && !self.justCheckOnce) { - // Whenever a directory changes, scan it soon as we notice, - // but then scan it again one secord later just to make sure - // that we haven't missed any changes. See commentary at - // #WorkAroundLowPrecisionMtimes - // XXX not sure why this uses a different strategy than files - var timerId = self.nextTimerId++; - self.timers[timerId] = setTimeout(function () { - delete self.timers[timerId]; - if (! self.stopped) - self._fireIfDirectoryChanged(info, true); - }, 1000); - } - return false; }, @@ -418,7 +406,19 @@ _.extend(Watcher.prototype, { _startDirectoryWatches: function () { var self = this; - // Set up a watch for each directory + // fs.watchFile doesn't work for directories (as tested on ubuntu) + // and fs.watch has serious issues on MacOS (at least in node 0.10) + // https://github.com/meteor/meteor/issues/1483 + // https://groups.google.com/forum/#!topic/meteor-talk/Zy1XxEcxe8o + // https://github.com/joyent/node/issues/5463 + // https://github.com/joyent/libuv/commit/38df93cf + // + // Instead, just use setInterval. _fireIfDirectoryChanged already + // does checking to see if anything really changed. When node has a + // stable directory watching API that is more efficient than just + // polling, look at the history for this file around release 0.6.5 + // for a version that uses fs.watch. + _.each(self.watchSet.directories, function (info) { if (self.stopped) return; @@ -431,24 +431,12 @@ _.extend(Watcher.prototype, { if (self.stopped || self.justCheckOnce) return; - // fs.watchFile doesn't work for directories (as tested on ubuntu) // Notice that we poll very frequently (500 ms) - try { - self.directoryWatches.push( - fs.watch(info.absPath, {interval: 500}, function () { - self._fireIfDirectoryChanged(info); - }) - ); - } catch (e) { - // Can happen if the directory doesn't exist, in which case we should - // fire if it should be there. - if (e && e.code === "ENOENT") { - if (info.contents !== null) - self._fire(); - return; - } - throw e; - } + self.directoryWatches.push( + setInterval(fiberHelpers.inFiber(function () { + self._fireIfDirectoryChanged(info, true); + }), 500) + ); }); }, @@ -480,7 +468,7 @@ _.extend(Watcher.prototype, { // Clean up directory watches _.each(self.directoryWatches, function (watch) { - watch.close(); + clearInterval(watch); }); self.directoryWatches = []; } @@ -536,6 +524,25 @@ var sha1 = function (contents) { return hash.digest('hex'); }; +// XXX We should eventually rewrite the whole meteor tools to use yielding fs +// calls instead of sync (so that meteor is responsive to C-c during bundling, +// so that the proxy accepts connections, etc) but we don't want to do this in +// the point release in which we are adding these functions. +var readdirSyncOrYield = function (path, yielding) { + if (yielding) { + return Future.wrap(fs.readdir)(path).wait(); + } else { + return fs.readdirSync(path); + } +}; +var statSyncOrYield = function (path, yielding) { + if (yielding) { + return Future.wrap(fs.stat)(path).wait(); + } else { + return fs.statSync(path); + } +}; + _.extend(exports, { WatchSet: WatchSet, Watcher: Watcher, From 47babd170fd702b155f68395a855681f9df3b4cc Mon Sep 17 00:00:00 2001 From: Nick Martin Date: Fri, 11 Oct 2013 17:02:42 -0700 Subject: [PATCH 2/5] banner and notices for 0.6.6.1 --- scripts/admin/banner.txt | 6 ++---- scripts/admin/notices.json | 3 +++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/scripts/admin/banner.txt b/scripts/admin/banner.txt index 5596513cdb..aa8e5f804f 100644 --- a/scripts/admin/banner.txt +++ b/scripts/admin/banner.txt @@ -1,6 +1,4 @@ -=> Meteor 0.6.6: Mongo upsert and $near queries, new browser-policy - package for security headers, NodeJS 0.10, and much more. For - details, see: https://github.com/meteor/meteor/blob/devel/History.md +=> Meteor 0.6.6.1: Fix file watching on MacOS (work around Node issue #6251) This is being downloaded in the background. Update your project - to Meteor 0.6.6 by running 'meteor update'. + to Meteor 0.6.6.1 by running 'meteor update'. diff --git a/scripts/admin/notices.json b/scripts/admin/notices.json index 7e5e104bcf..a575ed522e 100644 --- a/scripts/admin/notices.json +++ b/scripts/admin/notices.json @@ -57,6 +57,9 @@ { "release": "0.6.6" }, + { + "release": "0.6.6.1" + }, { "release": "NEXT" } From cb1d30c7e87cd342f7f637acd2c3e2a3472b9c82 Mon Sep 17 00:00:00 2001 From: Nick Martin Date: Fri, 11 Oct 2013 17:19:27 -0700 Subject: [PATCH 3/5] History for 0.6.6.1 --- History.md | 5 +++++ scripts/admin/banner.txt | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/History.md b/History.md index a512225cef..6db60365d5 100644 --- a/History.md +++ b/History.md @@ -1,5 +1,10 @@ ## vNEXT +## v0.6.6.1 + +* Fix file watching on OSX. Work around Node issue #6251 by not using + fs.watch. #1483 + ## v0.6.6 #### Security diff --git a/scripts/admin/banner.txt b/scripts/admin/banner.txt index aa8e5f804f..d80bda39c5 100644 --- a/scripts/admin/banner.txt +++ b/scripts/admin/banner.txt @@ -1,4 +1,4 @@ -=> Meteor 0.6.6.1: Fix file watching on MacOS (work around Node issue #6251) +=> Meteor 0.6.6.1: Fix file watching on OSX (work around Node issue #6251) This is being downloaded in the background. Update your project to Meteor 0.6.6.1 by running 'meteor update'. From ca0e7c3c075105790d905e79a9b9f809260c4f93 Mon Sep 17 00:00:00 2001 From: Nick Martin Date: Fri, 11 Oct 2013 18:20:06 -0700 Subject: [PATCH 4/5] Docs version bump. --- docs/lib/release-override.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/lib/release-override.js b/docs/lib/release-override.js index 8aa654904a..153af4fdb2 100644 --- a/docs/lib/release-override.js +++ b/docs/lib/release-override.js @@ -1,5 +1,5 @@ // While galaxy apps are on their own special meteor releases, override // Meteor.release here. if (Meteor.isClient) { - Meteor.release = Meteor.release ? "0.6.6" : undefined; + Meteor.release = Meteor.release ? "0.6.6.1" : undefined; } From 958a5475132ba24490bcaada9cd41ec1aa8ed17a Mon Sep 17 00:00:00 2001 From: Nick Martin Date: Fri, 11 Oct 2013 18:43:13 -0700 Subject: [PATCH 5/5] update examples to 0.6.6.1 --- examples/leaderboard/.meteor/release | 2 +- examples/parties/.meteor/release | 2 +- examples/todos/.meteor/release | 2 +- examples/wordplay/.meteor/release | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/leaderboard/.meteor/release b/examples/leaderboard/.meteor/release index 05e8a4593f..f6895d22a5 100644 --- a/examples/leaderboard/.meteor/release +++ b/examples/leaderboard/.meteor/release @@ -1 +1 @@ -0.6.6 +0.6.6.1 diff --git a/examples/parties/.meteor/release b/examples/parties/.meteor/release index 05e8a4593f..f6895d22a5 100644 --- a/examples/parties/.meteor/release +++ b/examples/parties/.meteor/release @@ -1 +1 @@ -0.6.6 +0.6.6.1 diff --git a/examples/todos/.meteor/release b/examples/todos/.meteor/release index 05e8a4593f..f6895d22a5 100644 --- a/examples/todos/.meteor/release +++ b/examples/todos/.meteor/release @@ -1 +1 @@ -0.6.6 +0.6.6.1 diff --git a/examples/wordplay/.meteor/release b/examples/wordplay/.meteor/release index 05e8a4593f..f6895d22a5 100644 --- a/examples/wordplay/.meteor/release +++ b/examples/wordplay/.meteor/release @@ -1 +1 @@ -0.6.6 +0.6.6.1