diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
15 files changed, 91 insertions, 197 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelDiffAwarenessModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelDiffAwarenessModule.java index b5a85d3c4f..4140a7d5e7 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelDiffAwarenessModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelDiffAwarenessModule.java @@ -19,7 +19,6 @@ import com.google.devtools.build.lib.runtime.BlazeModule; import com.google.devtools.build.lib.runtime.WorkspaceBuilder; import com.google.devtools.build.lib.skyframe.DiffAwareness; import com.google.devtools.build.lib.skyframe.LocalDiffAwareness; -import com.google.devtools.common.options.OptionsBase; /** * Provides the {@link DiffAwareness} implementation that uses the Java watch service. @@ -27,12 +26,8 @@ import com.google.devtools.common.options.OptionsBase; public class BazelDiffAwarenessModule extends BlazeModule { @Override public void workspaceInit(BlazeDirectories directories, WorkspaceBuilder builder) { - // Order here is important - LocalDiffAwareness creation always succeeds, so it must be last. - builder.addDiffAwarenessFactory(new LocalDiffAwareness.Factory(ImmutableList.<String>of())); - } - - @Override - public Iterable<Class<? extends OptionsBase>> getCommonCommandOptions() { - return ImmutableList.<Class<? extends OptionsBase>>of(LocalDiffAwareness.Options.class); + if (builder.enableWatchFs()) { + builder.addDiffAwarenessFactory(new LocalDiffAwareness.Factory(ImmutableList.<String>of())); + } } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/FetchCommand.java b/src/main/java/com/google/devtools/build/lib/bazel/commands/FetchCommand.java index 0d78adcb5f..6f6d48a5ea 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/FetchCommand.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/FetchCommand.java @@ -71,7 +71,9 @@ public final class FetchCommand implements BlazeCommand { } try { - env.setupPackageCache(options, runtime.getDefaultsPackageContent()); + env.setupPackageCache( + options.getOptions(PackageCacheOptions.class), + runtime.getDefaultsPackageContent()); } catch (InterruptedException e) { env.getReporter().handle(Event.error("fetch interrupted")); return ExitCode.INTERRUPTED; diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java index 9857283f62..e0f9682061 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java @@ -134,7 +134,8 @@ public final class BuildTool { validateOptions(request); BuildOptions buildOptions = runtime.createBuildOptions(request); // Sync the package manager before sending the BuildStartingEvent in runLoadingPhase() - env.setupPackageCache(request, DefaultsPackage.getDefaultsPackageContent(buildOptions)); + env.setupPackageCache(request.getPackageCacheOptions(), + DefaultsPackage.getDefaultsPackageContent(buildOptions)); ExecutionTool executionTool = null; LoadingResult loadingResult = null; diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java index dc67a17f9d..27e2ecc6d6 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java @@ -189,7 +189,9 @@ public final class BlazeRuntime { public void initWorkspace(BlazeDirectories directories, BinTools binTools) throws AbruptExitException { Preconditions.checkState(this.workspace == null); - WorkspaceBuilder builder = new WorkspaceBuilder(directories, binTools); + boolean watchFS = startupOptionsProvider != null + && startupOptionsProvider.getOptions(BlazeServerStartupOptions.class).watchFS; + WorkspaceBuilder builder = new WorkspaceBuilder(directories, binTools, watchFS); for (BlazeModule module : blazeModules) { module.workspaceInit(directories, builder); } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java index e22b09b007..a68226d446 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java @@ -51,7 +51,6 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.common.options.OptionPriority; -import com.google.devtools.common.options.OptionsClassProvider; import com.google.devtools.common.options.OptionsParser; import com.google.devtools.common.options.OptionsParsingException; import com.google.devtools.common.options.OptionsProvider; @@ -471,7 +470,7 @@ public final class CommandEnvironment { * * @see DefaultsPackage */ - public void setupPackageCache(OptionsClassProvider options, + public void setupPackageCache(PackageCacheOptions packageCacheOptions, String defaultsPackageContents) throws InterruptedException, AbruptExitException { SkyframeExecutor skyframeExecutor = getSkyframeExecutor(); if (!skyframeExecutor.hasIncrementalState()) { @@ -479,7 +478,7 @@ public final class CommandEnvironment { } skyframeExecutor.sync( reporter, - options.getOptions(PackageCacheOptions.class), + packageCacheOptions, getOutputBase(), getWorkingDirectory(), defaultsPackageContents, @@ -487,8 +486,7 @@ public final class CommandEnvironment { // TODO(bazel-team): this optimization disallows rule-specified additional dependencies // on the client environment! getWhitelistedClientEnv(), - timestampGranularityMonitor, - options); + timestampGranularityMonitor); } public void recordLastExecutionTime() { @@ -518,15 +516,6 @@ public final class CommandEnvironment { CommonCommandOptions options, long execStartTimeNanos, long waitTimeInMs) throws AbruptExitException { commandStartTime -= options.startupTime; - if (runtime.getStartupOptionsProvider().getOptions(BlazeServerStartupOptions.class).watchFS) { - try { - // TODO(ulfjack): Get rid of the startup option and drop this code. - optionsParser.parse("--watchfs"); - } catch (OptionsParsingException e) { - // This should never happen. - throw new IllegalStateException(e); - } - } eventBus.post(new GotOptionsEvent(runtime.getStartupOptionsProvider(), optionsParser)); throwPendingException(); diff --git a/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java b/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java index 241d5690a1..d7f0bfd2f2 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java @@ -36,6 +36,7 @@ import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionName; + import java.util.Map; /** @@ -45,6 +46,7 @@ import java.util.Map; public final class WorkspaceBuilder { private final BlazeDirectories directories; private final BinTools binTools; + private final boolean watchFs; private SkyframeExecutorFactory skyframeExecutorFactory; private WorkspaceStatusAction.Factory workspaceStatusActionFactory; @@ -61,9 +63,10 @@ public final class WorkspaceBuilder { private final ImmutableList.Builder<SkyValueDirtinessChecker> customDirtinessCheckers = ImmutableList.builder(); - WorkspaceBuilder(BlazeDirectories directories, BinTools binTools) { + WorkspaceBuilder(BlazeDirectories directories, BinTools binTools, boolean watchFs) { this.directories = directories; this.binTools = binTools; + this.watchFs = watchFs; } BlazeWorkspace build( @@ -101,6 +104,10 @@ public final class WorkspaceBuilder { workspaceStatusActionFactory, binTools); } + public boolean enableWatchFs() { + return watchFs; + } + /** * Sets a factory for creating {@link SkyframeExecutor} objects. Note that only one factory per * workspace is allowed. @@ -129,9 +136,7 @@ public final class WorkspaceBuilder { /** * Add a {@link DiffAwareness} factory. These will be used to determine which files, if any, - * changed between Blaze commands. Note that these factories are attempted in the order in which - * they are added to this class, so order matters - in order to guarantee a specific order, only - * a single module should add such factories. + * changed between Blaze commands. */ public WorkspaceBuilder addDiffAwarenessFactory(DiffAwareness.Factory factory) { this.diffAwarenessFactories.add(Preconditions.checkNotNull(factory)); diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java index 38c7d78ea8..b5e596fb88 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.analysis.NoBuildEvent; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.pkgcache.PackageCacheOptions; import com.google.devtools.build.lib.runtime.BlazeCommand; import com.google.devtools.build.lib.runtime.BlazeRuntime; import com.google.devtools.build.lib.runtime.Command; @@ -113,7 +114,8 @@ public class InfoCommand implements BlazeCommand { // package path. Since info inherits all the build options, all the necessary information // is available here. env.setupPackageCache( - optionsProvider, runtime.getDefaultsPackageContent(optionsProvider)); + optionsProvider.getOptions(PackageCacheOptions.class), + runtime.getDefaultsPackageContent(optionsProvider)); // TODO(bazel-team): What if there are multiple configurations? [multi-config] configuration = env .getConfigurations(optionsProvider) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java index c52a3fe0d9..6c689d2baa 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/QueryCommand.java @@ -83,7 +83,9 @@ public final class QueryCommand implements BlazeCommand { QueryOptions queryOptions = options.getOptions(QueryOptions.class); try { - env.setupPackageCache(options, runtime.getDefaultsPackageContent()); + env.setupPackageCache( + options.getOptions(PackageCacheOptions.class), + runtime.getDefaultsPackageContent()); } catch (InterruptedException e) { env.getReporter().handle(Event.error("query interrupted")); return ExitCode.INTERRUPTED; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DiffAwareness.java b/src/main/java/com/google/devtools/build/lib/skyframe/DiffAwareness.java index 64c90338d0..f51cdfe281 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/DiffAwareness.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/DiffAwareness.java @@ -15,8 +15,9 @@ package com.google.devtools.build.lib.skyframe; import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.common.options.OptionsClassProvider; + import java.io.Closeable; + import javax.annotation.Nullable; /** @@ -54,7 +55,7 @@ public interface DiffAwareness extends Closeable { * {@link DiffAwareness} instance. The {@link DiffAwareness} is expected to close itself in * this case. */ - View getCurrentView(OptionsClassProvider options) throws BrokenDiffAwarenessException; + View getCurrentView() throws BrokenDiffAwarenessException; /** * Returns the set of files of interest that have been modified between the given two views. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DiffAwarenessManager.java b/src/main/java/com/google/devtools/build/lib/skyframe/DiffAwarenessManager.java index 76f00a7fb4..02870970d4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/DiffAwarenessManager.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/DiffAwarenessManager.java @@ -13,16 +13,17 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; -import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.skyframe.DiffAwareness.View; import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.common.options.OptionsClassProvider; + import java.util.Map; import java.util.logging.Logger; + import javax.annotation.Nullable; /** @@ -33,13 +34,11 @@ public final class DiffAwarenessManager { private static final Logger LOG = Logger.getLogger(DiffAwarenessManager.class.getName()); - // The manager attempts to instantiate these in the order in which they are passed to the - // constructor; this is critical in the case where a factory always succeeds. - private final ImmutableList<? extends DiffAwareness.Factory> diffAwarenessFactories; + private final ImmutableSet<? extends DiffAwareness.Factory> diffAwarenessFactories; private Map<Path, DiffAwarenessState> currentDiffAwarenessStates = Maps.newHashMap(); public DiffAwarenessManager(Iterable<? extends DiffAwareness.Factory> diffAwarenessFactories) { - this.diffAwarenessFactories = ImmutableList.copyOf(diffAwarenessFactories); + this.diffAwarenessFactories = ImmutableSet.copyOf(diffAwarenessFactories); } private static class DiffAwarenessState { @@ -80,8 +79,7 @@ public final class DiffAwarenessManager { * Gets the set of changed files since the last call with this path entry, or * {@code ModifiedFileSet.EVERYTHING_MODIFIED} if this is the first such call. */ - public ProcessableModifiedFileSet getDiff( - EventHandler eventHandler, Path pathEntry, OptionsClassProvider options) { + public ProcessableModifiedFileSet getDiff(EventHandler eventHandler, Path pathEntry) { DiffAwarenessState diffAwarenessState = maybeGetDiffAwarenessState(pathEntry); if (diffAwarenessState == null) { return BrokenProcessableModifiedFileSet.INSTANCE; @@ -89,7 +87,7 @@ public final class DiffAwarenessManager { DiffAwareness diffAwareness = diffAwarenessState.diffAwareness; View newView; try { - newView = diffAwareness.getCurrentView(options); + newView = diffAwareness.getCurrentView(); } catch (BrokenDiffAwarenessException e) { handleBrokenDiffAwareness(eventHandler, pathEntry, e); return BrokenProcessableModifiedFileSet.INSTANCE; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/LocalDiffAwareness.java b/src/main/java/com/google/devtools/build/lib/skyframe/LocalDiffAwareness.java index 753f920cab..cae5cdebd4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/LocalDiffAwareness.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/LocalDiffAwareness.java @@ -14,20 +14,18 @@ package com.google.devtools.build.lib.skyframe; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.common.options.Option; -import com.google.devtools.common.options.OptionsBase; + import java.io.IOException; import java.nio.file.FileSystems; import java.nio.file.Path; +import java.nio.file.WatchService; import java.util.Set; /** @@ -40,19 +38,6 @@ import java.util.Set; * {@link WatchServiceDiffAwareness}. */ public abstract class LocalDiffAwareness implements DiffAwareness { - /** - * Option to enable / disable local diff awareness. - */ - public static final class Options extends OptionsBase { - @Option( - name = "watchfs", - defaultValue = "false", - category = "server startup", - help = "If true, %{product} tries to use the operating system's file watch service for " - + "local changes instead of scanning every file for a change." - ) - public boolean watchFS; - } /** Factory for creating {@link LocalDiffAwareness} instances. */ public static class Factory implements DiffAwareness.Factory { @@ -88,26 +73,16 @@ public abstract class LocalDiffAwareness implements DiffAwareness { return new MacOSXFsEventsDiffAwareness(resolvedPathEntryFragment.toString()); } - return new WatchServiceDiffAwareness(resolvedPathEntryFragment.toString()); + WatchService watchService; + try { + watchService = FileSystems.getDefault().newWatchService(); + } catch (IOException e) { + return null; + } + return new WatchServiceDiffAwareness(resolvedPathEntryFragment.toString(), watchService); } } - /** - * A view that results in any subsequent getDiff calls returning - * {@link ModifiedFileSet#EVERYTHING_MODIFIED}. Use this if --watchFs is disabled. - * - * <p>The position is set to -2 in order for {@link #areInSequence} below to always return false - * if this view is passed to it. Any negative number would work; we don't use -1 as the other - * view may have a position of 0. - */ - protected static final View EVERYTHING_MODIFIED = - new SequentialView(/*owner=*/null, /*position=*/-2, ImmutableSet.<Path>of()); - - public static boolean areInSequence(SequentialView oldView, SequentialView newView) { - // Keep this in sync with the EVERYTHING_MODIFIED View above. - return oldView.owner == newView.owner && (oldView.position + 1) == newView.position; - } - private int numGetCurrentViewCalls = 0; /** Root directory to watch. This is an absolute path. */ @@ -121,8 +96,7 @@ public abstract class LocalDiffAwareness implements DiffAwareness { * The WatchService is inherently sequential and side-effectful, so we enforce this by only * supporting {@link #getDiff} calls that happen to be sequential. */ - @VisibleForTesting - static class SequentialView implements DiffAwareness.View { + private static class SequentialView implements DiffAwareness.View { private final LocalDiffAwareness owner; private final int position; private final Set<Path> modifiedAbsolutePaths; @@ -133,6 +107,10 @@ public abstract class LocalDiffAwareness implements DiffAwareness { this.modifiedAbsolutePaths = modifiedAbsolutePaths; } + public static boolean areInSequence(SequentialView oldView, SequentialView newView) { + return oldView.owner == newView.owner && (oldView.position + 1) == newView.position; + } + @Override public String toString() { return String.format("SequentialView[owner=%s, position=%d, modifiedAbsolutePaths=%s]", owner, @@ -141,7 +119,7 @@ public abstract class LocalDiffAwareness implements DiffAwareness { } /** - * Returns true on any call before first call to {@link #newView}. + * Returns true on any call before first call to {@link #newView(Set<Path>)}. */ protected boolean isFirstCall() { return numGetCurrentViewCalls == 0; @@ -151,7 +129,8 @@ public abstract class LocalDiffAwareness implements DiffAwareness { * Create a new views using a list of modified absolute paths. This will increase the view * counter. */ - protected SequentialView newView(Set<Path> modifiedAbsolutePaths) { + protected SequentialView newView(Set<Path> modifiedAbsolutePaths) + throws BrokenDiffAwarenessException { numGetCurrentViewCalls++; return new SequentialView(this, numGetCurrentViewCalls, modifiedAbsolutePaths); } @@ -167,7 +146,7 @@ public abstract class LocalDiffAwareness implements DiffAwareness { } catch (ClassCastException e) { throw new IncompatibleViewException("Given views are not from LocalDiffAwareness"); } - if (!areInSequence(oldSequentialView, newSequentialView)) { + if (!SequentialView.areInSequence(oldSequentialView, newSequentialView)) { return ModifiedFileSet.EVERYTHING_MODIFIED; } return ModifiedFileSet.builder() 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 f56611aca3..cf0fc621b5 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,15 +37,23 @@ 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); - this.latency = latency; + 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(); } /** @@ -65,35 +73,15 @@ 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. */ @@ -105,36 +93,11 @@ public final class MacOSXFsEventsDiffAwareness extends LocalDiffAwareness { private native String[] poll(); static { - 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; + UnixJniLoader.loadJni(); } @Override - 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; - } + public synchronized View getCurrentView() throws BrokenDiffAwarenessException { Preconditions.checkState(!closed); ImmutableSet.Builder<Path> paths = ImmutableSet.builder(); for (String path : poll()) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java index 7a45eeded1..3b0bbb4562 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java @@ -67,7 +67,6 @@ import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; -import com.google.devtools.common.options.OptionsClassProvider; import java.io.PrintStream; import java.util.ArrayList; import java.util.Collection; @@ -267,12 +266,11 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { String defaultsPackageContents, UUID commandId, Map<String, String> clientEnv, - TimestampGranularityMonitor tsgm, - OptionsClassProvider options) + TimestampGranularityMonitor tsgm) throws InterruptedException, AbruptExitException { super.sync(eventHandler, packageCacheOptions, outputBase, workingDirectory, - defaultsPackageContents, commandId, clientEnv, tsgm, options); - handleDiffs(eventHandler, packageCacheOptions.checkOutputFiles, options); + defaultsPackageContents, commandId, clientEnv, tsgm); + handleDiffs(eventHandler, packageCacheOptions.checkOutputFiles); } /** @@ -327,12 +325,11 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { */ @VisibleForTesting public void handleDiffs(EventHandler eventHandler) throws InterruptedException { - handleDiffs(eventHandler, /*checkOutputFiles=*/false, OptionsClassProvider.EMPTY); + handleDiffs(eventHandler, /*checkOutputFiles=*/ false); } - private void handleDiffs( - EventHandler eventHandler, boolean checkOutputFiles, OptionsClassProvider options) - throws InterruptedException { + private void handleDiffs(EventHandler eventHandler, boolean checkOutputFiles) + throws InterruptedException { if (lastAnalysisDiscarded) { // Values were cleared last build, but they couldn't be deleted because they were needed for // the execution phase. We can delete them now. @@ -347,7 +344,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { pathEntriesWithoutDiffInformation = Sets.newHashSet(); for (Path pathEntry : pkgLocator.get().getPathEntries()) { DiffAwarenessManager.ProcessableModifiedFileSet modifiedFileSet = - diffAwarenessManager.getDiff(eventHandler, pathEntry, options); + diffAwarenessManager.getDiff(eventHandler, pathEntry); if (modifiedFileSet.getModifiedFileSet().treatEverythingAsModified()) { pathEntriesWithoutDiffInformation.add(Pair.of(pathEntry, modifiedFileSet)); } else { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 146c468aaf..f2bdf9cb48 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -1673,8 +1673,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { String defaultsPackageContents, UUID commandId, Map<String, String> clientEnv, - TimestampGranularityMonitor tsgm, - OptionsClassProvider options) + TimestampGranularityMonitor tsgm) throws InterruptedException, AbruptExitException { preparePackageLoading( createPackageLocator( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WatchServiceDiffAwareness.java b/src/main/java/com/google/devtools/build/lib/skyframe/WatchServiceDiffAwareness.java index e425648825..b6d274dc5a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/WatchServiceDiffAwareness.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/WatchServiceDiffAwareness.java @@ -17,10 +17,9 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.collect.HashBiMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.util.Preconditions; -import com.google.devtools.common.options.OptionsClassProvider; + import java.io.IOException; import java.nio.file.ClosedWatchServiceException; -import java.nio.file.FileSystems; import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.LinkOption; @@ -40,6 +39,7 @@ import java.util.Set; * two consecutive calls. Uses the standard Java WatchService, which uses 'inotify' on Linux. */ public final class WatchServiceDiffAwareness extends LocalDiffAwareness { + /** * Bijection from WatchKey to the (absolute) Path being watched. WatchKeys don't have this * functionality built-in so we do it ourselves. @@ -49,52 +49,13 @@ public final class WatchServiceDiffAwareness extends LocalDiffAwareness { /** Every directory is registered under this watch service. */ private WatchService watchService; - WatchServiceDiffAwareness(String watchRoot) { + WatchServiceDiffAwareness(String watchRoot, WatchService watchService) { super(watchRoot); - } - - private void init() { - Preconditions.checkState(watchService == null); - try { - watchService = FileSystems.getDefault().newWatchService(); - } catch (IOException ignored) { - // According to the docs, this can never happen with the default file system provider. - } + this.watchService = watchService; } @Override - public View getCurrentView(OptionsClassProvider options) throws BrokenDiffAwarenessException { - // We need to consider 4 cases for watchFs: - // previous view current view - // disabled disabled -> EVERYTHING_MODIFIED - // disabled enabled -> valid View (1) - // enabled disabled -> throw BrokenDiffAwarenessException - // enabled enabled -> valid View - // - // (1) When watchFs gets enabled, we need to consider both the delta from the previous view - // to the current view (1a), and from the current view to the next view (1b). - // (1a) If watchFs was previously disabled, then previous view was either EVERYTHING_MODIFIED, - // or we threw a BrokenDiffAwarenessException. The first is safe because comparing it to - // any view results in ModifiedFileSet.EVERYTHING_MODIFIED. The second is safe because - // the previous diff awareness gets closed and we're now in a new instance; comparisons - // between views with different owners always results in - // ModifiedFileSet.EVERYTHING_MODIFIED. - // (1b) On the next run, we want to see the files that were modified between the current and the - // next run. For that, the view we return needs to be valid; however, it's ok for it to - // contain files that are modified between init() and poll() below, because those are - // already taken into account for the current build, as we ended up with - // ModifiedFileSet.EVERYTHING_MODIFIED in the current build. - boolean watchFs = options.getOptions(Options.class).watchFS; - if (watchFs && watchService == null) { - init(); - } else if (!watchFs && (watchService != null)) { - close(); - throw new BrokenDiffAwarenessException("Switched off --watchfs again"); - } - // If init() failed, then this if also applies. - if (watchService == null) { - return EVERYTHING_MODIFIED; - } + public View getCurrentView() throws BrokenDiffAwarenessException { Set<Path> modifiedAbsolutePaths; if (isFirstCall()) { try { @@ -125,12 +86,10 @@ public final class WatchServiceDiffAwareness extends LocalDiffAwareness { @Override public void close() { - if (watchService != null) { - try { - watchService.close(); - } catch (IOException ignored) { - // Nothing we can do here. - } + try { + watchService.close(); + } catch (IOException ignored) { + // Nothing we can do here. } } |