Windows miscellany

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)
 }
 

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([], {})

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;

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);
   }
 

src/worker/recent_file_cache.cpp

@@ -44,7 +44,7 @@ shared_ptr<StatResult> 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<StatResult> 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++;

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;
 

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<bool>("Unable to open directory handle");
+      ostringstream msg;
+      msg << "Unable to open directory handle " << root_path;
+      return windows_error_result<bool>(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<Subscription *>(overlapped->hEvent);
   Result<> r = sub->get_platform()->handle_fs_event(error_code, num_bytes, sub);

src/worker/worker_thread.cpp

@@ -39,7 +39,7 @@ Result<Thread::CommandOutcome> WorkerThread::handle_add_command(const CommandPay
 {
   Result<bool> 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<CommandOutcome>();
 }
 
 Result<Thread::CommandOutcome> WorkerThread::handle_remove_command(const CommandPayload *payload)

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()

test/helper.js

@@ -4,31 +4,28 @@ 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 () {
     const rootDir = path.join(__dirname, '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)

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()
   })

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`)
+      }
+    })
   })
 })

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')
     })
   })

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)
   })
 })

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')