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