Fix watch on Windows (#236)

sake92 · web-flow · commit 7ab5c0fee190 · 2023-11-17T19:29:26.000+01:00
There are 2 parts to this fix: 1. the `FILE_TREE` modifier, which tells Windows to watch the whole folder recursively. This prevents errors when trying to move/rename a folder, since otherwise it locks them and we get errors.. Got this from here https://stackoverflow.com/a/6265860/4496364 2. preventing dynamic registration of subfolders in the `recursiveWatches` method
diff --git a/Readme.adoc b/Readme.adoc
@@ -1757,8 +1757,6 @@ paths changed: /Users/lihaoyi/Github/Ammonite/out/i am,/Users/lihaoyi/Github/Amm
 paths changed: /Users/lihaoyi/Github/Ammonite/out/version/log,/Users/lihaoyi/Github/Ammonite/out/version/meta.json,/Users/lihaoyi/Github/Ammonite/out/version
 ----
 
-`watch` currently only supports Linux and Mac-OSX, and not Windows.
-
 == Data Types
 
 === `os.Path`
diff --git a/os/watch/src/WatchServiceWatcher.scala b/os/watch/src/WatchServiceWatcher.scala
@@ -5,11 +5,11 @@ import java.io.IOException
 import java.nio.file.ClosedWatchServiceException
 import java.util.concurrent.atomic.AtomicBoolean
 import java.nio.file.StandardWatchEventKinds.{ENTRY_CREATE, ENTRY_DELETE, ENTRY_MODIFY, OVERFLOW}
-
-import com.sun.nio.file.SensitivityWatchEventModifier
+import com.sun.nio.file.{ExtendedWatchEventModifier, SensitivityWatchEventModifier}
 
 import scala.collection.mutable
 import collection.JavaConverters._
+import scala.util.Properties.isWin
 
 class WatchServiceWatcher(
     roots: Seq[os.Path],
@@ -33,12 +33,17 @@ class WatchServiceWatcher(
     val isDir = os.isDir(p, followLinks = false)
     logger("WATCH", (p, isDir))
     if (isDir) {
+      // https://stackoverflow.com/a/6265860/4496364
+      // on Windows we watch only the root directory
+      val modifiers: Array[WatchEvent.Modifier] = if (isWin)
+        Array(SensitivityWatchEventModifier.HIGH, ExtendedWatchEventModifier.FILE_TREE)
+      else Array(SensitivityWatchEventModifier.HIGH)
       currentlyWatchedPaths.put(
         p,
         p.toNIO.register(
           nioWatchService,
           Array[WatchEvent.Kind[_]](ENTRY_CREATE, ENTRY_DELETE, ENTRY_MODIFY, OVERFLOW),
-          SensitivityWatchEventModifier.HIGH
+          modifiers: _*
         )
       )
       newlyWatchedPaths.append(p)
@@ -68,13 +73,21 @@ class WatchServiceWatcher(
   }
 
   def recursiveWatches() = {
-    while (newlyWatchedPaths.nonEmpty) {
-      val top = newlyWatchedPaths.remove(newlyWatchedPaths.length - 1)
-      val listing =
-        try os.list(top)
-        catch { case e: java.nio.file.NotDirectoryException => Nil }
-      for (p <- listing) watchSinglePath(p)
-      bufferedEvents.add(top)
+    // no need to recursively watch each folder on windows
+    // https://stackoverflow.com/a/64030685/4496364
+    if (isWin) {
+      // noop
+    } else {
+      while (newlyWatchedPaths.nonEmpty) {
+        val top = newlyWatchedPaths.remove(newlyWatchedPaths.length - 1)
+        val listing =
+          try os.list(top)
+          catch {
+            case e: java.nio.file.NotDirectoryException => Nil
+          }
+        for (p <- listing) watchSinglePath(p)
+        bufferedEvents.add(top)
+      }
     }
   }
 
diff --git a/os/watch/src/package.scala b/os/watch/src/package.scala
@@ -24,18 +24,15 @@ package object watch {
    * changes happening within the watched roots folder, apart from the path
    * at which the change happened. It is up to the `onEvent` handler to query
    * the filesystem and figure out what happened, and what it wants to do.
-   *
-   * `watch` currently only supports Linux and Mac-OSX, and not Windows.
    */
   def watch(
       roots: Seq[os.Path],
       onEvent: Set[os.Path] => Unit,
       logger: (String, Any) => Unit = (_, _) => ()
   ): AutoCloseable = {
     val watcher = System.getProperty("os.name") match {
-      case "Linux" => new os.watch.WatchServiceWatcher(roots, onEvent, logger)
       case "Mac OS X" => new os.watch.FSEventsWatcher(roots, onEvent, logger, 0.05)
-      case osName => throw new Exception(s"watch not supported on operating system: $osName")
+      case _ => new os.watch.WatchServiceWatcher(roots, onEvent, logger)
     }
 
     val thread = new Thread {
diff --git a/os/watch/test/src/WatchTests.scala b/os/watch/test/src/WatchTests.scala
@@ -1,5 +1,7 @@
 package test.os.watch
 
+import scala.util.Properties.isWin
+import scala.util.Random
 import utest._
 
 object WatchTests extends TestSuite with TestSuite.Retries {
@@ -13,127 +15,135 @@ object WatchTests extends TestSuite with TestSuite.Retries {
 
   val tests = Tests {
     test("singleFolder") - _root_.test.os.TestUtil.prep { wd =>
-      if (_root_.test.os.Unix()) {
-        val changedPaths = collection.mutable.Set.empty[os.Path]
-        _root_.os.watch.watch(
-          Seq(wd),
-          onEvent = _.foreach(changedPaths.add)
-        )
+      val changedPaths = collection.mutable.Set.empty[os.Path]
+      _root_.os.watch.watch(
+        Seq(wd),
+        onEvent = _.foreach(changedPaths.add)
+      )
 
 //      os.write(wd / "lols", "")
 //      Thread.sleep(100)
 
-        changedPaths.clear()
-
-        def checkFileManglingChanges(p: os.Path) = {
-
-          checkChanges(
-            os.write(p, ""),
-            Set(p.subRelativeTo(wd))
-          )
-
-          checkChanges(
-            os.write.append(p, "hello"),
-            Set(p.subRelativeTo(wd))
-          )
-
-          checkChanges(
-            os.write.over(p, "world"),
-            Set(p.subRelativeTo(wd))
-          )
-
-          checkChanges(
-            os.truncate(p, 1),
-            Set(p.subRelativeTo(wd))
-          )
+      changedPaths.clear()
 
-          checkChanges(
-            os.remove(p),
-            Set(p.subRelativeTo(wd))
-          )
-        }
-        def checkChanges(action: => Unit, expectedChangedPaths: Set[os.SubPath]) = synchronized {
-          changedPaths.clear()
-          action
-          Thread.sleep(200)
-          val changedSubPaths = changedPaths.map(_.subRelativeTo(wd))
-          assert(expectedChangedPaths == changedSubPaths)
-        }
-
-        checkFileManglingChanges(wd / "test")
+      def checkFileManglingChanges(p: os.Path) = {
 
         checkChanges(
-          os.remove(wd / "File.txt"),
-          Set(os.sub / "File.txt")
+          os.write(p, Random.nextString(100)),
+          Set(p.subRelativeTo(wd))
         )
 
         checkChanges(
-          os.makeDir(wd / "my-new-folder"),
-          Set(os.sub / "my-new-folder")
+          os.write.append(p, "hello"),
+          Set(p.subRelativeTo(wd))
         )
 
-        checkFileManglingChanges(wd / "my-new-folder" / "test")
-
         checkChanges(
-          os.move(wd / "folder2", wd / "folder3"),
-          Set(
-            os.sub / "folder2",
-            os.sub / "folder3",
-            os.sub / "folder3" / "nestedA",
-            os.sub / "folder3" / "nestedA" / "a.txt",
-            os.sub / "folder3" / "nestedB",
-            os.sub / "folder3" / "nestedB" / "b.txt"
-          )
+          os.write.over(p, "world"),
+          Set(p.subRelativeTo(wd))
         )
 
         checkChanges(
-          os.copy(wd / "folder3", wd / "folder4"),
-          Set(
-            os.sub / "folder4",
-            os.sub / "folder4" / "nestedA",
-            os.sub / "folder4" / "nestedA" / "a.txt",
-            os.sub / "folder4" / "nestedB",
-            os.sub / "folder4" / "nestedB" / "b.txt"
-          )
+          os.truncate(p, 1),
+          Set(p.subRelativeTo(wd))
         )
 
         checkChanges(
-          os.remove.all(wd / "folder4"),
-          Set(
-            os.sub / "folder4",
-            os.sub / "folder4" / "nestedA",
-            os.sub / "folder4" / "nestedA" / "a.txt",
-            os.sub / "folder4" / "nestedB",
-            os.sub / "folder4" / "nestedB" / "b.txt"
-          )
+          os.remove(p),
+          Set(p.subRelativeTo(wd))
         )
+      }
+      def checkChanges(action: => Unit, expectedChangedPaths: Set[os.SubPath]) = synchronized {
+        changedPaths.clear()
+        action
+        Thread.sleep(200)
+        val changedSubPaths = changedPaths.map(_.subRelativeTo(wd))
+        // on Windows sometimes we get more changes
+        if (isWin) assert(expectedChangedPaths.subsetOf(changedSubPaths))
+        else assert(expectedChangedPaths == changedSubPaths)
+      }
 
-        checkFileManglingChanges(wd / "folder3" / "nestedA" / "double-nested-file")
-        checkFileManglingChanges(wd / "folder3" / "nestedB" / "double-nested-file")
+      checkFileManglingChanges(wd / "test")
 
-        checkChanges(
-          os.symlink(wd / "newlink", wd / "doesntexist"),
-          Set(os.sub / "newlink")
-        )
+      checkChanges(
+        os.remove(wd / "File.txt"),
+        Set(os.sub / "File.txt")
+      )
 
-        checkChanges(
-          os.symlink(wd / "newlink2", wd / "folder3"),
-          Set(os.sub / "newlink2")
-        )
+      checkChanges(
+        os.makeDir(wd / "my-new-folder"),
+        Set(os.sub / "my-new-folder")
+      )
+
+      checkFileManglingChanges(wd / "my-new-folder" / "test")
 
+      locally {
+        val expectedChanges = if (isWin) Set(
+          os.sub / "folder2",
+          os.sub / "folder3"
+        )
+        else Set(
+          os.sub / "folder2",
+          os.sub / "folder3",
+          os.sub / "folder3" / "nestedA",
+          os.sub / "folder3" / "nestedA" / "a.txt",
+          os.sub / "folder3" / "nestedB",
+          os.sub / "folder3" / "nestedB" / "b.txt"
+        )
         checkChanges(
-          os.hardlink(wd / "newlink3", wd / "folder3" / "nestedA" / "a.txt"),
-          System.getProperty("os.name") match {
-            case "Linux" => Set(os.sub / "newlink3")
-            case "Mac OS X" =>
-              Set(
-                os.sub / "newlink3",
-                os.sub / "folder3" / "nestedA",
-                os.sub / "folder3" / "nestedA" / "a.txt"
-              )
-          }
+          os.move(wd / "folder2", wd / "folder3"),
+          expectedChanges
         )
       }
+
+      checkChanges(
+        os.copy(wd / "folder3", wd / "folder4"),
+        Set(
+          os.sub / "folder4",
+          os.sub / "folder4" / "nestedA",
+          os.sub / "folder4" / "nestedA" / "a.txt",
+          os.sub / "folder4" / "nestedB",
+          os.sub / "folder4" / "nestedB" / "b.txt"
+        )
+      )
+
+      checkChanges(
+        os.remove.all(wd / "folder4"),
+        Set(
+          os.sub / "folder4",
+          os.sub / "folder4" / "nestedA",
+          os.sub / "folder4" / "nestedA" / "a.txt",
+          os.sub / "folder4" / "nestedB",
+          os.sub / "folder4" / "nestedB" / "b.txt"
+        )
+      )
+
+      checkFileManglingChanges(wd / "folder3" / "nestedA" / "double-nested-file")
+      checkFileManglingChanges(wd / "folder3" / "nestedB" / "double-nested-file")
+
+      checkChanges(
+        os.symlink(wd / "newlink", wd / "doesntexist"),
+        Set(os.sub / "newlink")
+      )
+
+      checkChanges(
+        os.symlink(wd / "newlink2", wd / "folder3"),
+        Set(os.sub / "newlink2")
+      )
+
+      checkChanges(
+        os.hardlink(wd / "newlink3", wd / "folder3" / "nestedA" / "a.txt"),
+        System.getProperty("os.name") match {
+          case "Mac OS X" =>
+            Set(
+              os.sub / "newlink3",
+              os.sub / "folder3" / "nestedA",
+              os.sub / "folder3" / "nestedA" / "a.txt"
+            )
+          case _ => Set(os.sub / "newlink3")
+        }
+      )
+
     }
   }
 }
diff --git a/testJarWriter/src/TestJarWriter.java b/testJarWriter/src/TestJarWriter.java
@@ -2,7 +2,6 @@
 import java.lang.InterruptedException;
 
 public class TestJarWriter {
-    private static boolean isWindows = System.getProperty("os.name").toLowerCase().contains("windows");
     public static void main(String[] args) throws InterruptedException {
         Scanner scanner = new Scanner(System.in);
         int writeN = Integer.parseInt(args[0]);
