Make --watchfs a common command option.

Adding an options parameter to DiffAwareness#getCurrentView seems like the
simplest way to achieve that.

Alternatives considered:
1. Making the diff awareness modules stateful. However, I did not want to do so
as I've also been working on improving the module API to reduce state, or at
least to have a proper lifecycle management for any necessary state.

2. Making the watchFs flag a constructor parameter. However, that would also
invalidate any implementations that don't use the flag (of which we have
several).

3. Only passing in a single boolean flag instead of an options class provider;
however, this is a more principled, futureproof API, which allows other
modules / awareness implementations to use their own options.

RELNOTES: --watchfs is now a command option; the startup option of the same
    name is deprecated. I.e., use bazel build --watchfs, not blaze --watchfs
    build.

--
MOS_MIGRATED_REVID=136026835

1 files changed, 54 insertions, 17 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java b/src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java
index cf0fc621b5..f56611aca3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java
@@ -17,7 +17,7 @@ package com.google.devtools.build.lib.skyframe;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.UnixJniLoader;
-
+import com.google.devtools.common.options.OptionsClassProvider;
 import java.io.File;
 import java.nio.file.Path;
 
@@ -25,11 +25,11 @@ import java.nio.file.Path;
  * A {@link DiffAwareness} that use fsevents to watch the filesystem to use in lieu of
  * {@link LocalDiffAwareness}.
  *
- * <p>
- * On OS X, the local diff awareness cannot work because WatchService is dummy and do polling, which
- * is slow (https://bugs.openjdk.java.net/browse/JDK-7133447).
+ * <p>On OS X, the local diff awareness cannot work because WatchService is dummy and do polling,
+ * which is slow (https://bugs.openjdk.java.net/browse/JDK-7133447).
  */
 public final class MacOSXFsEventsDiffAwareness extends LocalDiffAwareness {
+  private final double latency;
 
   private boolean closed;
 
@@ -37,23 +37,15 @@ public final class MacOSXFsEventsDiffAwareness extends LocalDiffAwareness {
   // structure).
   private long nativePointer;
 
+  private boolean opened;
+
   /**
    * Watch changes on the file system under <code>watchRoot</code> with a granularity of
    * <code>delay</code> seconds.
    */
   MacOSXFsEventsDiffAwareness(String watchRoot, double latency) {
     super(watchRoot);
-    create(new String[] {watchRootPath.toAbsolutePath().toString()}, latency);
-
-    // Start a thread that just contains the OS X run loop.
-    new Thread(
-            new Runnable() {
-              @Override
-              public void run() {
-                MacOSXFsEventsDiffAwareness.this.run();
-              }
-            })
-        .start();
+    this.latency = latency;
   }
 
   /**
@@ -73,15 +65,35 @@ public final class MacOSXFsEventsDiffAwareness extends LocalDiffAwareness {
    */
   private native void run();
 
+  private void init() {
+    // The code below is based on the assumption that init() can never fail, which is currently the
+    // case; if you change init(), then you also need to update {@link #getCurrentView}.
+    Preconditions.checkState(!opened);
+    opened = true;
+    create(new String[] {watchRootPath.toAbsolutePath().toString()}, latency);
+    // Start a thread that just contains the OS X run loop.
+    new Thread(
+            new Runnable() {
+              @Override
+              public void run() {
+                MacOSXFsEventsDiffAwareness.this.run();
+              }
+            })
+        .start();
+  }
+
   /**
    * Close this watch service, this service should not be used any longer after closing.
    */
+  @Override
   public synchronized void close() {
     Preconditions.checkState(!closed);
     closed = true;
     doClose();
   }
 
+  private static final boolean JNI_AVAILABLE;
+
   /**
    * JNI code stopping the main loop and shutting down listening to FSEvents.
    */
@@ -93,11 +105,36 @@ public final class MacOSXFsEventsDiffAwareness extends LocalDiffAwareness {
   private native String[] poll();
 
   static {
-    UnixJniLoader.loadJni();
+    boolean loadJniWorked = false;
+    try {
+      UnixJniLoader.loadJni();
+      loadJniWorked = true;
+    } catch (UnsatisfiedLinkError ignored) {
+      // Unfortunately, we compile this class into the Bazel bootstrap binary, which doesn't have
+      // access to the JNI code (to simplify bootstrap). This is the quick and dirty way to
+      // hard-disable --watchfs in the bootstrap binary.
+    }
+    JNI_AVAILABLE = loadJniWorked;
   }
 
   @Override
-  public synchronized View getCurrentView() throws BrokenDiffAwarenessException {
+  public synchronized View getCurrentView(OptionsClassProvider options)
+      throws BrokenDiffAwarenessException {
+    if (!JNI_AVAILABLE) {
+      return EVERYTHING_MODIFIED;
+    }
+    // See WatchServiceDiffAwareness#getCurrentView for an explanation of this logic.
+    boolean watchFs = options.getOptions(Options.class).watchFS;
+    if (watchFs && !opened) {
+      init();
+    } else if (!watchFs && opened) {
+      close();
+      throw new BrokenDiffAwarenessException("Switched off --watchfs again");
+    } else if (!opened) {
+      // The only difference with WatchServiceDiffAwareness#getCurrentView is this if; the init()
+      // call above can never fail, so we don't need to re-check the opened flag after init().
+      return EVERYTHING_MODIFIED;
+    }
     Preconditions.checkState(!closed);
     ImmutableSet.Builder<Path> paths = ImmutableSet.builder();
     for (String path : poll()) {