diff options
author | Ulf Adams <ulfjack@google.com> | 2016-10-14 14:20:31 +0000 |
---|---|---|
committer | Yun Peng <pcloudy@google.com> | 2016-10-14 20:25:05 +0000 |
commit | de14adebfcc0d66ff48aec9ace3a4d36343200f5 (patch) | |
tree | 21505128f8d0aacf9379e1ca378f0c9814f8e410 /src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java | |
parent | 8d0be8960e49e09c0cbcc8173ce538edfa3cb208 (diff) |
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=136154395
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/skyframe/MacOSXFsEventsDiffAwareness.java | 83 |
1 files changed, 61 insertions, 22 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..17a9413dcc 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,19 +65,41 @@ 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. */ - public synchronized void close() { - Preconditions.checkState(!closed); - closed = true; - doClose(); + @Override + public void close() { + if (opened) { + Preconditions.checkState(!closed); + closed = true; + doClose(); + } } + private static final boolean JNI_AVAILABLE; + /** * JNI code stopping the main loop and shutting down listening to FSEvents. */ - private synchronized native void doClose(); + private native void doClose(); /** * JNI code returning the list of absolute path modified since last call. @@ -93,11 +107,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 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()) { |