diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index 766cb2db7b2a9e..9b4512f40ee2ca 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.analysis; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Supplier; @@ -197,6 +198,7 @@ public AnalysisResult update( Set multiCpu, ImmutableSet explicitTargetPatterns, List aspects, + ImmutableMap aspectsParameters, AnalysisOptions viewOptions, boolean keepGoing, boolean checkForActionConflicts, @@ -348,7 +350,7 @@ public AnalysisResult update( if (!aspectClasses.isEmpty()) { aspectsKeys.add( AspectKeyCreator.createTopLevelAspectsKey( - aspectClasses, targetSpec.getLabel(), configuration)); + aspectClasses, targetSpec.getLabel(), configuration, aspectsParameters)); } } 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 7cd92bf88468b9..c812f2380e32fc 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 @@ -186,6 +186,7 @@ private static AnalysisAndExecutionResult runAnalysisAndExecutionPhase( multiCpu, explicitTargetPatterns, request.getAspects(), + request.getAspectsParameters(), request.getViewOptions(), request.getKeepGoing(), request.getCheckForActionConflicts(), 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 612494440fc96c..985e8d49b12ec0 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 @@ -220,6 +220,7 @@ private static AnalysisResult runAnalysisPhase( multiCpu, explicitTargetPatterns, request.getAspects(), + request.getAspectsParameters(), request.getViewOptions(), request.getKeepGoing(), request.getCheckForActionConflicts(), diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java index 6f99524d7881ea..497b68c3b9ee0a 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java @@ -13,15 +13,19 @@ // limitations under the License. package com.google.devtools.build.lib.buildtool; +import static com.google.common.collect.ImmutableMap.toImmutableMap; + import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.LoadingCache; import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedSet; import com.google.devtools.build.lib.analysis.AnalysisOptions; import com.google.devtools.build.lib.analysis.OutputGroupInfo; import com.google.devtools.build.lib.analysis.TopLevelArtifactContext; +import com.google.devtools.build.lib.analysis.ViewCreationFailedException; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions; import com.google.devtools.build.lib.exec.ExecutionOptions; @@ -31,6 +35,8 @@ import com.google.devtools.build.lib.runtime.KeepGoingOption; import com.google.devtools.build.lib.runtime.LoadingPhaseThreadsOption; import com.google.devtools.build.lib.runtime.UiOptions; +import com.google.devtools.build.lib.server.FailureDetails.Analysis; +import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.util.OptionsUtils; import com.google.devtools.build.lib.util.io.OutErr; import com.google.devtools.common.options.OptionsBase; @@ -40,6 +46,7 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import javax.annotation.Nullable; /** * A BuildRequest represents a single invocation of the build tool by a user. @@ -383,6 +390,24 @@ public ImmutableList getAspects() { return result.build(); } + @Nullable + public ImmutableMap getAspectsParameters() throws ViewCreationFailedException { + List> aspectsParametersList = getBuildOptions().aspectsParameters; + try { + return aspectsParametersList.stream() + .collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); + } catch (IllegalArgumentException e) { + String errorMessage = "Error in top-level aspects parameters"; + throw new ViewCreationFailedException( + errorMessage, + FailureDetail.newBuilder() + .setMessage(errorMessage) + .setAnalysis(Analysis.newBuilder().setCode(Analysis.Code.ASPECT_CREATION_FAILED)) + .build(), + e); + } + } + /** Whether {@value #VALIDATION_ASPECT_NAME} is in use. */ public boolean useValidationAspect() { return validationMode() == OutputGroupInfo.ValidationMode.ASPECT; diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java index 816c30a3114179..28f081352f3cbe 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java @@ -34,6 +34,7 @@ import com.google.devtools.common.options.OptionsParsingException; import com.google.devtools.common.options.RegexPatternOption; import java.util.List; +import java.util.Map; import javax.annotation.Nullable; /** @@ -318,6 +319,23 @@ public class BuildRequestOptions extends OptionsBase { + " 'my_aspect' is a top-level value from a file tools/my_def.bzl") public List aspects; + @Option( + name = "aspects_parameters", + converter = Converters.AssignmentConverter.class, + defaultValue = "null", + documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + allowMultiple = true, + help = + "Specifies the values of the command-line aspects parameters. Each parameter value is" + + " specified via =, for example 'my_param=my_val' where" + + " 'my_param' is a parameter of some aspect in --aspects list or required by an" + + " aspect in the list. This option can be used multiple times. However, it is not" + + " allowed to assign values to the same parameter more than once. This option is" + + " only be effective under the experimental flag" + + " --experimental_allow_top_level_aspects_parameters.") + public List> aspectsParameters; + public BuildRequestOptions() throws OptionsParsingException {} public String getSymlinkPrefix(String productName) { diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectsListBuilder.java b/src/main/java/com/google/devtools/build/lib/packages/AspectsListBuilder.java index 5f9d288749a02f..2863aab5ce749e 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AspectsListBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AspectsListBuilder.java @@ -14,8 +14,12 @@ package com.google.devtools.build.lib.packages; +import static com.google.common.collect.ImmutableList.toImmutableList; + import com.google.common.base.Function; +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.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization; @@ -48,20 +52,64 @@ public ImmutableList> getAspectsDetails() { return ImmutableList.copyOf(aspects.values()); } - /** - * Returns a list of Aspect objects for top level aspects. - * - *

Since top level aspects do not have parameters, a rule is not required to create their - * Aspect objects. - */ - public ImmutableList buildAspects() { + /** Returns a list of Aspect objects for top level aspects. */ + public ImmutableList buildAspects(ImmutableMap aspectsParameters) + throws EvalException { + Preconditions.checkArgument(aspectsParameters != null, "aspectsParameters cannot be null"); + ImmutableList.Builder aspectsList = ImmutableList.builder(); for (AspectDetails aspect : aspects.values()) { - aspectsList.add(aspect.getAspect(null)); + aspectsList.add(aspect.getTopLevelAspect(aspectsParameters)); } return aspectsList.build(); } + /** + * Validates top-level aspects parameters and reports error in the following cases: + * + *

If a parameter name is specified in command line but no aspect has a parameter with that + * name. + * + *

If a mandatory aspect attribute is not given a value in the top-level parameters list. + */ + public boolean validateTopLevelAspectsParameters(ImmutableMap aspectsParameters) + throws EvalException { + Preconditions.checkArgument(aspectsParameters != null, "aspectsParameters cannot be null"); + + ImmutableSet.Builder usedParametersBuilder = ImmutableSet.builder(); + for (AspectDetails aspectDetails : aspects.values()) { + if (aspectDetails instanceof StarlarkAspectDetails) { + ImmutableList aspectAttributes = + ((StarlarkAspectDetails) aspectDetails).aspect.getAttributes(); + for (Attribute attr : aspectAttributes) { + if (attr.isImplicit() || attr.isLateBound()) { + continue; + } + String attrName = attr.getName(); + if (aspectsParameters.containsKey(attrName)) { + usedParametersBuilder.add(attrName); + } else if (attr.isMandatory()) { + throw Starlark.errorf( + "Missing mandatory attribute '%s' for aspect '%s'.", + attrName, aspectDetails.getName()); + } + } + } + } + ImmutableSet usedParameters = usedParametersBuilder.build(); + ImmutableList unusedParameters = + aspectsParameters.keySet().stream() + .filter(p -> !usedParameters.contains(p)) + .collect(toImmutableList()); + if (!unusedParameters.isEmpty()) { + throw Starlark.errorf( + "Parameters '%s' are not parameters of any of the top-level aspects but they are" + + " specified in --aspects_parameters.", + unusedParameters); + } + return true; + } + /** Wraps the information necessary to construct an Aspect. */ @VisibleForSerialization abstract static class AspectDetails { @@ -92,7 +140,10 @@ ImmutableSet getRequiredParameters() { return ImmutableSet.of(); } - protected abstract Aspect getAspect(@Nullable Rule rule); + protected abstract Aspect getAspect(Rule rule); + + protected abstract Aspect getTopLevelAspect(ImmutableMap aspectParameters) + throws EvalException; C getAspectClass() { return aspectClass; @@ -114,14 +165,16 @@ private static class NativeAspectDetails extends AspectDetails aspectParameters) + throws EvalException { + // Native aspects ignore their top-level parameters values for now. + return Aspect.forNative(aspectClass, AspectParameters.EMPTY); + } } @VisibleForSerialization @@ -142,12 +195,14 @@ public ImmutableSet getRequiredParameters() { @Override public Aspect getAspect(Rule rule) { - AspectParameters params; - if (rule == null) { - params = AspectParameters.EMPTY; - } else { - params = parametersExtractor.apply(rule); - } + AspectParameters params = parametersExtractor.apply(rule); + return Aspect.forStarlark(aspectClass, aspect.getDefinition(params), params); + } + + @Override + public Aspect getTopLevelAspect(ImmutableMap aspectParameters) + throws EvalException { + AspectParameters params = aspect.extractTopLevelParameters(aspectParameters); return Aspect.forStarlark(aspectClass, aspect.getDefinition(params), params); } } @@ -165,6 +220,12 @@ private static class PredefinedAspectDetails extends AspectDetails public Aspect getAspect(Rule rule) { return aspect; } + + @Override + public Aspect getTopLevelAspect(ImmutableMap aspectParameters) + throws EvalException { + return aspect; + } } @SerializationConstant @AutoCodec.VisibleForSerialization diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java index da9d580c1c800b..279a1c6a17d60a 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java @@ -17,6 +17,7 @@ import com.google.common.base.Function; 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.devtools.build.lib.analysis.config.transitions.ConfigurationTransition; import com.google.devtools.build.lib.cmdline.Label; @@ -219,6 +220,38 @@ public Function getDefaultParametersExtractor() { }; } + public AspectParameters extractTopLevelParameters(ImmutableMap parametersValues) + throws EvalException { + AspectParameters.Builder builder = new AspectParameters.Builder(); + for (Attribute aspectParameter : attributes) { + String parameterName = aspectParameter.getName(); + if (Attribute.isImplicit(parameterName) || Attribute.isLateBound(parameterName)) { + // These attributes are the private matters of the aspect + continue; + } + + Preconditions.checkArgument( + aspectParameter.getType() == Type.STRING, + "Cannot pass value of non-string attribute '%s' in aspect %s.", + getName(), + parameterName); + + String parameterValue = + parametersValues.getOrDefault( + parameterName, (String) aspectParameter.getDefaultValue(null)); + + PredicateWithMessage allowedValues = aspectParameter.getAllowedValues(); + if (allowedValues.apply(parameterValue)) { + builder.addAttribute(parameterName, parameterValue); + } else { + throw Starlark.errorf( + "%s: invalid value in '%s' attribute: %s", + getName(), parameterName, allowedValues.getErrorReason(parameterValue)); + } + } + return builder.build(); + } + public ImmutableList