diff --git a/lib/registry/helper.js b/lib/registry/helper.js index 314e5c2f..8aa0f2f6 100644 --- a/lib/registry/helper.js +++ b/lib/registry/helper.js @@ -2,7 +2,8 @@ const path = require('path') // Private: re-join the segments split from an absolute path to form another absolute path. function absolute (...parts) { - const candidate = path.join(...parts) + let candidate = parts.length !== 1 ? path.join(...parts) : parts[0] + if (process.platform === 'win32' && /^[A-Z]:$/.test(candidate)) candidate += '\\' return path.isAbsolute(candidate) ? candidate : path.join(path.sep, candidate) } diff --git a/lib/registry/tree.js b/lib/registry/tree.js index c4675bdd..1cd10736 100644 --- a/lib/registry/tree.js +++ b/lib/registry/tree.js @@ -75,14 +75,19 @@ class Tree { }, children: (children, immediate) => { // One or more NativeWatchers exist on child directories of the requested path. Create a new native watcher - // on the parent directory, note the subscribed child paths, and cleanly stop the child native watchers. + // on the parent directory. const newNative = attachToNew(children, immediate) - for (let i = 0; i < children.length; i++) { - const childNode = children[i].node - const childNative = childNode.getNativeWatcher() - childNative.reattachTo(newNative, absolutePath, options) - childNative.stop() + if (options.recursive) { + // Create a new native watcher on the parent directory, note the subscribed child paths, and cleanly stop the + // child native watchers. + + for (let i = 0; i < children.length; i++) { + const childNode = children[i].node + const childNative = childNode.getNativeWatcher() + childNative.reattachTo(newNative, absolutePath, options) + childNative.stop() + } } }, missing: () => attachToNew([], {}) diff --git a/src/message.cpp b/src/message.cpp index f9cb73d3..8c81be03 100644 --- a/src/message.cpp +++ b/src/message.cpp @@ -129,6 +129,8 @@ string CommandPayload::describe() const break; case COMMAND_REMOVE: builder << "remove channel " << arg; break; case COMMAND_LOG_FILE: builder << "log to file " << root; break; + case COMMAND_LOG_STDERR: builder << "log to stderr" << root; break; + case COMMAND_LOG_STDOUT: builder << "log to stdout" << root; break; case COMMAND_LOG_DISABLE: builder << "disable logging"; break; case COMMAND_POLLING_INTERVAL: builder << "polling interval " << arg; break; case COMMAND_POLLING_THROTTLE: builder << "polling throttle " << arg; break; diff --git a/src/worker/macos/macos_worker_platform.cpp b/src/worker/macos/macos_worker_platform.cpp index e2ae8a8d..b2d9eb86 100644 --- a/src/worker/macos/macos_worker_platform.cpp +++ b/src/worker/macos/macos_worker_platform.cpp @@ -150,7 +150,7 @@ class MacOSWorkerPlatform : public WorkerPlatform subscriptions.emplace(channel_id, Subscription(channel_id, recursive, string(root_path), move(event_stream))); - cache.prepopulate(root_path, DEFAULT_CACHE_PREPOPULATION); + cache.prepopulate(root_path, DEFAULT_CACHE_PREPOPULATION, recursive); return ok_result(true); } diff --git a/src/worker/recent_file_cache.cpp b/src/worker/recent_file_cache.cpp index 0d342b43..ebf755f1 100644 --- a/src/worker/recent_file_cache.cpp +++ b/src/worker/recent_file_cache.cpp @@ -44,7 +44,7 @@ shared_ptr StatResult::at(string &&path, bool file_hint, bool direct // (d) have a path component that is (no longer) a directory // Log any other errno that we see. if (lstat_err != UV_ENOENT && lstat_err != UV_EACCES && lstat_err != UV_ELOOP && lstat_err != UV_ENAMETOOLONG - && lstat_err != UV_ENOTDIR) { + && lstat_err != UV_ENOTDIR && lstat_err != UV_EBUSY && lstat_err != UV_EPERM) { LOGGER << "lstat(" << path << ") failed: " << uv_strerror(lstat_err) << "." << endl; } @@ -299,16 +299,16 @@ void RecentFileCache::prune() << " remain." << endl; } -void RecentFileCache::prepopulate(const string &root, size_t max) +void RecentFileCache::prepopulate(const string &root, size_t max, bool recursive) { size_t bounded_max = max > maximum_size ? maximum_size : max; - size_t entries = prepopulate_helper(root, bounded_max); + size_t entries = prepopulate_helper(root, bounded_max, recursive); apply(); LOGGER << "Pre-populated cache with " << entries << " entries." << endl; } -size_t RecentFileCache::prepopulate_helper(const string &root, size_t max) +size_t RecentFileCache::prepopulate_helper(const string &root, size_t max, bool recursive) { size_t count = 0; size_t entries = 0; @@ -339,7 +339,7 @@ size_t RecentFileCache::prepopulate_helper(const string &root, size_t max) shared_ptr r = current_at_path(entry_path, file_hint, dir_hint); if (r->is_present()) { entries++; - if (r->get_entry_kind() == KIND_DIRECTORY) next_roots.push(entry_path); + if (recursive && r->get_entry_kind() == KIND_DIRECTORY) next_roots.push(entry_path); } count++; diff --git a/src/worker/recent_file_cache.h b/src/worker/recent_file_cache.h index 2989fef9..74810416 100644 --- a/src/worker/recent_file_cache.h +++ b/src/worker/recent_file_cache.h @@ -125,7 +125,7 @@ class RecentFileCache void prune(); - void prepopulate(const std::string &root, size_t max); + void prepopulate(const std::string &root, size_t max, bool recursive); void resize(size_t maximum_size); @@ -137,7 +137,7 @@ class RecentFileCache RecentFileCache &operator=(RecentFileCache &&) = delete; private: - size_t prepopulate_helper(const std::string &root, size_t max); + size_t prepopulate_helper(const std::string &root, size_t max, bool recursive); size_t maximum_size; diff --git a/src/worker/windows/windows_worker_platform.cpp b/src/worker/windows/windows_worker_platform.cpp index 3ec085c6..111d114b 100644 --- a/src/worker/windows/windows_worker_platform.cpp +++ b/src/worker/windows/windows_worker_platform.cpp @@ -121,7 +121,9 @@ class WindowsWorkerPlatform : public WorkerPlatform NULL // template file ); if (root == INVALID_HANDLE_VALUE) { - return windows_error_result("Unable to open directory handle"); + ostringstream msg; + msg << "Unable to open directory handle " << root_path; + return windows_error_result(msg.str()); } // Allocate and persist the subscription @@ -148,7 +150,7 @@ class WindowsWorkerPlatform : public WorkerPlatform .propagate(false); } - cache.prepopulate(root_path, DEFAULT_CACHE_PREPOPULATION); + cache.prepopulate(root_path, DEFAULT_CACHE_PREPOPULATION, recursive); return ok_result(true); } @@ -341,9 +343,12 @@ class WindowsWorkerPlatform : public WorkerPlatform messages.created(move(path), kind); break; case FILE_ACTION_MODIFIED: + logline << "FILE_ACTION_MODIFIED " << kind; if (kind != KIND_DIRECTORY) { - logline << "FILE_ACTION_MODIFIED " << kind << "." << endl; + logline << "." << endl; messages.modified(move(path), kind); + } else { + logline << " (ignored)." << endl; } break; case FILE_ACTION_REMOVED: @@ -403,7 +408,7 @@ void CALLBACK command_perform_helper(__in ULONG_PTR payload) platform->handle_commands(); } -static void CALLBACK event_helper(DWORD error_code, DWORD num_bytes, LPOVERLAPPED overlapped) +void CALLBACK event_helper(DWORD error_code, DWORD num_bytes, LPOVERLAPPED overlapped) { Subscription *sub = static_cast(overlapped->hEvent); Result<> r = sub->get_platform()->handle_fs_event(error_code, num_bytes, sub); diff --git a/src/worker/worker_thread.cpp b/src/worker/worker_thread.cpp index fea72ab6..e13838a9 100644 --- a/src/worker/worker_thread.cpp +++ b/src/worker/worker_thread.cpp @@ -39,7 +39,7 @@ Result WorkerThread::handle_add_command(const CommandPay { Result r = platform->handle_add_command( payload->get_id(), payload->get_channel_id(), payload->get_root(), payload->get_recursive()); - return r.propagate(r.get_value() ? ACK : NOTHING); + return r.is_ok() ? r.propagate(r.get_value() ? ACK : NOTHING) : r.propagate(); } Result WorkerThread::handle_remove_command(const CommandPayload *payload) diff --git a/test/errors.test.js b/test/errors.test.js index 6fcd9486..f4605296 100644 --- a/test/errors.test.js +++ b/test/errors.test.js @@ -2,14 +2,12 @@ const {Fixture} = require('./helper') const {EventMatcher} = require('./matcher') -const {watchPath} = require('../lib') describe('error reporting', function () { let fixture, matcher beforeEach(async function () { fixture = new Fixture() - fixture.createWatcherWith(watchPath) await fixture.before() await fixture.log() diff --git a/test/helper.js b/test/helper.js index 4558654e..466ad67d 100644 --- a/test/helper.js +++ b/test/helper.js @@ -4,19 +4,13 @@ const path = require('path') const fs = require('fs-extra') const {CompositeDisposable} = require('event-kit') -const {NativeWatcher} = require('../lib/native-watcher') -const {configure, DISABLE} = require('../lib/binding') +const {watchPath, configure, DISABLE} = require('../lib') class Fixture { constructor () { this.watchers = [] this.subs = new CompositeDisposable() - this.createWatcher = async (watchRoot, options, callback) => { - const w = new NativeWatcher(watchRoot, options) - w.onDidChange(callback) - await w.start() - return w - } + this.createWatcher = watchPath } async before () { @@ -24,11 +18,14 @@ class Fixture { this.fixtureDir = await fs.mkdtemp(path.join(rootDir, 'watched-')) this.watchDir = path.join(this.fixtureDir, 'root') - this.mainLogFile = path.join(this.fixtureDir, 'main.test.log') - this.workerLogFile = path.join(this.fixtureDir, 'worker.test.log') - this.pollingLogFile = path.join(this.fixtureDir, 'polling.test.log') + this.mainLogFile = path.join(this.fixtureDir, 'logs', 'main.test.log') + this.workerLogFile = path.join(this.fixtureDir, 'logs', 'worker.test.log') + this.pollingLogFile = path.join(this.fixtureDir, 'logs', 'polling.test.log') - await fs.mkdirs(this.watchDir) + await Promise.all([ + fs.mkdirs(this.watchDir), + fs.mkdirs(path.join(this.fixtureDir, 'logs')) + ]) return Promise.all([ [this.mainLogFile, this.workerLogFile, this.pollingLogFile].map(fname => { fs.unlink(fname, {encoding: 'utf8'}).catch(() => '') @@ -44,10 +41,6 @@ class Fixture { }) } - createWatcherWith (cb) { - this.createWatcher = cb - } - fixturePath (...subPath) { return path.join(this.fixtureDir, ...subPath) } @@ -58,7 +51,7 @@ class Fixture { async watch (subPath, options, callback) { const watchRoot = this.watchPath(...subPath) - const watcher = await this.createWatcher(watchRoot, options, events => callback(null, events)) + const watcher = await watchPath(watchRoot, options, events => callback(null, events)) this.subs.add(watcher.onDidError(err => callback(err))) this.watchers.push(watcher) diff --git a/test/index.test.js b/test/index.test.js index 794f66d4..15384ca3 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -3,14 +3,12 @@ const fs = require('fs-extra') const {Fixture} = require('./helper') const {EventMatcher} = require('./matcher') -const {watchPath} = require('../lib') describe('exported functions', function () { let fixture beforeEach(async function () { fixture = new Fixture() - fixture.createWatcherWith(watchPath) await fixture.before() await fixture.log() }) diff --git a/test/native-watcher-registry.test.js b/test/native-watcher-registry.test.js index cd444fca..6a37c040 100644 --- a/test/native-watcher-registry.test.js +++ b/test/native-watcher-registry.test.js @@ -533,5 +533,47 @@ describe('NativeWatcherRegistry', function () { assert.strictEqual(childWatcher.native, CHILD) assert.strictEqual(parentWatcher.native, PARENT) }) + + it('does not adopt PathWatchers on child directories', async function () { + const parentDir = absolute('existing', 'path') + const recursiveChild = absolute('existing', 'path', 'recursive') + const nonrecursiveChild = absolute('existing', 'path', 'nonrecursive') + + const PARENT = new MockNative('parent') + const RCHILD = new MockNative('recursive child') + const NRCHILD = new MockNative('non-recursive child') + + createNative = (thePath, opts) => { + if (thePath === recursiveChild && opts.recursive) { + return RCHILD + } else if (thePath === nonrecursiveChild && !opts.recursive) { + return NRCHILD + } else if (thePath === parentDir && !opts.recursive) { + return PARENT + } + + return new MockNative('oops') + } + + const rChildWatcher = new MockWatcher(recursiveChild, {recursive: true}) + await registry.attach(rChildWatcher) + + const nrChildWatcher = new MockWatcher(nonrecursiveChild, {recursive: false}) + await registry.attach(nrChildWatcher) + + assert.strictEqual(rChildWatcher.native, RCHILD) + assert.strictEqual(nrChildWatcher.native, NRCHILD) + + const parentWatcher = new MockWatcher(parentDir, {recursive: false}) + await registry.attach(parentWatcher) + + assert.strictEqual(parentWatcher.native, PARENT) + assert.strictEqual(rChildWatcher.native, RCHILD) + assert.strictEqual(nrChildWatcher.native, NRCHILD) + + for (const native of [PARENT, RCHILD, NRCHILD]) { + assert.isFalse(native.stopped, `Native watcher ${native.name} was stopped`) + } + }) }) }) diff --git a/test/polling.test.js b/test/polling.test.js index 11d07aa4..bf4587e4 100644 --- a/test/polling.test.js +++ b/test/polling.test.js @@ -25,7 +25,7 @@ describe('polling', function () { const s = await status() assert.equal(s.pollingThreadState, 'running') - await watcher.stop() + await watcher.getNativeWatcher().stop(false) await until(async () => (await status()).pollingThreadState === 'stopped') }) }) diff --git a/test/unwatching.test.js b/test/unwatching.test.js index 45625de1..181e1873 100644 --- a/test/unwatching.test.js +++ b/test/unwatching.test.js @@ -29,7 +29,7 @@ describe('unwatching a directory', function () { await until('the event arrives', () => events.some(event => event.path === filePath)) assert.isNull(error) - await watcher.stop() + await watcher.getNativeWatcher().stop(false) const eventCount = events.length await fs.writeFile(filePath, 'the modification') @@ -46,10 +46,12 @@ describe('unwatching a directory', function () { const watcher = await fixture.watch([], {}, err => (error = err)) assert.isNull(error) - await watcher.stop() + const native = watcher.getNativeWatcher() + await native.stop(false) assert.isNull(error) + assert.isNull(watcher.getNativeWatcher()) - await watcher.stop() + await native.stop(false) assert.isNull(error) }) }) diff --git a/test/watching.test.js b/test/watching.test.js index 6ce2fd65..4416e42b 100644 --- a/test/watching.test.js +++ b/test/watching.test.js @@ -126,7 +126,7 @@ describe('watching a directory', function () { await parent.watch([], {}) const child = new EventMatcher(fixture) - const nw = await child.watch(['subdir'], {}) + const w = await child.watch(['subdir'], {}) await fs.appendFile(rootFile, 'change 0\n') await fs.appendFile(subFile, 'change 0\n') @@ -139,7 +139,7 @@ describe('watching a directory', function () { {path: subFile} )) - await nw.stop() + w.dispose() parent.reset() await fs.appendFile(rootFile, 'change 1\n')