diff options
Diffstat (limited to 'src')
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() { |