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
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/DiffAwarenessManagerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/DiffAwarenessManagerTest.java
index b58abc1..e2aebba 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/DiffAwarenessManagerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/DiffAwarenessManagerTest.java
@@ -28,6 +28,7 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import com.google.devtools.common.options.OptionsClassProvider;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
@@ -42,7 +43,6 @@
*/
@RunWith(JUnit4.class)
public class DiffAwarenessManagerTest {
-
private FileSystem fs;
private Path root;
protected EventCollectionApparatus events;
@@ -67,7 +67,8 @@
assertEquals(
"Expected EVERYTHING_MODIFIED since there are no factories",
ModifiedFileSet.EVERYTHING_MODIFIED,
- manager.getDiff(events.reporter(), pathEntry).getModifiedFileSet());
+ manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY)
+ .getModifiedFileSet());
events.assertNoWarningsOrErrors();
}
@@ -80,12 +81,12 @@
DiffAwarenessFactoryStub factory = new DiffAwarenessFactoryStub();
factory.inject(pathEntry, diffAwareness1);
DiffAwarenessManager manager = new DiffAwarenessManager(ImmutableList.of(factory));
- manager.getDiff(events.reporter(), pathEntry);
+ manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY);
assertFalse("diffAwareness1 shouldn't have been closed yet", diffAwareness1.closed());
manager.reset();
assertTrue("diffAwareness1 should have been closed by reset", diffAwareness1.closed());
factory.inject(pathEntry, diffAwareness2);
- manager.getDiff(events.reporter(), pathEntry);
+ manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY);
assertFalse("diffAwareness2 shouldn't have been closed yet", diffAwareness2.closed());
events.assertNoWarningsOrErrors();
}
@@ -101,21 +102,26 @@
DiffAwarenessFactoryStub factory = new DiffAwarenessFactoryStub();
factory.inject(pathEntry, diffAwareness);
DiffAwarenessManager manager = new DiffAwarenessManager(ImmutableList.of(factory));
- ProcessableModifiedFileSet firstProcessableDiff = manager.getDiff(events.reporter(), pathEntry);
+ ProcessableModifiedFileSet firstProcessableDiff =
+ manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY);
assertEquals(
"Expected EVERYTHING_MODIFIED on first call to getDiff",
ModifiedFileSet.EVERYTHING_MODIFIED,
firstProcessableDiff.getModifiedFileSet());
firstProcessableDiff.markProcessed();
- ProcessableModifiedFileSet processableDiff1 = manager.getDiff(events.reporter(), pathEntry);
+ ProcessableModifiedFileSet processableDiff1 =
+ manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY);
assertEquals(diff1, processableDiff1.getModifiedFileSet());
- ProcessableModifiedFileSet processableDiff2 = manager.getDiff(events.reporter(), pathEntry);
+ ProcessableModifiedFileSet processableDiff2 =
+ manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY);
assertEquals(ModifiedFileSet.union(diff1, diff2), processableDiff2.getModifiedFileSet());
processableDiff2.markProcessed();
- ProcessableModifiedFileSet processableDiff3 = manager.getDiff(events.reporter(), pathEntry);
+ ProcessableModifiedFileSet processableDiff3 =
+ manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY);
assertEquals(diff3, processableDiff3.getModifiedFileSet());
events.assertNoWarningsOrErrors();
- ProcessableModifiedFileSet processableDiff4 = manager.getDiff(events.reporter(), pathEntry);
+ ProcessableModifiedFileSet processableDiff4 =
+ manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY);
assertEquals(ModifiedFileSet.EVERYTHING_MODIFIED, processableDiff4.getModifiedFileSet());
events.assertContainsWarning("error");
}
@@ -139,7 +145,8 @@
DiffAwarenessManager manager =
new DiffAwarenessManager(ImmutableList.of(factory1, factory2, factory3));
- ProcessableModifiedFileSet processableDiff = manager.getDiff(events.reporter(), pathEntry);
+ ProcessableModifiedFileSet processableDiff =
+ manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY);
events.assertNoWarningsOrErrors();
assertEquals(
"Expected EVERYTHING_MODIFIED on first call to getDiff for diffAwareness1",
@@ -147,7 +154,7 @@
processableDiff.getModifiedFileSet());
processableDiff.markProcessed();
- processableDiff = manager.getDiff(events.reporter(), pathEntry);
+ processableDiff = manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY);
events.assertContainsEventWithFrequency("error in getCurrentView", 1);
assertEquals(
"Expected EVERYTHING_MODIFIED because of broken getCurrentView",
@@ -156,18 +163,18 @@
processableDiff.markProcessed();
factory1.remove(pathEntry);
- processableDiff = manager.getDiff(events.reporter(), pathEntry);
+ processableDiff = manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY);
assertEquals(
"Expected EVERYTHING_MODIFIED on first call to getDiff for diffAwareness2",
ModifiedFileSet.EVERYTHING_MODIFIED,
processableDiff.getModifiedFileSet());
processableDiff.markProcessed();
- processableDiff = manager.getDiff(events.reporter(), pathEntry);
+ processableDiff = manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY);
assertEquals(diff2, processableDiff.getModifiedFileSet());
processableDiff.markProcessed();
- processableDiff = manager.getDiff(events.reporter(), pathEntry);
+ processableDiff = manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY);
events.assertContainsEventWithFrequency("error in getDiff", 1);
assertEquals(
"Expected EVERYTHING_MODIFIED because of broken getDiff",
@@ -176,14 +183,14 @@
processableDiff.markProcessed();
factory2.remove(pathEntry);
- processableDiff = manager.getDiff(events.reporter(), pathEntry);
+ processableDiff = manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY);
assertEquals(
"Expected EVERYTHING_MODIFIED on first call to getDiff for diffAwareness3",
ModifiedFileSet.EVERYTHING_MODIFIED,
processableDiff.getModifiedFileSet());
processableDiff.markProcessed();
- processableDiff = manager.getDiff(events.reporter(), pathEntry);
+ processableDiff = manager.getDiff(events.reporter(), pathEntry, OptionsClassProvider.EMPTY);
assertEquals(diff3, processableDiff.getModifiedFileSet());
processableDiff.markProcessed();
}
@@ -235,7 +242,7 @@
}
@Override
- public View getCurrentView() throws BrokenDiffAwarenessException {
+ public View getCurrentView(OptionsClassProvider options) throws BrokenDiffAwarenessException {
if (curSequenceNum == brokenViewNum) {
throw new BrokenDiffAwarenessException("error in getCurrentView");
}