Target pattern parsing cleanup
================
* `TargetPattern#renameRepository` is merged into `TargetPattern.Parser#parse`. This is much less error-prone -- repo mapping should not be an afterthought, but something that should always be applied if possible (sometimes it's not, for example for `blaze query`).
* This also fixes the bug where calling `native.register_toolchains("//:all")` in `@some_repo//:defs.bzl` registers `@//:all` instead of `@some_repo//:all` (see change in RegisteredExecutionPlatformsTest)
* A new class `SignedTargetPattern` is introduced, which can store whether the pattern is positive or negative (with the `sign` method).
* `TargetPatternValue#keys` is greatly simplified thanks to the changes above; the exception throwing is confined to the parsing step, and the construction of `TargetPatternKey` can happen as a separate step, obviating the whole "skykey or exception" dance.
* Following from the above, the //external package now stores registered toolchains and execution platforms as parsed target patterns, instead of simple strings.
Among other things, this would help implement toolchain registration in bzlmod.
PiperOrigin-RevId: 400720457
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 7bb24a88..20f77b8 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
@@ -15,6 +15,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Interner;
+import com.google.devtools.build.lib.cmdline.SignedTargetPattern;
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;
@@ -23,12 +24,10 @@
import com.google.devtools.build.lib.pkgcache.FilteringPolicies;
import com.google.devtools.build.lib.server.FailureDetails.TargetPatterns;
import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
-import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternSkyKeyOrException;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.AbstractSkyKey;
import com.google.devtools.build.skyframe.SkyFunctionName;
-import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.List;
@@ -92,16 +91,15 @@
ImmutableList.builder();
ImmutableList.Builder<PrepareDepsOfPatternSkyKeyException> resultExceptionsBuilder =
ImmutableList.builder();
- Iterable<TargetPatternSkyKeyOrException> keysMaybe =
- TargetPatternValue.keys(patterns, FilteringPolicies.NO_FILTER, offset);
+ TargetPattern.Parser parser = TargetPattern.mainRepoParser(offset);
ImmutableList.Builder<TargetPatternKey> targetPatternKeysBuilder = ImmutableList.builder();
- for (TargetPatternSkyKeyOrException keyMaybe : keysMaybe) {
+ for (String pattern : patterns) {
try {
- SkyKey key = keyMaybe.getSkyKey();
- targetPatternKeysBuilder.add((TargetPatternKey) key.argument());
+ targetPatternKeysBuilder.add(
+ TargetPatternValue.key(
+ SignedTargetPattern.parse(pattern, parser), FilteringPolicies.NO_FILTER));
} catch (TargetParsingException e) {
- resultExceptionsBuilder.add(
- new PrepareDepsOfPatternSkyKeyException(e, keyMaybe.getOriginalPattern()));
+ resultExceptionsBuilder.add(new PrepareDepsOfPatternSkyKeyException(e, pattern));
}
}
// This code path is evaluated only for query universe preloading, and the quadratic cost of
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 8a94bce..9d40971 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
@@ -87,6 +87,7 @@
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
ExtendedEventHandler eventHandler = env.getListener();
+ // TODO(wyv): use the repo mapping of the main repo here.
ImmutableList<SkyKey> skyKeys = getSkyKeys(skyKey, eventHandler);
Map<SkyKey, ValueOrException<TargetParsingException>> tokensByKey =
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunction.java
index fb5cca9..107620b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunction.java
@@ -16,7 +16,6 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
-import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -28,7 +27,10 @@
import com.google.devtools.build.lib.bazel.bzlmod.ExternalDepsException;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelConstants;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.cmdline.SignedTargetPattern;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
+import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
@@ -37,8 +39,10 @@
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.Toolchain;
import com.google.devtools.build.lib.skyframe.PlatformLookupUtil.InvalidPlatformException;
+import com.google.devtools.build.lib.skyframe.TargetPatternUtil.InvalidTargetPatternException;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
@@ -52,6 +56,10 @@
/** {@link SkyFunction} that returns all registered execution platforms available. */
public class RegisteredExecutionPlatformsFunction implements SkyFunction {
+ @AutoCodec
+ static final FilteringPolicy HAS_PLATFORM_INFO =
+ (Target target, boolean explicit) -> explicit || PlatformLookupUtil.hasPlatformInfo(target);
+
@Nullable
@Override
public SkyValue compute(SkyKey skyKey, Environment env)
@@ -60,34 +68,47 @@
BuildConfigurationValue buildConfigurationValue =
(BuildConfigurationValue)
env.getValue(((RegisteredExecutionPlatformsValue.Key) skyKey).getConfigurationKey());
+ RepositoryMappingValue mainRepoMapping =
+ (RepositoryMappingValue) env.getValue(RepositoryMappingValue.key(RepositoryName.MAIN));
if (env.valuesMissing()) {
return null;
}
BuildConfiguration configuration = buildConfigurationValue.getConfiguration();
- ImmutableList.Builder<String> targetPatternBuilder = new ImmutableList.Builder<>();
+ TargetPattern.Parser mainRepoParser =
+ new TargetPattern.Parser(
+ PathFragment.EMPTY_FRAGMENT,
+ RepositoryName.MAIN,
+ mainRepoMapping.getRepositoryMapping());
+ ImmutableList.Builder<SignedTargetPattern> targetPatternBuilder = new ImmutableList.Builder<>();
// Get the execution platforms from the configuration.
PlatformConfiguration platformConfiguration =
configuration.getFragment(PlatformConfiguration.class);
if (platformConfiguration != null) {
- targetPatternBuilder.addAll(platformConfiguration.getExtraExecutionPlatforms());
+ try {
+ targetPatternBuilder.addAll(
+ TargetPatternUtil.parseAllSigned(
+ platformConfiguration.getExtraExecutionPlatforms(), mainRepoParser));
+ } catch (InvalidTargetPatternException e) {
+ throw new RegisteredExecutionPlatformsFunctionException(
+ new InvalidExecutionPlatformLabelException(e), Transience.PERSISTENT);
+ }
}
// Get the registered execution platforms from the WORKSPACE.
- List<String> workspaceExecutionPlatforms = getWorkspaceExecutionPlatforms(env);
+ ImmutableList<TargetPattern> workspaceExecutionPlatforms = getWorkspaceExecutionPlatforms(env);
if (workspaceExecutionPlatforms == null) {
return null;
}
- targetPatternBuilder.addAll(workspaceExecutionPlatforms);
-
- ImmutableList<String> targetPatterns = targetPatternBuilder.build();
+ targetPatternBuilder.addAll(TargetPatternUtil.toSigned(workspaceExecutionPlatforms));
// Expand target patterns.
ImmutableList<Label> platformLabels;
try {
platformLabels =
- TargetPatternUtil.expandTargetPatterns(env, targetPatterns, HasPlatformInfo.create());
+ TargetPatternUtil.expandTargetPatterns(
+ env, targetPatternBuilder.build(), HAS_PLATFORM_INFO);
if (env.valuesMissing()) {
return null;
}
@@ -113,7 +134,7 @@
*/
@Nullable
@VisibleForTesting
- public static ImmutableList<String> getWorkspaceExecutionPlatforms(Environment env)
+ public static ImmutableList<TargetPattern> getWorkspaceExecutionPlatforms(Environment env)
throws InterruptedException {
PackageValue externalPackageValue =
(PackageValue) env.getValue(PackageValue.key(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER));
@@ -231,24 +252,4 @@
super(cause, transience);
}
}
-
- // This class uses AutoValue solely to get default equals/hashCode behavior, which is needed to
- // make skyframe serialization work properly.
- @AutoValue
- @AutoCodec
- abstract static class HasPlatformInfo extends FilteringPolicy {
-
- @Override
- public boolean shouldRetain(Target target, boolean explicit) {
- if (explicit) {
- return true;
- }
- return PlatformLookupUtil.hasPlatformInfo(target);
- }
-
- @AutoCodec.Instantiator
- static HasPlatformInfo create() {
- return new AutoValue_RegisteredExecutionPlatformsFunction_HasPlatformInfo();
- }
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
index 0b7400c..bd33592 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
@@ -27,10 +27,15 @@
import com.google.devtools.build.lib.bazel.bzlmod.ExternalDepsException;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelConstants;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.cmdline.SignedTargetPattern;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
+import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.pkgcache.FilteringPolicies;
import com.google.devtools.build.lib.server.FailureDetails.Toolchain.Code;
+import com.google.devtools.build.lib.skyframe.TargetPatternUtil.InvalidTargetPatternException;
+import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
@@ -54,33 +59,45 @@
BuildConfigurationValue buildConfigurationValue =
(BuildConfigurationValue)
env.getValue(((RegisteredToolchainsValue.Key) skyKey).getConfigurationKey());
+ RepositoryMappingValue mainRepoMapping =
+ (RepositoryMappingValue) env.getValue(RepositoryMappingValue.key(RepositoryName.MAIN));
if (env.valuesMissing()) {
return null;
}
BuildConfiguration configuration = buildConfigurationValue.getConfiguration();
- ImmutableList.Builder<String> targetPatternBuilder = new ImmutableList.Builder<>();
+ TargetPattern.Parser mainRepoParser =
+ new TargetPattern.Parser(
+ PathFragment.EMPTY_FRAGMENT,
+ RepositoryName.MAIN,
+ mainRepoMapping.getRepositoryMapping());
+ ImmutableList.Builder<SignedTargetPattern> targetPatternBuilder = new ImmutableList.Builder<>();
// Get the toolchains from the configuration.
PlatformConfiguration platformConfiguration =
configuration.getFragment(PlatformConfiguration.class);
- targetPatternBuilder.addAll(platformConfiguration.getExtraToolchains());
+ try {
+ targetPatternBuilder.addAll(
+ TargetPatternUtil.parseAllSigned(
+ platformConfiguration.getExtraToolchains(), mainRepoParser));
+ } catch (InvalidTargetPatternException e) {
+ throw new RegisteredToolchainsFunctionException(
+ new InvalidToolchainLabelException(e), Transience.PERSISTENT);
+ }
// Get the registered toolchains from the WORKSPACE.
- ImmutableList<String> workspaceToolchains = getWorkspaceToolchains(env);
+ ImmutableList<TargetPattern> workspaceToolchains = getWorkspaceToolchains(env);
if (workspaceToolchains == null) {
return null;
}
- targetPatternBuilder.addAll(workspaceToolchains);
-
- ImmutableList<String> targetPatterns = targetPatternBuilder.build();
+ targetPatternBuilder.addAll(TargetPatternUtil.toSigned(workspaceToolchains));
// Expand target patterns.
ImmutableList<Label> toolchainLabels;
try {
toolchainLabels =
TargetPatternUtil.expandTargetPatterns(
- env, targetPatterns, FilteringPolicies.ruleType("toolchain", true));
+ env, targetPatternBuilder.build(), FilteringPolicies.ruleTypeExplicit("toolchain"));
if (env.valuesMissing()) {
return null;
}
@@ -106,7 +123,7 @@
*/
@Nullable
@VisibleForTesting
- public static ImmutableList<String> getWorkspaceToolchains(Environment env)
+ public static ImmutableList<TargetPattern> getWorkspaceToolchains(Environment env)
throws InterruptedException {
PackageValue externalPackageValue =
(PackageValue) env.getValue(PackageValue.key(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER));
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java
index fe79140..081dfd9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.ResolvedTargets;
+import com.google.devtools.build.lib.cmdline.SignedTargetPattern;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.events.Event;
@@ -146,7 +147,9 @@
throws TargetParsingException {
try {
TargetPatternKey key =
- TargetPatternValue.key(targetPattern, FilteringPolicies.NO_FILTER, offset);
+ TargetPatternValue.key(
+ SignedTargetPattern.parse(targetPattern, TargetPattern.mainRepoParser(offset)),
+ FilteringPolicies.NO_FILTER);
return isSimple(key.getParsedPattern())
? new SimpleLookup(targetPattern, key)
: new NormalLookup(targetPattern, key);
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
index 4069ea6..ef23fd2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetExcludingFilteringPolicy.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetExcludingFilteringPolicy.java
@@ -17,12 +17,13 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
+import java.util.Objects;
/**
* 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 {
+class TargetExcludingFilteringPolicy implements FilteringPolicy {
private final ImmutableSet<Label> excludedSingleTargets;
TargetExcludingFilteringPolicy(ImmutableSet<Label> excludedSingleTargets) {
@@ -38,4 +39,21 @@
public String toString() {
return String.format("excludedTargets%s", excludedSingleTargets);
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (!(o instanceof TargetExcludingFilteringPolicy)) {
+ return false;
+ }
+ TargetExcludingFilteringPolicy that = (TargetExcludingFilteringPolicy) o;
+ return Objects.equals(excludedSingleTargets, that.excludedSingleTargets);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(excludedSingleTargets);
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
index 54d1833..8e8722f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
@@ -29,6 +29,7 @@
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.cmdline.ResolvedTargets;
+import com.google.devtools.build.lib.cmdline.SignedTargetPattern;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.events.Event;
@@ -41,6 +42,7 @@
import com.google.devtools.build.lib.pkgcache.AbstractRecursivePackageProvider.MissingDepException;
import com.google.devtools.build.lib.pkgcache.CompileOneDependencyTransformer;
import com.google.devtools.build.lib.pkgcache.FilteringPolicies;
+import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
import com.google.devtools.build.lib.pkgcache.LoadingPhaseCompleteEvent;
import com.google.devtools.build.lib.pkgcache.ParsingFailedEvent;
import com.google.devtools.build.lib.pkgcache.TargetParsingCompleteEvent;
@@ -48,7 +50,6 @@
import com.google.devtools.build.lib.repository.ExternalPackageHelper;
import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue.TargetPatternPhaseKey;
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.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
@@ -107,8 +108,13 @@
// set as build list as well.
ResolvedTargets<Target> testTargets = null;
if (options.getDetermineTests() || options.getBuildTestsOnly()) {
- testTargets = determineTests(env,
- options.getTargetPatterns(), options.getOffset(), options.getTestFilter());
+ testTargets =
+ determineTests(
+ env,
+ options.getTargetPatterns(),
+ options.getOffset(),
+ repositoryMappingValue.getRepositoryMapping(),
+ options.getTestFilter());
Preconditions.checkState(env.valuesMissing() || (testTargets != null));
}
@@ -274,43 +280,34 @@
RepositoryMapping repoMapping,
List<String> failedPatterns)
throws InterruptedException {
-
- ImmutableList.Builder<String> canonicalPatterns = new ImmutableList.Builder<>();
- for (String rawPattern : options.getTargetPatterns()) {
- canonicalPatterns.add(TargetPattern.renameRepository(rawPattern, repoMapping));
- }
-
+ TargetPattern.Parser parser =
+ new TargetPattern.Parser(options.getOffset(), RepositoryName.MAIN, repoMapping);
+ FilteringPolicy policy =
+ options.getBuildManualTests()
+ ? FilteringPolicies.NO_FILTER
+ : FilteringPolicies.FILTER_MANUAL;
List<TargetPatternKey> patternSkyKeys = new ArrayList<>(options.getTargetPatterns().size());
- for (TargetPatternSkyKeyOrException keyOrException :
- TargetPatternValue.keys(
- canonicalPatterns.build(),
- options.getBuildManualTests()
- ? FilteringPolicies.NO_FILTER
- : FilteringPolicies.FILTER_MANUAL,
- options.getOffset())) {
+ for (String pattern : options.getTargetPatterns()) {
try {
- patternSkyKeys.add(keyOrException.getSkyKey());
+ patternSkyKeys.add(
+ TargetPatternValue.key(SignedTargetPattern.parse(pattern, parser), policy));
} catch (TargetParsingException e) {
- failedPatterns.add(keyOrException.getOriginalPattern());
+ failedPatterns.add(pattern);
// We post a PatternExpandingError here - the pattern could not be parsed, so we don't even
// get to run TargetPatternFunction.
- env.getListener().post(
- PatternExpandingError.failed(keyOrException.getOriginalPattern(), e.getMessage()));
+ env.getListener().post(PatternExpandingError.failed(pattern, e.getMessage()));
// We generally skip patterns that don't parse. We report a parsing failed exception to the
// event bus here, but not in determineTests below, which goes through the same list. Note
// that the TargetPatternFunction otherwise reports these events (but only if the target
// pattern could be parsed successfully).
- env.getListener().post(
- new ParsingFailedEvent(keyOrException.getOriginalPattern(), e.getMessage()));
+ env.getListener().post(new ParsingFailedEvent(pattern, e.getMessage()));
try {
env.getValueOrThrow(
TargetPatternErrorFunction.key(e.getMessage()), TargetParsingException.class);
} catch (TargetParsingException ignore) {
// We ignore this. Keep going is active.
}
- env.getListener().handle(
- Event.error(
- "Skipping '" + keyOrException.getOriginalPattern() + "': " + e.getMessage()));
+ env.getListener().handle(Event.error("Skipping '" + pattern + "': " + e.getMessage()));
}
}
@@ -398,16 +395,24 @@
* of targets, handling the filter flags, and expanding test suites.
*
* @param targetPatterns the list of command-line target patterns specified by the user
+ * @param repoMapping the repository mapping to apply to repos in the patterns
* @param testFilter the test filter
*/
private static ResolvedTargets<Target> determineTests(
- Environment env, List<String> targetPatterns, PathFragment offset, TestFilter testFilter)
+ Environment env,
+ List<String> targetPatterns,
+ PathFragment offset,
+ RepositoryMapping repoMapping,
+ TestFilter testFilter)
throws InterruptedException {
+ TargetPattern.Parser parser =
+ new TargetPattern.Parser(offset, RepositoryName.MAIN, repoMapping);
List<TargetPatternKey> patternSkyKeys = new ArrayList<>();
- for (TargetPatternSkyKeyOrException keyOrException :
- TargetPatternValue.keys(targetPatterns, FilteringPolicies.FILTER_TESTS, offset)) {
+ for (String pattern : targetPatterns) {
try {
- patternSkyKeys.add(keyOrException.getSkyKey());
+ patternSkyKeys.add(
+ TargetPatternValue.key(
+ SignedTargetPattern.parse(pattern, parser), FilteringPolicies.FILTER_TESTS));
} catch (TargetParsingException e) {
// Skip.
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternUtil.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternUtil.java
index fd5e61c..411d24e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternUtil.java
@@ -14,70 +14,48 @@
package com.google.devtools.build.lib.skyframe;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.SignedTargetPattern;
+import com.google.devtools.build.lib.cmdline.SignedTargetPattern.Sign;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
-import com.google.devtools.build.lib.pkgcache.FilteringPolicies;
+import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
-import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.ValueOrException;
-import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
-/**
- * Utility class to help with evaluating target patterns.
- */
+/** Utility class to help with evaluating target patterns. */
public class TargetPatternUtil {
/**
- * Expand the given {@code targetPatterns}. This handles the needed underlying Skyframe calls (via
- * {@code env}), and will return {@code null} to signal a Skyframe restart.
- */
- @Nullable
- public static ImmutableList<Label> expandTargetPatterns(
- Environment env, List<String> targetPatterns)
- throws InvalidTargetPatternException, InterruptedException {
-
- return expandTargetPatterns(env, targetPatterns, FilteringPolicies.NO_FILTER);
- }
-
- /**
* Expand the given {@code targetPatterns}, using the {@code filteringPolicy}. This handles the
* needed underlying Skyframe calls (via {@code env}), and will return {@code null} to signal a
* Skyframe restart.
*/
@Nullable
public static ImmutableList<Label> expandTargetPatterns(
- Environment env, List<String> targetPatterns, FilteringPolicy filteringPolicy)
+ Environment env, List<SignedTargetPattern> targetPatterns, FilteringPolicy filteringPolicy)
throws InvalidTargetPatternException, InterruptedException {
if (targetPatterns.isEmpty()) {
return ImmutableList.of();
}
- // First parse the patterns, and throw any errors immediately.
- List<TargetPatternValue.TargetPatternKey> patternKeys = new ArrayList<>();
- for (TargetPatternValue.TargetPatternSkyKeyOrException keyOrException :
- TargetPatternValue.keys(targetPatterns, filteringPolicy, PathFragment.EMPTY_FRAGMENT)) {
-
- try {
- patternKeys.add(keyOrException.getSkyKey());
- } catch (TargetParsingException e) {
- throw new InvalidTargetPatternException(keyOrException.getOriginalPattern(), e);
- }
- }
-
- // Then, resolve the patterns.
+ Iterable<TargetPatternKey> targetPatternKeys =
+ TargetPatternValue.keys(targetPatterns, filteringPolicy);
Map<SkyKey, ValueOrException<TargetParsingException>> resolvedPatterns =
- env.getValuesOrThrow(patternKeys, TargetParsingException.class);
+ env.getValuesOrThrow(targetPatternKeys, TargetParsingException.class);
boolean valuesMissing = env.valuesMissing();
ImmutableList.Builder<Label> labels = valuesMissing ? null : new ImmutableList.Builder<>();
- for (TargetPatternValue.TargetPatternKey pattern : patternKeys) {
+ for (TargetPatternKey pattern : targetPatternKeys) {
TargetPatternValue value;
try {
value = (TargetPatternValue) resolvedPatterns.get(pattern).get();
@@ -96,8 +74,31 @@
return labels.build();
}
+ // TODO(bazel-team): look into moving this into SignedTargetPattern itself.
+ public static ImmutableList<SignedTargetPattern> parseAllSigned(
+ List<String> patterns, TargetPattern.Parser parser) throws InvalidTargetPatternException {
+ ImmutableList.Builder<SignedTargetPattern> parsedPatterns = ImmutableList.builder();
+ for (String pattern : patterns) {
+ try {
+ parsedPatterns.add(SignedTargetPattern.parse(pattern, parser));
+ } catch (TargetParsingException e) {
+ throw new InvalidTargetPatternException(pattern, e);
+ }
+ }
+ return parsedPatterns.build();
+ }
+
+ /** Converts patterns to signed patterns, considering all input patterns positive. */
+ public static ImmutableList<SignedTargetPattern> toSigned(List<TargetPattern> patterns) {
+ return patterns.stream()
+ .map(pattern -> SignedTargetPattern.create(pattern, Sign.POSITIVE))
+ .collect(toImmutableList());
+ }
+
/** Exception used when an error occurs in {@link #expandTargetPatterns}. */
- static final class InvalidTargetPatternException extends Exception {
+ // TODO(bazel-team): Consolidate this and TargetParsingException. Just have the latter store the
+ // original unparsed pattern too.
+ public static final class InvalidTargetPatternException extends Exception {
private String invalidPattern;
private TargetParsingException tpe;
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 fecdf3d..5fecef1 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
@@ -13,15 +13,17 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.ResolvedTargets;
-import com.google.devtools.build.lib.cmdline.TargetParsingException;
+import com.google.devtools.build.lib.cmdline.SignedTargetPattern;
+import com.google.devtools.build.lib.cmdline.SignedTargetPattern.Sign;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.cmdline.TargetPattern.TargetsBelowDirectory;
import com.google.devtools.build.lib.cmdline.TargetPattern.TargetsBelowDirectory.ContainsResult;
@@ -102,55 +104,28 @@
}
/**
- * Create a target pattern {@link SkyKey}. Throws {@link TargetParsingException} if the provided
- * {@code pattern} cannot be parsed.
+ * Create a target pattern {@link SkyKey}.
*
- * @param pattern The pattern, eg "-foo/biz...". If the first character is "-", the pattern is
- * treated as a negative pattern.
+ * @param pattern The pattern, eg "-foo/biz...".
* @param policy The filtering policy, eg "only return test targets"
- * @param offset The offset to apply to relative target patterns.
*/
@ThreadSafe
- public static TargetPatternKey key(String pattern, FilteringPolicy policy, PathFragment offset)
- throws TargetParsingException {
- return Iterables.getOnlyElement(keys(ImmutableList.of(pattern), policy, offset)).getSkyKey();
+ public static TargetPatternKey key(SignedTargetPattern pattern, FilteringPolicy policy) {
+ return new TargetPatternKey(
+ pattern, pattern.sign() == Sign.POSITIVE ? policy : FilteringPolicies.NO_FILTER);
}
/**
- * Returns an iterable of {@link TargetPatternSkyKeyOrException}, with {@link TargetPatternKey}
- * arguments, in the same order as the list of patterns provided as input. If a provided pattern
- * fails to parse, the element in the returned iterable corresponding to it will throw when its
- * {@link TargetPatternSkyKeyOrException#getSkyKey} method is called.
+ * Returns an iterable of {@link TargetPatternKey}s, in the same order as the list of patterns
+ * provided as input.
*
- * @param patterns The list of patterns, eg "-foo/biz...". If a pattern's first character is "-",
- * it is treated as a negative pattern.
+ * @param patterns The list of patterns, eg "-foo/biz...".
* @param policy The filtering policy, eg "only return test targets"
- * @param offset The offset to apply to relative target patterns.
*/
@ThreadSafe
- public static Iterable<TargetPatternSkyKeyOrException> keys(
- List<String> patterns, FilteringPolicy policy, PathFragment offset) {
- TargetPattern.Parser parser = new TargetPattern.Parser(offset);
- ImmutableList.Builder<TargetPatternSkyKeyOrException> builder = ImmutableList.builder();
- for (String pattern : patterns) {
- boolean positive = !pattern.startsWith("-");
- String absoluteValueOfPattern = positive ? pattern : pattern.substring(1);
- TargetPattern targetPattern;
- try {
- targetPattern = parser.parse(absoluteValueOfPattern);
- } catch (TargetParsingException e) {
- builder.add(new TargetPatternSkyKeyException(e, absoluteValueOfPattern));
- continue;
- }
- TargetPatternKey targetPatternKey =
- new TargetPatternKey(
- targetPattern,
- positive ? policy : FilteringPolicies.NO_FILTER,
- /*isNegative=*/ !positive,
- offset);
- builder.add(new TargetPatternSkyKeyValue(targetPatternKey));
- }
- return builder.build();
+ public static Iterable<TargetPatternKey> keys(
+ List<SignedTargetPattern> patterns, FilteringPolicy policy) {
+ return patterns.stream().map(pattern -> key(pattern, policy)).collect(toImmutableList());
}
@ThreadSafe
@@ -189,12 +164,7 @@
policy =
FilteringPolicies.and(policy, new TargetExcludingFilteringPolicy(excludedSingleTargets));
}
- return new TargetPatternKey(
- original.getParsedPattern(),
- policy,
- original.isNegative(),
- original.getOffset(),
- excludedSubdirectories);
+ return new TargetPatternKey(original.getSignedParsedPattern(), policy, excludedSubdirectories);
}
private static class TargetPatternKeyWithExclusionsResult {
@@ -281,36 +251,26 @@
*/
@ThreadSafe
public static class TargetPatternKey implements SkyKey, Serializable {
- private final TargetPattern parsedPattern;
+ private final SignedTargetPattern signedParsedPattern;
private final FilteringPolicy policy;
- private final boolean isNegative;
- private final PathFragment offset;
/**
- * Must be "compatible" with {@link #parsedPattern}: if {@link #parsedPattern} is a {@link
- * TargetsBelowDirectory} object, then {@link TargetsBelowDirectory#containedIn} is false for
- * every element of {@code excludedSubdirectories}.
+ * Must be "compatible" with {@link #signedParsedPattern}: if {@link #signedParsedPattern} is a
+ * {@link TargetsBelowDirectory} object, then {@link TargetsBelowDirectory#containedIn} is false
+ * for every element of {@code excludedSubdirectories}.
*/
private final ImmutableSet<PathFragment> excludedSubdirectories;
- public TargetPatternKey(
- TargetPattern parsedPattern,
- FilteringPolicy policy,
- boolean isNegative,
- PathFragment offset) {
- this(parsedPattern, policy, isNegative, offset, ImmutableSet.of());
+ public TargetPatternKey(SignedTargetPattern signedParsedPattern, FilteringPolicy policy) {
+ this(signedParsedPattern, policy, ImmutableSet.of());
}
private TargetPatternKey(
- TargetPattern parsedPattern,
+ SignedTargetPattern signedParsedPattern,
FilteringPolicy policy,
- boolean isNegative,
- PathFragment offset,
ImmutableSet<PathFragment> excludedSubdirectories) {
- this.parsedPattern = Preconditions.checkNotNull(parsedPattern);
+ this.signedParsedPattern = Preconditions.checkNotNull(signedParsedPattern);
this.policy = Preconditions.checkNotNull(policy);
- this.isNegative = isNegative;
- this.offset = offset;
this.excludedSubdirectories = Preconditions.checkNotNull(excludedSubdirectories);
}
@@ -320,25 +280,25 @@
}
public String getPattern() {
- return parsedPattern.getOriginalPattern();
+ return signedParsedPattern.pattern().getOriginalPattern();
}
public TargetPattern getParsedPattern() {
- return parsedPattern;
+ return signedParsedPattern.pattern();
+ }
+
+ private SignedTargetPattern getSignedParsedPattern() {
+ return signedParsedPattern;
}
public boolean isNegative() {
- return isNegative;
+ return signedParsedPattern.sign() == Sign.NEGATIVE;
}
public FilteringPolicy getPolicy() {
return policy;
}
- public PathFragment getOffset() {
- return offset;
- }
-
public ImmutableSet<PathFragment> getExcludedSubdirectories() {
return excludedSubdirectories;
}
@@ -347,15 +307,14 @@
public String toString() {
return String.format(
"%s, excludedSubdirs=%s, filteringPolicy=%s",
- (isNegative ? "-" : "") + parsedPattern.getOriginalPattern(),
+ (isNegative() ? "-" : "") + signedParsedPattern.pattern().getOriginalPattern(),
excludedSubdirectories,
getPolicy());
}
@Override
public int hashCode() {
- return Objects.hash(parsedPattern, isNegative, policy, offset,
- excludedSubdirectories);
+ return Objects.hash(signedParsedPattern, policy, excludedSubdirectories);
}
@Override
@@ -365,69 +324,9 @@
}
TargetPatternKey other = (TargetPatternKey) obj;
- return other.isNegative == this.isNegative && other.parsedPattern.equals(this.parsedPattern)
- && other.offset.equals(this.offset) && other.policy.equals(this.policy)
+ return other.signedParsedPattern.equals(this.signedParsedPattern)
+ && other.policy.equals(this.policy)
&& other.excludedSubdirectories.equals(this.excludedSubdirectories);
}
}
-
- /**
- * Wrapper for a target pattern {@link SkyKey} or the {@link TargetParsingException} thrown when
- * trying to compute it.
- */
- public interface TargetPatternSkyKeyOrException {
-
- /**
- * Returns the stored {@link SkyKey} or throws {@link TargetParsingException} if one was thrown
- * when computing the key.
- */
- TargetPatternKey getSkyKey() throws TargetParsingException;
-
- /**
- * Returns the pattern that resulted in the stored {@link SkyKey} or {@link
- * TargetParsingException}.
- */
- String getOriginalPattern();
- }
-
- private static final class TargetPatternSkyKeyValue implements TargetPatternSkyKeyOrException {
-
- private final TargetPatternKey value;
-
- private TargetPatternSkyKeyValue(TargetPatternKey value) {
- this.value = value;
- }
-
- @Override
- public TargetPatternKey getSkyKey() throws TargetParsingException {
- return value;
- }
-
- @Override
- public String getOriginalPattern() {
- return ((TargetPatternKey) value.argument()).getPattern();
- }
- }
-
- private static final class TargetPatternSkyKeyException implements
- TargetPatternSkyKeyOrException {
-
- private final TargetParsingException exception;
- private final String originalPattern;
-
- private TargetPatternSkyKeyException(TargetParsingException exception, String originalPattern) {
- this.exception = exception;
- this.originalPattern = originalPattern;
- }
-
- @Override
- public TargetPatternKey getSkyKey() throws TargetParsingException {
- throw exception;
- }
-
- @Override
- public String getOriginalPattern() {
- return originalPattern;
- }
- }
}
