commit f9fdc8dfced8b2b14561720623126a91e04b22cb
author Kristina Chodorow <kchodorow@google.com> Tue Dec 08 12:49:31 2015 +0000
committer Lukacs Berki <lberki@google.com> Tue Dec 08 13:17:04 2015 +0000
tree 3dd323db49d99bcf61bef0cf9abe4b93a8c1f84b
parent 258af7b294706bde162a965390ab39f9917a87b1

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

src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
index 6adf32f..e270f41 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
@@ -20,7 +20,6 @@
 import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
 import com.google.devtools.build.lib.util.Pair;
-import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.RootedPath;
@@ -30,7 +29,6 @@
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.TreeSet;
 import java.util.concurrent.atomic.AtomicReference;
@@ -46,15 +44,9 @@
  */
 public class FileFunction implements SkyFunction {
   private final AtomicReference<PathPackageLocator> pkgLocator;
-  private final TimestampGranularityMonitor tsgm;
-  private final ExternalFilesHelper externalFilesHelper;
 
-  public FileFunction(AtomicReference<PathPackageLocator> pkgLocator,
-      TimestampGranularityMonitor tsgm,
-      ExternalFilesHelper externalFilesHelper) {
+  public FileFunction(AtomicReference<PathPackageLocator> pkgLocator) {
     this.pkgLocator = pkgLocator;
-    this.tsgm = tsgm;
-    this.externalFilesHelper = externalFilesHelper;
   }
 
   @Override
@@ -101,33 +93,13 @@
     while (realFileStateValue.getType().equals(FileStateValue.Type.SYMLINK)) {
       symlinkChain.add(realRootedPath);
       orderedSeenPaths.add(realRootedPath.asPath());
-      if (externalFilesHelper.shouldAssumeImmutable(realRootedPath)) {
-        // If the file is assumed to be immutable, we want to resolve the symlink chain without
-        // adding dependencies since we don't care about incremental correctness.
-        try {
-          Path realPath = rootedPath.asPath().resolveSymbolicLinks();
-          realRootedPath = RootedPath.toRootedPathMaybeUnderRoot(realPath,
-              pkgLocator.get().getPathEntries());
-          realFileStateValue = FileStateValue.create(realRootedPath, tsgm);
-        } catch (IOException e) {
-          RootedPath root = RootedPath.toRootedPath(
-              rootedPath.asPath().getFileSystem().getRootDirectory(),
-              rootedPath.asPath().getFileSystem().getRootDirectory());
-          return FileValue.value(
-              rootedPath, fileStateValue,
-              root, FileStateValue.NONEXISTENT_FILE_STATE_NODE);
-        } catch (InconsistentFilesystemException e) {
-          throw new FileFunctionException(e, Transience.TRANSIENT);
-        }
-      } else {
-        Pair<RootedPath, FileStateValue> resolvedState = getSymlinkTargetRootedPath(realRootedPath,
-            realFileStateValue.getSymlinkTarget(), orderedSeenPaths, symlinkChain, env);
-        if (resolvedState == null) {
-          return null;
-        }
-        realRootedPath = resolvedState.getFirst();
-        realFileStateValue = resolvedState.getSecond();
+      Pair<RootedPath, FileStateValue> resolvedState = getSymlinkTargetRootedPath(realRootedPath,
+          realFileStateValue.getSymlinkTarget(), orderedSeenPaths, symlinkChain, env);
+      if (resolvedState == null) {
+        return null;
       }
+      realRootedPath = resolvedState.getFirst();
+      realFileStateValue = resolvedState.getSecond();
     }
     return FileValue.value(rootedPath, fileStateValue, realRootedPath, realFileStateValue);
   }
@@ -142,10 +114,7 @@
     PathFragment relativePath = rootedPath.getRelativePath();
     RootedPath realRootedPath = rootedPath;
     FileValue parentFileValue = null;
-    // We only resolve ancestors if the file is not assumed to be immutable (handling ancestors
-    // would be too aggressive).
-    if (!externalFilesHelper.shouldAssumeImmutable(rootedPath)
-        && !relativePath.equals(PathFragment.EMPTY_FRAGMENT)) {
+    if (!relativePath.equals(PathFragment.EMPTY_FRAGMENT)) {
       RootedPath parentRootedPath = RootedPath.toRootedPath(rootedPath.getRoot(),
           relativePath.getParentDirectory());
       parentFileValue = (FileValue) env.getValue(FileValue.key(parentRootedPath));