Modify SkyQueryEnvironment#evalTargetPatternKey to take into account the filtering policy specified in the TargetPatternKey.
Add a TargetExcludingFilteringPolicy that can filter multiple single targets
Allow for TargetPatternValue.combineTargetsBelowDirectoryWithNegativePatterns the option of excluding single targets.
Improve toString of some filtering policies.
PiperOrigin-RevId: 230566477
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/FilteringPolicies.java b/src/main/java/com/google/devtools/build/lib/pkgcache/FilteringPolicies.java
index 9936f33..a4644a5 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/FilteringPolicies.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/FilteringPolicies.java
@@ -33,6 +33,12 @@
/** Returns the result of applying y, if target passes x. */
public static FilteringPolicy and(final FilteringPolicy x, final FilteringPolicy y) {
+ if (x.equals(NO_FILTER)) {
+ return y;
+ }
+ if (y.equals(NO_FILTER)) {
+ return x;
+ }
return new AndFilteringPolicy(x, y);
}
@@ -69,6 +75,11 @@
public boolean shouldRetain(Target target, boolean explicit) {
return true;
}
+
+ @Override
+ public String toString() {
+ return "[]";
+ }
}
private static class FilterManual extends AbstractFilteringPolicy {
@@ -157,5 +168,10 @@
AndFilteringPolicy other = (AndFilteringPolicy) obj;
return other.firstPolicy.equals(firstPolicy) && other.secondPolicy.equals(secondPolicy);
}
+
+ @Override
+ public String toString() {
+ return String.format("and_filter(%s, %s)", firstPolicy, secondPolicy);
+ }
}
}
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 30fc480..08c44e9 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
@@ -15,6 +15,7 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
+import static com.google.devtools.build.lib.pkgcache.FilteringPolicies.NO_FILTER;
import com.google.common.base.Ascii;
import com.google.common.base.Function;
@@ -725,13 +726,24 @@
reportBuildFileError(owner, exn.getMessage());
return Futures.immediateFuture(null);
};
- ListenableFuture<Void> evalFuture = patternToEval.evalAsync(
- resolver,
- blacklistedSubdirectoriesToExclude,
- additionalSubdirectoriesToExclude,
- callback,
- QueryException.class,
- executor);
+ Callback<Target> filteredCallback = callback;
+ if (!targetPatternKey.getPolicy().equals(NO_FILTER)) {
+ filteredCallback =
+ targets ->
+ callback.process(
+ Iterables.filter(
+ targets,
+ target ->
+ targetPatternKey.getPolicy().shouldRetain(target, /*explicit=*/ false)));
+ }
+ ListenableFuture<Void> evalFuture =
+ patternToEval.evalAsync(
+ resolver,
+ blacklistedSubdirectoriesToExclude,
+ additionalSubdirectoriesToExclude,
+ filteredCallback,
+ QueryException.class,
+ executor);
return QueryTaskFutureImpl.ofDelegate(
Futures.catchingAsync(
evalFuture,
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 2c43726..bfbb45d 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
@@ -108,8 +108,8 @@
// subdirectory exclusion) is only acceptable because all the use cases for query universe
// preloading involve short (<10 items) pattern sequences.
Iterable<TargetPatternKey> combinedTargetPatternKeys =
- TargetPatternValue.combineNegativeTargetsBelowDirectoryPatterns(
- targetPatternKeysBuilder.build());
+ TargetPatternValue.combineTargetsBelowDirectoryWithNegativePatterns(
+ targetPatternKeysBuilder.build(), /*excludeSingleTargets=*/ false);
for (TargetPatternKey targetPatternKey : combinedTargetPatternKeys) {
if (targetPatternKey.isNegative()
&& !targetPatternKey
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetExcludingFilteringPolicy.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetExcludingFilteringPolicy.java
new file mode 100644
index 0000000..4069ea6
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetExcludingFilteringPolicy.java
@@ -0,0 +1,41 @@
+// Copyright 2018 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.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
+
+/**
+ * A filtering policy that excludes multiple single targets. These are not expected to be a part of
+ * any SkyKey and it's expected that the number of targets is not too large.
+ */
+class TargetExcludingFilteringPolicy extends FilteringPolicy {
+ private final ImmutableSet<Label> excludedSingleTargets;
+
+ TargetExcludingFilteringPolicy(ImmutableSet<Label> excludedSingleTargets) {
+ this.excludedSingleTargets = excludedSingleTargets;
+ }
+
+ @Override
+ public boolean shouldRetain(Target target, boolean explicit) {
+ return !excludedSingleTargets.contains(target.getLabel());
+ }
+
+ @Override
+ public String toString() {
+ return String.format("excludedTargets%s", excludedSingleTargets);
+ }
+}
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 9e3d0ef..0237af1 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
@@ -25,6 +25,7 @@
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;
import com.google.devtools.build.lib.pkgcache.FilteringPolicies;
@@ -145,8 +146,8 @@
TargetPatternKey targetPatternKey =
new TargetPatternKey(
targetPattern,
- positive ? policy : FilteringPolicies.NO_FILTER, /*isNegative=*/
- !positive,
+ positive ? policy : FilteringPolicies.NO_FILTER,
+ /*isNegative=*/ !positive,
offset,
ImmutableSet.<PathFragment>of());
builder.add(new TargetPatternSkyKeyValue(targetPatternKey));
@@ -155,25 +156,24 @@
}
@ThreadSafe
- public static ImmutableList<TargetPatternKey> combineNegativeTargetsBelowDirectoryPatterns(
- List<TargetPatternKey> keys) {
+ public static ImmutableList<TargetPatternKey> combineTargetsBelowDirectoryWithNegativePatterns(
+ List<TargetPatternKey> keys, boolean excludeSingleTargets) {
ImmutableList.Builder<TargetPatternKey> builder = ImmutableList.builder();
+ // We use indicesOfNegativePatternsThatNeedToBeIncluded to avoid adding negative TBD or single
+ // target patterns that have already been combined with previous patterns as an excluded
+ // directory or excluded single target.
HashSet<Integer> indicesOfNegativePatternsThatNeedToBeIncluded = new HashSet<>();
+ boolean positivePatternSeen = false;
for (int i = 0; i < keys.size(); i++) {
TargetPatternKey targetPatternKey = keys.get(i);
if (targetPatternKey.isNegative()) {
- if (!targetPatternKey
- .getParsedPattern()
- .getType()
- .equals(TargetPattern.Type.TARGETS_BELOW_DIRECTORY)
- || indicesOfNegativePatternsThatNeedToBeIncluded.contains(i)) {
+ if (indicesOfNegativePatternsThatNeedToBeIncluded.contains(i) || !positivePatternSeen) {
builder.add(targetPatternKey);
}
- // Otherwise it's a negative TBD pattern which was combined with previous patterns as an
- // excluded directory.
} else {
+ positivePatternSeen = true;
TargetPatternKeyWithExclusionsResult result =
- computeTargetPatternKeyWithExclusions(targetPatternKey, i, keys);
+ computeTargetPatternKeyWithExclusions(targetPatternKey, i, keys, excludeSingleTargets);
result.targetPatternKeyMaybe.ifPresent(builder::add);
indicesOfNegativePatternsThatNeedToBeIncluded.addAll(
result.indicesOfNegativePatternsThatNeedToBeIncluded);
@@ -182,10 +182,21 @@
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 TargetPatternKey setExcludedDirectoriesAndTargets(
+ TargetPatternKey original,
+ ImmutableSet<PathFragment> excludedSubdirectories,
+ ImmutableSet<Label> excludedSingleTargets) {
+ FilteringPolicy policy = original.getPolicy();
+ if (!excludedSingleTargets.isEmpty()) {
+ policy =
+ FilteringPolicies.and(policy, new TargetExcludingFilteringPolicy(excludedSingleTargets));
+ }
+ return new TargetPatternKey(
+ original.getParsedPattern(),
+ policy,
+ original.isNegative(),
+ original.getOffset(),
+ excludedSubdirectories);
}
private static class TargetPatternKeyWithExclusionsResult {
@@ -204,16 +215,20 @@
private static TargetPatternKeyWithExclusionsResult computeTargetPatternKeyWithExclusions(
TargetPatternKey targetPatternKey,
int position,
- List<TargetPatternKey> keys) {
+ List<TargetPatternKey> keys,
+ boolean excludeSingleTargets) {
TargetPattern targetPattern = targetPatternKey.getParsedPattern();
ImmutableSet.Builder<PathFragment> excludedDirectoriesBuilder = ImmutableSet.builder();
+ ImmutableSet.Builder<Label> excludedSingleTargetsBuilder = 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() == TargetPattern.Type.TARGETS_BELOW_DIRECTORY) {
+ if (!laterTargetPatternKey.isNegative()) {
+ continue;
+ }
+ if (laterParsedPattern.getType() == Type.TARGETS_BELOW_DIRECTORY) {
if (laterParsedPattern.containsTBDForTBD(targetPattern)
== ContainsTBDForTBDResult.DIRECTORY_EXCLUSION_WOULD_BE_EXACT) {
return new TargetPatternKeyWithExclusionsResult(Optional.empty(), ImmutableList.of());
@@ -230,12 +245,27 @@
default:
// Nothing to do with this pattern.
}
-
}
+ } else if (excludeSingleTargets && laterParsedPattern.getType() == Type.SINGLE_TARGET) {
+ try {
+ Label label =
+ Label.parseAbsolute(
+ laterParsedPattern.getSingleTargetPath(),
+ /*repositoryMapping=*/ ImmutableMap.of());
+ excludedSingleTargetsBuilder.add(label);
+ } catch (LabelSyntaxException e) {
+ indicesOfNegativePatternsThatNeedToBeIncludedBuilder.add(j);
+ }
+ } else {
+ indicesOfNegativePatternsThatNeedToBeIncludedBuilder.add(j);
}
}
return new TargetPatternKeyWithExclusionsResult(
- Optional.of(setExcludedDirectories(targetPatternKey, excludedDirectoriesBuilder.build())),
+ Optional.of(
+ setExcludedDirectoriesAndTargets(
+ targetPatternKey,
+ excludedDirectoriesBuilder.build(),
+ excludedSingleTargetsBuilder.build())),
indicesOfNegativePatternsThatNeedToBeIncludedBuilder.build());
}
@@ -312,7 +342,7 @@
InterruptibleSupplier<? extends Iterable<PathFragment>> blacklistedPackagePrefixes)
throws InterruptedException {
ImmutableSet.Builder<PathFragment> blacklistedPathsBuilder = ImmutableSet.builder();
- if (parsedPattern.getType() == TargetPattern.Type.TARGETS_BELOW_DIRECTORY) {
+ if (parsedPattern.getType() == Type.TARGETS_BELOW_DIRECTORY) {
for (PathFragment blacklistedPackagePrefix : blacklistedPackagePrefixes.get()) {
PackageIdentifier pkgIdForBlacklistedDirectorPrefix = PackageIdentifier.create(
parsedPattern.getDirectoryForTargetsUnderDirectory().getRepository(),
@@ -328,7 +358,11 @@
@Override
public String toString() {
- return (isNegative ? "-" : "") + parsedPattern.getOriginalPattern();
+ return String.format(
+ "%s, excludedSubdirs=%s, filteringPolicy=%s",
+ (isNegative ? "-" : "") + parsedPattern.getOriginalPattern(),
+ excludedSubdirectories,
+ getPolicy());
}
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java
index 0c59fbe..03b7c51 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java
@@ -206,8 +206,8 @@
// Then a event is published that says that negative non-TBD patterns are skipped.
assertContainsEvent(
- "Skipping '-//foo/bar': Negative target patterns of types other than \"targets below "
- + "directory\" are not permitted.");
+ "Skipping '-//foo/bar, excludedSubdirs=[], filteringPolicy=[]': Negative target patterns of"
+ + " types other than \"targets below directory\" are not permitted.");
}
// Helpers:
