Skip to content

Commit f64f071

Browse files
mai93mai93
committed
Add required_providers attribute to Starlark defined aspects.
`required_providers` attribute allows the aspect to limit its propagation to only the targets whose rules advertise the required providers. It accepts a list of either providers or providers lists. To make some rule targets visible to an aspect, the rule must advertise all providers from at least one of the lists specified in the aspect `required_providers`. This CL also adds incompatible flag `incompatible_top_level_aspects_require_providers` which when set allows the top level aspects to only run on top level targets that advertise its required providers. It is needed to avoid breaking existing usages on command line aspects on targets not advertising its required providers. PiperOrigin-RevId: 373738497
1 parent f013c84 commit f64f071

File tree

11 files changed

+960
-3
lines changed

11 files changed

+960
-3
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,7 @@ public StarlarkAspect aspect(
521521
StarlarkFunction implementation,
522522
Sequence<?> attributeAspects,
523523
Object attrs,
524+
Sequence<?> requiredProvidersArg,
524525
Sequence<?> requiredAspectProvidersArg,
525526
Sequence<?> providesArg,
526527
Sequence<?> fragments,
@@ -610,6 +611,7 @@ public StarlarkAspect aspect(
610611
implementation,
611612
attrAspects.build(),
612613
attributes.build(),
614+
StarlarkAttrModule.buildProviderPredicate(requiredProvidersArg, "required_providers"),
613615
StarlarkAttrModule.buildProviderPredicate(
614616
requiredAspectProvidersArg, "required_aspect_providers"),
615617
StarlarkAttrModule.getStarlarkProviderIdentifiers(providesArg),

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,20 @@ public Builder requireProviders(Class<? extends TransitiveInfoProvider>... provi
283283
return this;
284284
}
285285

286+
/**
287+
* Asserts that this aspect can only be evaluated for rules that supply all of the providers
288+
* from at least one set of required providers.
289+
*/
290+
public Builder requireStarlarkProviderSets(
291+
Iterable<ImmutableSet<StarlarkProviderIdentifier>> providerSets) {
292+
for (ImmutableSet<StarlarkProviderIdentifier> providerSet : providerSets) {
293+
if (!providerSet.isEmpty()) {
294+
requiredProviders.addStarlarkSet(providerSet);
295+
}
296+
}
297+
return this;
298+
}
299+
286300
/**
287301
* Asserts that this aspect can only be evaluated for rules that supply all of the specified
288302
* Starlark providers.

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public class StarlarkDefinedAspect implements StarlarkExportable, StarlarkAspect
3636
private final StarlarkCallable implementation;
3737
private final ImmutableList<String> attributeAspects;
3838
private final ImmutableList<Attribute> attributes;
39+
private final ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredProviders;
3940
private final ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredAspectProviders;
4041
private final ImmutableSet<StarlarkProviderIdentifier> provides;
4142
private final ImmutableSet<String> paramAttributes;
@@ -52,6 +53,7 @@ public StarlarkDefinedAspect(
5253
StarlarkCallable implementation,
5354
ImmutableList<String> attributeAspects,
5455
ImmutableList<Attribute> attributes,
56+
ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredProviders,
5557
ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredAspectProviders,
5658
ImmutableSet<StarlarkProviderIdentifier> provides,
5759
ImmutableSet<String> paramAttributes,
@@ -65,6 +67,7 @@ public StarlarkDefinedAspect(
6567
this.implementation = implementation;
6668
this.attributeAspects = attributeAspects;
6769
this.attributes = attributes;
70+
this.requiredProviders = requiredProviders;
6871
this.requiredAspectProviders = requiredAspectProviders;
6972
this.provides = provides;
7073
this.paramAttributes = paramAttributes;
@@ -83,6 +86,7 @@ public StarlarkDefinedAspect(
8386
StarlarkCallable implementation,
8487
ImmutableList<String> attributeAspects,
8588
ImmutableList<Attribute> attributes,
89+
ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredProviders,
8690
ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredAspectProviders,
8791
ImmutableSet<StarlarkProviderIdentifier> provides,
8892
ImmutableSet<String> paramAttributes,
@@ -98,6 +102,7 @@ public StarlarkDefinedAspect(
98102
implementation,
99103
attributeAspects,
100104
attributes,
105+
requiredProviders,
101106
requiredAspectProviders,
102107
provides,
103108
paramAttributes,
@@ -165,7 +170,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParams) {
165170
builder.propagateAlongAttribute(attributeAspect);
166171
}
167172
}
168-
173+
169174
for (Attribute attribute : attributes) {
170175
Attribute attr = attribute; // Might be reassigned.
171176
if (!aspectParams.getAttribute(attr.getName()).isEmpty()) {
@@ -182,6 +187,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParams) {
182187
}
183188
builder.add(attr);
184189
}
190+
builder.requireStarlarkProviderSets(requiredProviders);
185191
builder.requireAspectsWithProviders(requiredAspectProviders);
186192
ImmutableList.Builder<StarlarkProviderIdentifier> advertisedStarlarkProviders =
187193
ImmutableList.builder();
@@ -272,6 +278,7 @@ public boolean equals(Object o) {
272278
return Objects.equals(implementation, that.implementation)
273279
&& Objects.equals(attributeAspects, that.attributeAspects)
274280
&& Objects.equals(attributes, that.attributes)
281+
&& Objects.equals(requiredProviders, that.requiredProviders)
275282
&& Objects.equals(requiredAspectProviders, that.requiredAspectProviders)
276283
&& Objects.equals(provides, that.provides)
277284
&& Objects.equals(paramAttributes, that.paramAttributes)
@@ -289,6 +296,7 @@ public int hashCode() {
289296
implementation,
290297
attributeAspects,
291298
attributes,
299+
requiredProviders,
292300
requiredAspectProviders,
293301
provides,
294302
paramAttributes,

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,21 @@ public class BuildLanguageOptions extends OptionsBase implements Serializable {
586586
+ " (zero means no limit).")
587587
public long maxComputationSteps;
588588

589+
@Option(
590+
name = "incompatible_top_level_aspects_require_providers",
591+
defaultValue = "false",
592+
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
593+
metadataTags = {
594+
OptionMetadataTag.INCOMPATIBLE_CHANGE,
595+
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
596+
},
597+
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
598+
help =
599+
"If set to true, the top level aspect will honor its required providers and only run on"
600+
+ " top level targets whose rules' advertised providers satisfy the required"
601+
+ " providers of the aspect.")
602+
public boolean incompatibleTopLevelAspectsRequireProviders;
603+
589604
/**
590605
* An interner to reduce the number of StarlarkSemantics instances. A single Blaze instance should
591606
* never accumulate a large number of these and being able to shortcut on object identity makes a
@@ -656,6 +671,9 @@ public StarlarkSemantics toStarlarkSemantics() {
656671
INCOMPATIBLE_OBJC_PROVIDER_REMOVE_COMPILE_INFO,
657672
incompatibleObjcProviderRemoveCompileInfo)
658673
.set(MAX_COMPUTATION_STEPS, maxComputationSteps)
674+
.setBool(
675+
INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS,
676+
incompatibleTopLevelAspectsRequireProviders)
659677
.build();
660678
return INTERNER.intern(semantics);
661679
}
@@ -729,6 +747,8 @@ public StarlarkSemantics toStarlarkSemantics() {
729747
"-incompatible_visibility_private_attributes_at_definition";
730748
public static final String RECORD_RULE_INSTANTIATION_CALLSTACK =
731749
"+record_rule_instantiation_callstack";
750+
public static final String INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS =
751+
"-incompatible_top_level_aspects_require_providers";
732752

733753
// non-booleans
734754
public static final StarlarkSemantics.Key<String> EXPERIMENTAL_BUILTINS_BZL_PATH =

‎src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,14 @@
5858
import com.google.devtools.build.lib.packages.NoSuchThingException;
5959
import com.google.devtools.build.lib.packages.OutputFile;
6060
import com.google.devtools.build.lib.packages.Package;
61+
import com.google.devtools.build.lib.packages.Rule;
6162
import com.google.devtools.build.lib.packages.RuleClassProvider;
6263
import com.google.devtools.build.lib.packages.StarlarkAspect;
6364
import com.google.devtools.build.lib.packages.StarlarkAspectClass;
6465
import com.google.devtools.build.lib.packages.StarlarkDefinedAspect;
6566
import com.google.devtools.build.lib.packages.Target;
6667
import com.google.devtools.build.lib.packages.Type.ConversionException;
68+
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
6769
import com.google.devtools.build.lib.profiler.memory.CurrentRuleTracker;
6870
import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
6971
import com.google.devtools.build.lib.skyframe.BzlLoadFunction.BzlLoadFailedException;
@@ -78,6 +80,7 @@
7880
import java.util.HashMap;
7981
import java.util.Map;
8082
import javax.annotation.Nullable;
83+
import net.starlark.java.eval.StarlarkSemantics;
8184

8285
/**
8386
* The Skyframe function that generates aspects.
@@ -324,6 +327,34 @@ public SkyValue compute(SkyKey skyKey, Environment env)
324327
baseConfiguredTargetValue.getConfiguredTarget());
325328
}
326329

330+
// If the incompatible flag is set, the top-level aspect should not be applied on top-level
331+
// targets whose rules do not advertise the aspect's required providers. The aspect should not
332+
// also propagate to these targets dependencies.
333+
StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
334+
if (starlarkSemantics == null) {
335+
return null;
336+
}
337+
boolean checkRuleAdvertisedProviders =
338+
starlarkSemantics.getBool(
339+
BuildLanguageOptions.INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS);
340+
if (checkRuleAdvertisedProviders) {
341+
Target target = associatedConfiguredTargetAndData.getTarget();
342+
if (target instanceof Rule) {
343+
if (!aspect
344+
.getDefinition()
345+
.getRequiredProviders()
346+
.isSatisfiedBy(((Rule) target).getRuleClassObject().getAdvertisedProviders())) {
347+
return new AspectValue(
348+
key,
349+
aspect,
350+
target.getLocation(),
351+
ConfiguredAspect.forNonapplicableTarget(),
352+
/*transitivePackagesForPackageRootResolution=*/ NestedSetBuilder
353+
.<Package>stableOrder()
354+
.build());
355+
}
356+
}
357+
}
327358

