Rollback of commit 0c7a42a09d85ddffd9b860bcb31e4c43a00632c1.

*** Reason for rollback ***

[]

*** Original change description ***

Slight refactor of ExternalFilesHelper:
-Make FileType and ExternalFileAction public.
-Have producers use ExternalFileAction, rather than a boolean, to specify the desired behavior.

And a big change in semantics (doesn't affect Bazel):
-Replace ExternalFileAction.ERROR_OUT with ExternalFileAction.ASSUME_NON_EXISTENT_AND_IMMUTABLE, which does what it sounds like. This new action, like the old ERROR_OUT, is _not_ used in Bazel.

--
MOS_MIGRATED_REVID=136206810

9 files changed, 188 insertions, 77 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java
index 307c74b22a..aa1fe760e2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java
@@ -45,10 +45,8 @@ public class DirectoryListingStateFunction implements SkyFunction {
         return null;
       }
       return DirectoryListingStateValue.create(dirRootedPath);
-    } catch (ExternalFilesHelper.NonexistentImmutableExternalFileException e) {
-      // DirectoryListingStateValue.key assumes the path exists. This exception here is therefore
-      // indicative of a programming bug.
-      throw new IllegalStateException(dirRootedPath.toString(), e);
+    } catch (FileOutsidePackageRootsException e) {
+      throw new DirectoryListingStateFunctionException(e);
     } catch (IOException e) {
       throw new DirectoryListingStateFunctionException(e);
     }
