Rollback of commit b776d6c12e952eb358c1a036cc9d93d8944c4c77.

*** Reason for rollback ***

--
MOS_MIGRATED_REVID=136448178

1 files changed, 53 insertions, 40 deletions

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 664cf9888a..e757c27574 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 files outside the package roots. */
+/** Common utilities for dealing with paths outside the package roots. */
 public class ExternalFilesHelper {
   private final AtomicReference<PathPackageLocator> pkgLocator;
   private final ExternalFileAction externalFileAction;
@@ -37,23 +37,8 @@ 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, 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,
+      AtomicReference<PathPackageLocator> pkgLocator,
       ExternalFileAction externalFileAction,
       BlazeDirectories directories) {
     this.pkgLocator = pkgLocator;
@@ -61,25 +46,42 @@ public class ExternalFilesHelper {
     this.directories = directories;
   }
 
-  private enum ExternalFileAction {
-    /** Re-check the files when the WORKSPACE file changes. */
-    DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_FILES,
+  /**
+   * 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,
 
-    /** Throw an exception if there is an external file. */
-    ERROR_OUT,
+    /**
+     * 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,
   }
 
-  enum FileType {
-    /** A file inside the package roots or in an external repository. */
+  /** Classification of a path encountered by Bazel. */
+  public enum FileType {
+    /** A path inside the package roots or in an external repository. */
     INTERNAL,
 
-    /** A file outside the package roots about which we may make no other assumptions. */
-    EXTERNAL_MUTABLE,
+    /**
+     * A non {@link #EXTERNAL_REPO} path outside the package roots about which we may make no other
+     * assumptions.
+     */
+    EXTERNAL,
 
     /**
-     * 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.
+     * 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.
      *
      * <p>Note that {@link ExternalFilesHelper#maybeHandleExternalFile} is only used for {@link
      * FileStateValue} and {@link DirectoryListingStateValue}, and also note that output files do
@@ -93,12 +95,19 @@ public class ExternalFilesHelper {
     OUTPUT,
 
     /**
-     * A file in the part of Bazel's output tree that contains (/ symlinks to) to external
+     * A path 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;
@@ -134,7 +143,7 @@ public class ExternalFilesHelper {
     Path outputBase = packageLocator.getOutputBase();
     if (outputBase == null) {
       anyNonOutputExternalFilesSeen = true;
-      return FileType.EXTERNAL_MUTABLE;
+      return FileType.EXTERNAL;
     }
     if (rootedPath.asPath().startsWith(outputBase)) {
       Path externalRepoDir = outputBase.getRelative(Label.EXTERNAL_PACKAGE_NAME);
@@ -147,29 +156,33 @@ public class ExternalFilesHelper {
       }
     }
     anyNonOutputExternalFilesSeen = true;
-    return FileType.EXTERNAL_MUTABLE;
+    return FileType.EXTERNAL;
   }
 
   /**
-   * 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.
+   * 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.
    */
   @ThreadSafe
   void maybeHandleExternalFile(RootedPath rootedPath, SkyFunction.Environment env)
-      throws IOException, InterruptedException {
+      throws NonexistentImmutableExternalFileException, IOException, InterruptedException {
     FileType fileType = getAndNoteFileType(rootedPath);
     if (fileType == FileType.INTERNAL) {
       return;
     }
-    if (fileType == FileType.OUTPUT || fileType == FileType.EXTERNAL_MUTABLE) {
-      if (externalFileAction == ExternalFileAction.ERROR_OUT) {
-        throw new FileOutsidePackageRootsException(rootedPath);
+    if (fileType == FileType.OUTPUT || fileType == FileType.EXTERNAL) {
+      if (externalFileAction
+          == ExternalFileAction.ASSUME_NON_EXISTENT_AND_IMMUTABLE_FOR_EXTERNAL_PATHS) {
+        throw new NonexistentImmutableExternalFileException();
       }
       return;
     }
     Preconditions.checkState(
-        externalFileAction == ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_FILES,
+        externalFileAction == ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS,
         externalFileAction);
     RepositoryFunction.addExternalFilesDependencies(rootedPath, directories, env);
   }