Generalize some of methods in TargetPattern, PrepareDepsOfPatternValue, and RecursivePackageProvider dealing with the concept of "excluded directories".
RELNOTES: None
PiperOrigin-RevId: 163074794
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
index be3bcaa..43f77c8 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
@@ -143,19 +143,32 @@
}
/**
- * Evaluates the current target pattern, excluding targets under directories in
- * {@code excludedSubdirectories}, and returns the result.
+ * Evaluates the current target pattern, excluding targets under directories in both
+ * {@code blacklistedSubdirectories} and {@code excludedSubdirectories}, and returns the result.
*
- * @throws IllegalArgumentException if {@code excludedSubdirectories} is nonempty and this
- * pattern does not have type {@code Type.TARGETS_BELOW_DIRECTORY}.
+ * @throws IllegalArgumentException if either {@code blacklistedSubdirectories} or
+ * {@code excludedSubdirectories} is nonempty and this pattern does not have type
+ * {@code Type.TARGETS_BELOW_DIRECTORY}.
*/
public abstract <T, E extends Exception> void eval(
TargetPatternResolver<T> resolver,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
BatchCallback<T, E> callback,
Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException;
+ protected void assertBlacklistedAndExcludedSubdirectoriesEmpty(
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
+ ImmutableSet<PathFragment> excludedSubdirectories) {
+ Preconditions.checkArgument(blacklistedSubdirectories.isEmpty(),
+ "Target pattern %s of type %s cannot be evaluated with blacklisted subdirectories: %s.",
+ getOriginalPattern(), getType(), blacklistedSubdirectories);
+ Preconditions.checkArgument(excludedSubdirectories.isEmpty(),
+ "Target pattern %s of type %s cannot be evaluated with excluded subdirectories: %s.",
+ getOriginalPattern(), getType(), excludedSubdirectories);
+ }
+
/**
* Evaluates this {@link TargetPattern} synchronously, feeding the result to the given
* {@code callback}, and then returns an appropriate immediate {@link ListenableFuture}.
@@ -166,11 +179,12 @@
*/
public final <T, E extends Exception> ListenableFuture<Void> evalAdaptedForAsync(
TargetPatternResolver<T> resolver,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
ThreadSafeBatchCallback<T, E> callback,
Class<E> exceptionClass) {
try {
- eval(resolver, excludedSubdirectories, callback, exceptionClass);
+ eval(resolver, blacklistedSubdirectories, excludedSubdirectories, callback, exceptionClass);
return Futures.immediateFuture(null);
} catch (TargetParsingException e) {
return Futures.immediateFailedFuture(e);
@@ -194,11 +208,13 @@
*/
public <T, E extends Exception> ListenableFuture<Void> evalAsync(
TargetPatternResolver<T> resolver,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
ThreadSafeBatchCallback<T, E> callback,
Class<E> exceptionClass,
ListeningExecutorService executor) {
- return evalAdaptedForAsync(resolver, excludedSubdirectories, callback, exceptionClass);
+ return evalAdaptedForAsync(
+ resolver, blacklistedSubdirectories, excludedSubdirectories, callback, exceptionClass);
}
/**
@@ -210,19 +226,45 @@
*/
public abstract boolean containsAllTransitiveSubdirectoriesForTBD(PackageIdentifier directory);
+ /** A tristate return value for {@link #containsTBDForTBD}. */
+ public enum ContainsTBDForTBDResult {
+ /**
+ * Evaluating this TBD pattern with a directory exclusion of the other TBD pattern's directory
+ * would result in exactly the same set of targets as evaluating the subtraction of the other
+ * TBD pattern from this one.
+ */
+ DIRECTORY_EXCLUSION_WOULD_BE_EXACT,
+ /**
+ * A directory exclusion of the other TBD pattern's directory would be too broad because this
+ * TBD pattern is not "rules only" and the other one is, meaning that this TBD pattern
+ * potentially matches more targets underneath the directory in question than the other one
+ * does. Thus, a directory exclusion would incorrectly exclude non-rule targets.
+ */
+ DIRECTORY_EXCLUSION_WOULD_BE_TOO_BROAD,
+ /**
+ * None of the above. Perhaps the other pattern isn't a TBD pattern or perhaps it's not
+ * contained by this pattern.
+ */
+ OTHER,
+ }
+
/**
- * Returns {@code true} iff both this pattern and {@code containedPattern} have type
- * {@code Type.TARGETS_BELOW_DIRECTORY} and the directory in question for {@code containedPattern}
- * is underneath the directory in question for this pattern.
- *
- * <p>That is, when this method returns {@code true} it means every target matched by
- * {@code containedPattern} is also matched by this pattern.
+ * Determines how, if it all, the evaluation of this TBD pattern with a directory exclusion of the
+ * given TBD {@containedPattern}'s directory relates to the evaluation of the subtraction of the
+ * given {@link containedPattern} from this one.
*/
- public boolean containsDirectoryOfTBDForTBD(TargetPattern containedPattern) {
- return containedPattern.getType() != Type.TARGETS_BELOW_DIRECTORY
- ? false
- : containsAllTransitiveSubdirectoriesForTBD(
- containedPattern.getDirectoryForTargetsUnderDirectory());
+ public ContainsTBDForTBDResult containsTBDForTBD(TargetPattern containedPattern) {
+ if (containedPattern.getType() != Type.TARGETS_BELOW_DIRECTORY) {
+ return ContainsTBDForTBDResult.OTHER;
+ } else if (containsAllTransitiveSubdirectoriesForTBD(
+ containedPattern.getDirectoryForTargetsUnderDirectory())) {
+ return !getRulesOnly() && containedPattern.getRulesOnly()
+ ? ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_TOO_BROAD
+ : ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT;
+ } else {
+ return ContainsTBDForTBDResult.OTHER;
+ }
+
}
/**
@@ -282,12 +324,12 @@
@Override
public <T, E extends Exception> void eval(
TargetPatternResolver<T> resolver,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
BatchCallback<T, E> callback,
Class<E> exceptionClass) throws TargetParsingException, E, InterruptedException {
- Preconditions.checkArgument(excludedSubdirectories.isEmpty(),
- "Target pattern \"%s\" of type %s cannot be evaluated with excluded subdirectories: %s.",
- getOriginalPattern(), getType(), excludedSubdirectories);
+ assertBlacklistedAndExcludedSubdirectoriesEmpty(
+ blacklistedSubdirectories, excludedSubdirectories);
callback.process(resolver.getExplicitTarget(label(targetName)).getTargets());
}
@@ -336,12 +378,12 @@
@Override
public <T, E extends Exception> void eval(
TargetPatternResolver<T> resolver,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
BatchCallback<T, E> callback, Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException {
- Preconditions.checkArgument(excludedSubdirectories.isEmpty(),
- "Target pattern \"%s\" of type %s cannot be evaluated with excluded subdirectories: %s.",
- getOriginalPattern(), getType(), excludedSubdirectories);
+ assertBlacklistedAndExcludedSubdirectoriesEmpty(
+ blacklistedSubdirectories, excludedSubdirectories);
if (resolver.isPackage(PackageIdentifier.createInMainRepo(path))) {
// User has specified a package name. lookout for default target.
callback.process(resolver.getExplicitTarget(label("//" + path)).getTargets());
@@ -423,12 +465,12 @@
@Override
public <T, E extends Exception> void eval(
TargetPatternResolver<T> resolver,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
BatchCallback<T, E> callback, Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException {
- Preconditions.checkArgument(excludedSubdirectories.isEmpty(),
- "Target pattern \"%s\" of type %s cannot be evaluated with excluded subdirectories: %s.",
- getOriginalPattern(), getType(), excludedSubdirectories);
+ assertBlacklistedAndExcludedSubdirectoriesEmpty(
+ blacklistedSubdirectories, excludedSubdirectories);
if (checkWildcardConflict) {
ResolvedTargets<T> targets = getWildcardConflict(resolver);
if (targets != null) {
@@ -534,6 +576,7 @@
@Override
public <T, E extends Exception> void eval(
TargetPatternResolver<T> resolver,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
BatchCallback<T, E> callback,
Class<E> exceptionClass)
@@ -543,6 +586,7 @@
getOriginalPattern(),
directory.getPackageFragment().getPathString(),
rulesOnly,
+ blacklistedSubdirectories,
excludedSubdirectories,
callback,
exceptionClass);
@@ -551,6 +595,7 @@
@Override
public <T, E extends Exception> ListenableFuture<Void> evalAsync(
TargetPatternResolver<T> resolver,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
ThreadSafeBatchCallback<T, E> callback,
Class<E> exceptionClass,
@@ -560,6 +605,7 @@
getOriginalPattern(),
directory.getPackageFragment().getPathString(),
rulesOnly,
+ blacklistedSubdirectories,
excludedSubdirectories,
callback,
exceptionClass,
@@ -667,6 +713,10 @@
this.relativeDirectory = relativeDirectory;
}
+ public String getRelativeDirectory() {
+ return relativeDirectory;
+ }
+
/**
* Parses the given pattern, and throws an exception if the pattern is invalid.
*
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java
index 38b866b..b3e1465 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java
@@ -81,8 +81,10 @@
* @param originalPattern the original target pattern for error reporting purposes
* @param directory the directory in which to look for packages
* @param rulesOnly whether to return rules only
- * @param excludedSubdirectories a set of transitive subdirectories beneath {@code directory}
+ * @param blacklistedSubdirectories a set of transitive subdirectories beneath {@code directory}
* to ignore
+ * @param excludedSubdirectories another set of transitive subdirectories beneath
+ * {@code directory} to ignore
* @param callback the callback to receive the result, possibly in multiple batches.
* @param exceptionClass The class type of the parameterized exception.
* @throws TargetParsingException under implementation-specific failure conditions
@@ -92,6 +94,7 @@
String originalPattern,
String directory,
boolean rulesOnly,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
BatchCallback<T, E> callback,
Class<E> exceptionClass)
@@ -106,6 +109,7 @@
String originalPattern,
String directory,
boolean rulesOnly,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
ThreadSafeBatchCallback<T, E> callback,
Class<E> exceptionClass,
@@ -116,6 +120,7 @@
originalPattern,
directory,
rulesOnly,
+ blacklistedSubdirectories,
excludedSubdirectories,
callback,
exceptionClass);
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java
index 9b857d9..1a16b6c 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java
@@ -37,6 +37,8 @@
* @param eventHandler any errors emitted during package lookup and loading for {@code directory}
* and non-excluded directories beneath it will be reported here
* @param directory a {@link RootedPath} specifying the directory to search
+ * @param blacklistedSubdirectories a set of {@link PathFragment}s, all of which are beneath
+ * {@code directory}, specifying transitive subdirectories to that have been blacklisted
* @param excludedSubdirectories a set of {@link PathFragment}s, all of which are beneath {@code
* directory}, specifying transitive subdirectories to exclude
*/
@@ -44,6 +46,7 @@
ExtendedEventHandler eventHandler,
RepositoryName repository,
PathFragment directory,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories)
throws InterruptedException;
diff --git a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
index 6cc8fd1..8055158 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
@@ -685,24 +685,19 @@
return new UniquifierImpl<>(ReverseDepSkyKeyKeyExtractor.INSTANCE, DEFAULT_THREAD_COUNT);
}
- private Pair<TargetPattern, ImmutableSet<PathFragment>> getPatternAndExcludes(String pattern)
- throws TargetParsingException, InterruptedException {
- TargetPatternKey targetPatternKey =
- TargetPatternValue.key(
- pattern, TargetPatternEvaluator.DEFAULT_FILTERING_POLICY, parserPrefix);
- ImmutableSet<PathFragment> subdirectoriesToExclude =
- targetPatternKey.getAllSubdirectoriesToExclude(blacklistPatternsSupplier);
- return Pair.of(targetPatternKey.getParsedPattern(), subdirectoriesToExclude);
+ private ImmutableSet<PathFragment> getBlacklistedExcludes(TargetPatternKey targetPatternKey)
+ throws InterruptedException {
+ return targetPatternKey.getAllBlacklistedSubdirectoriesToExclude(blacklistPatternsSupplier);
}
@ThreadSafe
@Override
public QueryTaskFuture<Void> getTargetsMatchingPattern(
- final QueryExpression owner, String pattern, Callback<Target> callback) {
- // Directly evaluate the target pattern, making use of packages in the graph.
- Pair<TargetPattern, ImmutableSet<PathFragment>> patternToEvalAndSubdirectoriesToExclude;
+ QueryExpression owner, String pattern, Callback<Target> callback) {
+ TargetPatternKey targetPatternKey;
try {
- patternToEvalAndSubdirectoriesToExclude = getPatternAndExcludes(pattern);
+ targetPatternKey = TargetPatternValue.key(
+ pattern, TargetPatternEvaluator.DEFAULT_FILTERING_POLICY, parserPrefix);
} catch (TargetParsingException tpe) {
try {
reportBuildFileError(owner, tpe.getMessage());
@@ -710,12 +705,22 @@
return immediateFailedFuture(qe);
}
return immediateSuccessfulFuture(null);
+ }
+ return evalTargetPatternKey(owner, targetPatternKey, callback);
+ }
+
+ @ThreadSafe
+ public QueryTaskFuture<Void> evalTargetPatternKey(
+ QueryExpression owner, TargetPatternKey targetPatternKey, Callback<Target> callback) {
+ ImmutableSet<PathFragment> blacklistedSubdirectoriesToExclude;
+ try {
+ blacklistedSubdirectoriesToExclude = getBlacklistedExcludes(targetPatternKey);
} catch (InterruptedException ie) {
return immediateCancelledFuture();
}
- TargetPattern patternToEval = patternToEvalAndSubdirectoriesToExclude.getFirst();
- ImmutableSet<PathFragment> subdirectoriesToExclude =
- patternToEvalAndSubdirectoriesToExclude.getSecond();
+ TargetPattern patternToEval = targetPatternKey.getParsedPattern();
+ ImmutableSet<PathFragment> additionalSubdirectoriesToExclude =
+ targetPatternKey.getExcludedSubdirectories();
AsyncFunction<TargetParsingException, Void> reportBuildFileErrorAsyncFunction =
exn -> {
reportBuildFileError(owner, exn.getMessage());
@@ -723,7 +728,8 @@
};
ListenableFuture<Void> evalFuture = patternToEval.evalAsync(
resolver,
- subdirectoriesToExclude,
+ blacklistedSubdirectoriesToExclude,
+ additionalSubdirectoriesToExclude,
callback,
QueryException.class,
executor);
diff --git a/src/main/java/com/google/devtools/build/lib/query2/engine/SetExpression.java b/src/main/java/com/google/devtools/build/lib/query2/engine/SetExpression.java
index e1eadf3..57db015 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/engine/SetExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/engine/SetExpression.java
@@ -38,11 +38,11 @@
*
* <pre>expr ::= SET '(' WORD * ')'</pre>
*/
-class SetExpression extends QueryExpression {
+public class SetExpression extends QueryExpression {
private final List<TargetLiteral> words;
- SetExpression(List<TargetLiteral> words) {
+ public SetExpression(List<TargetLiteral> words) {
this.words = words;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java
index 4baa59d..e070b65 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java
@@ -19,7 +19,6 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryName;
-import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
@@ -37,6 +36,7 @@
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyKey;
import java.util.ArrayList;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
@@ -115,6 +115,7 @@
ExtendedEventHandler eventHandler,
RepositoryName repository,
PathFragment directory,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories)
throws MissingDepException, InterruptedException {
PathPackageLocator packageLocator = PrecomputedValue.PATH_PACKAGE_LOCATOR.get(env);
@@ -140,11 +141,11 @@
roots.add(repositoryValue.getPath());
}
- NestedSetBuilder<String> packageNames = NestedSetBuilder.stableOrder();
+ LinkedHashSet<PathFragment> packageNames = new LinkedHashSet<>();
for (Path root : roots) {
- PathFragment.checkAllPathsAreUnder(excludedSubdirectories, directory);
+ PathFragment.checkAllPathsAreUnder(blacklistedSubdirectories, directory);
RecursivePkgValue lookup = (RecursivePkgValue) env.getValue(RecursivePkgValue.key(
- repository, RootedPath.toRootedPath(root, directory), excludedSubdirectories));
+ repository, RootedPath.toRootedPath(root, directory), blacklistedSubdirectories));
if (lookup == null) {
// Typically a null value from Environment.getValue(k) means that either the key k is
// missing a dependency or an exception was thrown during evaluation of k. Here, if this
@@ -156,11 +157,19 @@
throw new MissingDepException();
}
- packageNames.addTransitive(lookup.getPackages());
+ for (String packageName : lookup.getPackages()) {
+ // TODO(bazel-team): Make RecursivePkgValue return NestedSet<PathFragment> so this transform
+ // is unnecessary.
+ PathFragment packageNamePathFragment = PathFragment.create(packageName);
+ if (!Iterables.any(
+ excludedSubdirectories,
+ excludedSubdirectory -> packageNamePathFragment.startsWith(excludedSubdirectory))) {
+ packageNames.add(packageNamePathFragment);
+ }
+ }
}
- // TODO(bazel-team): Make RecursivePkgValue return NestedSet<PathFragment> so this transform is
- // unnecessary.
- return Iterables.transform(packageNames.build(), PathFragment::create);
+
+ return packageNames;
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java
index b5ba1da..0cc2097 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java
@@ -166,10 +166,16 @@
ExtendedEventHandler eventHandler,
RepositoryName repository,
PathFragment directory,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories)
throws InterruptedException {
+ PathFragment.checkAllPathsAreUnder(blacklistedSubdirectories, directory);
PathFragment.checkAllPathsAreUnder(excludedSubdirectories, directory);
+ if (excludedSubdirectories.contains(directory)) {
+ return ImmutableList.of();
+ }
+
// Check that this package is covered by at least one of our universe patterns.
boolean inUniverse = false;
for (TargetPatternKey patternKey : universeTargetPatternKeys) {
@@ -206,7 +212,8 @@
ImmutableList.Builder<PathFragment> builder = ImmutableList.builder();
for (Path root : roots) {
RootedPath rootedDir = RootedPath.toRootedPath(root, directory);
- TraversalInfo info = new TraversalInfo(rootedDir, excludedSubdirectories);
+ TraversalInfo info =
+ new TraversalInfo(rootedDir, blacklistedSubdirectories, excludedSubdirectories);
collectPackagesUnder(eventHandler, repository, ImmutableSet.of(info), builder);
}
return builder.build();
@@ -225,7 +232,7 @@
@Override
public SkyKey apply(TraversalInfo traversalInfo) {
return CollectPackagesUnderDirectoryValue.key(
- repository, traversalInfo.rootedDir, traversalInfo.excludedSubdirectories);
+ repository, traversalInfo.rootedDir, traversalInfo.blacklistedSubdirectories);
}
});
Map<SkyKey, SkyValue> values = graph.getSuccessfulValues(traversalToKeyMap.values());
@@ -251,11 +258,19 @@
for (RootedPath subdirectory : subdirectoryTransitivelyContainsPackages.keySet()) {
if (subdirectoryTransitivelyContainsPackages.get(subdirectory)) {
PathFragment subdirectoryRelativePath = subdirectory.getRelativePath();
+ ImmutableSet<PathFragment> blacklistedSubdirectoriesBeneathThisSubdirectory =
+ PathFragment.filterPathsStartingWith(
+ info.blacklistedSubdirectories, subdirectoryRelativePath);
ImmutableSet<PathFragment> excludedSubdirectoriesBeneathThisSubdirectory =
PathFragment.filterPathsStartingWith(
info.excludedSubdirectories, subdirectoryRelativePath);
- subdirTraversalBuilder.add(
- new TraversalInfo(subdirectory, excludedSubdirectoriesBeneathThisSubdirectory));
+ if (!excludedSubdirectoriesBeneathThisSubdirectory.contains(subdirectoryRelativePath)) {
+ subdirTraversalBuilder.add(
+ new TraversalInfo(
+ subdirectory,
+ blacklistedSubdirectoriesBeneathThisSubdirectory,
+ excludedSubdirectoriesBeneathThisSubdirectory));
+ }
}
}
}
@@ -275,16 +290,28 @@
private static final class TraversalInfo {
private final RootedPath rootedDir;
+ // Set of blacklisted directories. The graph is assumed to be prepopulated with
+ // CollectPackagesUnderDirectoryValue nodes whose keys have blacklisted packages embedded in
+ // them. Therefore, we need to be careful to request and use the same sort of keys here in our
+ // traversal.
+ private final ImmutableSet<PathFragment> blacklistedSubdirectories;
+ // Set of directories, targets under which should be excluded from the traversal results.
+ // Excluded directory information isn't part of the graph keys in the prepopulated graph, so we
+ // need to perform the filtering ourselves.
private final ImmutableSet<PathFragment> excludedSubdirectories;
- private TraversalInfo(RootedPath rootedDir, ImmutableSet<PathFragment> excludedSubdirectories) {
+ private TraversalInfo(
+ RootedPath rootedDir,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
+ ImmutableSet<PathFragment> excludedSubdirectories) {
this.rootedDir = rootedDir;
+ this.blacklistedSubdirectories = blacklistedSubdirectories;
this.excludedSubdirectories = excludedSubdirectories;
}
@Override
public int hashCode() {
- return Objects.hashCode(rootedDir, excludedSubdirectories);
+ return Objects.hashCode(rootedDir, blacklistedSubdirectories, excludedSubdirectories);
}
@Override
@@ -295,6 +322,7 @@
if (obj instanceof TraversalInfo) {
TraversalInfo otherTraversal = (TraversalInfo) obj;
return Objects.equal(rootedDir, otherTraversal.rootedDir)
+ && Objects.equal(blacklistedSubdirectories, otherTraversal.blacklistedSubdirectories)
&& Objects.equal(excludedSubdirectories, otherTraversal.excludedSubdirectories);
}
return false;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
index a4a3b1b..b2ce5b6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
@@ -84,15 +84,22 @@
if (blacklist == null) {
return null;
}
- ImmutableSet<PathFragment> subdirectoriesToExclude =
+ // This SkyFunction is used to load the universe, so we want both the blacklisted directories
+ // from the global blacklist and the excluded directories from the TargetPatternKey itself to be
+ // embedded in the SkyKeys created and used by the DepsOfPatternPreparer. The
+ // DepsOfPatternPreparer ignores excludedSubdirectories and embeds blacklistedSubdirectories in
+ // the SkyKeys it creates and uses.
+ ImmutableSet<PathFragment> blacklistedSubdirectories =
patternKey.getAllSubdirectoriesToExclude(blacklist.getPatterns());
+ ImmutableSet<PathFragment> excludedSubdirectories = ImmutableSet.of();
DepsOfPatternPreparer preparer = new DepsOfPatternPreparer(env, pkgPath.get());
try {
parsedPattern.eval(
preparer,
- subdirectoriesToExclude,
+ blacklistedSubdirectories,
+ excludedSubdirectories,
NullCallback.<Void>instance(),
RuntimeException.class);
} catch (TargetParsingException e) {
@@ -227,10 +234,12 @@
String originalPattern,
String directory,
boolean rulesOnly,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
BatchCallback<Void, E> callback,
Class<E> exceptionClass)
throws TargetParsingException, E, InterruptedException {
+ Preconditions.checkArgument(excludedSubdirectories.isEmpty(), excludedSubdirectories);
FilteringPolicy policy =
rulesOnly ? FilteringPolicies.RULES_ONLY : FilteringPolicies.NO_FILTER;
PathFragment pathFragment = TargetPatternResolverUtil.getPathFragment(directory);
@@ -257,9 +266,9 @@
env.getValues(
ImmutableList.of(
PrepareDepsOfTargetsUnderDirectoryValue.key(
- repository, rootedPath, excludedSubdirectories, policy),
+ repository, rootedPath, blacklistedSubdirectories, policy),
CollectPackagesUnderDirectoryValue.key(
- repository, rootedPath, excludedSubdirectories)));
+ repository, rootedPath, blacklistedSubdirectories)));
if (env.valuesMissing()) {
throw new MissingDepException();
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java
index f54604d..1dc932d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java
@@ -14,15 +14,12 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
-import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.cmdline.TargetPattern.Type;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.pkgcache.FilteringPolicies;
import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternSkyKeyOrException;
-import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.LegacySkyKey;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
@@ -58,123 +55,97 @@
return 42;
}
+
/**
- * Returns an iterable of {@link PrepareDepsOfPatternSkyKeyOrException}, with {@link
- * TargetPatternKey} arguments. Negative target patterns of type other than {@link
- * Type#TARGETS_BELOW_DIRECTORY} are not permitted. If a provided pattern fails to parse or is
- * negative but not a {@link Type#TARGETS_BELOW_DIRECTORY}, an element in the returned iterable
- * will throw when its {@link PrepareDepsOfPatternSkyKeyOrException#getSkyKey} method is called
- * and will return the failing pattern when its {@link
- * PrepareDepsOfPatternSkyKeyOrException#getOriginalPattern} method is called.
+ * Returns a {@link PrepareDepsOfPatternSkyKeysAndExceptions}, containing
+ * {@link PrepareDepsOfPatternSkyKeyValue} and {@link PrepareDepsOfPatternSkyKeyException}
+ * instances that have {@link TargetPatternKey} arguments. Negative target patterns of type other
+ * than {@link Type#TARGETS_BELOW_DIRECTORY} are not permitted. If a provided pattern fails to
+ * parse or is negative but not a {@link Type#TARGETS_BELOW_DIRECTORY}, there will be a
+ * corresponding {@link PrepareDepsOfPatternSkyKeyException} in the iterable returned by
+ * {@link PrepareDepsOfPatternSkyKeysAndExceptions#getExceptions} whose
+ * {@link PrepareDepsOfPatternSkyKeyException#getException} and
+ * {@link PrepareDepsOfPatternSkyKeyException#getOriginalPattern} methods return the
+ * {@link TargetParsingException} and original pattern, respectively.
*
- * <p>There may be fewer returned elements than patterns provided as input. This function will
- * combine negative {@link Type#TARGETS_BELOW_DIRECTORY} patterns with preceding patterns to
- * return an iterable of SkyKeys that avoids loading excluded directories during evaluation.
+ * <p>There may be fewer returned elements in
+ * {@link PrepareDepsOfPatternSkyKeysAndExceptions#getValues} than patterns provided as input.
+ * This function will combine negative {@link Type#TARGETS_BELOW_DIRECTORY} patterns with
+ * preceding patterns to return an iterable of SkyKeys that avoids loading excluded directories
+ * during evaluation.
*
* @param patterns The list of patterns, e.g. [//foo/..., -//foo/biz/...]. If a pattern's first
* character is "-", it is treated as a negative pattern.
* @param offset The offset to apply to relative target patterns.
*/
@ThreadSafe
- public static Iterable<PrepareDepsOfPatternSkyKeyOrException> keys(List<String> patterns,
- String offset) {
- List<TargetPatternSkyKeyOrException> keysMaybe =
- ImmutableList.copyOf(TargetPatternValue.keys(patterns, FilteringPolicies.NO_FILTER,
- offset));
-
+ public static PrepareDepsOfPatternSkyKeysAndExceptions keys(
+ List<String> patterns, String offset) {
+ ImmutableList.Builder<PrepareDepsOfPatternSkyKeyValue> resultValuesBuilder =
+ ImmutableList.builder();
+ ImmutableList.Builder<PrepareDepsOfPatternSkyKeyException> resultExceptionsBuilder =
+ ImmutableList.builder();
+ Iterable<TargetPatternSkyKeyOrException> keysMaybe =
+ TargetPatternValue.keys(patterns, FilteringPolicies.NO_FILTER, offset);
+ ImmutableList.Builder<TargetPatternKey> targetPatternKeysBuilder = ImmutableList.builder();
+ for (TargetPatternSkyKeyOrException keyMaybe : keysMaybe) {
+ try {
+ SkyKey key = keyMaybe.getSkyKey();
+ targetPatternKeysBuilder.add((TargetPatternKey) key.argument());
+ } catch (TargetParsingException e) {
+ resultExceptionsBuilder.add(
+ new PrepareDepsOfPatternSkyKeyException(e, keyMaybe.getOriginalPattern()));
+ }
+ }
// This code path is evaluated only for query universe preloading, and the quadratic cost of
// the code below (i.e. for each pattern, consider each later pattern as a candidate for
// subdirectory exclusion) is only acceptable because all the use cases for query universe
// preloading involve short (<10 items) pattern sequences.
- ImmutableList.Builder<PrepareDepsOfPatternSkyKeyOrException> builder = ImmutableList.builder();
- for (int i = 0; i < keysMaybe.size(); i++) {
- TargetPatternSkyKeyOrException keyMaybe = keysMaybe.get(i);
- TargetPatternKey targetPatternKey;
- try {
- targetPatternKey = keyMaybe.getSkyKey();
- } catch (TargetParsingException e) {
- // keyMaybe.getSkyKey() may throw TargetParsingException if its corresponding pattern
- // failed to parse. If so, wrap the exception and return it, so that our caller can
- // deal with it.
- targetPatternKey = null;
- builder.add(new PrepareDepsOfPatternSkyKeyException(e, keyMaybe.getOriginalPattern()));
- }
- if (targetPatternKey != null) {
- if (targetPatternKey.isNegative()) {
- if (!targetPatternKey.getParsedPattern().getType().equals(Type.TARGETS_BELOW_DIRECTORY)) {
- builder.add(
- new PrepareDepsOfPatternSkyKeyException(
- new TargetParsingException(
- "Negative target patterns of types other than \"targets below directory\""
- + " are not permitted."), targetPatternKey.toString()));
- }
- // Otherwise it's a negative TBD pattern which was combined with previous patterns as an
- // excluded directory. These can be skipped because there's no PrepareDepsOfPattern work
- // to be done for them.
- } else {
- builder.add(new PrepareDepsOfPatternSkyKeyValue(setExcludedDirectories(targetPatternKey,
- excludedDirectoriesBeneath(targetPatternKey, i, keysMaybe))));
- }
+ Iterable<TargetPatternKey> combinedTargetPatternKeys =
+ TargetPatternValue.combineNegativeTargetsBelowDirectoryPatterns(
+ targetPatternKeysBuilder.build());
+ for (TargetPatternKey targetPatternKey : combinedTargetPatternKeys) {
+ if (targetPatternKey.isNegative()
+ && !targetPatternKey.getParsedPattern().getType().equals(Type.TARGETS_BELOW_DIRECTORY)) {
+ resultExceptionsBuilder.add(
+ new PrepareDepsOfPatternSkyKeyException(
+ new TargetParsingException(
+ "Negative target patterns of types other than \"targets below directory\""
+ + " are not permitted."), targetPatternKey.toString()));
+ } else {
+ resultValuesBuilder.add(new PrepareDepsOfPatternSkyKeyValue(targetPatternKey));
}
}
- return builder.build();
- }
-
- private static TargetPatternKey setExcludedDirectories(
- TargetPatternKey original, ImmutableSet<PathFragment> excludedSubdirectories) {
- return new TargetPatternKey(original.getParsedPattern(), original.getPolicy(),
- original.isNegative(), original.getOffset(), excludedSubdirectories);
- }
-
- private static ImmutableSet<PathFragment> excludedDirectoriesBeneath(
- TargetPatternKey targetPatternKey,
- int position,
- List<TargetPatternSkyKeyOrException> keysMaybe) {
- ImmutableSet.Builder<PathFragment> excludedDirectoriesBuilder = ImmutableSet.builder();
- for (int j = position + 1; j < keysMaybe.size(); j++) {
- TargetPatternSkyKeyOrException laterPatternMaybe = keysMaybe.get(j);
- TargetPatternKey laterTargetPatternKey;
- try {
- laterTargetPatternKey = laterPatternMaybe.getSkyKey();
- } catch (TargetParsingException ignored) {
- laterTargetPatternKey = null;
- }
- if (laterTargetPatternKey != null) {
- TargetPattern laterParsedPattern = laterTargetPatternKey.getParsedPattern();
- if (laterTargetPatternKey.isNegative()
- && laterParsedPattern.getType() == Type.TARGETS_BELOW_DIRECTORY
- && targetPatternKey.getParsedPattern().containsDirectoryOfTBDForTBD(
- laterParsedPattern)) {
- excludedDirectoriesBuilder.add(
- laterParsedPattern.getDirectoryForTargetsUnderDirectory().getPackageFragment());
- }
- }
- }
- return excludedDirectoriesBuilder.build();
+ return new PrepareDepsOfPatternSkyKeysAndExceptions(
+ resultValuesBuilder.build(), resultExceptionsBuilder.build());
}
/**
- * Wrapper for a prepare deps of pattern {@link SkyKey} or the {@link TargetParsingException}
- * thrown when trying to create it.
+ * A pair of {@link Iterable<PrepareDepsOfPatternSkyKeyValue>} and
+ * {@link Iterable<PrepareDepsOfPatternSkyKeyException>}.
*/
- public interface PrepareDepsOfPatternSkyKeyOrException {
+ public static class PrepareDepsOfPatternSkyKeysAndExceptions {
+ private final Iterable<PrepareDepsOfPatternSkyKeyValue> values;
+ private final Iterable<PrepareDepsOfPatternSkyKeyException> exceptions;
- /**
- * Returns the stored {@link SkyKey} or throws {@link TargetParsingException} if one was thrown
- * when creating the key.
- */
- SkyKey getSkyKey() throws TargetParsingException;
+ public PrepareDepsOfPatternSkyKeysAndExceptions(
+ Iterable<PrepareDepsOfPatternSkyKeyValue> values,
+ Iterable<PrepareDepsOfPatternSkyKeyException> exceptions) {
+ this.values = values;
+ this.exceptions = exceptions;
+ }
- /**
- * Returns the pattern that resulted in the stored {@link SkyKey} or {@link
- * TargetParsingException}.
- */
- String getOriginalPattern();
+ public Iterable<PrepareDepsOfPatternSkyKeyValue> getValues() {
+ return values;
+ }
+
+ public Iterable<PrepareDepsOfPatternSkyKeyException> getExceptions() {
+ return exceptions;
+ }
}
-
- private static class PrepareDepsOfPatternSkyKeyException implements
- PrepareDepsOfPatternSkyKeyOrException {
+ /** Represents a {@link TargetParsingException} when parsing a target pattern string. */
+ public static class PrepareDepsOfPatternSkyKeyException {
private final TargetParsingException exception;
private final String originalPattern;
@@ -185,19 +156,19 @@
this.originalPattern = originalPattern;
}
- @Override
- public SkyKey getSkyKey() throws TargetParsingException {
- throw exception;
+ public TargetParsingException getException() {
+ return exception;
}
- @Override
public String getOriginalPattern() {
return originalPattern;
}
}
- private static class PrepareDepsOfPatternSkyKeyValue implements
- PrepareDepsOfPatternSkyKeyOrException {
+ /**
+ * Represents the successful parsing of a target pattern string into a {@link TargetPatternKey}.
+ */
+ public static class PrepareDepsOfPatternSkyKeyValue {
private final TargetPatternKey targetPatternKey;
@@ -205,12 +176,10 @@
this.targetPatternKey = targetPatternKey;
}
- @Override
- public SkyKey getSkyKey() throws TargetParsingException {
+ public SkyKey getSkyKey() {
return LegacySkyKey.create(SkyFunctions.PREPARE_DEPS_OF_PATTERN, targetPatternKey);
}
- @Override
public String getOriginalPattern() {
return targetPatternKey.getPattern();
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java
index 422462e..3e48fab 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java
@@ -20,7 +20,9 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.pkgcache.ParsingFailedEvent;
-import com.google.devtools.build.lib.skyframe.PrepareDepsOfPatternValue.PrepareDepsOfPatternSkyKeyOrException;
+import com.google.devtools.build.lib.skyframe.PrepareDepsOfPatternValue.PrepareDepsOfPatternSkyKeyException;
+import com.google.devtools.build.lib.skyframe.PrepareDepsOfPatternValue.PrepareDepsOfPatternSkyKeyValue;
+import com.google.devtools.build.lib.skyframe.PrepareDepsOfPatternValue.PrepareDepsOfPatternSkyKeysAndExceptions;
import com.google.devtools.build.lib.skyframe.PrepareDepsOfPatternsValue.TargetPatternSequence;
import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
import com.google.devtools.build.lib.util.Preconditions;
@@ -39,22 +41,24 @@
public static ImmutableList<SkyKey> getSkyKeys(SkyKey skyKey, ExtendedEventHandler eventHandler) {
TargetPatternSequence targetPatternSequence = (TargetPatternSequence) skyKey.argument();
- Iterable<PrepareDepsOfPatternSkyKeyOrException> keysMaybe =
+ PrepareDepsOfPatternSkyKeysAndExceptions prepareDepsOfPatternSkyKeysAndExceptions =
PrepareDepsOfPatternValue.keys(targetPatternSequence.getPatterns(),
targetPatternSequence.getOffset());
ImmutableList.Builder<SkyKey> skyKeyBuilder = ImmutableList.builder();
- for (PrepareDepsOfPatternSkyKeyOrException skyKeyOrException : keysMaybe) {
- try {
- skyKeyBuilder.add(skyKeyOrException.getSkyKey());
- } catch (TargetParsingException e) {
- // We post an event here rather than in handleTargetParsingException because the
- // TargetPatternFunction already posts an event unless the pattern cannot be parsed, in
- // which case the caller (i.e., us) needs to post an event.
- eventHandler.post(
- new ParsingFailedEvent(skyKeyOrException.getOriginalPattern(), e.getMessage()));
- handleTargetParsingException(eventHandler, skyKeyOrException.getOriginalPattern(), e);
- }
+ for (PrepareDepsOfPatternSkyKeyValue skyKeyValue
+ : prepareDepsOfPatternSkyKeysAndExceptions.getValues()) {
+ skyKeyBuilder.add(skyKeyValue.getSkyKey());
+ }
+ for (PrepareDepsOfPatternSkyKeyException skyKeyException
+ : prepareDepsOfPatternSkyKeysAndExceptions.getExceptions()) {
+ TargetParsingException e = skyKeyException.getException();
+ // We post an event here rather than in handleTargetParsingException because the
+ // TargetPatternFunction already posts an event unless the pattern cannot be parsed, in
+ // which case the caller (i.e., us) needs to post an event.
+ eventHandler.post(
+ new ParsingFailedEvent(skyKeyException.getOriginalPattern(), e.getMessage()));
+ handleTargetParsingException(eventHandler, skyKeyException.getOriginalPattern(), e);
}
return skyKeyBuilder.build();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
index 2fb7486..4ba2122 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
@@ -216,6 +216,7 @@
final String originalPattern,
String directory,
boolean rulesOnly,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
BatchCallback<Target, E> callback,
Class<E> exceptionClass)
@@ -226,6 +227,7 @@
originalPattern,
directory,
rulesOnly,
+ blacklistedSubdirectories,
excludedSubdirectories,
new SynchronizedBatchCallback<Target, E>(callback),
MoreExecutors.newDirectExecutorService()).get();
@@ -242,6 +244,7 @@
String originalPattern,
String directory,
boolean rulesOnly,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
ThreadSafeBatchCallback<Target, E> callback,
Class<E> exceptionClass,
@@ -251,6 +254,7 @@
originalPattern,
directory,
rulesOnly,
+ blacklistedSubdirectories,
excludedSubdirectories,
callback,
executor);
@@ -261,6 +265,7 @@
final String originalPattern,
String directory,
boolean rulesOnly,
+ ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories,
final ThreadSafeBatchCallback<Target, E> callback,
ListeningExecutorService executor) {
@@ -273,7 +278,11 @@
pathFragment = TargetPatternResolverUtil.getPathFragment(directory);
packagesUnderDirectory =
recursivePackageProvider.getPackagesUnderDirectory(
- eventHandler, repository, pathFragment, excludedSubdirectories);
+ eventHandler,
+ repository,
+ pathFragment,
+ blacklistedSubdirectories,
+ excludedSubdirectories);
} catch (TargetParsingException e) {
return Futures.immediateFailedFuture(e);
} catch (InterruptedException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
index 62b537c..197e0f7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
@@ -69,7 +69,12 @@
Iterables.addAll(results, partialResult);
}
};
- parsedPattern.eval(resolver, excludedSubdirectories, callback, RuntimeException.class);
+ parsedPattern.eval(
+ resolver,
+ /*blacklistedSubdirectories=*/ ImmutableSet.of(),
+ excludedSubdirectories,
+ callback,
+ RuntimeException.class);
resolvedTargets = ResolvedTargets.<Target>builder().addAll(results).build();
} catch (TargetParsingException e) {
env.getListener().post(new ParsingFailedEvent(patternKey.getPattern(), e.getMessage()));
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java
index f36a439..f28cdfd 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java
@@ -23,6 +23,7 @@
import com.google.devtools.build.lib.cmdline.ResolvedTargets.Builder;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPattern;
+import com.google.devtools.build.lib.cmdline.TargetPattern.ContainsTBDForTBDResult;
import com.google.devtools.build.lib.cmdline.TargetPattern.Type;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
@@ -39,8 +40,10 @@
import java.io.ObjectOutputStream;
import java.io.Serializable;
import java.util.ArrayList;
+import java.util.HashSet;
import java.util.List;
import java.util.Objects;
+import java.util.Optional;
/**
* A value referring to a computed set of resolved targets. This is used for the results of target
@@ -152,6 +155,88 @@
return builder.build();
}
+ @ThreadSafe
+ public static ImmutableList<TargetPatternKey> combineNegativeTargetsBelowDirectoryPatterns(
+ List<TargetPatternKey> keys) {
+ ImmutableList.Builder<TargetPatternKey> builder = ImmutableList.builder();
+ HashSet<Integer> indicesOfNegativePatternsThatNeedToBeIncluded = new HashSet<>();
+ for (int i = 0; i < keys.size(); i++) {
+ TargetPatternKey targetPatternKey = keys.get(i);
+ if (targetPatternKey.isNegative()) {
+ if (!targetPatternKey.getParsedPattern().getType().equals(Type.TARGETS_BELOW_DIRECTORY)
+ || indicesOfNegativePatternsThatNeedToBeIncluded.contains(i)) {
+ builder.add(targetPatternKey);
+ }
+ // Otherwise it's a negative TBD pattern which was combined with previous patterns as an
+ // excluded directory.
+ } else {
+ TargetPatternKeyWithExclusionsResult result =
+ computeTargetPatternKeyWithExclusions(targetPatternKey, i, keys);
+ result.targetPatternKeyMaybe.ifPresent(builder::add);
+ indicesOfNegativePatternsThatNeedToBeIncluded.addAll(
+ result.indicesOfNegativePatternsThatNeedToBeIncluded);
+ }
+ }
+ return builder.build();
+ }
+
+ private static TargetPatternKey setExcludedDirectories(
+ TargetPatternKey original, ImmutableSet<PathFragment> excludedSubdirectories) {
+ return new TargetPatternKey(original.getParsedPattern(), original.getPolicy(),
+ original.isNegative(), original.getOffset(), excludedSubdirectories);
+ }
+
+ private static class TargetPatternKeyWithExclusionsResult {
+ private final Optional<TargetPatternKey> targetPatternKeyMaybe;
+ private final ImmutableList<Integer> indicesOfNegativePatternsThatNeedToBeIncluded;
+
+ private TargetPatternKeyWithExclusionsResult(
+ Optional<TargetPatternKey> targetPatternKeyMaybe,
+ ImmutableList<Integer> indicesOfNegativePatternsThatNeedToBeIncluded) {
+ this.targetPatternKeyMaybe = targetPatternKeyMaybe;
+ this.indicesOfNegativePatternsThatNeedToBeIncluded =
+ indicesOfNegativePatternsThatNeedToBeIncluded;
+ }
+ }
+
+ private static TargetPatternKeyWithExclusionsResult computeTargetPatternKeyWithExclusions(
+ TargetPatternKey targetPatternKey,
+ int position,
+ List<TargetPatternKey> keys) {
+ TargetPattern targetPattern = targetPatternKey.getParsedPattern();
+ ImmutableSet.Builder<PathFragment> excludedDirectoriesBuilder = ImmutableSet.builder();
+ ImmutableList.Builder<Integer> indicesOfNegativePatternsThatNeedToBeIncludedBuilder =
+ ImmutableList.builder();
+ for (int j = position + 1; j < keys.size(); j++) {
+ TargetPatternKey laterTargetPatternKey = keys.get(j);
+ TargetPattern laterParsedPattern = laterTargetPatternKey.getParsedPattern();
+ if (laterTargetPatternKey.isNegative()
+ && laterParsedPattern.getType() == Type.TARGETS_BELOW_DIRECTORY) {
+ if (laterParsedPattern.containsTBDForTBD(targetPattern)
+ == ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT) {
+ return new TargetPatternKeyWithExclusionsResult(Optional.empty(), ImmutableList.of());
+ } else {
+ switch (targetPattern.containsTBDForTBD(laterParsedPattern)) {
+ case DIRECTORY_EXCLUSION_WOULD_BE_EXACT:
+ excludedDirectoriesBuilder.add(
+ laterParsedPattern.getDirectoryForTargetsUnderDirectory().getPackageFragment());
+ break;
+ case DIRECTORY_EXCLUSION_WOULD_BE_TOO_BROAD:
+ indicesOfNegativePatternsThatNeedToBeIncludedBuilder.add(j);
+ break;
+ case OTHER:
+ default:
+ // Nothing to do with this pattern.
+ }
+
+ }
+ }
+ }
+ return new TargetPatternKeyWithExclusionsResult(
+ Optional.of(setExcludedDirectories(targetPatternKey, excludedDirectoriesBuilder.build())),
+ indicesOfNegativePatternsThatNeedToBeIncludedBuilder.build());
+ }
+
public ResolvedTargets<Label> getTargets() {
return targets;
}
@@ -214,15 +299,17 @@
ImmutableSet<PathFragment> getAllSubdirectoriesToExclude(
Iterable<PathFragment> blacklistedPackagePrefixes) throws InterruptedException {
- return getAllSubdirectoriesToExclude(
- new InterruptibleSupplier.Instance<>(blacklistedPackagePrefixes));
- }
-
- public ImmutableSet<PathFragment> getAllSubdirectoriesToExclude(
- InterruptibleSupplier<? extends Iterable<PathFragment>> blacklistedPackagePrefixes)
- throws InterruptedException {
ImmutableSet.Builder<PathFragment> excludedPathsBuilder = ImmutableSet.builder();
excludedPathsBuilder.addAll(getExcludedSubdirectories());
+ excludedPathsBuilder.addAll(getAllBlacklistedSubdirectoriesToExclude(
+ new InterruptibleSupplier.Instance<>(blacklistedPackagePrefixes)));
+ return excludedPathsBuilder.build();
+ }
+
+ public ImmutableSet<PathFragment> getAllBlacklistedSubdirectoriesToExclude(
+ InterruptibleSupplier<? extends Iterable<PathFragment>> blacklistedPackagePrefixes)
+ throws InterruptedException {
+ ImmutableSet.Builder<PathFragment> blacklistedPathsBuilder = ImmutableSet.builder();
if (parsedPattern.getType() == Type.TARGETS_BELOW_DIRECTORY) {
for (PathFragment blacklistedPackagePrefix : blacklistedPackagePrefixes.get()) {
PackageIdentifier pkgIdForBlacklistedDirectorPrefix = PackageIdentifier.create(
@@ -230,11 +317,11 @@
blacklistedPackagePrefix);
if (parsedPattern.containsAllTransitiveSubdirectoriesForTBD(
pkgIdForBlacklistedDirectorPrefix)) {
- excludedPathsBuilder.add(blacklistedPackagePrefix);
+ blacklistedPathsBuilder.add(blacklistedPackagePrefix);
}
}
}
- return excludedPathsBuilder.build();
+ return blacklistedPathsBuilder.build();
}
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java
index 1804abb..bcc9e78 100644
--- a/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java
+++ b/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
+import com.google.devtools.build.lib.cmdline.TargetPattern.ContainsTBDForTBDResult;
import com.google.devtools.build.lib.cmdline.TargetPattern.Type;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -86,15 +87,48 @@
}
@Test
+ public void testTargetsBelowDirectoryContainsColonStar() throws Exception {
+ // Given an outer pattern '//foo/...', that matches rules only,
+ TargetPattern outerPattern = parseAsExpectedType("//foo/...", Type.TARGETS_BELOW_DIRECTORY);
+ // And a nested inner pattern '//foo/bar/...:*', that matches all targets,
+ TargetPattern innerPattern =
+ parseAsExpectedType("//foo/bar/...:*", Type.TARGETS_BELOW_DIRECTORY);
+ // Then a directory exclusion would exactly describe the subtraction of the inner pattern from
+ // the outer pattern,
+ assertThat(outerPattern.containsTBDForTBD(innerPattern))
+ .isEqualTo(ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT);
+ // And the inner pattern does not contain the outer pattern.
+ assertThat(innerPattern.containsTBDForTBD(outerPattern))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
+ }
+
+ @Test
+ public void testTargetsBelowDirectoryColonStarContains() throws Exception {
+ // Given an outer pattern '//foo/...:*', that matches all targets,
+ TargetPattern outerPattern = parseAsExpectedType("//foo/...:*", Type.TARGETS_BELOW_DIRECTORY);
+ // And a nested inner pattern '//foo/bar/...', that matches rules only,
+ TargetPattern innerPattern =
+ parseAsExpectedType("//foo/bar/...", Type.TARGETS_BELOW_DIRECTORY);
+ // Then a directory exclusion would be too broad,
+ assertThat(outerPattern.containsTBDForTBD(innerPattern))
+ .isEqualTo(ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_TOO_BROAD);
+ // And the inner pattern does not contain the outer pattern.
+ assertThat(innerPattern.containsTBDForTBD(outerPattern))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
+ }
+
+ @Test
public void testTargetsBelowDirectoryContainsNestedPatterns() throws Exception {
// Given an outer pattern '//foo/...',
TargetPattern outerPattern = parseAsExpectedType("//foo/...", Type.TARGETS_BELOW_DIRECTORY);
// And a nested inner pattern '//foo/bar/...',
TargetPattern innerPattern = parseAsExpectedType("//foo/bar/...", Type.TARGETS_BELOW_DIRECTORY);
- // Then the outer pattern contains the inner pattern,,
- assertThat(outerPattern.containsDirectoryOfTBDForTBD(innerPattern)).isTrue();
+ // Then the outer pattern contains the inner pattern,
+ assertThat(outerPattern.containsTBDForTBD(innerPattern))
+ .isEqualTo(ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT);
// And the inner pattern does not contain the outer pattern.
- assertThat(innerPattern.containsDirectoryOfTBDForTBD(outerPattern)).isFalse();
+ assertThat(innerPattern.containsTBDForTBD(outerPattern))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
}
@Test
@@ -104,8 +138,8 @@
// And a pattern '//bar/...',
TargetPattern patternBar = parseAsExpectedType("//bar/...", Type.TARGETS_BELOW_DIRECTORY);
// Then neither pattern contains the other.
- assertThat(patternFoo.containsDirectoryOfTBDForTBD(patternBar)).isFalse();
- assertThat(patternBar.containsDirectoryOfTBDForTBD(patternFoo)).isFalse();
+ assertThat(patternFoo.containsTBDForTBD(patternBar)).isEqualTo(ContainsTBDForTBDResult.OTHER);
+ assertThat(patternBar.containsTBDForTBD(patternFoo)).isEqualTo(ContainsTBDForTBDResult.OTHER);
}
@Test
@@ -120,15 +154,21 @@
TargetPattern targetsInPackagePattern = parseAsExpectedType("foo:all", Type.TARGETS_IN_PACKAGE);
// Then the non-TargetsBelowDirectory patterns do not contain tbdFoo.
- assertThat(pathAsTargetPattern.containsDirectoryOfTBDForTBD(tbdFoo)).isFalse();
+ assertThat(pathAsTargetPattern.containsTBDForTBD(tbdFoo))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
// And are not considered to be a contained directory of the TargetsBelowDirectory pattern.
- assertThat(tbdFoo.containsDirectoryOfTBDForTBD(pathAsTargetPattern)).isFalse();
+ assertThat(tbdFoo.containsTBDForTBD(pathAsTargetPattern))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
- assertThat(singleTargetPattern.containsDirectoryOfTBDForTBD(tbdFoo)).isFalse();
- assertThat(tbdFoo.containsDirectoryOfTBDForTBD(singleTargetPattern)).isFalse();
+ assertThat(singleTargetPattern.containsTBDForTBD(tbdFoo))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
+ assertThat(tbdFoo.containsTBDForTBD(singleTargetPattern))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
- assertThat(targetsInPackagePattern.containsDirectoryOfTBDForTBD(tbdFoo)).isFalse();
- assertThat(tbdFoo.containsDirectoryOfTBDForTBD(targetsInPackagePattern)).isFalse();
+ assertThat(targetsInPackagePattern.containsTBDForTBD(tbdFoo))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
+ assertThat(tbdFoo.containsTBDForTBD(targetsInPackagePattern))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
}
@Test
@@ -146,10 +186,14 @@
parseAsExpectedType("food:all", Type.TARGETS_IN_PACKAGE);
// Then the non-TargetsBelowDirectory patterns are not contained by tbdFoo.
- assertThat(tbdFoo.containsDirectoryOfTBDForTBD(targetsBelowDirectoryPattern)).isFalse();
- assertThat(tbdFoo.containsDirectoryOfTBDForTBD(pathAsTargetPattern)).isFalse();
- assertThat(tbdFoo.containsDirectoryOfTBDForTBD(singleTargetPattern)).isFalse();
- assertThat(tbdFoo.containsDirectoryOfTBDForTBD(targetsInPackagePattern)).isFalse();
+ assertThat(tbdFoo.containsTBDForTBD(targetsBelowDirectoryPattern))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
+ assertThat(tbdFoo.containsTBDForTBD(pathAsTargetPattern))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
+ assertThat(tbdFoo.containsTBDForTBD(singleTargetPattern))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
+ assertThat(tbdFoo.containsTBDForTBD(targetsInPackagePattern))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
}
@Test
@@ -165,17 +209,24 @@
TargetPattern targetsInPackagePattern = parseAsExpectedType("foo:all", Type.TARGETS_IN_PACKAGE);
// Then the patterns are contained by tbdDepot, and do not contain tbdDepot.
- assertThat(tbdDepot.containsDirectoryOfTBDForTBD(tbdFoo)).isTrue();
- assertThat(tbdFoo.containsDirectoryOfTBDForTBD(tbdDepot)).isFalse();
+ assertThat(tbdDepot.containsTBDForTBD(tbdFoo))
+ .isEqualTo(ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT);
+ assertThat(tbdFoo.containsTBDForTBD(tbdDepot)).isEqualTo(ContainsTBDForTBDResult.OTHER);
- assertThat(tbdDepot.containsDirectoryOfTBDForTBD(pathAsTargetPattern)).isFalse();
- assertThat(pathAsTargetPattern.containsDirectoryOfTBDForTBD(tbdDepot)).isFalse();
+ assertThat(tbdDepot.containsTBDForTBD(pathAsTargetPattern))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
+ assertThat(pathAsTargetPattern.containsTBDForTBD(tbdDepot))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
- assertThat(tbdDepot.containsDirectoryOfTBDForTBD(singleTargetPattern)).isFalse();
- assertThat(singleTargetPattern.containsDirectoryOfTBDForTBD(tbdDepot)).isFalse();
+ assertThat(tbdDepot.containsTBDForTBD(singleTargetPattern)).
+ isEqualTo(ContainsTBDForTBDResult.OTHER);
+ assertThat(singleTargetPattern.containsTBDForTBD(tbdDepot))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
- assertThat(tbdDepot.containsDirectoryOfTBDForTBD(targetsInPackagePattern)).isFalse();
- assertThat(targetsInPackagePattern.containsDirectoryOfTBDForTBD(tbdDepot)).isFalse();
+ assertThat(tbdDepot.containsTBDForTBD(targetsInPackagePattern))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
+ assertThat(targetsInPackagePattern.containsTBDForTBD(tbdDepot))
+ .isEqualTo(ContainsTBDForTBDResult.OTHER);
}
private static TargetPattern parse(String pattern) throws TargetParsingException {