@@ -68,5 +66,9 @@ public class DirectoryListingStateFunction implements SkyFunction {
     public DirectoryListingStateFunctionException(IOException e) {
       super(e, Transience.TRANSIENT);
     }
+
+    public DirectoryListingStateFunctionException(FileOutsidePackageRootsException e) {
+      super(e, Transience.PERSISTENT);
+    }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
index e757c27574..664cf9888a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java
@@ -26,7 +26,7 @@ import com.google.devtools.build.skyframe.SkyFunction;
 import java.io.IOException;
 import java.util.concurrent.atomic.AtomicReference;
 
-/** Common utilities for dealing with paths outside the package roots. */
+/** Common utilities for dealing with files outside the package roots. */
 public class ExternalFilesHelper {
   private final AtomicReference<PathPackageLocator> pkgLocator;
   private final ExternalFileAction externalFileAction;
@@ -37,8 +37,23 @@ public class ExternalFilesHelper {
   private boolean anyOutputFilesSeen = false;
   private boolean anyNonOutputExternalFilesSeen = false;
 
+  /**
+   * @param pkgLocator an {@link AtomicReference} to a {@link PathPackageLocator} used to
+   *    determine what files are internal.
+   * @param errorOnExternalFiles If files outside of package paths should be allowed.
+   */
   public ExternalFilesHelper(
-      AtomicReference<PathPackageLocator> pkgLocator,
+      AtomicReference<PathPackageLocator> pkgLocator, boolean errorOnExternalFiles,
+      BlazeDirectories directories) {
+    this(
+        pkgLocator,
+        errorOnExternalFiles
+            ? ExternalFileAction.ERROR_OUT
+            : ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_FILES,
+        directories);
+  }
+
+  private ExternalFilesHelper(AtomicReference<PathPackageLocator> pkgLocator,
       ExternalFileAction externalFileAction,
       BlazeDirectories directories) {
     this.pkgLocator = pkgLocator;
@@ -46,42 +61,25 @@ public class ExternalFilesHelper {
     this.directories = directories;
   }
 
-  /**
-   * The action to take when an external path is encountered. See {@link FileType} for the
-   * definition of "external".
-   */
-  public enum ExternalFileAction {
-    /**
-     * For paths of type {@link FileType#EXTERNAL_REPO}, introduce a Skyframe dependency on the
-     * 'external' package.
-     *
-     * <p>This is the default for Bazel, since it's required for correctness of the external
-     * repositories feature.
-     */
-    DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS,
+  private enum ExternalFileAction {
+    /** Re-check the files when the WORKSPACE file changes. */
+    DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_FILES,
 
-    /**
-     * For paths of type {@link FileType#EXTERNAL} or {@link FileType#OUTPUT}, assume the path does
-     * not exist and will never exist.
-     */
-    ASSUME_NON_EXISTENT_AND_IMMUTABLE_FOR_EXTERNAL_PATHS,
+    /** Throw an exception if there is an external file. */
+    ERROR_OUT,
   }
 
-  /** Classification of a path encountered by Bazel. */
-  public enum FileType {
-    /** A path inside the package roots or in an external repository. */
+  enum FileType {
+    /** A file inside the package roots or in an external repository. */
     INTERNAL,
 
-    /**
-     * A non {@link #EXTERNAL_REPO} path outside the package roots about which we may make no other
-     * assumptions.
-     */
-    EXTERNAL,
+    /** A file outside the package roots about which we may make no other assumptions. */
+    EXTERNAL_MUTABLE,
 
     /**
-     * A path in Bazel's output tree that's a proper output of an action (*not* a source file in an
-     * external repository). Such files are theoretically mutable, but certain Bazel flags may tell
-     * Bazel to assume these paths are immutable.
+     * A file in Bazel's output tree that's a proper output of an action (*not* a source file in an
+     * external repository). Such files are theoretically mutable, but certain Blaze flags may tell
+     * Blaze to assume these files are immutable.
      *
      * <p>Note that {@link ExternalFilesHelper#maybeHandleExternalFile} is only used for {@link
      * FileStateValue} and {@link DirectoryListingStateValue}, and also note that output files do
@@ -95,19 +93,12 @@ public class ExternalFilesHelper {
     OUTPUT,
 
     /**
-     * A path in the part of Bazel's output tree that contains (/ symlinks to) to external
+     * A file in the part of Bazel's output tree that contains (/ symlinks to) to external
      * repositories.
      */
     EXTERNAL_REPO,
   }
 
-  /**
-   * Thrown by {@link #maybeHandleExternalFile} when an applicable path is processed (see
-   * {@link ExternalFileAction#ASSUME_NON_EXISTENT_AND_IMMUTABLE_FOR_EXTERNAL_PATHS}.
-   */
-  static class NonexistentImmutableExternalFileException extends Exception {
-  }
-
   static class ExternalFilesKnowledge {
     final boolean anyOutputFilesSeen;
     final boolean anyNonOutputExternalFilesSeen;
@@ -143,7 +134,7 @@ public class ExternalFilesHelper {
     Path outputBase = packageLocator.getOutputBase();
     if (outputBase == null) {
       anyNonOutputExternalFilesSeen = true;
-      return FileType.EXTERNAL;
+      return FileType.EXTERNAL_MUTABLE;
     }
     if (rootedPath.asPath().startsWith(outputBase)) {
       Path externalRepoDir = outputBase.getRelative(Label.EXTERNAL_PACKAGE_NAME);
@@ -156,33 +147,29 @@ public class ExternalFilesHelper {
       }
     }
     anyNonOutputExternalFilesSeen = true;
-    return FileType.EXTERNAL;
+    return FileType.EXTERNAL_MUTABLE;
   }
 
   /**
-   * If this instance is configured with
-   * {@link ExternalFileAction#DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS} and
-   * {@code rootedPath} isn't under a package root then this adds a dependency on the //external
-   * package. If the action is
-   * {@link ExternalFileAction#ASSUME_NON_EXISTENT_AND_IMMUTABLE_FOR_EXTERNAL_PATHS}, it will throw
-   * a {@link NonexistentImmutableExternalFileException} instead.
+   * If this instance is configured with DEPEND_ON_EXTERNAL_PKG and rootedPath is a file that isn't
+   * under a package root then this adds a dependency on the //external package. If the action is
+   * ERROR_OUT, it will throw an error instead.
    */
   @ThreadSafe
   void maybeHandleExternalFile(RootedPath rootedPath, SkyFunction.Environment env)
-      throws NonexistentImmutableExternalFileException, IOException, InterruptedException {
+      throws IOException, InterruptedException {
     FileType fileType = getAndNoteFileType(rootedPath);
     if (fileType == FileType.INTERNAL) {
       return;
     }
-    if (fileType == FileType.OUTPUT || fileType == FileType.EXTERNAL) {
-      if (externalFileAction
-          == ExternalFileAction.ASSUME_NON_EXISTENT_AND_IMMUTABLE_FOR_EXTERNAL_PATHS) {
-        throw new NonexistentImmutableExternalFileException();
+    if (fileType == FileType.OUTPUT || fileType == FileType.EXTERNAL_MUTABLE) {
+      if (externalFileAction == ExternalFileAction.ERROR_OUT) {
+        throw new FileOutsidePackageRootsException(rootedPath);
       }
       return;
     }
     Preconditions.checkState(
-        externalFileAction == ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS,
+        externalFileAction == ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_FILES,
         externalFileAction);
     RepositoryFunction.addExternalFilesDependencies(rootedPath, directories, env);
   }
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 86bb9e6aab..8404f53aa7 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
@@ -62,7 +62,19 @@ public class FileFunction implements SkyFunction {
     // symlink cycle, we want to detect that quickly as it gives a more informative error message
     // than we'd get doing bogus filesystem operations.
     if (!relativePath.equals(PathFragment.EMPTY_FRAGMENT)) {
-      Pair<RootedPath, FileStateValue> resolvedState = resolveFromAncestors(rootedPath, env);
+      Pair<RootedPath, FileStateValue> resolvedState = null;
+
+      try {
+        resolvedState = resolveFromAncestors(rootedPath, env);
+      } catch (FileOutsidePackageRootsException e) {
+        // When getting a FileOutsidePackageRootsException caused by an external file symlink
+        // somewhere in this file's path, rethrow an exception with this file's path, so the error
+        // message mentions this file instead of the first ancestor path where the external file
+        // error is observed
+        throw new FileFunctionException(
+            new FileOutsidePackageRootsException(rootedPath), Transience.PERSISTENT);
+      }
+
       if (resolvedState == null) {
         return null;
       }
@@ -70,7 +82,18 @@ public class FileFunction implements SkyFunction {
       realFileStateValue = resolvedState.getSecond();
     }
 
-    FileStateValue fileStateValue = (FileStateValue) env.getValue(FileStateValue.key(rootedPath));
+    FileStateValue fileStateValue = null;
+
+    try {
+      fileStateValue =
+          (FileStateValue)
+              env.getValueOrThrow(
+                  FileStateValue.key(rootedPath), FileOutsidePackageRootsException.class);
+    } catch (FileOutsidePackageRootsException e) {
+      throw new FileFunctionException(
+          new FileOutsidePackageRootsException(rootedPath), Transience.PERSISTENT);
+    }
+
     if (fileStateValue == null) {
       return null;
     }
@@ -110,7 +133,7 @@ public class FileFunction implements SkyFunction {
   @Nullable
   private static Pair<RootedPath, FileStateValue> resolveFromAncestors(
       RootedPath rootedPath, Environment env)
-      throws FileFunctionException, InterruptedException {
+      throws FileFunctionException, FileOutsidePackageRootsException, InterruptedException {
     PathFragment relativePath = rootedPath.getRelativePath();
     RootedPath realRootedPath = rootedPath;
     FileValue parentFileValue = null;
@@ -118,7 +141,11 @@ public class FileFunction implements SkyFunction {
       RootedPath parentRootedPath = RootedPath.toRootedPath(rootedPath.getRoot(),
           relativePath.getParentDirectory());
 
-      parentFileValue = (FileValue) env.getValue(FileValue.key(parentRootedPath));
+      parentFileValue =
+          (FileValue)
+              env.getValueOrThrow(
+                  FileValue.key(parentRootedPath), FileOutsidePackageRootsException.class);
+
       if (parentFileValue == null) {
         return null;
       }
@@ -134,7 +161,8 @@ public class FileFunction implements SkyFunction {
     }
     FileStateValue realFileStateValue =
         (FileStateValue)
-            env.getValue(FileStateValue.key(realRootedPath));
+            env.getValueOrThrow(
+                FileStateValue.key(realRootedPath), FileOutsidePackageRootsException.class);
 
     if (realFileStateValue == null) {
       return null;
@@ -250,7 +278,17 @@ public class FileFunction implements SkyFunction {
       throw new FileFunctionException(Preconditions.checkNotNull(fse, rootedPath));
     }
 
-    return resolveFromAncestors(symlinkTargetRootedPath, env);
+    try {
+      return resolveFromAncestors(symlinkTargetRootedPath, env);
+    } catch (FileOutsidePackageRootsException e) {
+      // At this point we know this file node is a symlink leading to an external file. Mark the
+      // exception to be a specific SymlinkOutsidePackageRootsException. The error will be bubbled
+      // up further but no path information will be updated again. This allows preserving the
+      // information about the symlink crossing the internal/external boundary.
+      throw new FileFunctionException(
+          new SymlinkOutsidePackageRootsException(rootedPath, symlinkTargetRootedPath),
+          Transience.PERSISTENT);
+    }
   }
 
   private static final Predicate<RootedPath> isPathPredicate(final Path path) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileOutsidePackageRootsException.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileOutsidePackageRootsException.java
new file mode 100644
index 0000000000..ce9e27c0e7
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileOutsidePackageRootsException.java
@@ -0,0 +1,35 @@
+// Copyright 2015 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.devtools.build.lib.vfs.RootedPath;
+
+import java.io.IOException;
+
+/**
+ * <p>This is an implementation detail of {@link FileFunction} to signal attempt to evaluate a file
+ * outside of known/allowed directory structures.
+ *
+ * <p>Extends {@link IOException} to ensure it is properly handled by consumers of
+ * {@link FileValue}.
+ */
+// TODO(bazel-team): Don't piggyback on existing handling of IOExceptions and instead implement
+// the desired semantics.
+class FileOutsidePackageRootsException extends IOException {
+
+  /** @param outsidePath the {@link RootedPath} that triggered this exception. */
+  public FileOutsidePackageRootsException(RootedPath outsidePath) {
+    super("Encountered reference to external mutable " + outsidePath);
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java
index dece88c313..f2dfff824c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java
@@ -50,8 +50,8 @@ public class FileStateFunction implements SkyFunction {
         return null;
       }
       return FileStateValue.create(rootedPath, tsgm.get());
-    } catch (ExternalFilesHelper.NonexistentImmutableExternalFileException e) {
-      return FileStateValue.NONEXISTENT_FILE_STATE_NODE;
+    } catch (FileOutsidePackageRootsException e) {
+      throw new FileStateFunctionException(e);
     } catch (IOException e) {
       throw new FileStateFunctionException(e);
     } catch (InconsistentFilesystemException e) {
@@ -76,5 +76,9 @@ public class FileStateFunction implements SkyFunction {
     public FileStateFunctionException(InconsistentFilesystemException e) {
       super(e, Transience.TRANSIENT);
     }
+
+    public FileStateFunctionException(FileOutsidePackageRootsException e) {
+      super(e, Transience.PERSISTENT);
+    }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index 4573a511e9..9db19e5b4e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -221,7 +221,8 @@ public class PackageFunction implements SkyFunction {
       Iterable<SkyKey> depKeys,
       Environment env,
       boolean packageWasInError)
-      throws InternalInconsistentFilesystemException, InterruptedException {
+      throws InternalInconsistentFilesystemException, FileOutsidePackageRootsException,
+          SymlinkOutsidePackageRootsException, InterruptedException {
     Preconditions.checkState(
         Iterables.all(depKeys, SkyFunctions.isSkyFunction(SkyFunctions.FILE)), depKeys);
     boolean packageShouldBeInError = packageWasInError;
@@ -230,6 +231,8 @@ public class PackageFunction implements SkyFunction {
             FileSymlinkException.class, InconsistentFilesystemException.class).entrySet()) {
       try {
         entry.getValue().get();
+      } catch (FileOutsidePackageRootsException | SymlinkOutsidePackageRootsException e) {
+        throw e;
       } catch (IOException e) {
         maybeThrowFilesystemInconsistency(packageIdentifier, e, packageWasInError);
       } catch (FileSymlinkException e) {
@@ -251,7 +254,8 @@ public class PackageFunction implements SkyFunction {
       Iterable<SkyKey> depKeys,
       Environment env,
       boolean packageWasInError)
-      throws InternalInconsistentFilesystemException, InterruptedException {
+      throws InternalInconsistentFilesystemException, FileOutsidePackageRootsException,
+          SymlinkOutsidePackageRootsException, InterruptedException {
     Preconditions.checkState(
         Iterables.all(depKeys, SkyFunctions.isSkyFunction(SkyFunctions.GLOB)), depKeys);
     boolean packageShouldBeInError = packageWasInError;
@@ -261,6 +265,8 @@ public class PackageFunction implements SkyFunction {
             FileSymlinkException.class, InconsistentFilesystemException.class).entrySet()) {
       try {
         entry.getValue().get();
+      } catch (FileOutsidePackageRootsException | SymlinkOutsidePackageRootsException e) {
+        throw e;
       } catch (IOException | BuildFileNotFoundException e) {
         maybeThrowFilesystemInconsistency(packageIdentifier, e, packageWasInError);
       } catch (FileSymlinkException e) {
@@ -288,7 +294,8 @@ public class PackageFunction implements SkyFunction {
       Map<Label, Path> subincludes,
       PackageIdentifier packageIdentifier,
       boolean containsErrors)
-      throws InternalInconsistentFilesystemException, InterruptedException {
+      throws InternalInconsistentFilesystemException, FileOutsidePackageRootsException,
+          SymlinkOutsidePackageRootsException, InterruptedException {
     boolean packageShouldBeInError = containsErrors;
 
     // TODO(bazel-team): This means that many packages will have to be preprocessed twice. Ouch!
@@ -543,6 +550,11 @@ public class PackageFunction implements SkyFunction {
       throw new PackageFunctionException(
           e.toNoSuchPackageException(),
           e.isTransient() ? Transience.TRANSIENT : Transience.PERSISTENT);
+    } catch (FileOutsidePackageRootsException | SymlinkOutsidePackageRootsException e) {
+      packageFunctionCache.invalidate(packageId);
+      throw new PackageFunctionException(
+          new NoSuchPackageException(packageId, "Encountered file outside package roots", e),
+          Transience.PERSISTENT);
     }
     if (env.valuesMissing()) {
       return null;
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..3eee0c1056 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
@@ -44,7 +44,6 @@ import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.BasicFilesys
 import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.ExternalDirtinessChecker;
 import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.MissingDiffDirtinessChecker;
 import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.UnionDirtinessChecker;
-import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
 import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFilesKnowledge;
 import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.FileType;
 import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
@@ -124,7 +123,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
         preprocessorFactorySupplier,
         extraSkyFunctions,
         extraPrecomputedValues,
-        ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS,
+        false,
         blacklistedPackagePrefixesFile,
         productName,
         crossRepositoryLabelViolationStrategy);
@@ -422,8 +421,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
         externalFilesHelper.cloneWithFreshExternalFilesKnowledge();
     // See the comment for FileType.OUTPUT for why we need to consider output files here.
     EnumSet<FileType> fileTypesToCheck = checkOutputFiles
-        ? EnumSet.of(FileType.EXTERNAL, FileType.EXTERNAL_REPO, FileType.OUTPUT)
-        : EnumSet.of(FileType.EXTERNAL, FileType.EXTERNAL_REPO);
+        ? EnumSet.of(FileType.EXTERNAL_MUTABLE, FileType.EXTERNAL_REPO, FileType.OUTPUT)
+        : EnumSet.of(FileType.EXTERNAL_MUTABLE, FileType.EXTERNAL_REPO);
     Differencer.Diff diff =
         fsvc.getDirtyKeys(
             memoizingEvaluator.getValues(),
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..ce4c74847f 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
@@ -105,7 +105,6 @@ import com.google.devtools.build.lib.pkgcache.TransitivePackageLoader;
 import com.google.devtools.build.lib.profiler.AutoProfiler;
 import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey;
 import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.FileDirtinessChecker;
-import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
 import com.google.devtools.build.lib.skyframe.PackageFunction.CacheEntryWithGlobDeps;
 import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
 import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ActionCompletedReceiver;
@@ -255,7 +254,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   protected int outputDirtyFiles;
   protected int modifiedFilesDuringPreviousBuild;
   private final Predicate<PathFragment> allowedMissingInputs;
-  private final ExternalFileAction externalFileAction;
+  private final boolean errorOnExternalFiles;
 
   private final ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions;
   private final ImmutableList<PrecomputedValue.Injected> extraPrecomputedValues;
@@ -288,7 +287,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       Preprocessor.Factory.Supplier preprocessorFactorySupplier,
       ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions,
       ImmutableList<PrecomputedValue.Injected> extraPrecomputedValues,
-      ExternalFileAction externalFileAction,
+      boolean errorOnExternalFiles,
       PathFragment blacklistedPackagePrefixesFile,
       String productName,
       CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy) {
@@ -310,7 +309,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     this.preprocessorFactorySupplier = preprocessorFactorySupplier;
     this.extraSkyFunctions = extraSkyFunctions;
     this.extraPrecomputedValues = extraPrecomputedValues;
-    this.externalFileAction = externalFileAction;
+    this.errorOnExternalFiles = errorOnExternalFiles;
     this.blacklistedPackagePrefixesFile = blacklistedPackagePrefixesFile;
     this.binTools = binTools;
 
@@ -322,7 +321,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
         (ConfiguredRuleClassProvider) ruleClassProvider);
     this.artifactFactory.set(skyframeBuildView.getArtifactFactory());
     this.externalFilesHelper = new ExternalFilesHelper(
-        pkgLocator, this.externalFileAction, directories);
+        pkgLocator, this.errorOnExternalFiles, directories);
     this.productName = productName;
     this.crossRepositoryLabelViolationStrategy = crossRepositoryLabelViolationStrategy;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SymlinkOutsidePackageRootsException.java b/src/main/java/com/google/devtools/build/lib/skyframe/SymlinkOutsidePackageRootsException.java
new file mode 100644
index 0000000000..3f20ce0a48
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SymlinkOutsidePackageRootsException.java
@@ -0,0 +1,35 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.devtools.build.lib.vfs.RootedPath;
+
+import java.io.IOException;
+
+/**
+ * <p>This is an implementation detail of {@link FileFunction} to to evaluate a symlink linking to
+ * file outside of known/allowed directory structures.
+ *
+ * <p>Extends {@link IOException} to ensure it is properly handled by consumers of
+ * {@link FileValue}.
+ */
+class SymlinkOutsidePackageRootsException extends IOException {
+  /**
+   * @param symlinkPath the {@link RootedPath} that links to an outside path.
+   * @param outsidePath the {@link RootedPath} that triggered this exception.
+   */
+  public SymlinkOutsidePackageRootsException(RootedPath symlinkPath, RootedPath outsidePath) {
+    super("Encountered symlink " + symlinkPath + " linking to external mutable " + outsidePath);
+  }
+}