Skip to content

Commit 3771072

Browse files
mai93copybara-github
mai93
authored and
copybara-github
committed
Enable command-line aspects parameters
This CL adds `--aspects_parameters` option to pass values for command-line aspects specified via `--aspects` option. The new option `--aspects_parameters` is only effective with `--experimental_allow_top_level_aspects_parameters` flag. PiperOrigin-RevId: 412476585
1 parent 4f3bf33 commit 3771072

17 files changed

Lines changed: 1025 additions & 46 deletions

‎src/main/java/com/google/devtools/build/lib/analysis/BuildView.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.devtools.build.lib.analysis;
1616

17+
1718
import com.google.common.annotations.VisibleForTesting;
1819
import com.google.common.base.Preconditions;
1920
import com.google.common.base.Supplier;
@@ -197,6 +198,7 @@ public AnalysisResult update(
197198
Set<String> multiCpu,
198199
ImmutableSet<String> explicitTargetPatterns,
199200
List<String> aspects,
201+
ImmutableMap<String, String> aspectsParameters,
200202
AnalysisOptions viewOptions,
201203
boolean keepGoing,
202204
boolean checkForActionConflicts,
@@ -348,7 +350,7 @@ public AnalysisResult update(
348350
if (!aspectClasses.isEmpty()) {
349351
aspectsKeys.add(
350352
AspectKeyCreator.createTopLevelAspectsKey(
351-
aspectClasses, targetSpec.getLabel(), configuration));
353+
aspectClasses, targetSpec.getLabel(), configuration, aspectsParameters));
352354
}
353355
}
354356

