From 35da19ab4e2c3ca297c7e6aae55a68fc079900a8 Mon Sep 17 00:00:00 2001
From: Ben Newman <ben@meteor.com>
Date: Fri, 21 Oct 2016 21:08:01 -0400
Subject: [PATCH] Avoid "The handle(...) returned by watching..." errors on
 Linux.

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.
---
 tools/fs/safe-watcher.js | 44 ++++++++++++++++++++++++++++------------
 tools/tests/old.js       | 16 +++++++++++++++
 2 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/tools/fs/safe-watcher.js b/tools/fs/safe-watcher.js
index 16d91705e3..d3df4b3c3a 100644
--- a/tools/fs/safe-watcher.js
+++ b/tools/fs/safe-watcher.js
@@ -1,6 +1,7 @@
 import * as watchLibrary from "pathwatcher";
 import { Profile } from "../tool-env/profile.js";
 import {
+  statOrNull,
   pathDirname,
   convertToOSPath,
   convertToStandardPath,
@@ -28,6 +29,11 @@ const WATCHER_CLEANUP_DELAY_MS = 30000;
 
 const watchers = Object.create(null);
 
+// Pathwatcher complains (using console.error, ugh) if you try to watch
+// two files with the same stat.ino number but different paths, so we have
+// to deduplicate files by ino.
+const watchersByIno = new Map;
+
 function acquireWatcher(absPath, callback) {
   const entry = watchers[absPath] || (
     watchers[absPath] = startNewWatcher(absPath));
@@ -45,6 +51,22 @@ function acquireWatcher(absPath, callback) {
 }
 
 function startNewWatcher(absPath) {
+  const stat = statOrNull(absPath);
+  const ino = stat && stat.ino;
+  if (ino > 0 && watchersByIno.has(ino)) {
+    return watchersByIno.get(ino);
+  }
+
+  function safeUnwatch() {
+    if (watcher) {
+      watcher.close();
+      watcher = null;
+      if (ino > 0) {
+        watchersByIno.delete(ino);
+      }
+    }
+  }
+
   let lastWatcherEventTime = +new Date;
   const callbacks = new Set;
   let watcherCleanupTimer = null;
@@ -53,15 +75,12 @@ function startNewWatcher(absPath) {
 
   function fire(event) {
     if (event !== "change") {
-      if (watcher) {
-        // When we receive a "delete" or "rename" event, the watcher is
-        // probably not going to generate any more notifications for this
-        // file, so we close and nullify the watcher to ensure that
-        // entry.rewatch() will attempt to reestablish the watcher the
-        // next time we call safeWatcher.watch.
-        watcher.close();
-        watcher = null;
-      }
+      // When we receive a "delete" or "rename" event, the watcher is
+      // probably not going to generate any more notifications for this
+      // file, so we close and nullify the watcher to ensure that
+      // entry.rewatch() will attempt to reestablish the watcher the next
+      // time we call safeWatcher.watch.
+      safeUnwatch();
 
       // Make sure we don't throttle the watchFile callback after a
       // "delete" or "rename" event, since it is now our only reliable
@@ -166,15 +185,14 @@ function startNewWatcher(absPath) {
         watcherCleanupTimer = null;
       }
 
-      if (watcher) {
-        watcher.close();
-        watcher = null;
-      }
+      safeUnwatch();
 
       unwatchFile(absPath, watchFileWrapper);
     }
   };
 
+  watchersByIno.set(ino, entry);
+
   return entry;
 }
 
diff --git a/tools/tests/old.js b/tools/tests/old.js
index 5abe06f96c..670c0d7436 100644
--- a/tools/tests/old.js
+++ b/tools/tests/old.js
@@ -51,6 +51,22 @@ var runOldTest = function (filename, extraEnv) {
 // will take another look at them later, but it is not worth that much more time
 // before 0.9.0.
 //
+selftest.define("watch", ["slow"], function () {
+  var runFuture = runOldTest.future();
+  var futures = [
+    // Run with pathwatcher (if possible)
+    runFuture('test-watch.js'),
+    // Run with fs.watchFile fallback
+    runFuture('test-watch.js', {
+      METEOR_WATCH_FORCE_POLLING: 1
+    })
+  ];
+  Future.wait(futures);
+  // Throw if any threw.
+  _.each(futures, function (f) {
+    f.get();
+  });
+});
 
 selftest.define("bundler-assets", ["checkout"], function () {
   runOldTest('test-bundler-assets.js');