328359
ImmutableList.Builder<Aspect> aspectPathBuilder = ImmutableList.builder();
329360

‎src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,31 @@ StarlarkCallable rule(
416416
+ "the <code>values</code> restriction. Explicit attributes restrict the "
417417
+ "aspect to only be used with rules that have attributes of the same "
418418
+ "name, type, and valid values according to the restriction."),
419+
@Param(
420+
name = "required_providers",
421+
named = true,
422+
defaultValue = "[]",
423+
doc =
424+
"This attribute allows the aspect to limit its propagation to only the targets "
425+
+ "whose rules advertise its required providers. The value must be a "
426+
+ "list containing either individual providers or lists of providers but not "
427+
+ "both. For example, <code>[[FooInfo], [BarInfo], [BazInfo, QuxInfo]]</code> "
428+
+ "is a valid value while <code>[FooInfo, BarInfo, [BazInfo, QuxInfo]]</code> "
429+
+ "is not valid."
430+
+ ""
431+
+ "<p>An unnested list of providers will automatically be converted to a list "
432+
+ "containing one list of providers. That is, <code>[FooInfo, BarInfo]</code> "
433+
+ "will automatically be converted to <code>[[FooInfo, BarInfo]]</code>."
434+
+ ""
435+
+ "<p>To make some rule (e.g. <code>some_rule</code>) targets visible to an "
436+
+ "aspect, <code>some_rule</code> must advertise all providers from at least "
437+
+ "one of the required providers lists. For example, if the "
438+
+ "<code>required_providers</code> of an aspect are "
439+
+ "<code>[[FooInfo], [BarInfo], [BazInfo, QuxInfo]]</code>, this aspect can "
440+
+ "only see <code>some_rule</code> targets if and only if "
441+
+ "<code>some_rule</code> provides <code>FooInfo</code> *or* "
442+
+ "<code>BarInfo</code> *or* both <code>BazInfo</code> *and* "
443+
+ "<code>QuxInfo</code>."),
419444
@Param(
420445
name = "required_aspect_providers",
421446
named = true,
@@ -500,6 +525,7 @@ StarlarkAspectApi aspect(
500525
StarlarkFunction implementation,
501526
Sequence<?> attributeAspects,
502527
Object attrs,
528+
Sequence<?> requiredProvidersArg,
503529
Sequence<?> requiredAspectProvidersArg,
504530
Sequence<?> providesArg,
505531
Sequence<?> fragments,

‎src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ public StarlarkAspectApi aspect(
180180
StarlarkFunction implementation,
181181
Sequence<?> attributeAspects,
182182
Object attrs,
183+
Sequence<?> requiredProvidersArg,
183184
Sequence<?> requiredAspectProvidersArg,
184185
Sequence<?> providesArg,
185186
Sequence<?> fragments,

‎src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java‎

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
import com.google.devtools.build.lib.packages.BuildType;
3434
import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
3535
import com.google.devtools.build.lib.packages.NativeAspectClass;
36+
import com.google.devtools.build.lib.packages.StarlarkProvider;
37+
import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier;
3638
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
3739
import com.google.devtools.build.lib.util.FileTypeSet;
3840
import net.starlark.java.annot.StarlarkBuiltin;
@@ -55,6 +57,20 @@ private static final class P3 implements TransitiveInfoProvider {}
5557

5658
private static final class P4 implements TransitiveInfoProvider {}
5759

60+
private static final Label FAKE_LABEL = Label.parseAbsoluteUnchecked("//fake/label.bzl");
61+
62+
private static final StarlarkProviderIdentifier STARLARK_P1 =
63+
StarlarkProviderIdentifier.forKey(new StarlarkProvider.Key(FAKE_LABEL, "STARLARK_P1"));
64+
65+
private static final StarlarkProviderIdentifier STARLARK_P2 =
66+
StarlarkProviderIdentifier.forKey(new StarlarkProvider.Key(FAKE_LABEL, "STARLARK_P2"));
67+
68+
private static final StarlarkProviderIdentifier STARLARK_P3 =
69+
StarlarkProviderIdentifier.forKey(new StarlarkProvider.Key(FAKE_LABEL, "STARLARK_P3"));
70+
71+
private static final StarlarkProviderIdentifier STARLARK_P4 =
72+
StarlarkProviderIdentifier.forKey(new StarlarkProvider.Key(FAKE_LABEL, "STARLARK_P4"));
73+
5874
/**
5975
* A dummy aspect factory. Is there to demonstrate how to define aspects and so that we can test
6076
* {@code attributeAspect}.
@@ -150,7 +166,7 @@ public void testAttributeAspect_allAttributes() throws Exception {
150166
}
151167

152168
@Test
153-
public void testRequireProvider_addsToSetOfRequiredProvidersAndNames() throws Exception {
169+
public void testRequireBuiltinProviders_addsToSetOfRequiredProvidersAndNames() throws Exception {
154170
AspectDefinition requiresProviders =
155171
new AspectDefinition.Builder(TEST_ASPECT_CLASS)
156172
.requireProviders(P1.class, P2.class)
@@ -176,7 +192,8 @@ public void testRequireProvider_addsToSetOfRequiredProvidersAndNames() throws Ex
176192
}
177193

178194
@Test
179-
public void testRequireProvider_addsTwoSetsOfRequiredProvidersAndNames() throws Exception {
195+
public void testRequireBuiltinProviders_addsTwoSetsOfRequiredProvidersAndNames()
196+
throws Exception {
180197
AspectDefinition requiresProviders =
181198
new AspectDefinition.Builder(TEST_ASPECT_CLASS)
182199
.requireProviderSets(
@@ -202,6 +219,73 @@ public void testRequireProvider_addsTwoSetsOfRequiredProvidersAndNames() throws
202219

203220
}
204221

222+
@Test
223+
public void testRequireStarlarkProviders_addsFlatSetOfRequiredProviders() throws Exception {
224+
AspectDefinition requiresProviders =
225+
new AspectDefinition.Builder(TEST_ASPECT_CLASS)
226+
.requireStarlarkProviders(STARLARK_P1, STARLARK_P2)
227+
.build();
228+
229+
AdvertisedProviderSet expectedOkSet =
230+
AdvertisedProviderSet.builder()
231+
.addStarlark(STARLARK_P1)
232+
.addStarlark(STARLARK_P2)
233+
.addStarlark(STARLARK_P3)
234+
.build();
235+
assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(expectedOkSet)).isTrue();
236+
237+
AdvertisedProviderSet expectedFailSet =
238+
AdvertisedProviderSet.builder().addStarlark(STARLARK_P1).build();
239+
assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(expectedFailSet)).isFalse();
240+
241+
assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(AdvertisedProviderSet.ANY))
242+
.isTrue();
243+
assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(AdvertisedProviderSet.EMPTY))
244+
.isFalse();
245+
}
246+
247+
@Test
248+
public void testRequireStarlarkProviders_addsTwoSetsOfRequiredProviders() throws Exception {
249+
AspectDefinition requiresProviders =
250+
new AspectDefinition.Builder(TEST_ASPECT_CLASS)
251+
.requireStarlarkProviderSets(
252+
ImmutableList.of(
253+
ImmutableSet.of(STARLARK_P1, STARLARK_P2), ImmutableSet.of(STARLARK_P3)))
254+
.build();
255+
256+
AdvertisedProviderSet expectedOkSet1 =
257+
AdvertisedProviderSet.builder().addStarlark(STARLARK_P1).addStarlark(STARLARK_P2).build();
258+
assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(expectedOkSet1)).isTrue();
259+
260+
AdvertisedProviderSet expectedOkSet2 =
261+
AdvertisedProviderSet.builder().addStarlark(STARLARK_P3).build();
262+
assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(expectedOkSet2)).isTrue();
263+
264+
AdvertisedProviderSet expectedFailSet =
265+
AdvertisedProviderSet.builder().addStarlark(STARLARK_P4).build();
266+
assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(expectedFailSet)).isFalse();
267+
268+
assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(AdvertisedProviderSet.ANY))
269+
.isTrue();
270+
assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(AdvertisedProviderSet.EMPTY))
271+
.isFalse();
272+
}
273+
274+
@Test
275+
public void testRequireProviders_defaultAcceptsEverything() {
276+
AspectDefinition noRequiredProviders = new AspectDefinition.Builder(TEST_ASPECT_CLASS).build();
277+
278+
AdvertisedProviderSet expectedOkSet =
279+
AdvertisedProviderSet.builder().addBuiltin(P4.class).addStarlark(STARLARK_P4).build();
280+
assertThat(noRequiredProviders.getRequiredProviders().isSatisfiedBy(expectedOkSet)).isTrue();
281+
282+
assertThat(noRequiredProviders.getRequiredProviders().isSatisfiedBy(AdvertisedProviderSet.ANY))
283+
.isTrue();
284+
assertThat(
285+
noRequiredProviders.getRequiredProviders().isSatisfiedBy(AdvertisedProviderSet.EMPTY))
286+
.isTrue();
287+
}
288+
205289
@Test
206290
public void testRequireAspectClass_defaultAcceptsNothing() {
207291
AspectDefinition noAspects = new AspectDefinition.Builder(TEST_ASPECT_CLASS)

0 commit comments

Comments
 (0)