‎src/main/java/com/google/devtools/build/lib/buildtool/AnalysisAndExecutionPhaseRunner.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ private static AnalysisAndExecutionResult runAnalysisAndExecutionPhase(
186186
multiCpu,
187187
explicitTargetPatterns,
188188
request.getAspects(),
189+
request.getAspectsParameters(),
189190
request.getViewOptions(),
190191
request.getKeepGoing(),
191192
request.getCheckForActionConflicts(),

‎src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ private static AnalysisResult runAnalysisPhase(
220220
multiCpu,
221221
explicitTargetPatterns,
222222
request.getAspects(),
223+
request.getAspectsParameters(),
223224
request.getViewOptions(),
224225
request.getKeepGoing(),
225226
request.getCheckForActionConflicts(),

‎src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,19 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.buildtool;
1515

16+
import static com.google.common.collect.ImmutableMap.toImmutableMap;
17+
1618
import com.github.benmanes.caffeine.cache.Caffeine;
1719
import com.github.benmanes.caffeine.cache.LoadingCache;
1820
import com.google.common.base.Optional;
1921
import com.google.common.base.Preconditions;
2022
import com.google.common.collect.ImmutableList;
23+
import com.google.common.collect.ImmutableMap;
2124
import com.google.common.collect.ImmutableSortedSet;
2225
import com.google.devtools.build.lib.analysis.AnalysisOptions;
2326
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
2427
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
28+
import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
2529
import com.google.devtools.build.lib.analysis.config.BuildOptions;
2630
import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
2731
import com.google.devtools.build.lib.exec.ExecutionOptions;
@@ -31,6 +35,8 @@
3135
import com.google.devtools.build.lib.runtime.KeepGoingOption;
3236
import com.google.devtools.build.lib.runtime.LoadingPhaseThreadsOption;
3337
import com.google.devtools.build.lib.runtime.UiOptions;
38+
import com.google.devtools.build.lib.server.FailureDetails.Analysis;
39+
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
3440
import com.google.devtools.build.lib.util.OptionsUtils;
3541
import com.google.devtools.build.lib.util.io.OutErr;
3642
import com.google.devtools.common.options.OptionsBase;
@@ -40,6 +46,7 @@
4046
import java.util.List;
4147
import java.util.Map;
4248
import java.util.UUID;
49+
import javax.annotation.Nullable;
4350

4451
/**
4552
* A BuildRequest represents a single invocation of the build tool by a user.
@@ -383,6 +390,24 @@ public ImmutableList<String> getAspects() {
383390
return result.build();
384391
}
385392

393+
@Nullable
394+
public ImmutableMap<String, String> getAspectsParameters() throws ViewCreationFailedException {
395+
List<Map.Entry<String, String>> aspectsParametersList = getBuildOptions().aspectsParameters;
396+
try {
397+
return aspectsParametersList.stream()
398+
.collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue));
399+
} catch (IllegalArgumentException e) {
400+
String errorMessage = "Error in top-level aspects parameters";
401+
throw new ViewCreationFailedException(
402+
errorMessage,
403+
FailureDetail.newBuilder()
404+
.setMessage(errorMessage)
405+
.setAnalysis(Analysis.newBuilder().setCode(Analysis.Code.ASPECT_CREATION_FAILED))
406+
.build(),
407+
e);
408+
}
409+
}
410+
386411
/** Whether {@value #VALIDATION_ASPECT_NAME} is in use. */
387412
public boolean useValidationAspect() {
388413
return validationMode() == OutputGroupInfo.ValidationMode.ASPECT;

‎src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.google.devtools.common.options.OptionsParsingException;
3535
import com.google.devtools.common.options.RegexPatternOption;
3636
import java.util.List;
37+
import java.util.Map;
3738
import javax.annotation.Nullable;
3839

3940
/**
@@ -318,6 +319,23 @@ public class BuildRequestOptions extends OptionsBase {
318319
+ " 'my_aspect' is a top-level value from a file tools/my_def.bzl")
319320
public List<String> aspects;
320321

322+
@Option(
323+
name = "aspects_parameters",
324+
converter = Converters.AssignmentConverter.class,
325+
defaultValue = "null",
326+
documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS,
327+
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
328+
allowMultiple = true,
329+
help =
330+
"Specifies the values of the command-line aspects parameters. Each parameter value is"
331+
+ " specified via <param_name>=<param_value>, for example 'my_param=my_val' where"
332+
+ " 'my_param' is a parameter of some aspect in --aspects list or required by an"
333+
+ " aspect in the list. This option can be used multiple times. However, it is not"
334+
+ " allowed to assign values to the same parameter more than once. This option is"
335+
+ " only be effective under the experimental flag"
336+
+ " --experimental_allow_top_level_aspects_parameters.")
337+
public List<Map.Entry<String, String>> aspectsParameters;
338+
321339
public BuildRequestOptions() throws OptionsParsingException {}
322340

323341
public String getSymlinkPrefix(String productName) {

‎src/main/java/com/google/devtools/build/lib/packages/AspectsListBuilder.java‎

Lines changed: 82 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,12 @@
1414

1515
package com.google.devtools.build.lib.packages;
1616

17+
import static com.google.common.collect.ImmutableList.toImmutableList;
18+
1719
import com.google.common.base.Function;
20+
import com.google.common.base.Preconditions;
1821
import com.google.common.collect.ImmutableList;
22+
import com.google.common.collect.ImmutableMap;
1923
import com.google.common.collect.ImmutableSet;
2024
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
2125
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
@@ -48,20 +52,64 @@ public ImmutableList<AspectDetails<?>> getAspectsDetails() {
4852
return ImmutableList.copyOf(aspects.values());
4953
}
5054

51-
/**
52-
* Returns a list of Aspect objects for top level aspects.
53-
*
54-
* <p>Since top level aspects do not have parameters, a rule is not required to create their
55-
* Aspect objects.
56-
*/
57-
public ImmutableList<Aspect> buildAspects() {
55+
/** Returns a list of Aspect objects for top level aspects. */
56+
public ImmutableList<Aspect> buildAspects(ImmutableMap<String, String> aspectsParameters)
57+
throws EvalException {
58+
Preconditions.checkArgument(aspectsParameters != null, "aspectsParameters cannot be null");
59+
5860
ImmutableList.Builder<Aspect> aspectsList = ImmutableList.builder();
5961
for (AspectDetails<?> aspect : aspects.values()) {
60-
aspectsList.add(aspect.getAspect(null));
62+
aspectsList.add(aspect.getTopLevelAspect(aspectsParameters));
6163
}
6264
return aspectsList.build();
6365
}
6466

67+
/**
68+
* Validates top-level aspects parameters and reports error in the following cases:
69+
*
70+
* <p>If a parameter name is specified in command line but no aspect has a parameter with that
71+
* name.
72+
*
73+
* <p>If a mandatory aspect attribute is not given a value in the top-level parameters list.
74+
*/
75+
public boolean validateTopLevelAspectsParameters(ImmutableMap<String, String> aspectsParameters)
76+
throws EvalException {
77+
Preconditions.checkArgument(aspectsParameters != null, "aspectsParameters cannot be null");
78+
79+
ImmutableSet.Builder<String> usedParametersBuilder = ImmutableSet.builder();
80+
for (AspectDetails<?> aspectDetails : aspects.values()) {
81+
if (aspectDetails instanceof StarlarkAspectDetails) {
82+
ImmutableList<Attribute> aspectAttributes =
83+
((StarlarkAspectDetails) aspectDetails).aspect.getAttributes();
84+
for (Attribute attr : aspectAttributes) {
85+
if (attr.isImplicit() || attr.isLateBound()) {
86+
continue;
87+
}
88+
String attrName = attr.getName();
89+
if (aspectsParameters.containsKey(attrName)) {
90+
usedParametersBuilder.add(attrName);
91+
} else if (attr.isMandatory()) {
92+
throw Starlark.errorf(
93+
"Missing mandatory attribute '%s' for aspect '%s'.",
94+
attrName, aspectDetails.getName());
95+
}
96+
}
97+
}
98+
}
99+
ImmutableSet<String> usedParameters = usedParametersBuilder.build();
100+
ImmutableList<String> unusedParameters =
101+
aspectsParameters.keySet().stream()
102+
.filter(p -> !usedParameters.contains(p))
103+
.collect(toImmutableList());
104+
if (!unusedParameters.isEmpty()) {
105+
throw Starlark.errorf(
106+
"Parameters '%s' are not parameters of any of the top-level aspects but they are"
107+
+ " specified in --aspects_parameters.",
108+
unusedParameters);
109+
}
110+
return true;
111+
}
112+
65113
/** Wraps the information necessary to construct an Aspect. */
66114
@VisibleForSerialization
67115
abstract static class AspectDetails<C extends AspectClass> {
@@ -92,7 +140,10 @@ ImmutableSet<String> getRequiredParameters() {
92140
return ImmutableSet.of();
93141
}
94142

95-
protected abstract Aspect getAspect(@Nullable Rule rule);
143+
protected abstract Aspect getAspect(Rule rule);
144+
145+
protected abstract Aspect getTopLevelAspect(ImmutableMap<String, String> aspectParameters)
146+
throws EvalException;
96147

97148
C getAspectClass() {
98149
return aspectClass;
@@ -114,14 +165,16 @@ private static class NativeAspectDetails extends AspectDetails<NativeAspectClass
114165

115166
@Override
116167
public Aspect getAspect(Rule rule) {
117-
AspectParameters params;
118-
if (rule == null) {
119-
params = AspectParameters.EMPTY;
120-
} else {
121-
params = parametersExtractor.apply(rule);
122-
}
168+
AspectParameters params = parametersExtractor.apply(rule);
123169
return params == null ? null : Aspect.forNative(aspectClass, params);
124170
}
171+
172+
@Override
173+
protected Aspect getTopLevelAspect(ImmutableMap<String, String> aspectParameters)
174+
throws EvalException {
175+
// Native aspects ignore their top-level parameters values for now.
176+
return Aspect.forNative(aspectClass, AspectParameters.EMPTY);
177+
}
125178
}
126179

127180
@VisibleForSerialization
@@ -142,12 +195,14 @@ public ImmutableSet<String> getRequiredParameters() {
142195

143196
@Override
144197
public Aspect getAspect(Rule rule) {
145-
AspectParameters params;
146-
if (rule == null) {
147-
params = AspectParameters.EMPTY;
148-
} else {
149-
params = parametersExtractor.apply(rule);
150-
}
198+
AspectParameters params = parametersExtractor.apply(rule);
199+
return Aspect.forStarlark(aspectClass, aspect.getDefinition(params), params);
200+
}
201+
202+
@Override
203+
public Aspect getTopLevelAspect(ImmutableMap<String, String> aspectParameters)
204+
throws EvalException {
205+
AspectParameters params = aspect.extractTopLevelParameters(aspectParameters);
151206
return Aspect.forStarlark(aspectClass, aspect.getDefinition(params), params);
152207
}
153208
}
@@ -165,6 +220,12 @@ private static class PredefinedAspectDetails extends AspectDetails<AspectClass>
165220
public Aspect getAspect(Rule rule) {
166221
return aspect;
167222
}
223+
224+
@Override
225+
public Aspect getTopLevelAspect(ImmutableMap<String, String> aspectParameters)
226+
throws EvalException {
227+
return aspect;
228+
}
168229
}
169230

170231
@SerializationConstant @AutoCodec.VisibleForSerialization

‎src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import com.google.common.base.Function;
1818
import com.google.common.base.Preconditions;
1919
import com.google.common.collect.ImmutableList;
20+
import com.google.common.collect.ImmutableMap;
2021
import com.google.common.collect.ImmutableSet;
2122
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
2223
import com.google.devtools.build.lib.cmdline.Label;
@@ -219,6 +220,38 @@ public Function<Rule, AspectParameters> getDefaultParametersExtractor() {
219220
};
220221
}
221222

223+
public AspectParameters extractTopLevelParameters(ImmutableMap<String, String> parametersValues)
224+
throws EvalException {
225+
AspectParameters.Builder builder = new AspectParameters.Builder();
226+
for (Attribute aspectParameter : attributes) {
227+
String parameterName = aspectParameter.getName();
228+
if (Attribute.isImplicit(parameterName) || Attribute.isLateBound(parameterName)) {
229+
// These attributes are the private matters of the aspect
230+
continue;
231+
}
232+
233+
Preconditions.checkArgument(
234+
aspectParameter.getType() == Type.STRING,
235+
"Cannot pass value of non-string attribute '%s' in aspect %s.",
236+
getName(),
237+
parameterName);
238+
239+
String parameterValue =
240+
parametersValues.getOrDefault(
241+
parameterName, (String) aspectParameter.getDefaultValue(null));
242+
243+
PredicateWithMessage<Object> allowedValues = aspectParameter.getAllowedValues();
244+
if (allowedValues.apply(parameterValue)) {
245+
builder.addAttribute(parameterName, parameterValue);
246+
} else {
247+
throw Starlark.errorf(
248+
"%s: invalid value in '%s' attribute: %s",
249+
getName(), parameterName, allowedValues.getErrorReason(parameterValue));
250+
}
251+
}
252+
return builder.build();
253+
}
254+
222255
public ImmutableList<Label> getRequiredToolchains() {
223256
return requiredToolchains;
224257
}

‎src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,17 @@ public final class BuildLanguageOptions extends OptionsBase {
538538
+ " providers of the aspect.")
539539
public boolean incompatibleTopLevelAspectsRequireProviders;
540540

541+
@Option(
542+
name = "experimental_allow_top_level_aspects_parameters",
543+
defaultValue = "false",
544+
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
545+
metadataTags = {OptionMetadataTag.EXPERIMENTAL},
546+
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
547+
help =
548+
"If set to true, top level aspects will accept values for their parameters passed via"
549+
+ " --aspects_parameters option.")
550+
public boolean experimentalAllowTopLevelAspectsParameters;
551+
541552
/**
542553
* An interner to reduce the number of StarlarkSemantics instances. A single Blaze instance should
543554
* never accumulate a large number of these and being able to shortcut on object identity makes a
@@ -605,6 +616,9 @@ public StarlarkSemantics toStarlarkSemantics() {
605616
.setBool(
606617
INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS,
607618
incompatibleTopLevelAspectsRequireProviders)
619+
.setBool(
620+
EXPERIMENTAL_ALLOW_TOP_LEVEL_ASPECTS_PARAMETERS,
621+
experimentalAllowTopLevelAspectsParameters)
608622
.build();
609623
return INTERNER.intern(semantics);
610624
}
@@ -673,6 +687,8 @@ public StarlarkSemantics toStarlarkSemantics() {
673687
"-incompatible_visibility_private_attributes_at_definition";
674688
public static final String INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS =
675689
"-incompatible_top_level_aspects_require_providers";
690+
public static final String EXPERIMENTAL_ALLOW_TOP_LEVEL_ASPECTS_PARAMETERS =
691+
"-experimental_allow_top_level_aspects_parameters";
676692

677693
// non-booleans
678694
public static final StarlarkSemantics.Key<String> EXPERIMENTAL_BUILTINS_BZL_PATH =

0 commit comments

Comments
 (0)