Don't treat external files as immutable
Fixes #352.
RELNOTES: Files in external repositories are now treated as mutable, which will make the correctness guarantees of using external repositories stronger (existent), but may cause performance penalties.
--
MOS_MIGRATED_REVID=109676408
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
index ae5de27..96dc02a 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
@@ -19,7 +19,6 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -34,10 +33,14 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.common.testing.EqualsTester;
+import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.events.NullEventHandler;
import com.google.devtools.build.lib.events.StoredEventHandler;
+import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.testutil.ManualClock;
+import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.lib.testutil.TestUtils;
import com.google.devtools.build.lib.util.BlazeClock;
import com.google.devtools.build.lib.util.Pair;
@@ -56,6 +59,7 @@
import com.google.devtools.build.skyframe.MemoizingEvaluator;
import com.google.devtools.build.skyframe.RecordingDifferencer;
import com.google.devtools.build.skyframe.SequentialBuildDriver;
+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;
@@ -121,15 +125,26 @@
differencer = new RecordingDifferencer();
MemoizingEvaluator evaluator =
new InMemoryMemoizingEvaluator(
- ImmutableMap.of(
- SkyFunctions.FILE_STATE, new FileStateFunction(tsgm, externalFilesHelper),
- SkyFunctions.FILE_SYMLINK_CYCLE_UNIQUENESS,
- new FileSymlinkCycleUniquenessFunction(),
- SkyFunctions.FILE_SYMLINK_INFINITE_EXPANSION_UNIQUENESS,
- new FileSymlinkInfiniteExpansionUniquenessFunction(),
- SkyFunctions.FILE, new FileFunction(pkgLocatorRef, tsgm, externalFilesHelper)),
+ ImmutableMap.<SkyFunctionName, SkyFunction>builder()
+ .put(SkyFunctions.FILE_STATE, new FileStateFunction(tsgm, externalFilesHelper))
+ .put(SkyFunctions.FILE_SYMLINK_CYCLE_UNIQUENESS,
+ new FileSymlinkCycleUniquenessFunction())
+ .put(SkyFunctions.FILE_SYMLINK_INFINITE_EXPANSION_UNIQUENESS,
+ new FileSymlinkInfiniteExpansionUniquenessFunction())
+ .put(SkyFunctions.FILE, new FileFunction(pkgLocatorRef))
+ .put(SkyFunctions.PACKAGE,
+ new PackageFunction(null, null, null, null, null, null, null))
+ .put(SkyFunctions.PACKAGE_LOOKUP,
+ new PackageLookupFunction(new AtomicReference<>(
+ ImmutableSet.<PackageIdentifier>of())))
+ .put(SkyFunctions.WORKSPACE_FILE,
+ new WorkspaceFileFunction(TestRuleClassProvider.getRuleClassProvider(),
+ new PackageFactory(TestRuleClassProvider.getRuleClassProvider()),
+ new BlazeDirectories(pkgRoot, outputBase, pkgRoot)))
+ .build(),
differencer);
PrecomputedValue.BUILD_ID.set(differencer, UUID.randomUUID());
+ PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator);
return new SequentialBuildDriver(evaluator);
}
@@ -265,6 +280,7 @@
getFilesSeenAndAssertValueChangesIfContentsOfFileChanges("../outside", true, "a"));
assertThat(seenFiles)
.containsExactly(
+ rootedPath("WORKSPACE"),
rootedPath("a"),
rootedPath(""),
RootedPath.toRootedPath(fs.getRootDirectory(), PathFragment.EMPTY_FRAGMENT),
@@ -282,6 +298,7 @@
getFilesSeenAndAssertValueChangesIfContentsOfFileChanges("/absolute", true, "a"));
assertThat(seenFiles)
.containsExactly(
+ rootedPath("WORKSPACE"),
rootedPath("a"),
rootedPath(""),
RootedPath.toRootedPath(fs.getRootDirectory(), PathFragment.EMPTY_FRAGMENT),
@@ -537,7 +554,7 @@
}
@Test
- public void testFilesOutsideRootHasDepOnBuildID() throws Exception {
+ public void testFilesOutsideRootIsReEvaluated() throws Exception {
Path file = file("/outsideroot");
SequentialBuildDriver driver = makeDriver();
SkyKey key = skyKey("/outsideroot");
@@ -552,6 +569,7 @@
assertTrue(oldValue.exists());
file.delete();
+ differencer.invalidate(ImmutableList.of(fileStateSkyKey("/outsideroot")));
result =
driver.evaluate(
ImmutableList.of(key), false, DEFAULT_THREAD_COUNT, NullEventHandler.INSTANCE);
@@ -559,16 +577,6 @@
fail(String.format("Evaluation error for %s: %s", key, result.getError()));
}
FileValue newValue = (FileValue) result.get(key);
- assertSame(oldValue, newValue);
-
- PrecomputedValue.BUILD_ID.set(differencer, UUID.randomUUID());
- result =
- driver.evaluate(
- ImmutableList.of(key), false, DEFAULT_THREAD_COUNT, NullEventHandler.INSTANCE);
- if (result.hasError()) {
- fail(String.format("Evaluation error for %s: %s", key, result.getError()));
- }
- newValue = (FileValue) result.get(key);
assertNotSame(oldValue, newValue);
assertFalse(newValue.exists());
}
