aboutsummaryrefslogtreecommitdiffhomepage
path: root/src
diff options
context:
space:
mode:
authorGravatar John Cater <jcater@google.com>2018-03-12 18:50:56 -0700
committerGravatar Copybara-Service <copybara-piper@google.com>2018-03-12 18:53:23 -0700
commit0084e16b55ad54f7aeeffd6d003ea3506039d957 (patch)
tree306210efed020fe8a7fd8645e383d8fe4e3b2947 /src
parentcf37b4f3e44564a154ea0535efa61c6c673bab52 (diff)
Fix toolchain and execution platform registration to use patterns.
This allows more flexibility in registering toolchains and execution platforms, both in the WORKSPACE and from the command-line. Change-Id: I6fe75507d1a74de74085b7c927fdf093c152b894 PiperOrigin-RevId: 188813688
Diffstat (limited to 'src')
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/PlatformConfiguration.java22
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java15
-rw-r--r--src/main/java/com/google/devtools/build/lib/packages/Package.java29
-rw-r--r--src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java49
-rw-r--r--src/main/java/com/google/devtools/build/lib/pkgcache/FilteringPolicies.java33
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunction.java54
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java64
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java63
-rw-r--r--src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java36
-rw-r--r--src/test/java/com/google/devtools/build/lib/repository/ExternalPackageUtilTest.java34
-rw-r--r--src/test/java/com/google/devtools/build/lib/skyframe/RegisteredExecutionPlatformsFunctionTest.java48
-rw-r--r--src/test/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunctionTest.java70
-rwxr-xr-xsrc/test/shell/bazel/toolchain_test.sh2
13 files changed, 380 insertions, 139 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PlatformConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/PlatformConfiguration.java
index fefc871457..7b94b9379d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/PlatformConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/PlatformConfiguration.java
@@ -34,17 +34,17 @@ import java.util.List;
)
public class PlatformConfiguration extends BuildConfiguration.Fragment {
private final Label hostPlatform;
- private final ImmutableList<Label> extraExecutionPlatforms;
+ private final ImmutableList<String> extraExecutionPlatforms;
private final ImmutableList<Label> targetPlatforms;
- private final ImmutableList<Label> extraToolchains;
+ private final ImmutableList<String> extraToolchains;
private final ImmutableList<Label> enabledToolchainTypes;
@AutoCodec.Instantiator
PlatformConfiguration(
Label hostPlatform,
- ImmutableList<Label> extraExecutionPlatforms,
+ ImmutableList<String> extraExecutionPlatforms,
ImmutableList<Label> targetPlatforms,
- ImmutableList<Label> extraToolchains,
+ ImmutableList<String> extraToolchains,
ImmutableList<Label> enabledToolchainTypes) {
this.hostPlatform = hostPlatform;
this.extraExecutionPlatforms = extraExecutionPlatforms;
@@ -58,8 +58,11 @@ public class PlatformConfiguration extends BuildConfiguration.Fragment {
return hostPlatform;
}
- /** Additional platforms that are available for action execution. */
- public ImmutableList<Label> getExtraExecutionPlatforms() {
+ /**
+ * Target patterns that select additional platforms that will be made available for action
+ * execution.
+ */
+ public ImmutableList<String> getExtraExecutionPlatforms() {
return extraExecutionPlatforms;
}
@@ -68,8 +71,11 @@ public class PlatformConfiguration extends BuildConfiguration.Fragment {
return targetPlatforms;
}
- /** Additional toolchains that should be considered during toolchain resolution. */
- public ImmutableList<Label> getExtraToolchains() {
+ /**
+ * Target patterns that select additional toolchains that will be considered during toolchain
+ * resolution.
+ */
+ public ImmutableList<String> getExtraToolchains() {
return extraToolchains;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java
index 688e418fc4..dd8fff432d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java
@@ -20,6 +20,7 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration.LabelLis
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.common.options.Converters.CommaSeparatedOptionListConverter;
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionDocumentationCategory;
import com.google.devtools.common.options.OptionEffectTag;
@@ -58,16 +59,17 @@ public class PlatformOptions extends FragmentOptions {
@Option(
name = "extra_execution_platforms",
- converter = LabelListConverter.class,
+ converter = CommaSeparatedOptionListConverter.class,
defaultValue = "",
documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
effectTags = {OptionEffectTag.EXECUTION},
help =
- "The labels of platforms that are available as execution platforms to run actions. "
+ "The platforms that are available as execution platforms to run actions. "
+ + "Platforms can be specified by exact target, or as a target pattern. "
+ "These platforms will be considered before those declared in the WORKSPACE file by "
+ "register_execution_platforms()."
)
- public List<Label> extraExecutionPlatforms;
+ public List<String> extraExecutionPlatforms;
@Option(
name = "platforms",
@@ -87,8 +89,8 @@ public class PlatformOptions extends FragmentOptions {
@Option(
name = "extra_toolchains",
- converter = LabelListConverter.class,
defaultValue = "",
+ converter = CommaSeparatedOptionListConverter.class,
documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
effectTags = {
OptionEffectTag.AFFECTS_OUTPUTS,
@@ -96,11 +98,12 @@ public class PlatformOptions extends FragmentOptions {
OptionEffectTag.LOADING_AND_ANALYSIS
},
help =
- "The labels of toolchain rules to be considered during toolchain resolution. "
+ "The toolchain rules to be considered during toolchain resolution. "
+ + "Toolchains can be specified by exact target, or as a target pattern. "
+ "These toolchains will be considered before those declared in the WORKSPACE file by "
+ "register_toolchains()."
)
- public List<Label> extraToolchains;
+ public List<String> extraToolchains;
@Option(
name = "toolchain_resolution_override",
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 03f2de60e0..25c375e6a0 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
@@ -203,8 +203,8 @@ public class Package {
private ImmutableList<Event> events;
private ImmutableList<Postable> posts;
- private ImmutableList<Label> registeredExecutionPlatformLabels;
- private ImmutableList<Label> registeredToolchainLabels;
+ private ImmutableList<String> registeredExecutionPlatforms;
+ private ImmutableList<String> registeredToolchains;
/**
* Package initialization, part 1 of 3: instantiates a new package with the
@@ -340,9 +340,8 @@ public class Package {
this.features = ImmutableSortedSet.copyOf(builder.features);
this.events = ImmutableList.copyOf(builder.events);
this.posts = ImmutableList.copyOf(builder.posts);
- this.registeredExecutionPlatformLabels =
- ImmutableList.copyOf(builder.registeredExecutionPlatformLabels);
- this.registeredToolchainLabels = ImmutableList.copyOf(builder.registeredToolchainLabels);
+ this.registeredExecutionPlatforms = ImmutableList.copyOf(builder.registeredExecutionPlatforms);
+ this.registeredToolchains = ImmutableList.copyOf(builder.registeredToolchains);
}
/**
@@ -659,12 +658,12 @@ public class Package {
return defaultRestrictedTo;
}
- public ImmutableList<Label> getRegisteredExecutionPlatformLabels() {
- return registeredExecutionPlatformLabels;
+ public ImmutableList<String> getRegisteredExecutionPlatforms() {
+ return registeredExecutionPlatforms;
}
- public ImmutableList<Label> getRegisteredToolchainLabels() {
- return registeredToolchainLabels;
+ public ImmutableList<String> getRegisteredToolchains() {
+ return registeredToolchains;
}
@Override
@@ -789,8 +788,8 @@ public class Package {
protected Map<Label, Path> subincludes = null;
protected ImmutableList<Label> skylarkFileDependencies = ImmutableList.of();
- protected List<Label> registeredExecutionPlatformLabels = new ArrayList<>();
- protected List<Label> registeredToolchainLabels = new ArrayList<>();
+ protected List<String> registeredExecutionPlatforms = new ArrayList<>();
+ protected List<String> registeredToolchains = new ArrayList<>();
/**
* True iff the "package" function has already been called in this package.
@@ -1313,12 +1312,12 @@ public class Package {
addRuleUnchecked(rule);
}
- public void addRegisteredExecutionPlatformLabels(List<Label> platforms) {
- this.registeredExecutionPlatformLabels.addAll(platforms);
+ public void addRegisteredExecutionPlatforms(List<String> platforms) {
+ this.registeredExecutionPlatforms.addAll(platforms);
}
- void addRegisteredToolchainLabels(List<Label> toolchains) {
- this.registeredToolchainLabels.addAll(toolchains);
+ void addRegisteredToolchains(List<String> toolchains) {
+ this.registeredToolchains.addAll(toolchains);
}
private Builder beforeBuild(boolean discoverAssumedInputFiles)
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
index 702f6afb25..4e7af5f001 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -53,9 +53,7 @@ import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
import com.google.devtools.build.lib.vfs.Path;
import java.io.File;
-import java.util.ArrayList;
import java.util.HashMap;
-import java.util.List;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -275,8 +273,8 @@ public class WorkspaceFactory {
if (aPackage.containsErrors()) {
builder.setContainsErrors();
}
- builder.addRegisteredExecutionPlatformLabels(aPackage.getRegisteredExecutionPlatformLabels());
- builder.addRegisteredToolchainLabels(aPackage.getRegisteredToolchainLabels());
+ builder.addRegisteredExecutionPlatforms(aPackage.getRegisteredExecutionPlatforms());
+ builder.addRegisteredToolchains(aPackage.getRegisteredToolchains());
for (Rule rule : aPackage.getTargets(Rule.class)) {
try {
// The old rule references another Package instance and we wan't to keep the invariant that
@@ -416,25 +414,10 @@ public class WorkspaceFactory {
SkylarkList<String> platformLabels, Location location, Environment env)
throws EvalException, InterruptedException {
- // Collect the platform labels.
- List<Label> platforms = new ArrayList<>();
- for (String rawLabel : platformLabels.getContents(String.class, "platform_labels")) {
- try {
- platforms.add(Label.parseAbsolute(rawLabel));
- } catch (LabelSyntaxException e) {
- throw new EvalException(
- location,
- String.format(
- "In register_execution_platforms: unable to parse platform label %s: %s",
- rawLabel, e.getMessage()),
- e);
- }
- }
-
// Add to the package definition for later.
- Package.Builder builder =
- PackageFactory.getContext(env, location).pkgBuilder;
- builder.addRegisteredExecutionPlatformLabels(platforms);
+ Package.Builder builder = PackageFactory.getContext(env, location).pkgBuilder;
+ builder.addRegisteredExecutionPlatforms(
+ platformLabels.getContents(String.class, "platform_labels"));
return NONE;
}
@@ -468,26 +451,10 @@ public class WorkspaceFactory {
SkylarkList<String> toolchainLabels, Location location, Environment env)
throws EvalException, InterruptedException {
- // Collect the toolchain labels.
- List<Label> toolchains = new ArrayList<>();
- for (String rawLabel :
- toolchainLabels.getContents(String.class, "toolchain_labels")) {
- try {
- toolchains.add(Label.parseAbsolute(rawLabel));
- } catch (LabelSyntaxException e) {
- throw new EvalException(
- location,
- String.format(
- "In register_toolchains: unable to parse toolchain label %s: %s",
- rawLabel, e.getMessage()),
- e);
- }
- }
-
// Add to the package definition for later.
- Package.Builder builder =
- PackageFactory.getContext(env, location).pkgBuilder;
- builder.addRegisteredToolchainLabels(toolchains);
+ Package.Builder builder = PackageFactory.getContext(env, location).pkgBuilder;
+ builder.addRegisteredToolchains(
+ toolchainLabels.getContents(String.class, "toolchain_labels"));
return NONE;
}
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 e91439642b..9936f338b0 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
@@ -13,10 +13,12 @@
// limitations under the License.
package com.google.devtools.build.lib.pkgcache;
+import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.TargetUtils;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import java.util.Objects;
/**
@@ -34,6 +36,10 @@ public final class FilteringPolicies {
return new AndFilteringPolicy(x, y);
}
+ public static FilteringPolicy ruleType(String ruleName, boolean keepExplicit) {
+ return RuleTypeFilter.create(ruleName, keepExplicit);
+ }
+
private FilteringPolicies() {
}
@@ -87,6 +93,33 @@ public final class FilteringPolicies {
}
}
+ /** FilteringPolicy that only matches a specific rule name. */
+ @AutoValue
+ @AutoCodec
+ abstract static class RuleTypeFilter extends FilteringPolicy {
+ abstract String ruleName();
+
+ abstract boolean keepExplicit();
+
+ @Override
+ public boolean shouldRetain(Target target, boolean explicit) {
+ if (explicit && keepExplicit()) {
+ return true;
+ }
+
+ if (target.getAssociatedRule().getRuleClass().equals(ruleName())) {
+ return true;
+ }
+
+ return false;
+ }
+
+ @AutoCodec.Instantiator
+ static RuleTypeFilter create(String ruleName, boolean keepExplicit) {
+ return new AutoValue_FilteringPolicies_RuleTypeFilter(ruleName, keepExplicit);
+ }
+ }
+
/** FilteringPolicy for combining FilteringPolicies. */
public static class AndFilteringPolicy extends FilteringPolicy {
private final FilteringPolicy firstPolicy;
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 9837589f01..ad775ca9a5 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
@@ -22,7 +22,9 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.packages.Package;
+import com.google.devtools.build.lib.pkgcache.FilteringPolicies;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ConfiguredValueCreationException;
import com.google.devtools.build.lib.skyframe.ToolchainUtil.InvalidPlatformException;
import com.google.devtools.build.skyframe.SkyFunction;
@@ -51,26 +53,39 @@ public class RegisteredExecutionPlatformsFunction implements SkyFunction {
}
BuildConfiguration configuration = buildConfigurationValue.getConfiguration();
- ImmutableList.Builder<Label> registeredExecutionPlatformLabels = new ImmutableList.Builder<>();
+ ImmutableList.Builder<String> targetPatterns = new ImmutableList.Builder<>();
// Get the execution platforms from the configuration.
PlatformConfiguration platformConfiguration =
configuration.getFragment(PlatformConfiguration.class);
if (platformConfiguration != null) {
- registeredExecutionPlatformLabels.addAll(platformConfiguration.getExtraExecutionPlatforms());
+ targetPatterns.addAll(platformConfiguration.getExtraExecutionPlatforms());
}
// Get the registered execution platforms from the WORKSPACE.
- List<Label> workspaceExecutionPlatforms = getWorkspaceExecutionPlatforms(env);
+ List<String> workspaceExecutionPlatforms = getWorkspaceExecutionPlatforms(env);
if (workspaceExecutionPlatforms == null) {
return null;
}
- registeredExecutionPlatformLabels.addAll(workspaceExecutionPlatforms);
+ targetPatterns.addAll(workspaceExecutionPlatforms);
+
+ // Expand target patterns.
+ ImmutableList<Label> platformLabels;
+ try {
+ platformLabels =
+ ToolchainUtil.expandTargetPatterns(
+ env, targetPatterns.build(), FilteringPolicies.ruleType("platform", true));
+ if (env.valuesMissing()) {
+ return null;
+ }
+ } catch (ToolchainUtil.InvalidTargetPatternException e) {
+ throw new RegisteredExecutionPlatformsFunctionException(
+ new InvalidExecutionPlatformLabelException(e), Transience.PERSISTENT);
+ }
// Load the configured target for each, and get the declared execution platforms providers.
ImmutableList<ConfiguredTargetKey> registeredExecutionPlatformKeys =
- configureRegisteredExecutionPlatforms(
- env, configuration, registeredExecutionPlatformLabels.build());
+ configureRegisteredExecutionPlatforms(env, configuration, platformLabels);
if (env.valuesMissing()) {
return null;
}
@@ -85,7 +100,7 @@ public class RegisteredExecutionPlatformsFunction implements SkyFunction {
*/
@Nullable
@VisibleForTesting
- public static List<Label> getWorkspaceExecutionPlatforms(Environment env)
+ public static List<String> getWorkspaceExecutionPlatforms(Environment env)
throws InterruptedException {
PackageValue externalPackageValue =
(PackageValue) env.getValue(PackageValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER));
@@ -94,7 +109,7 @@ public class RegisteredExecutionPlatformsFunction implements SkyFunction {
}
Package externalPackage = externalPackageValue.getPackage();
- return externalPackage.getRegisteredExecutionPlatformLabels();
+ return externalPackage.getRegisteredExecutionPlatforms();
}
private ImmutableList<ConfiguredTargetKey> configureRegisteredExecutionPlatforms(
@@ -149,12 +164,35 @@ public class RegisteredExecutionPlatformsFunction implements SkyFunction {
}
/**
+ * Used to indicate that the given {@link Label} represents a {@link ConfiguredTarget} which is
+ * not a valid {@link PlatformInfo} provider.
+ */
+ static final class InvalidExecutionPlatformLabelException extends Exception {
+
+ public InvalidExecutionPlatformLabelException(ToolchainUtil.InvalidTargetPatternException e) {
+ this(e.getInvalidPattern(), e.getTpe());
+ }
+
+ public InvalidExecutionPlatformLabelException(String invalidPattern, TargetParsingException e) {
+ super(
+ String.format(
+ "invalid registered execution platform '%s': %s", invalidPattern, e.getMessage()),
+ e);
+ }
+ }
+
+ /**
* Used to declare all the exception types that can be wrapped in the exception thrown by {@link
* #compute}.
*/
private static class RegisteredExecutionPlatformsFunctionException extends SkyFunctionException {
private RegisteredExecutionPlatformsFunctionException(
+ InvalidExecutionPlatformLabelException cause, Transience transience) {
+ super(cause, transience);
+ }
+
+ private RegisteredExecutionPlatformsFunctionException(
InvalidPlatformException cause, Transience transience) {
super(cause, transience);
}
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 d6a8bfe1dd..5d3b580853 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
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.skyframe;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -21,7 +23,9 @@ import com.google.devtools.build.lib.analysis.PlatformConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.platform.DeclaredToolchainInfo;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.packages.Package;
+import com.google.devtools.build.lib.pkgcache.FilteringPolicies;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ConfiguredValueCreationException;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
@@ -51,22 +55,36 @@ public class RegisteredToolchainsFunction implements SkyFunction {
}
BuildConfiguration configuration = buildConfigurationValue.getConfiguration();
- ImmutableList.Builder<Label> registeredToolchainLabels = new ImmutableList.Builder<>();
+ ImmutableList.Builder<String> targetPatterns = new ImmutableList.Builder<>();
// Get the toolchains from the configuration.
PlatformConfiguration platformConfiguration =
configuration.getFragment(PlatformConfiguration.class);
- registeredToolchainLabels.addAll(platformConfiguration.getExtraToolchains());
+ targetPatterns.addAll(platformConfiguration.getExtraToolchains());
// Get the registered toolchains from the WORKSPACE.
- registeredToolchainLabels.addAll(getWorkspaceToolchains(env));
+ targetPatterns.addAll(getWorkspaceToolchains(env));
if (env.valuesMissing()) {
return null;
}
+ // Expand target patterns.
+ ImmutableList<Label> toolchainLabels;
+ try {
+ toolchainLabels =
+ ToolchainUtil.expandTargetPatterns(
+ env, targetPatterns.build(), FilteringPolicies.ruleType("toolchain", true));
+ if (env.valuesMissing()) {
+ return null;
+ }
+ } catch (ToolchainUtil.InvalidTargetPatternException e) {
+ throw new RegisteredToolchainsFunctionException(
+ new InvalidToolchainLabelException(e), Transience.PERSISTENT);
+ }
+
// Load the configured target for each, and get the declared toolchain providers.
ImmutableList<DeclaredToolchainInfo> registeredToolchains =
- configureRegisteredToolchains(env, configuration, registeredToolchainLabels.build());
+ configureRegisteredToolchains(env, configuration, toolchainLabels);
if (env.valuesMissing()) {
return null;
}
@@ -74,23 +92,23 @@ public class RegisteredToolchainsFunction implements SkyFunction {
return RegisteredToolchainsValue.create(registeredToolchains);
}
- private Iterable<? extends Label> getWorkspaceToolchains(Environment env)
+ private Iterable<? extends String> getWorkspaceToolchains(Environment env)
throws InterruptedException {
- List<Label> labels = getRegisteredToolchainLabels(env);
- if (labels == null) {
+ List<String> patterns = getRegisteredToolchains(env);
+ if (patterns == null) {
return ImmutableList.of();
}
- return labels;
+ return patterns;
}
/**
- * Loads the external package and then returns the registered toolchain labels.
+ * Loads the external package and then returns the registered toolchains.
*
* @param env the environment to use for lookups
*/
- @Nullable @VisibleForTesting
- public static List<Label> getRegisteredToolchainLabels(Environment env)
- throws InterruptedException {
+ @Nullable
+ @VisibleForTesting
+ public static List<String> getRegisteredToolchains(Environment env) throws InterruptedException {
PackageValue externalPackageValue =
(PackageValue) env.getValue(PackageValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER));
if (externalPackageValue == null) {
@@ -98,7 +116,7 @@ public class RegisteredToolchainsFunction implements SkyFunction {
}
Package externalPackage = externalPackageValue.getPackage();
- return externalPackage.getRegisteredToolchainLabels();
+ return externalPackage.getRegisteredToolchains();
}
private ImmutableList<DeclaredToolchainInfo> configureRegisteredToolchains(
@@ -108,7 +126,7 @@ public class RegisteredToolchainsFunction implements SkyFunction {
labels
.stream()
.map(label -> ConfiguredTargetKey.of(label, configuration))
- .collect(ImmutableList.toImmutableList());
+ .collect(toImmutableList());
Map<SkyKey, ValueOrException<ConfiguredValueCreationException>> values =
env.getValuesOrThrow(keys, ConfiguredValueCreationException.class);
@@ -156,25 +174,27 @@ public class RegisteredToolchainsFunction implements SkyFunction {
*/
public static final class InvalidToolchainLabelException extends Exception {
- private final Label invalidLabel;
-
public InvalidToolchainLabelException(Label invalidLabel) {
super(
String.format(
"invalid registered toolchain '%s': "
+ "target does not provide the DeclaredToolchainInfo provider",
invalidLabel));
- this.invalidLabel = invalidLabel;
}
- public InvalidToolchainLabelException(Label invalidLabel, ConfiguredValueCreationException e) {
+ public InvalidToolchainLabelException(ToolchainUtil.InvalidTargetPatternException e) {
+ this(e.getInvalidPattern(), e.getTpe());
+ }
+
+ public InvalidToolchainLabelException(String invalidPattern, TargetParsingException e) {
super(
- String.format("invalid registered toolchain '%s': %s", invalidLabel, e.getMessage()), e);
- this.invalidLabel = invalidLabel;
+ String.format("invalid registered toolchain '%s': %s", invalidPattern, e.getMessage()),
+ e);
}
- public Label getInvalidLabel() {
- return invalidLabel;
+ public InvalidToolchainLabelException(Label invalidLabel, ConfiguredValueCreationException e) {
+ super(
+ String.format("invalid registered toolchain '%s': %s", invalidLabel, e.getMessage()), e);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java
index 3b84c81353..e2317890e7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java
@@ -31,7 +31,9 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ConfiguredValueCreationException;
import com.google.devtools.build.lib.skyframe.RegisteredToolchainsFunction.InvalidToolchainLabelException;
import com.google.devtools.build.lib.skyframe.ToolchainResolutionFunction.NoToolchainFoundException;
@@ -377,6 +379,67 @@ public class ToolchainUtil {
toolchains);
}
+ @Nullable
+ static ImmutableList<Label> expandTargetPatterns(
+ Environment env, List<String> targetPatterns, FilteringPolicy filteringPolicy)
+ throws InvalidTargetPatternException, InterruptedException {
+
+ // First parse the patterns, and throw any errors immediately.
+ List<TargetPatternValue.TargetPatternKey> patternKeys = new ArrayList<>();
+ for (TargetPatternValue.TargetPatternSkyKeyOrException keyOrException :
+ TargetPatternValue.keys(targetPatterns, filteringPolicy, "")) {
+
+ try {
+ patternKeys.add(keyOrException.getSkyKey());
+ } catch (TargetParsingException e) {
+ throw new InvalidTargetPatternException(keyOrException.getOriginalPattern(), e);
+ }
+ }
+
+ // Then, resolve the patterns.
+ ImmutableList.Builder<Label> labels = new ImmutableList.Builder<>();
+ Map<SkyKey, ValueOrException<TargetParsingException>> resolvedPatterns =
+ env.getValuesOrThrow(patternKeys, TargetParsingException.class);
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ for (TargetPatternValue.TargetPatternKey pattern : patternKeys) {
+ TargetPatternValue value;
+ try {
+ value = (TargetPatternValue) resolvedPatterns.get(pattern).get();
+ labels.addAll(value.getTargets().getTargets());
+ } catch (TargetParsingException e) {
+ throw new InvalidTargetPatternException(pattern.getPattern(), e);
+ }
+ }
+
+ return labels.build();
+ }
+
+ /**
+ * Exception used when an error occurs in {@link #expandTargetPatterns(Environment, List,
+ * FilteringPolicy)}.
+ */
+ static final class InvalidTargetPatternException extends Exception {
+ private String invalidPattern;
+ private TargetParsingException tpe;
+
+ public InvalidTargetPatternException(String invalidPattern, TargetParsingException tpe) {
+ super(tpe);
+ this.invalidPattern = invalidPattern;
+ this.tpe = tpe;
+ }
+
+ public String getInvalidPattern() {
+ return invalidPattern;
+ }
+
+ public TargetParsingException getTpe() {
+ return tpe;
+ }
+ }
+
/** Exception used when a platform label is not a valid platform. */
static final class InvalidPlatformException extends Exception {
InvalidPlatformException(Label label) {
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 934308992a..8531dec5e0 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
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.packages;
import static com.google.common.truth.Truth.assertThat;
-import com.google.devtools.build.lib.cmdline.Label;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -73,17 +72,16 @@ public class WorkspaceFactoryTest {
@Test
public void testRegisterExecutionPlatforms() throws Exception {
WorkspaceFactoryTestHelper helper = parse("register_execution_platforms('//platform:ep1')");
- assertThat(helper.getPackage().getRegisteredExecutionPlatformLabels())
- .containsExactly(Label.parseAbsolute("//platform:ep1"));
+ assertThat(helper.getPackage().getRegisteredExecutionPlatforms())
+ .containsExactly("//platform:ep1");
}
@Test
public void testRegisterExecutionPlatforms_multipleLabels() throws Exception {
WorkspaceFactoryTestHelper helper =
parse("register_execution_platforms(", " '//platform:ep1',", " '//platform:ep2')");
- assertThat(helper.getPackage().getRegisteredExecutionPlatformLabels())
- .containsExactly(
- Label.parseAbsolute("//platform:ep1"), Label.parseAbsolute("//platform:ep2"));
+ assertThat(helper.getPackage().getRegisteredExecutionPlatforms())
+ .containsExactly("//platform:ep1", "//platform:ep2");
}
@Test
@@ -91,35 +89,35 @@ public class WorkspaceFactoryTest {
WorkspaceFactoryTestHelper helper =
parse(
"register_execution_platforms('//platform:ep1')",
- "register_execution_platforms('//platform:ep2')");
- assertThat(helper.getPackage().getRegisteredExecutionPlatformLabels())
- .containsExactly(
- Label.parseAbsolute("//platform:ep1"), Label.parseAbsolute("//platform:ep2"));
+ "register_execution_platforms('//platform:ep2')",
+ "register_execution_platforms('//platform/...')");
+ assertThat(helper.getPackage().getRegisteredExecutionPlatforms())
+ .containsExactly("//platform:ep1", "//platform:ep2", "//platform/...");
}
@Test
public void testRegisterToolchains() throws Exception {
WorkspaceFactoryTestHelper helper = parse("register_toolchains('//toolchain:tc1')");
- assertThat(helper.getPackage().getRegisteredToolchainLabels())
- .containsExactly(Label.parseAbsolute("//toolchain:tc1"));
+ assertThat(helper.getPackage().getRegisteredToolchains()).containsExactly("//toolchain:tc1");
}
@Test
public void testRegisterToolchains_multipleLabels() throws Exception {
WorkspaceFactoryTestHelper helper =
parse("register_toolchains(", " '//toolchain:tc1',", " '//toolchain:tc2')");
- assertThat(helper.getPackage().getRegisteredToolchainLabels())
- .containsExactly(
- Label.parseAbsolute("//toolchain:tc1"), Label.parseAbsolute("//toolchain:tc2"));
+ assertThat(helper.getPackage().getRegisteredToolchains())
+ .containsExactly("//toolchain:tc1", "//toolchain:tc2");
}
@Test
public void testRegisterToolchains_multipleCalls() throws Exception {
WorkspaceFactoryTestHelper helper =
- parse("register_toolchains('//toolchain:tc1')", "register_toolchains('//toolchain:tc2')");
- assertThat(helper.getPackage().getRegisteredToolchainLabels())
- .containsExactly(
- Label.parseAbsolute("//toolchain:tc1"), Label.parseAbsolute("//toolchain:tc2"));
+ parse(
+ "register_toolchains('//toolchain:tc1')",
+ "register_toolchains('//toolchain:tc2')",
+ "register_toolchains('//toolchain/...')");
+ assertThat(helper.getPackage().getRegisteredToolchains())
+ .containsExactly("//toolchain:tc1", "//toolchain:tc2", "//toolchain/...");
}
private WorkspaceFactoryTestHelper parse(String... args) {
diff --git a/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageUtilTest.java b/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageUtilTest.java
index 8d07983cd5..fefefea50e 100644
--- a/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageUtilTest.java
+++ b/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageUtilTest.java
@@ -24,7 +24,6 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ServerDirectories;
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.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.events.NullEventHandler;
import com.google.devtools.build.lib.packages.PackageFactory;
@@ -189,9 +188,9 @@ public class ExternalPackageUtilTest extends BuildViewTestCase {
assertThatEvaluationResult(result).hasNoError();
- assertThat(result.get(key).registeredToolchainLabels())
+ assertThat(result.get(key).registeredToolchains())
// There are default toolchains that are always registered, so just check for the ones added
- .containsAllOf(makeLabel("//toolchain:tc1"), makeLabel("//toolchain:tc2"))
+ .containsAllOf("//toolchain:tc1", "//toolchain:tc2")
.inOrder();
}
@@ -206,8 +205,8 @@ public class ExternalPackageUtilTest extends BuildViewTestCase {
assertThatEvaluationResult(result).hasNoError();
- assertThat(result.get(key).registeredExecutionPlatformLabels())
- .containsExactly(makeLabel("//platform:ep1"), makeLabel("//platform:ep2"))
+ assertThat(result.get(key).registeredExecutionPlatforms())
+ .containsExactly("//platform:ep1", "//platform:ep2")
.inOrder();
}
@@ -279,11 +278,11 @@ public class ExternalPackageUtilTest extends BuildViewTestCase {
@AutoValue
abstract static class GetRegisteredToolchainsValue implements SkyValue {
- abstract ImmutableList<Label> registeredToolchainLabels();
+ abstract ImmutableList<String> registeredToolchains();
- static GetRegisteredToolchainsValue create(Iterable<Label> registeredToolchainLabels) {
+ static GetRegisteredToolchainsValue create(Iterable<String> registeredToolchains) {
return new AutoValue_ExternalPackageUtilTest_GetRegisteredToolchainsValue(
- ImmutableList.copyOf(registeredToolchainLabels));
+ ImmutableList.copyOf(registeredToolchains));
}
}
@@ -293,12 +292,11 @@ public class ExternalPackageUtilTest extends BuildViewTestCase {
@Override
public SkyValue compute(SkyKey skyKey, Environment env)
throws SkyFunctionException, InterruptedException {
- List<Label> registeredToolchainLabels =
- RegisteredToolchainsFunction.getRegisteredToolchainLabels(env);
- if (registeredToolchainLabels == null) {
+ List<String> registeredToolchains = RegisteredToolchainsFunction.getRegisteredToolchains(env);
+ if (registeredToolchains == null) {
return null;
}
- return GetRegisteredToolchainsValue.create(registeredToolchainLabels);
+ return GetRegisteredToolchainsValue.create(registeredToolchains);
}
@Nullable
@@ -327,12 +325,12 @@ public class ExternalPackageUtilTest extends BuildViewTestCase {
@AutoValue
abstract static class GetRegisteredExecutionPlatformsValue implements SkyValue {
- abstract ImmutableList<Label> registeredExecutionPlatformLabels();
+ abstract ImmutableList<String> registeredExecutionPlatforms();
static GetRegisteredExecutionPlatformsValue create(
- Iterable<Label> registeredExecutionPlatformLabels) {
+ Iterable<String> registeredExecutionPlatforms) {
return new AutoValue_ExternalPackageUtilTest_GetRegisteredExecutionPlatformsValue(
- ImmutableList.copyOf(registeredExecutionPlatformLabels));
+ ImmutableList.copyOf(registeredExecutionPlatforms));
}
}
@@ -342,12 +340,12 @@ public class ExternalPackageUtilTest extends BuildViewTestCase {
@Override
public SkyValue compute(SkyKey skyKey, Environment env)
throws SkyFunctionException, InterruptedException {
- List<Label> registeredExecutionPlatformLabels =
+ List<String> registeredExecutionPlatforms =
RegisteredExecutionPlatformsFunction.getWorkspaceExecutionPlatforms(env);
- if (registeredExecutionPlatformLabels == null) {
+ if (registeredExecutionPlatforms == null) {
return null;
}
- return GetRegisteredExecutionPlatformsValue.create(registeredExecutionPlatformLabels);
+ return GetRegisteredExecutionPlatformsValue.create(registeredExecutionPlatforms);
}
@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 c00261969c..21d1229436 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
@@ -108,6 +108,54 @@ public class RegisteredExecutionPlatformsFunctionTest extends ToolchainTestCase
}
@Test
+ public void testRegisteredExecutionPlatforms_targetPattern_workspace() throws Exception {
+
+ // Add an extra execution platform.
+ scratch.file(
+ "extra/BUILD",
+ "platform(name = 'execution_platform_1')",
+ "platform(name = 'execution_platform_2')");
+
+ rewriteWorkspace("register_execution_platforms('//extra/...')");
+
+ SkyKey executionPlatformsKey = RegisteredExecutionPlatformsValue.key(targetConfigKey);
+ EvaluationResult<RegisteredExecutionPlatformsValue> result =
+ requestExecutionPlatformsFromSkyframe(executionPlatformsKey);
+ assertThatEvaluationResult(result).hasNoError();
+
+ // Verify that the target registered with the extra_execution_platforms flag is first in the
+ // list.
+ assertExecutionPlatformLabels(result.get(executionPlatformsKey))
+ .containsAllOf(
+ makeLabel("//extra:execution_platform_1"), makeLabel("//extra:execution_platform_2"))
+ .inOrder();
+ }
+
+ @Test
+ public void testRegisteredExecutionPlatforms_targetPattern_flagOverride() throws Exception {
+
+ // Add an extra execution platform.
+ scratch.file(
+ "extra/BUILD",
+ "platform(name = 'execution_platform_1')",
+ "platform(name = 'execution_platform_2')");
+
+ useConfiguration("--extra_execution_platforms=//extra/...");
+
+ SkyKey executionPlatformsKey = RegisteredExecutionPlatformsValue.key(targetConfigKey);
+ EvaluationResult<RegisteredExecutionPlatformsValue> result =
+ requestExecutionPlatformsFromSkyframe(executionPlatformsKey);
+ assertThatEvaluationResult(result).hasNoError();
+
+ // Verify that the target registered with the extra_execution_platforms flag is first in the
+ // list.
+ assertExecutionPlatformLabels(result.get(executionPlatformsKey))
+ .containsAllOf(
+ makeLabel("//extra:execution_platform_1"), makeLabel("//extra:execution_platform_2"))
+ .inOrder();
+ }
+
+ @Test
public void testRegisteredExecutionPlatforms_notExecutionPlatform() throws Exception {
rewriteWorkspace("register_execution_platforms(", " '//error:not_an_execution_platform')");
scratch.file("error/BUILD", "filegroup(name = 'not_an_execution_platform')");
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 d405ef8186..311cd5ad0a 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
@@ -41,7 +41,7 @@ public class RegisteredToolchainsFunctionTest extends ToolchainTestCase {
assertThatEvaluationResult(result).hasEntryThat(toolchainsKey).isNotNull();
RegisteredToolchainsValue value = result.get(toolchainsKey);
- // We have two registered toolchains, and a default for c++
+ // We have two registered toolchains, and a default toolchain for C++.
assertThat(value.registeredToolchains()).hasSize(3);
assertThat(
@@ -124,6 +124,74 @@ public class RegisteredToolchainsFunctionTest extends ToolchainTestCase {
}
@Test
+ public void testRegisteredToolchains_targetPattern_workspace() throws Exception {
+ scratch.appendFile("extra/BUILD", "filegroup(name = 'not_a_platform')");
+ addToolchain(
+ "extra",
+ "extra_toolchain1",
+ ImmutableList.of("//constraints:linux"),
+ ImmutableList.of("//constraints:linux"),
+ "foo");
+ addToolchain(
+ "extra",
+ "extra_toolchain2",
+ ImmutableList.of("//constraints:linux"),
+ ImmutableList.of("//constraints:mac"),
+ "bar");
+ addToolchain(
+ "extra/more",
+ "more_toolchain",
+ ImmutableList.of("//constraints:mac"),
+ ImmutableList.of("//constraints:linux"),
+ "baz");
+ rewriteWorkspace("register_toolchains('//extra/...')");
+
+ SkyKey toolchainsKey = RegisteredToolchainsValue.key(targetConfigKey);
+ EvaluationResult<RegisteredToolchainsValue> result =
+ requestToolchainsFromSkyframe(toolchainsKey);
+ assertThatEvaluationResult(result).hasNoError();
+ assertToolchainLabels(result.get(toolchainsKey))
+ .containsAllOf(
+ makeLabel("//extra:extra_toolchain1_impl"),
+ makeLabel("//extra:extra_toolchain2_impl"),
+ makeLabel("//extra/more:more_toolchain_impl"));
+ }
+
+ @Test
+ public void testRegisteredToolchains_targetPattern_flagOverride() throws Exception {
+ scratch.appendFile("extra/BUILD", "filegroup(name = 'not_a_platform')");
+ addToolchain(
+ "extra",
+ "extra_toolchain1",
+ ImmutableList.of("//constraints:linux"),
+ ImmutableList.of("//constraints:linux"),
+ "foo");
+ addToolchain(
+ "extra",
+ "extra_toolchain2",
+ ImmutableList.of("//constraints:linux"),
+ ImmutableList.of("//constraints:mac"),
+ "bar");
+ addToolchain(
+ "extra/more",
+ "more_toolchain",
+ ImmutableList.of("//constraints:mac"),
+ ImmutableList.of("//constraints:linux"),
+ "baz");
+ useConfiguration("--extra_toolchains=//extra/...");
+
+ SkyKey toolchainsKey = RegisteredToolchainsValue.key(targetConfigKey);
+ EvaluationResult<RegisteredToolchainsValue> result =
+ requestToolchainsFromSkyframe(toolchainsKey);
+ assertThatEvaluationResult(result).hasNoError();
+ assertToolchainLabels(result.get(toolchainsKey))
+ .containsAllOf(
+ makeLabel("//extra:extra_toolchain1_impl"),
+ makeLabel("//extra:extra_toolchain2_impl"),
+ makeLabel("//extra/more:more_toolchain_impl"));
+ }
+
+ @Test
public void testRegisteredToolchains_reload() throws Exception {
rewriteWorkspace("register_toolchains('//toolchain:toolchain_1')");
diff --git a/src/test/shell/bazel/toolchain_test.sh b/src/test/shell/bazel/toolchain_test.sh
index 24abfcc244..f50423b428 100755
--- a/src/test/shell/bazel/toolchain_test.sh
+++ b/src/test/shell/bazel/toolchain_test.sh
@@ -539,7 +539,7 @@ use_toolchain(
EOF
bazel build //demo:use &> $TEST_log && fail "Build failure expected"
- expect_log 'In register_toolchains: unable to parse toolchain label /:invalid:label:syntax'
+ expect_log "invalid registered toolchain '/:invalid:label:syntax': not a valid absolute pattern"
}
function test_register_toolchain_error_invalid_target() {