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=136063159
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 664cf98..e757c27 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 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 @@
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 @@
this.directories = directories;
}
- private enum ExternalFileAction {
- /** Re-check the files when the WORKSPACE file changes. */
- DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_FILES,
-
- /** Throw an exception if there is an external file. */
- ERROR_OUT,
- }
-
- enum FileType {
- /** A file 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,
+ /**
+ * 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,
/**
- * 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.
+ * 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,
+ }
+
+ /** Classification of a path encountered by Bazel. */
+ public enum FileType {
+ /** A path 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 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 @@
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 @@
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 @@
}
}
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);
}
