Skip to content

Commit 1bca1bd

Browse files
mai93copybara-github
authored andcommitted
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 88acef4 commit 1bca1bd

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
@@ -518,6 +518,7 @@ public StarlarkAspect aspect(
518518
StarlarkFunction implementation,
519519
Sequence<?> attributeAspects,
520520
Object attrs,
521+
Sequence<?> requiredProvidersArg,
521522
Sequence<?> requiredAspectProvidersArg,
522523
Sequence<?> providesArg,
523524
Sequence<?> fragments,
@@ -607,6 +608,7 @@ public StarlarkAspect aspect(
607608
implementation,
608609
attrAspects.build(),
609610
attributes.build(),
611+
StarlarkAttrModule.buildProviderPredicate(requiredProvidersArg, "required_providers"),
610612
StarlarkAttrModule.buildProviderPredicate(
611613
requiredAspectProvidersArg, "required_aspect_providers"),
612614
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
@@ -305,6 +305,20 @@ public Builder requireProviders(Class<? extends TransitiveInfoProvider>... provi
305305
return this;
306306
}
307307

308+
/**
309+
* Asserts that this aspect can only be evaluated for rules that supply all of the providers
310+
* from at least one set of required providers.
311+
*/
312+
public Builder requireStarlarkProviderSets(
313+
Iterable<ImmutableSet<StarlarkProviderIdentifier>> providerSets) {
314+
for (ImmutableSet<StarlarkProviderIdentifier> providerSet : providerSets) {
315+
if (!providerSet.isEmpty()) {
316+
requiredProviders.addStarlarkSet(providerSet);
317+
}
318+
}
319+
return this;
320+
}
321+
308322
/**
309323
* Asserts that this aspect can only be evaluated for rules that supply all of the specified
310324
* 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
@@ -37,6 +37,7 @@ public class StarlarkDefinedAspect implements StarlarkExportable, StarlarkAspect
3737
private final StarlarkCallable implementation;
3838
private final ImmutableList<String> attributeAspects;
3939
private final ImmutableList<Attribute> attributes;
40+
private final ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredProviders;
4041
private final ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredAspectProviders;
4142
private final ImmutableSet<StarlarkProviderIdentifier> provides;
4243
private final ImmutableSet<String> paramAttributes;
@@ -53,6 +54,7 @@ public StarlarkDefinedAspect(
5354
StarlarkCallable implementation,
5455
ImmutableList<String> attributeAspects,
5556
ImmutableList<Attribute> attributes,
57+
ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredProviders,
5658
ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredAspectProviders,
5759
ImmutableSet<StarlarkProviderIdentifier> provides,
5860
ImmutableSet<String> paramAttributes,
@@ -66,6 +68,7 @@ public StarlarkDefinedAspect(
6668
this.implementation = implementation;
6769
this.attributeAspects = attributeAspects;
6870
this.attributes = attributes;
71+
this.requiredProviders = requiredProviders;
6972
this.requiredAspectProviders = requiredAspectProviders;
7073
this.provides = provides;
7174
this.paramAttributes = paramAttributes;
@@ -84,6 +87,7 @@ public StarlarkDefinedAspect(
8487
StarlarkCallable implementation,
8588
ImmutableList<String> attributeAspects,
8689
ImmutableList<Attribute> attributes,
90+
ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredProviders,
8791
ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredAspectProviders,
8892
ImmutableSet<StarlarkProviderIdentifier> provides,
8993
ImmutableSet<String> paramAttributes,
@@ -99,6 +103,7 @@ public StarlarkDefinedAspect(
99103
implementation,
100104
attributeAspects,
101105
attributes,
106+
requiredProviders,
102107
requiredAspectProviders,
103108
provides,
104109
paramAttributes,
@@ -166,7 +171,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParams) {
166171
builder.propagateAlongAttribute(attributeAspect);
167172
}
168173
}
169-
174+
170175
for (Attribute attribute : attributes) {
171176
Attribute attr = attribute; // Might be reassigned.
172177
if (!aspectParams.getAttribute(attr.getName()).isEmpty()) {
@@ -183,6 +188,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParams) {
183188
}
184189
builder.add(attr);
185190
}
191+
builder.requireStarlarkProviderSets(requiredProviders);
186192
builder.requireAspectsWithProviders(requiredAspectProviders);
187193
ImmutableList.Builder<StarlarkProviderIdentifier> advertisedStarlarkProviders =
188194
ImmutableList.builder();
@@ -273,6 +279,7 @@ public boolean equals(Object o) {
273279
return Objects.equals(implementation, that.implementation)
274280
&& Objects.equals(attributeAspects, that.attributeAspects)
275281
&& Objects.equals(attributes, that.attributes)
282+
&& Objects.equals(requiredProviders, that.requiredProviders)
276283
&& Objects.equals(requiredAspectProviders, that.requiredAspectProviders)
277284
&& Objects.equals(provides, that.provides)
278285
&& Objects.equals(paramAttributes, that.paramAttributes)
@@ -290,6 +297,7 @@ public int hashCode() {
290297
implementation,
291298
attributeAspects,
292299
attributes,
300+
requiredProviders,
293301
requiredAspectProviders,
294302
provides,
295303
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
@@ -627,6 +627,21 @@ public class BuildLanguageOptions extends OptionsBase implements Serializable {
627627
+ " environment and inputs during execution.")
628628
public boolean experimentalShadowedAction;
629629

630+
@Option(
631+
name = "incompatible_top_level_aspects_require_providers",
632+
defaultValue = "false",
633+
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
634+
metadataTags = {
635+
OptionMetadataTag.INCOMPATIBLE_CHANGE,
636+
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
637+
},
638+
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
639+
help =
640+
"If set to true, the top level aspect will honor its required providers and only run on"
641+
+ " top level targets whose rules' advertised providers satisfy the required"
642+
+ " providers of the aspect.")
643+
public boolean incompatibleTopLevelAspectsRequireProviders;
644+
630645
/**
631646
* An interner to reduce the number of StarlarkSemantics instances. A single Blaze instance should
632647
* never accumulate a large number of these and being able to shortcut on object identity makes a
@@ -694,6 +709,9 @@ public StarlarkSemantics toStarlarkSemantics() {
694709
.set(MAX_COMPUTATION_STEPS, maxComputationSteps)
695710
.set(NESTED_SET_DEPTH_LIMIT, nestedSetDepthLimit)
696711
.setBool(EXPERIMENTAL_SHADOWED_ACTION, experimentalShadowedAction)
712+
.setBool(
713+
INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS,
714+
incompatibleTopLevelAspectsRequireProviders)
697715
.build();
698716
return INTERNER.intern(semantics);
699717
}
@@ -765,6 +783,8 @@ public StarlarkSemantics toStarlarkSemantics() {
765783
public static final String INCOMPATIBLE_VISIBILITY_PRIVATE_ATTRIBUTES_AT_DEFINITION =
766784
"-incompatible_visibility_private_attributes_at_definition";
767785
public static final String EXPERIMENTAL_SHADOWED_ACTION = "-experimental_shadowed_action";
786+
public static final String INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS =
787+
"-incompatible_top_level_aspects_require_providers";
768788

769789
// non-booleans
770790
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.
@@ -314,6 +317,34 @@ public SkyValue compute(SkyKey skyKey, Environment env)
314317
baseConfiguredTargetValue.getConfiguredTarget());
315318
}
316319

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

318349
ImmutableList.Builder<Aspect> aspectPathBuilder = ImmutableList.builder();
319350

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)