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/bazel/commands/SyncCommand.java b/src/main/java/com/google/devtools/build/lib/bazel/commands/SyncCommand.java
index 3b78846..ea765a6 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/commands/SyncCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/SyncCommand.java
@@ -13,6 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.bazel.commands;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -24,6 +26,7 @@
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler.ResolvedEvent;
import com.google.devtools.build.lib.packages.Rule;
@@ -166,12 +169,17 @@
env.getReporter()
.post(
genericArgsCall(
- "register_toolchains", fileValue.getPackage().getRegisteredToolchains()));
+ "register_toolchains",
+ fileValue.getPackage().getRegisteredToolchains().stream()
+ .map(TargetPattern::getOriginalPattern)
+ .collect(toImmutableList())));
env.getReporter()
.post(
genericArgsCall(
"register_execution_platforms",
- fileValue.getPackage().getRegisteredExecutionPlatforms()));
+ fileValue.getPackage().getRegisteredExecutionPlatforms().stream()
+ .map(TargetPattern::getOriginalPattern)
+ .collect(toImmutableList())));
env.getReporter().post(new RepositoryOrderEvent(repositoryOrder.build()));
// take all Starlark workspace rules and get their values
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisAndExecutionPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisAndExecutionPhaseRunner.java
index e9c3e2c..850bdec 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisAndExecutionPhaseRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisAndExecutionPhaseRunner.java
@@ -228,7 +228,7 @@
CommandEnvironment env, List<String> requestedTargetPatterns)
throws ViewCreationFailedException {
ImmutableSet.Builder<String> explicitTargetPatterns = ImmutableSet.builder();
- TargetPattern.Parser parser = new TargetPattern.Parser(env.getRelativeWorkingDirectory());
+ TargetPattern.Parser parser = TargetPattern.mainRepoParser(env.getRelativeWorkingDirectory());
for (String requestedTargetPattern : requestedTargetPatterns) {
if (requestedTargetPattern.startsWith("-")) {
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
index aeb2397..3e0b428 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
@@ -315,7 +315,7 @@
CommandEnvironment env, List<String> requestedTargetPatterns)
throws ViewCreationFailedException {
ImmutableSet.Builder<String> explicitTargetPatterns = ImmutableSet.builder();
- TargetPattern.Parser parser = new TargetPattern.Parser(env.getRelativeWorkingDirectory());
+ TargetPattern.Parser parser = TargetPattern.mainRepoParser(env.getRelativeWorkingDirectory());
for (String requestedTargetPattern : requestedTargetPatterns) {
if (requestedTargetPattern.startsWith("-")) {
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/BUILD b/src/main/java/com/google/devtools/build/lib/cmdline/BUILD
index bd3fff0..1f204fe 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/BUILD
@@ -16,6 +16,7 @@
"LabelConstants.java",
"LabelSerializationProxy.java",
"ResolvedTargets.java",
+ "SignedTargetPattern.java",
"TargetParsingException.java",
"TargetPattern.java",
"TargetPatternResolver.java",
@@ -38,6 +39,7 @@
"//src/main/java/net/starlark/java/annot",
"//src/main/java/net/starlark/java/eval",
"//src/main/protobuf:failure_details_java_proto",
+ "//third_party:auto_value",
"//third_party:error_prone_annotations",
"//third_party:guava",
"//third_party:jsr305",
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/SignedTargetPattern.java b/src/main/java/com/google/devtools/build/lib/cmdline/SignedTargetPattern.java
new file mode 100644
index 0000000..a8dc762
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/SignedTargetPattern.java
@@ -0,0 +1,44 @@
+// Copyright 2021 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.cmdline;
+
+import com.google.auto.value.AutoValue;
+
+/** A {@link TargetPattern} with a potential minus sign in front of it, signifying exclusion. */
+@AutoValue
+public abstract class SignedTargetPattern {
+ public abstract TargetPattern pattern();
+
+ public abstract Sign sign();
+
+ public static SignedTargetPattern create(TargetPattern pattern, Sign sign) {
+ return new AutoValue_SignedTargetPattern(pattern, sign);
+ }
+
+ /** Whether this target pattern begins with a minus sign (NEGATIVE) or not (POSITIVE). */
+ public enum Sign {
+ POSITIVE,
+ NEGATIVE
+ }
+
+ public static SignedTargetPattern parse(String pattern, TargetPattern.Parser parser)
+ throws TargetParsingException {
+ if (pattern.startsWith("-")) {
+ return create(parser.parse(pattern.substring(1)), SignedTargetPattern.Sign.NEGATIVE);
+ } else {
+ return create(parser.parse(pattern), SignedTargetPattern.Sign.POSITIVE);
+ }
+ }
+}
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 4015c1a..536c0f7 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
@@ -31,6 +31,7 @@
import com.google.devtools.build.lib.cmdline.LabelValidator.PackageAndTarget;
import com.google.devtools.build.lib.concurrent.BatchCallback;
import com.google.devtools.build.lib.server.FailureDetails.TargetPatterns;
+import com.google.devtools.build.lib.server.FailureDetails.TargetPatterns.Code;
import com.google.devtools.build.lib.supplier.InterruptibleSupplier;
import com.google.devtools.build.lib.util.StringUtilities;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -63,19 +64,28 @@
private static final Splitter SLASH_SPLITTER = Splitter.on('/');
private static final Joiner SLASH_JOINER = Joiner.on('/');
- private static final Parser DEFAULT_PARSER = new Parser(PathFragment.EMPTY_FRAGMENT);
+ private static final Parser DEFAULT_PARSER = mainRepoParser(PathFragment.EMPTY_FRAGMENT);
private final String originalPattern;
- private final PathFragment offset;
/**
- * Returns a parser with no offset. Note that the Parser class is immutable, so this method may
- * return the same instance on subsequent calls.
+ * Returns a parser defaulting to the main repo, with no offset or repo mapping. Note that the
+ * Parser class is immutable, so this method may return the same instance on subsequent calls.
*/
public static Parser defaultParser() {
return DEFAULT_PARSER;
}
+ /**
+ * Returns a parser defaulting to the main repo, with repo mapping, but using the given offset.
+ */
+ // NOTE(wyv): This is only strictly correct within a monorepo. If external repos exist, there
+ // should always be a proper repo mapping. We should audit calls to this function and add a repo
+ // mapping wherever appropriate.
+ public static Parser mainRepoParser(PathFragment offset) {
+ return new Parser(offset, RepositoryName.MAIN, RepositoryMapping.ALWAYS_FALLBACK);
+ }
+
private static String removeSuffix(String s, String suffix) {
if (s.endsWith(suffix)) {
return s.substring(0, s.length() - suffix.length());
@@ -119,10 +129,9 @@
return SLASH_JOINER.join(pieces);
}
- private TargetPattern(String originalPattern, PathFragment offset) {
+ private TargetPattern(String originalPattern) {
// Don't allow inheritance outside this class.
this.originalPattern = Preconditions.checkNotNull(originalPattern);
- this.offset = Preconditions.checkNotNull(offset);
}
/**
@@ -138,11 +147,6 @@
return originalPattern;
}
- /** Returns the offset this target pattern was parsed with. */
- public PathFragment getOffset() {
- return offset;
- }
-
/**
* Evaluates the current target pattern, excluding targets under directories in both {@code
* ignoredSubdirectories} and {@code excludedSubdirectories}, and returns the result.
@@ -163,7 +167,7 @@
* Evaluates this {@link TargetPattern} synchronously, feeding the result to the given {@code
* callback}, and then returns an appropriate immediate {@link ListenableFuture}.
*
- * <p>If the returned {@link ListenableFuture}'s {@link ListenableFuture#get} throws an {@link
+ * <p>If the returned {@link ListenableFuture}'s {@link ListenableFuture#get} throws an {@code
* ExecutionException}, the cause will be an instance of either {@link TargetParsingException} or
* the given {@code exceptionClass}.
*/
@@ -192,7 +196,7 @@
* Returns a {@link ListenableFuture} representing the asynchronous evaluation of this {@link
* TargetPattern} that feeds the results to the given {@code callback}.
*
- * <p>If the returned {@link ListenableFuture}'s {@link ListenableFuture#get} throws an {@link
+ * <p>If the returned {@link ListenableFuture}'s {@link ListenableFuture#get} throws an {@code
* ExecutionException}, the cause will be an instance of either {@link TargetParsingException} or
* the given {@code exceptionClass}.
*/
@@ -256,12 +260,8 @@
private final String targetName;
private final PackageIdentifier directory;
- private SingleTarget(
- String targetName,
- PackageIdentifier directory,
- String originalPattern,
- PathFragment offset) {
- super(originalPattern, offset);
+ private SingleTarget(String targetName, PackageIdentifier directory, String originalPattern) {
+ super(originalPattern);
this.targetName = Preconditions.checkNotNull(targetName);
this.directory = Preconditions.checkNotNull(directory);
}
@@ -323,8 +323,8 @@
private static final class InterpretPathAsTarget extends TargetPattern {
private final String path;
- private InterpretPathAsTarget(String path, String originalPattern, PathFragment offset) {
- super(originalPattern, offset);
+ private InterpretPathAsTarget(String path, String originalPattern) {
+ super(originalPattern);
this.path = normalize(Preconditions.checkNotNull(path));
}
@@ -412,13 +412,12 @@
private TargetsInPackage(
String originalPattern,
- PathFragment offset,
PackageIdentifier packageIdentifier,
String suffix,
boolean wasOriginallyAbsolute,
boolean rulesOnly,
boolean checkWildcardConflict) {
- super(originalPattern, offset);
+ super(originalPattern);
Preconditions.checkArgument(!packageIdentifier.getRepository().isDefault());
this.packageIdentifier = packageIdentifier;
this.suffix = Preconditions.checkNotNull(suffix);
@@ -476,16 +475,24 @@
return false;
}
TargetsInPackage that = (TargetsInPackage) o;
- return wasOriginallyAbsolute == that.wasOriginallyAbsolute && rulesOnly == that.rulesOnly
+ return wasOriginallyAbsolute == that.wasOriginallyAbsolute
+ && rulesOnly == that.rulesOnly
&& checkWildcardConflict == that.checkWildcardConflict
&& getOriginalPattern().equals(that.getOriginalPattern())
- && packageIdentifier.equals(that.packageIdentifier) && suffix.equals(that.suffix);
+ && packageIdentifier.equals(that.packageIdentifier)
+ && suffix.equals(that.suffix);
}
@Override
public int hashCode() {
- return Objects.hash(getType(), getOriginalPattern(), packageIdentifier, suffix,
- wasOriginallyAbsolute, rulesOnly, checkWildcardConflict);
+ return Objects.hash(
+ getType(),
+ getOriginalPattern(),
+ packageIdentifier,
+ suffix,
+ wasOriginallyAbsolute,
+ rulesOnly,
+ checkWildcardConflict);
}
/**
@@ -538,11 +545,8 @@
private final boolean rulesOnly;
private TargetsBelowDirectory(
- String originalPattern,
- PathFragment offset,
- PackageIdentifier directory,
- boolean rulesOnly) {
- super(originalPattern, offset);
+ String originalPattern, PackageIdentifier directory, boolean rulesOnly) {
+ super(originalPattern);
Preconditions.checkArgument(!directory.getRepository().isDefault());
this.directory = Preconditions.checkNotNull(directory);
this.rulesOnly = rulesOnly;
@@ -800,7 +804,8 @@
return false;
}
TargetsBelowDirectory that = (TargetsBelowDirectory) o;
- return rulesOnly == that.rulesOnly && getOriginalPattern().equals(that.getOriginalPattern())
+ return rulesOnly == that.rulesOnly
+ && getOriginalPattern().equals(that.getOriginalPattern())
&& directory.equals(that.directory);
}
@@ -810,42 +815,6 @@
}
}
- /**
- * Apply a renaming to the repository part of a pattern string, returning the renamed pattern
- * string. This function only looks at the repository part of the pattern string, not the rest; so
- * any syntactic errors will not be handled here, but simply remain. Similarly, if the repository
- * part of the pattern is not syntactically valid, the renaming simply does not match and the
- * string is returned unchanged.
- */
- public static String renameRepository(String pattern, RepositoryMapping renaming) {
- if (pattern.startsWith("-")) {
- return "-" + renameRepositoryInternal(pattern.substring(1), renaming);
- }
- return renameRepositoryInternal(pattern, renaming);
- }
-
- private static String renameRepositoryInternal(String pattern, RepositoryMapping renaming) {
- if (!pattern.startsWith("@")) {
- return pattern;
- }
- int pkgStart = pattern.indexOf("//");
- if (pkgStart < 0) {
- return pattern;
- }
- RepositoryName repository;
- try {
- repository = RepositoryName.create(pattern.substring(0, pkgStart));
- } catch (LabelSyntaxException e) {
- return pattern;
- }
- RepositoryName newRepository = renaming.get(repository);
- if (newRepository == null) {
- // No renaming required
- return pattern;
- }
- return newRepository.getName() + pattern.substring(pkgStart);
- }
-
@Immutable
public static final class Parser {
// A valid pattern either starts with exactly 0 slashes (relative pattern) or exactly two
@@ -902,9 +871,19 @@
*/
private final PathFragment relativeDirectory;
+ // The repo to use for any repo-relative target patterns (so "//foo" becomes
+ // "@currentRepo//foo").
+ private final RepositoryName currentRepo;
+
+ // The repo mapping to use for the @repo part of target patterns.
+ private final RepositoryMapping repoMapping;
+
/** Creates a new parser with the given offset for relative patterns. */
- public Parser(PathFragment relativeDirectory) {
+ public Parser(
+ PathFragment relativeDirectory, RepositoryName currentRepo, RepositoryMapping repoMapping) {
this.relativeDirectory = relativeDirectory;
+ this.currentRepo = currentRepo;
+ this.repoMapping = repoMapping;
}
/**
@@ -919,18 +898,27 @@
String originalPattern = pattern;
final boolean includesRepo = pattern.startsWith("@");
- RepositoryName repository = null;
- if (includesRepo) {
+ RepositoryName repository;
+ if (!includesRepo) {
+ repository = currentRepo;
+ } else {
int pkgStart = pattern.indexOf("//");
if (pkgStart < 0) {
throw new TargetParsingException(
"Couldn't find package in target " + pattern, TargetPatterns.Code.PACKAGE_NOT_FOUND);
}
try {
- repository = RepositoryName.create(pattern.substring(0, pkgStart));
+ repository = repoMapping.get(RepositoryName.create(pattern.substring(0, pkgStart)));
} catch (LabelSyntaxException e) {
throw new TargetParsingException(e.getMessage(), TargetPatterns.Code.LABEL_SYNTAX_ERROR);
}
+ if (!repository.isVisible()) {
+ throw new TargetParsingException(
+ String.format(
+ "%s is not visible from %s",
+ repository.getName(), repository.getOwnerRepoIfNotVisible()),
+ Code.PACKAGE_NOT_FOUND);
+ }
pattern = pattern.substring(pkgStart);
}
@@ -968,10 +956,6 @@
TargetPatterns.Code.PACKAGE_PART_CANNOT_END_IN_SLASH);
}
- if (repository == null) {
- repository = RepositoryName.MAIN;
- }
-
if (packagePart.endsWith("/...")) {
String realPackagePart = removeSuffix(packagePart, "/...");
PackageIdentifier packageIdentifier;
@@ -984,11 +968,9 @@
TargetPatterns.Code.LABEL_SYNTAX_ERROR);
}
if (targetPart.isEmpty() || ALL_RULES_IN_SUFFIXES.contains(targetPart)) {
- return new TargetsBelowDirectory(
- originalPattern, relativeDirectory, packageIdentifier, true);
+ return new TargetsBelowDirectory(originalPattern, packageIdentifier, true);
} else if (ALL_TARGETS_IN_SUFFIXES.contains(targetPart)) {
- return new TargetsBelowDirectory(
- originalPattern, relativeDirectory, packageIdentifier, false);
+ return new TargetsBelowDirectory(originalPattern, packageIdentifier, false);
}
}
@@ -1001,8 +983,8 @@
"Invalid package name '" + packagePart + "': " + e.getMessage(),
TargetPatterns.Code.LABEL_SYNTAX_ERROR);
}
- return new TargetsInPackage(originalPattern, relativeDirectory, packageIdentifier,
- targetPart, wasOriginallyAbsolute, true, true);
+ return new TargetsInPackage(
+ originalPattern, packageIdentifier, targetPart, wasOriginallyAbsolute, true, true);
}
if (ALL_TARGETS_IN_SUFFIXES.contains(targetPart)) {
@@ -1014,8 +996,8 @@
"Invalid package name '" + packagePart + "': " + e.getMessage(),
TargetPatterns.Code.LABEL_SYNTAX_ERROR);
}
- return new TargetsInPackage(originalPattern, relativeDirectory, packageIdentifier,
- targetPart, wasOriginallyAbsolute, false, true);
+ return new TargetsInPackage(
+ originalPattern, packageIdentifier, targetPart, wasOriginallyAbsolute, false, true);
}
if (includesRepo || wasOriginallyAbsolute || pattern.contains(":")) {
@@ -1030,7 +1012,7 @@
String error = "invalid target format '" + originalPattern + "': " + e.getMessage();
throw new TargetParsingException(error, TargetPatterns.Code.TARGET_FORMAT_INVALID);
}
- return new SingleTarget(fullLabel, packageIdentifier, originalPattern, relativeDirectory);
+ return new SingleTarget(fullLabel, packageIdentifier, originalPattern);
}
// This is a stripped-down version of interpretPathAsTarget that does no I/O. We have a basic
@@ -1050,7 +1032,7 @@
"Bad target pattern '" + originalPattern + "': " + e.getMessage(),
TargetPatterns.Code.LABEL_SYNTAX_ERROR);
}
- return new InterpretPathAsTarget(pattern, originalPattern, relativeDirectory);
+ return new InterpretPathAsTarget(pattern, originalPattern);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index 77b7298..25c5112 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -34,6 +34,7 @@
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.collect.CollectionUtils;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
import com.google.devtools.build.lib.events.Event;
@@ -238,8 +239,8 @@
private ImmutableSet<String> features;
- private ImmutableList<String> registeredExecutionPlatforms;
- private ImmutableList<String> registeredToolchains;
+ private ImmutableList<TargetPattern> registeredExecutionPlatforms;
+ private ImmutableList<TargetPattern> registeredToolchains;
private long computationSteps;
@@ -801,11 +802,11 @@
return defaultRestrictedTo;
}
- public ImmutableList<String> getRegisteredExecutionPlatforms() {
+ public ImmutableList<TargetPattern> getRegisteredExecutionPlatforms() {
return registeredExecutionPlatforms;
}
- public ImmutableList<String> getRegisteredToolchains() {
+ public ImmutableList<TargetPattern> getRegisteredToolchains() {
return registeredToolchains;
}
@@ -994,8 +995,8 @@
private ImmutableList<Label> starlarkFileDependencies = ImmutableList.of();
- private final List<String> registeredExecutionPlatforms = new ArrayList<>();
- private final List<String> registeredToolchains = new ArrayList<>();
+ private final List<TargetPattern> registeredExecutionPlatforms = new ArrayList<>();
+ private final List<TargetPattern> registeredToolchains = new ArrayList<>();
private ThirdPartyLicenseExistencePolicy thirdPartyLicenceExistencePolicy =
ThirdPartyLicenseExistencePolicy.USER_CONTROLLABLE;
@@ -1730,11 +1731,11 @@
ruleLabels.put(rule, labels);
}
- void addRegisteredExecutionPlatforms(List<String> platforms) {
+ void addRegisteredExecutionPlatforms(List<TargetPattern> platforms) {
this.registeredExecutionPlatforms.addAll(platforms);
}
- void addRegisteredToolchains(List<String> toolchains) {
+ void addRegisteredToolchains(List<TargetPattern> toolchains) {
this.registeredToolchains.addAll(toolchains);
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
index 06c1180..6e27b10 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
@@ -23,6 +23,8 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.cmdline.TargetParsingException;
+import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
import java.util.Map;
@@ -56,7 +58,18 @@
Label nameLabel = Label.parseAbsolute("//external:" + entry.getKey(), ImmutableMap.of());
addBindRule(pkg, bindRuleClass, nameLabel, entry.getValue(), semantics, callstack);
}
- pkg.addRegisteredToolchains(ruleClass.getToolchainsToRegisterFunction().apply(rule));
+ // NOTE(wyv): What is this madness?? This is the only instance where a repository rule can
+ // register toolchains upon being instantiated. We should look into converting
+ // android_{s,n}dk_repository into module extensions.
+ ImmutableList.Builder<TargetPattern> toolchains = new ImmutableList.Builder<>();
+ for (String pattern : ruleClass.getToolchainsToRegisterFunction().apply(rule)) {
+ try {
+ toolchains.add(TargetPattern.defaultParser().parse(pattern));
+ } catch (TargetParsingException e) {
+ throw new LabelSyntaxException(e.getMessage());
+ }
+ }
+ pkg.addRegisteredToolchains(toolchains.build());
return rule;
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
index dc4eb35..a531eb2 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
@@ -27,6 +27,7 @@
import com.google.devtools.build.lib.cmdline.LabelValidator;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
@@ -232,15 +233,23 @@
return label.getRepository();
}
- private static ImmutableList<String> renamePatterns(
- List<String> patterns, Package.Builder builder, StarlarkThread thread) {
+ private static ImmutableList<TargetPattern> parsePatterns(
+ List<String> patterns, Package.Builder builder, StarlarkThread thread) throws EvalException {
BazelModuleContext bzlModule =
BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread));
RepositoryName myName = getRepositoryName((bzlModule != null ? bzlModule.label() : null));
RepositoryMapping renaming = builder.getRepositoryMappingFor(myName);
- return patterns.stream()
- .map(patternEntry -> TargetPattern.renameRepository(patternEntry, renaming))
- .collect(ImmutableList.toImmutableList());
+ TargetPattern.Parser parser =
+ new TargetPattern.Parser(PathFragment.EMPTY_FRAGMENT, myName, renaming);
+ ImmutableList.Builder<TargetPattern> parsedPatterns = ImmutableList.builder();
+ for (String pattern : patterns) {
+ try {
+ parsedPatterns.add(parser.parse(pattern));
+ } catch (TargetParsingException e) {
+ throw Starlark.errorf("error parsing target pattern \"%s\": %s", pattern, e.getMessage());
+ }
+ }
+ return parsedPatterns.build();
}
@Override
@@ -249,7 +258,7 @@
// Add to the package definition for later.
Package.Builder builder = PackageFactory.getContext(thread).pkgBuilder;
List<String> patterns = Sequence.cast(platformLabels, String.class, "platform_labels");
- builder.addRegisteredExecutionPlatforms(renamePatterns(patterns, builder, thread));
+ builder.addRegisteredExecutionPlatforms(parsePatterns(patterns, builder, thread));
}
@Override
@@ -258,7 +267,7 @@
// Add to the package definition for later.
Package.Builder builder = PackageFactory.getContext(thread).pkgBuilder;
List<String> patterns = Sequence.cast(toolchainLabels, String.class, "toolchain_labels");
- builder.addRegisteredToolchains(renamePatterns(patterns, builder, thread));
+ builder.addRegisteredToolchains(parsePatterns(patterns, builder, thread));
}
@Override
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 327951c..3f21242 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
@@ -41,15 +41,15 @@
return new AndFilteringPolicy(x, y);
}
- public static FilteringPolicy ruleType(String ruleName, boolean keepExplicit) {
- return RuleTypeFilter.create(ruleName, keepExplicit);
+ public static FilteringPolicy ruleTypeExplicit(String ruleName) {
+ return RuleTypeFilter.create(ruleName, /*keepExplicit=*/ true);
}
private FilteringPolicies() {
}
/** Base class for singleton filtering policies. */
- private abstract static class AbstractFilteringPolicy extends FilteringPolicy {
+ private abstract static class AbstractFilteringPolicy implements FilteringPolicy {
private final int hashCode = getClass().getSimpleName().hashCode();
@Override
@@ -109,7 +109,7 @@
/** FilteringPolicy that only matches a specific rule name. */
@AutoValue
@AutoCodec
- abstract static class RuleTypeFilter extends FilteringPolicy {
+ abstract static class RuleTypeFilter implements FilteringPolicy {
abstract String ruleName();
abstract boolean keepExplicit();
@@ -134,7 +134,7 @@
}
/** FilteringPolicy for combining FilteringPolicies. */
- public static class AndFilteringPolicy extends FilteringPolicy {
+ public static class AndFilteringPolicy implements FilteringPolicy {
private final FilteringPolicy firstPolicy;
private final FilteringPolicy secondPolicy;
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/FilteringPolicy.java b/src/main/java/com/google/devtools/build/lib/pkgcache/FilteringPolicy.java
index 3790fbc..edb95b6 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/FilteringPolicy.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/FilteringPolicy.java
@@ -21,7 +21,7 @@
* A filtering policy defines how target patterns are matched. For instance, we may wish to select
* only tests or no tests.
*/
-public abstract class FilteringPolicy implements Serializable {
+public interface FilteringPolicy extends Serializable {
/**
* Returns true if this target should be retained.
@@ -30,5 +30,5 @@
* a wildcard.
*/
@ThreadSafety.ThreadSafe
- public abstract boolean shouldRetain(Target target, boolean explicit);
+ boolean shouldRetain(Target target, boolean explicit);
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/PostAnalysisQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/PostAnalysisQueryEnvironment.java
index fa5fd78..505298c 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/PostAnalysisQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/PostAnalysisQueryEnvironment.java
@@ -69,8 +69,6 @@
import com.google.devtools.build.lib.skyframe.SimplePackageIdentifierBatchingCallback;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
-import com.google.devtools.build.lib.skyframe.TargetPatternValue;
-import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
import com.google.devtools.build.lib.supplier.InterruptibleSupplier;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyKey;
@@ -249,10 +247,7 @@
protected abstract T getValueFromKey(SkyKey key) throws InterruptedException;
protected TargetPattern getPattern(String pattern) throws TargetParsingException {
- TargetPatternKey targetPatternKey =
- ((TargetPatternKey)
- TargetPatternValue.key(pattern, FilteringPolicies.NO_FILTER, parserPrefix).argument());
- return targetPatternKey.getParsedPattern();
+ return TargetPattern.mainRepoParser(parserPrefix).parse(pattern);
}
public ThreadSafeMutableSet<T> getFwdDeps(Iterable<T> targets) 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 677f485..c9247a8 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
@@ -40,6 +40,7 @@
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+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.TargetPatternResolver;
@@ -382,7 +383,7 @@
if (constantUniverseScopeList.size() != 1) {
return QueryExpressionMapper.identity();
}
- TargetPattern.Parser targetPatternParser = new TargetPattern.Parser(parserPrefix);
+ TargetPattern.Parser targetPatternParser = TargetPattern.mainRepoParser(parserPrefix);
String universeScopePatternString = Iterables.getOnlyElement(constantUniverseScopeList);
TargetPattern absoluteUniverseScopePattern = null;
try {
@@ -805,7 +806,10 @@
QueryExpression owner, String pattern, Callback<Target> callback) {
TargetPatternKey targetPatternKey;
try {
- targetPatternKey = TargetPatternValue.key(pattern, FilteringPolicies.NO_FILTER, parserPrefix);
+ targetPatternKey =
+ TargetPatternValue.key(
+ SignedTargetPattern.parse(pattern, TargetPattern.mainRepoParser(parserPrefix)),
+ FilteringPolicies.NO_FILTER);
} catch (TargetParsingException tpe) {
try {
handleError(owner, tpe.getMessage(), tpe.getDetailedExitCode());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
index fc0dff4..fd52445 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
@@ -44,6 +44,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
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.collect.compacthashset.CompactHashSet;
@@ -490,7 +491,8 @@
checkValidPatternType(pattern);
patternKeys.put(
TargetPatternValue.key(
- pattern, FilteringPolicies.NO_FILTER, PathFragment.EMPTY_FRAGMENT),
+ SignedTargetPattern.parse(pattern, TargetPattern.defaultParser()),
+ FilteringPolicies.NO_FILTER),
pattern);
}
Set<SkyKey> packageKeys = new HashSet<>();
@@ -553,8 +555,7 @@
}
private void checkValidPatternType(String pattern) throws TargetParsingException {
- TargetPattern.Type type =
- new TargetPattern.Parser(PathFragment.EMPTY_FRAGMENT).parse(pattern).getType();
+ TargetPattern.Type type = TargetPattern.defaultParser().parse(pattern).getType();
if (type == TargetPattern.Type.PATH_AS_TARGET) {
throw new TargetParsingException(
String.format("couldn't determine target from filename '%s'", pattern),
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;
- }
- }
}
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 eb24477..0e50105 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
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.TargetPattern.TargetsBelowDirectory;
import com.google.devtools.build.lib.cmdline.TargetPattern.TargetsBelowDirectory.ContainsResult;
+import com.google.devtools.build.lib.vfs.PathFragment;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -171,35 +172,26 @@
ImmutableMap.of(
RepositoryName.create("@foo"), RepositoryName.create("@bar"),
RepositoryName.create("@myworkspace"), RepositoryName.create("@")));
+ TargetPattern.Parser parser =
+ new TargetPattern.Parser(
+ PathFragment.EMPTY_FRAGMENT,
+ RepositoryName.createFromValidStrippedName("myrepo"),
+ renaming);
// Expecting renaming
- assertThat(TargetPattern.renameRepository("@foo//package:target", renaming))
- .isEqualTo("@bar//package:target");
- assertThat(TargetPattern.renameRepository("@myworkspace//package:target", renaming))
- .isEqualTo("@//package:target");
- assertThat(TargetPattern.renameRepository("@foo//foo/...", renaming))
- .isEqualTo("@bar//foo/...");
- assertThat(TargetPattern.renameRepository("@myworkspace//foo/...", renaming))
- .isEqualTo("@//foo/...");
-
- // Expecting renaming with the prefix negation operator -
- assertThat(TargetPattern.renameRepository("-@foo//package:target", renaming))
- .isEqualTo("-@bar//package:target");
- assertThat(TargetPattern.renameRepository("-@myworkspace//package:target", renaming))
- .isEqualTo("-@//package:target");
- assertThat(TargetPattern.renameRepository("-@foo//foo/...", renaming))
- .isEqualTo("-@bar//foo/...");
- assertThat(TargetPattern.renameRepository("-@myworkspace//foo/...", renaming))
- .isEqualTo("-@//foo/...");
+ assertThat(parser.parse("@foo//package:target").getRepository().strippedName())
+ .isEqualTo("bar");
+ assertThat(parser.parse("@myworkspace//package:target").getRepository().isMain()).isTrue();
+ assertThat(parser.parse("@foo//foo/...").getRepository().strippedName()).isEqualTo("bar");
+ assertThat(parser.parse("@myworkspace//foo/...").getRepository().isMain()).isTrue();
// No renaming should occur
- assertThat(TargetPattern.renameRepository("@//package:target", renaming))
- .isEqualTo("@//package:target");
- assertThat(TargetPattern.renameRepository("@unrelated//package:target", renaming))
- .isEqualTo("@unrelated//package:target");
- assertThat(TargetPattern.renameRepository("foo/package:target", renaming))
- .isEqualTo("foo/package:target");
- assertThat(TargetPattern.renameRepository("foo/...", renaming)).isEqualTo("foo/...");
+ assertThat(parser.parse("@//package:target").getRepository().isMain()).isTrue();
+ assertThat(parser.parse("@unrelated//package:target").getRepository().strippedName())
+ .isEqualTo("unrelated");
+ assertThat(parser.parse("foo/package:target").getRepository().strippedName())
+ .isEqualTo("myrepo");
+ assertThat(parser.parse("foo/...").getRepository().strippedName()).isEqualTo("myrepo");
}
private static TargetPattern parse(String pattern) throws TargetParsingException {
diff --git a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
index c153fd4..e48acfc 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.vfs.Root;
import org.junit.Before;
@@ -76,14 +77,16 @@
public void testRegisterExecutionPlatforms() throws Exception {
helper.parse("register_execution_platforms('//platform:ep1')");
assertThat(helper.getPackage().getRegisteredExecutionPlatforms())
- .containsExactly("//platform:ep1");
+ .containsExactly(TargetPattern.defaultParser().parse("//platform:ep1"));
}
@Test
public void testRegisterExecutionPlatforms_multipleLabels() throws Exception {
- helper.parse("register_execution_platforms(", " '//platform:ep1',", " '//platform:ep2')");
+ helper.parse("register_execution_platforms('//platform:ep1', '//platform:ep2')");
assertThat(helper.getPackage().getRegisteredExecutionPlatforms())
- .containsExactly("//platform:ep1", "//platform:ep2");
+ .containsExactly(
+ TargetPattern.defaultParser().parse("//platform:ep1"),
+ TargetPattern.defaultParser().parse("//platform:ep2"));
}
@Test
@@ -93,20 +96,26 @@
"register_execution_platforms('//platform:ep2')",
"register_execution_platforms('//platform/...')");
assertThat(helper.getPackage().getRegisteredExecutionPlatforms())
- .containsExactly("//platform:ep1", "//platform:ep2", "//platform/...");
+ .containsExactly(
+ TargetPattern.defaultParser().parse("//platform:ep1"),
+ TargetPattern.defaultParser().parse("//platform:ep2"),
+ TargetPattern.defaultParser().parse("//platform/..."));
}
@Test
public void testRegisterToolchains() throws Exception {
helper.parse("register_toolchains('//toolchain:tc1')");
- assertThat(helper.getPackage().getRegisteredToolchains()).containsExactly("//toolchain:tc1");
+ assertThat(helper.getPackage().getRegisteredToolchains())
+ .containsExactly(TargetPattern.defaultParser().parse("//toolchain:tc1"));
}
@Test
public void testRegisterToolchains_multipleLabels() throws Exception {
- helper.parse("register_toolchains(", " '//toolchain:tc1',", " '//toolchain:tc2')");
+ helper.parse("register_toolchains('//toolchain:tc1', '//toolchain:tc2')");
assertThat(helper.getPackage().getRegisteredToolchains())
- .containsExactly("//toolchain:tc1", "//toolchain:tc2");
+ .containsExactly(
+ TargetPattern.defaultParser().parse("//toolchain:tc1"),
+ TargetPattern.defaultParser().parse("//toolchain:tc2"));
}
@Test
@@ -116,7 +125,10 @@
"register_toolchains('//toolchain:tc2')",
"register_toolchains('//toolchain/...')");
assertThat(helper.getPackage().getRegisteredToolchains())
- .containsExactly("//toolchain:tc1", "//toolchain:tc2", "//toolchain/...");
+ .containsExactly(
+ TargetPattern.defaultParser().parse("//toolchain:tc1"),
+ TargetPattern.defaultParser().parse("//toolchain:tc2"),
+ TargetPattern.defaultParser().parse("//toolchain/..."));
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageHelperTest.java b/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageHelperTest.java
index 3ecf4a8..59fa1cb 100644
--- a/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageHelperTest.java
+++ b/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageHelperTest.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.repository;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.skyframe.EvaluationResultSubjectFactory.assertThatEvaluationResult;
import static org.mockito.Mockito.mock;
@@ -30,6 +31,7 @@
import com.google.devtools.build.lib.analysis.util.AnalysisMock;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.events.NullEventHandler;
import com.google.devtools.build.lib.packages.BuildFileName;
import com.google.devtools.build.lib.packages.Rule;
@@ -413,11 +415,15 @@
@Override
public SkyValue compute(SkyKey skyKey, Environment env)
throws SkyFunctionException, InterruptedException {
- List<String> registeredToolchains = RegisteredToolchainsFunction.getWorkspaceToolchains(env);
+ List<TargetPattern> registeredToolchains =
+ RegisteredToolchainsFunction.getWorkspaceToolchains(env);
if (registeredToolchains == null) {
return null;
}
- return GetRegisteredToolchainsValue.create(registeredToolchains);
+ return GetRegisteredToolchainsValue.create(
+ registeredToolchains.stream()
+ .map(TargetPattern::getOriginalPattern)
+ .collect(toImmutableList()));
}
@Nullable
@@ -447,12 +453,15 @@
@Override
public SkyValue compute(SkyKey skyKey, Environment env)
throws SkyFunctionException, InterruptedException {
- List<String> registeredExecutionPlatforms =
+ List<TargetPattern> registeredExecutionPlatforms =
RegisteredExecutionPlatformsFunction.getWorkspaceExecutionPlatforms(env);
if (registeredExecutionPlatforms == null) {
return null;
}
- return GetRegisteredExecutionPlatformsValue.create(registeredExecutionPlatforms);
+ return GetRegisteredExecutionPlatformsValue.create(
+ registeredExecutionPlatforms.stream()
+ .map(TargetPattern::getOriginalPattern)
+ .collect(toImmutableList()));
}
@Nullable
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunctionTest.java
index e83d783..89b96b5 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunctionTest.java
@@ -193,12 +193,8 @@
assertExecutionPlatformLabels(result.get(executionPlatformsKey))
.containsAtLeast(
- // We would expect:
- // Label.parseAbsoluteUnchecked("@myrepo//platforms:execution_platform_1"),
- // Label.parseAbsoluteUnchecked("@myrepo//platforms:execution_platform_2"))
- // but this is actually:
- Label.parseAbsoluteUnchecked("//platforms:linux"),
- Label.parseAbsoluteUnchecked("//platforms:mac"))
+ Label.parseAbsoluteUnchecked("@myrepo//platforms:execution_platform_1"),
+ Label.parseAbsoluteUnchecked("@myrepo//platforms:execution_platform_2"))
.inOrder();
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunctionTest.java
index b2adebe..71d59c7 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunctionTest.java
@@ -158,23 +158,6 @@
}
@Test
- public void testRegisteredToolchains_invalidPattern() throws Exception {
- rewriteWorkspace("register_toolchains('/:invalid:label:syntax')");
-
- // Request the toolchains.
- SkyKey toolchainsKey = RegisteredToolchainsValue.key(targetConfigKey);
- EvaluationResult<RegisteredToolchainsValue> result =
- requestToolchainsFromSkyframe(toolchainsKey);
- assertThatEvaluationResult(result)
- .hasErrorEntryForKeyThat(toolchainsKey)
- .hasExceptionThat()
- .hasMessageThat()
- .contains(
- "invalid registered toolchain '/:invalid:label:syntax': "
- + "not a valid absolute pattern");
- }
-
- @Test
public void testRegisteredToolchains_notToolchain() throws Exception {
rewriteWorkspace("register_toolchains('//error:not_a_toolchain')");
scratch.file("error/BUILD", "filegroup(name = 'not_a_toolchain')");
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
index 534ff72..0cd0436 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
@@ -430,6 +430,22 @@
}
@Test
+ public void testRegisterToolchainsInvalidPattern() throws Exception {
+ // Test intentionally introduces errors.
+ reporter.removeHandler(failFastHandler);
+
+ // //external:bar:baz is not a legal package.
+ String[] lines = {"register_toolchains('/:invalid:label:syntax')"};
+ createWorkspaceFile(lines);
+
+ SkyKey key = ExternalPackageFunction.key();
+ EvaluationResult<PackageValue> evaluationResult = eval(key);
+ Package pkg = evaluationResult.get(key).getPackage();
+ assertThat(pkg.containsErrors()).isTrue();
+ assertContainsEvent("not a valid absolute pattern");
+ }
+
+ @Test
public void testNoWorkspaceFile() throws Exception {
// Create and immediately delete to make sure we got the right file.
RootedPath workspacePath = createWorkspaceFile();
diff --git a/src/test/shell/bazel/toolchain_test.sh b/src/test/shell/bazel/toolchain_test.sh
index 37e1327..3fce211 100755
--- a/src/test/shell/bazel/toolchain_test.sh
+++ b/src/test/shell/bazel/toolchain_test.sh
@@ -747,7 +747,7 @@
EOF
bazel build //demo:use &> $TEST_log && fail "Build failure expected"
- expect_log "invalid registered toolchain '/:invalid:label:syntax': not a valid absolute pattern"
+ expect_log "error parsing target pattern \"/:invalid:label:syntax\": not a valid absolute pattern"
}
function test_register_toolchain_error_invalid_target() {
