Introduce a PackageBoundaryMode enum to FilesetTraversalParams.
This enables the introduction of a third behavior mode when
RecursiveFilesystemTraversalFunction encounters a package boundary: on top of
the existing cross/no-cross behavior, it can now bail out with an error.
This is preparatory work to support Skyframe-native Filesets.
The CL also contains a couple of prettifying cleanups (in ActionCacheChecker
and in FilesetManifestAction).
--
MOS_MIGRATED_REVID=86185105
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
index 4b970e3..07b9977 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
@@ -107,9 +107,9 @@
if (handler != null) {
if (verboseExplanations) {
String keyDescription = action.describeKey();
- reportRebuild(handler, action,
- keyDescription == null ? "action command has changed" :
- "action command has changed.\nNew action: " + keyDescription);
+ reportRebuild(handler, action, keyDescription == null
+ ? "action command has changed"
+ : "action command has changed.\nNew action: " + keyDescription);
} else {
reportRebuild(handler, action,
"action command has changed (try --verbose_explanations for more info)");
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java
index 1464f73..51f131e 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java
@@ -1,4 +1,4 @@
-// Copyright 2014 Google Inc. All rights reserved.
+// Copyright 2015 Google Inc. 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.
@@ -31,6 +31,29 @@
*/
public interface FilesetTraversalParams {
+ /** Desired behavior if the traversal hits a directory with a BUILD file, i.e. a subpackage. */
+ public enum PackageBoundaryMode {
+ /** The traversal should recurse into the directory, optionally reporting a warning. */
+ CROSS,
+
+ // TODO(bazel-team): deprecate CROSS and REPORT_ERROR in favor of DONT_CROSS. Clean up the depot
+ // and lock down the semantics of FilesetEntry.srcdir to only accept other Filesets or BUILD
+ // files of a package, in which case also require an explicit list of files.
+ /** The traversal should not recurse into the directory but silently skip it. */
+ DONT_CROSS,
+
+ /** The traversal should not recurse into the directory and report an error. */
+ REPORT_ERROR;
+
+ public static PackageBoundaryMode forStrictFilesetFlag(boolean flagEnabled) {
+ return flagEnabled ? REPORT_ERROR : CROSS;
+ }
+
+ public void fingerprint(Fingerprint fp) {
+ fp.addInt(ordinal());
+ }
+ }
+
/**
* Abstraction of the root directory of a {@link DirectTraversal}.
*
@@ -128,7 +151,7 @@
boolean isFollowingSymlinks();
/** Returns the desired behavior when the traversal hits a subpackage. */
- boolean getCrossPackageBoundary();
+ PackageBoundaryMode getPackageBoundaryMode();
}
/** Label of the Fileset rule that owns this traversal. */
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParamsFactory.java b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParamsFactory.java
index acc0e86..c26c603 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParamsFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParamsFactory.java
@@ -1,4 +1,4 @@
-// Copyright 2014 Google Inc. All rights reserved.
+// Copyright 2015 Google Inc. 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.
@@ -19,6 +19,7 @@
import com.google.common.collect.Ordering;
import com.google.devtools.build.lib.actions.FilesetTraversalParams.DirectTraversal;
import com.google.devtools.build.lib.actions.FilesetTraversalParams.DirectTraversalRoot;
+import com.google.devtools.build.lib.actions.FilesetTraversalParams.PackageBoundaryMode;
import com.google.devtools.build.lib.syntax.FilesetEntry.SymlinkBehavior;
import com.google.devtools.build.lib.syntax.Label;
import com.google.devtools.build.lib.util.Fingerprint;
@@ -46,15 +47,15 @@
* @param excludes optional; set of files directly under this package's directory to exclude;
* files in subdirectories cannot be excluded
* @param symlinkBehaviorMode what to do with symlinks
- * @param crossPkgBoundary whether to traverse a subdirectory if it's also a subpackage (contains
- * a BUILD file)
+ * @param pkgBoundaryMode what to do when the traversal hits a subdirectory that is also a
+ * subpackage (contains a BUILD file)
*/
public static FilesetTraversalParams recursiveTraversalOfPackage(Label ownerLabel,
Artifact buildFile, PathFragment destPath, @Nullable Set<String> excludes,
- SymlinkBehavior symlinkBehaviorMode, boolean crossPkgBoundary) {
+ SymlinkBehavior symlinkBehaviorMode, PackageBoundaryMode pkgBoundaryMode) {
Preconditions.checkState(buildFile.isSourceArtifact(), "%s", buildFile);
return new DirectoryTraversalParams(ownerLabel, DirectTraversalRootImpl.forPackage(buildFile),
- true, destPath, excludes, symlinkBehaviorMode, crossPkgBoundary, true, false);
+ true, destPath, excludes, symlinkBehaviorMode, pkgBoundaryMode, true, false);
}
/**
@@ -70,15 +71,15 @@
* @param excludes optional; set of files directly below this directory to exclude; files in
* subdirectories cannot be excluded
* @param symlinkBehaviorMode what to do with symlinks
- * @param crossPkgBoundary whether to traverse a subdirectory if it's also a subpackage (contains
- * a BUILD file)
+ * @param pkgBoundaryMode what to do when the traversal hits a subdirectory that is also a
+ * subpackage (contains a BUILD file)
*/
public static FilesetTraversalParams recursiveTraversalOfDirectory(Label ownerLabel,
Artifact directoryToTraverse, PathFragment destPath, @Nullable Set<String> excludes,
- SymlinkBehavior symlinkBehaviorMode, boolean crossPkgBoundary) {
+ SymlinkBehavior symlinkBehaviorMode, PackageBoundaryMode pkgBoundaryMode) {
return new DirectoryTraversalParams(ownerLabel,
DirectTraversalRootImpl.forFileOrDirectory(directoryToTraverse), false, destPath,
- excludes, symlinkBehaviorMode, crossPkgBoundary, true,
+ excludes, symlinkBehaviorMode, pkgBoundaryMode, true,
!directoryToTraverse.isSourceArtifact());
}
@@ -93,14 +94,15 @@
* @param destPath path in the Fileset's output directory that will be the name of this file's
* respective symlink there, or the root of files found (in case this is a directory)
* @param symlinkBehaviorMode what to do with symlinks
- * @param crossPkgBoundary whether to traverse a subdirectory if it's also a subpackage (contains
- * a BUILD file)
+ * @param pkgBoundaryMode what to do when the traversal hits a subdirectory that is also a
+ * subpackage (contains a BUILD file)
*/
public static FilesetTraversalParams fileTraversal(Label ownerLabel, Artifact fileToTraverse,
- PathFragment destPath, SymlinkBehavior symlinkBehaviorMode, boolean crossPkgBoundary) {
+ PathFragment destPath, SymlinkBehavior symlinkBehaviorMode,
+ PackageBoundaryMode pkgBoundaryMode) {
return new DirectoryTraversalParams(ownerLabel,
DirectTraversalRootImpl.forFileOrDirectory(fileToTraverse), false, destPath, null,
- symlinkBehaviorMode, crossPkgBoundary, false, !fileToTraverse.isSourceArtifact());
+ symlinkBehaviorMode, pkgBoundaryMode, false, !fileToTraverse.isSourceArtifact());
}
/**
@@ -162,16 +164,16 @@
private final DirectTraversalRoot root;
private final boolean isPackage;
private final boolean followSymlinks;
- private final boolean crossPkgBoundary;
+ private final PackageBoundaryMode pkgBoundaryMode;
private final boolean isRecursive;
private final boolean isGenerated;
DirectTraversalImpl(DirectTraversalRoot root, boolean isPackage, boolean followSymlinks,
- boolean crossPkgBoundary, boolean isRecursive, boolean isGenerated) {
+ PackageBoundaryMode pkgBoundaryMode, boolean isRecursive, boolean isGenerated) {
this.root = root;
this.isPackage = isPackage;
this.followSymlinks = followSymlinks;
- this.crossPkgBoundary = crossPkgBoundary;
+ this.pkgBoundaryMode = pkgBoundaryMode;
this.isRecursive = isRecursive;
this.isGenerated = isGenerated;
}
@@ -202,8 +204,8 @@
}
@Override
- public boolean getCrossPackageBoundary() {
- return crossPkgBoundary;
+ public PackageBoundaryMode getPackageBoundaryMode() {
+ return pkgBoundaryMode;
}
void fingerprint(Fingerprint fp) {
@@ -212,7 +214,7 @@
fp.addBoolean(followSymlinks);
fp.addBoolean(isRecursive);
fp.addBoolean(isGenerated);
- fp.addBoolean(crossPkgBoundary);
+ pkgBoundaryMode.fingerprint(fp);
}
}
@@ -225,12 +227,12 @@
PathFragment destPath,
@Nullable Set<String> excludes,
SymlinkBehavior symlinkBehaviorMode,
- boolean crossPkgBoundary,
+ PackageBoundaryMode pkgBoundaryMode,
boolean isRecursive,
boolean isGenerated) {
super(ownerLabel, destPath, excludes);
traversal = new DirectTraversalImpl(root, isPackage,
- symlinkBehaviorMode == SymlinkBehavior.DEREFERENCE, crossPkgBoundary, isRecursive,
+ symlinkBehaviorMode == SymlinkBehavior.DEREFERENCE, pkgBoundaryMode, isRecursive,
isGenerated);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
index a559206..67b3f40 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
@@ -1,4 +1,4 @@
-// Copyright 2014 Google Inc. All rights reserved.
+// Copyright 2015 Google Inc. 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.
@@ -222,7 +222,7 @@
DirectTraversal traversal) throws MissingDepException {
SkyKey depKey = RecursiveFilesystemTraversalValue.key(
new RecursiveFilesystemTraversalValue.TraversalRequest(traversal.getRoot().asRootedPath(),
- traversal.isGenerated(), traversal.getCrossPackageBoundary(), traversal.isPackage(),
+ traversal.isGenerated(), traversal.getPackageBoundaryMode(), traversal.isPackage(),
errorInfo));
RecursiveFilesystemTraversalValue v = (RecursiveFilesystemTraversalValue) env.getValue(depKey);
if (env.valuesMissing()) {
@@ -234,7 +234,7 @@
private static String createErrorInfo(FilesetTraversalParams t) {
if (t.getDirectTraversal().isPresent()) {
DirectTraversal direct = t.getDirectTraversal().get();
- return String.format("Fileset '%s' traversing %s %s", t.getOwnerLabel(),
+ return String.format("Fileset '%s' traversing %s '%s'", t.getOwnerLabel(),
direct.isPackage() ? "package" : "file (or directory)",
direct.getRoot().getRelativePart().getPathString());
} else {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
index d54e98f..ab2faa2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java
@@ -63,7 +63,7 @@
/**
* Thrown when the traversal encounters a subdirectory with a BUILD file but is not allowed to
- * recurse into it.
+ * recurse into it. See {@code PackageBoundaryMode#REPORT_ERROR}.
*/
public static final class CannotCrossPackageBoundaryException extends
RecursiveFilesystemTraversalException {
@@ -137,14 +137,22 @@
new GeneratedPathConflictException(traversal));
} else if (pkgLookupResult.isPackage() && !traversal.skipTestingForSubpackage) {
// The traversal was requested for a directory that defines a package.
- if (traversal.crossPkgBoundaries) {
- // We are free to traverse the subpackage but we need to display a warning.
- String msg = traversal.errorInfo + " crosses package boundary into package rooted at "
- + traversal.path.getRelativePath().getPathString();
- env.getListener().handle(new Event(EventKind.WARNING, null, msg));
- } else {
- // We cannot traverse the subpackage and should skip it silently. Return empty results.
- return RecursiveFilesystemTraversalValue.EMPTY;
+ String msg = traversal.errorInfo + " crosses package boundary into package rooted at "
+ + traversal.path.getRelativePath().getPathString();
+ switch (traversal.crossPkgBoundaries) {
+ case CROSS:
+ // We are free to traverse the subpackage but we need to display a warning.
+ env.getListener().handle(new Event(EventKind.WARNING, null, msg));
+ break;
+ case DONT_CROSS:
+ // We cannot traverse the subpackage and should skip it silently. Return empty results.
+ return RecursiveFilesystemTraversalValue.EMPTY;
+ case REPORT_ERROR:
+ // We cannot traverse the subpackage and should complain loudly (display an error).
+ throw new RecursiveFilesystemTraversalFunctionException(
+ new CannotCrossPackageBoundaryException(msg));
+ default:
+ throw new IllegalStateException(traversal.toString());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java
index 023b1cf..f72792f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalValue.java
@@ -17,6 +17,7 @@
import com.google.common.base.Objects;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
+import com.google.devtools.build.lib.actions.FilesetTraversalParams.PackageBoundaryMode;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
@@ -120,7 +121,7 @@
final boolean isGenerated;
/** Whether traversal should descend into directories that are roots of subpackages. */
- final boolean crossPkgBoundaries;
+ final PackageBoundaryMode crossPkgBoundaries;
/**
* Whether to skip checking if the root (if it's a directory) contains a BUILD file.
@@ -135,7 +136,7 @@
@Nullable final String errorInfo;
public TraversalRequest(RootedPath path, boolean isRootGenerated,
- boolean crossPkgBoundaries, boolean skipTestingForSubpackage,
+ PackageBoundaryMode crossPkgBoundaries, boolean skipTestingForSubpackage,
@Nullable String errorInfo) {
this.path = path;
this.isGenerated = isRootGenerated;
@@ -188,8 +189,8 @@
public String toString() {
return String.format(
"TraversalParams(root=%s, is_generated=%d, skip_testing_for_subpkg=%d,"
- + " pkg_boundaries=%d)", path, isGenerated ? 1 : 0, skipTestingForSubpackage ? 1 : 0,
- crossPkgBoundaries ? 1 : 0);
+ + " pkg_boundaries=%s)", path, isGenerated ? 1 : 0, skipTestingForSubpackage ? 1 : 0,
+ crossPkgBoundaries);
}
}
