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

1 files changed, 27 insertions, 19 deletions

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 ae5de2782b..96dc02af25 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.assertEquals;
 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.Lists;
 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.InMemoryMemoizingEvaluator;
 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 @@ public class FileFunctionTest {
     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 @@ public class FileFunctionTest {
         getFilesSeenAndAssertValueChangesIfContentsOfFileChanges("../outside", true, "a"));
     assertThat(seenFiles)
         .containsExactly(
+            rootedPath("WORKSPACE"),
             rootedPath("a"),
             rootedPath(""),
             RootedPath.toRootedPath(fs.getRootDirectory(), PathFragment.EMPTY_FRAGMENT),
@@ -282,6 +298,7 @@ public class FileFunctionTest {
         getFilesSeenAndAssertValueChangesIfContentsOfFileChanges("/absolute", true, "a"));
     assertThat(seenFiles)
         .containsExactly(
+            rootedPath("WORKSPACE"),
             rootedPath("a"),
             rootedPath(""),
             RootedPath.toRootedPath(fs.getRootDirectory(), PathFragment.EMPTY_FRAGMENT),
@@ -537,7 +554,7 @@ public class FileFunctionTest {
   }
 
   @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 @@ public class FileFunctionTest {
     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 @@ public class FileFunctionTest {
       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());
   }