diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java index a905d015e1ea0d..29c4febdb671ea 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java @@ -518,6 +518,7 @@ public StarlarkAspect aspect( StarlarkFunction implementation, Sequence attributeAspects, Object attrs, + Sequence requiredProvidersArg, Sequence requiredAspectProvidersArg, Sequence providesArg, Sequence fragments, @@ -607,6 +608,7 @@ public StarlarkAspect aspect( implementation, attrAspects.build(), attributes.build(), + StarlarkAttrModule.buildProviderPredicate(requiredProvidersArg, "required_providers"), StarlarkAttrModule.buildProviderPredicate( requiredAspectProvidersArg, "required_aspect_providers"), StarlarkAttrModule.getStarlarkProviderIdentifiers(providesArg), diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java index 9c50c80d7631a7..30ec83cb11c544 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java @@ -305,6 +305,20 @@ public Builder requireProviders(Class... provi return this; } + /** + * Asserts that this aspect can only be evaluated for rules that supply all of the providers + * from at least one set of required providers. + */ + public Builder requireStarlarkProviderSets( + Iterable> providerSets) { + for (ImmutableSet providerSet : providerSets) { + if (!providerSet.isEmpty()) { + requiredProviders.addStarlarkSet(providerSet); + } + } + return this; + } + /** * Asserts that this aspect can only be evaluated for rules that supply all of the specified * Starlark providers. 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 3991894e154c05..959d6ec967bf55 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 @@ -37,6 +37,7 @@ public class StarlarkDefinedAspect implements StarlarkExportable, StarlarkAspect private final StarlarkCallable implementation; private final ImmutableList attributeAspects; private final ImmutableList attributes; + private final ImmutableList> requiredProviders; private final ImmutableList> requiredAspectProviders; private final ImmutableSet provides; private final ImmutableSet paramAttributes; @@ -53,6 +54,7 @@ public StarlarkDefinedAspect( StarlarkCallable implementation, ImmutableList attributeAspects, ImmutableList attributes, + ImmutableList> requiredProviders, ImmutableList> requiredAspectProviders, ImmutableSet provides, ImmutableSet paramAttributes, @@ -66,6 +68,7 @@ public StarlarkDefinedAspect( this.implementation = implementation; this.attributeAspects = attributeAspects; this.attributes = attributes; + this.requiredProviders = requiredProviders; this.requiredAspectProviders = requiredAspectProviders; this.provides = provides; this.paramAttributes = paramAttributes; @@ -84,6 +87,7 @@ public StarlarkDefinedAspect( StarlarkCallable implementation, ImmutableList attributeAspects, ImmutableList attributes, + ImmutableList> requiredProviders, ImmutableList> requiredAspectProviders, ImmutableSet provides, ImmutableSet paramAttributes, @@ -99,6 +103,7 @@ public StarlarkDefinedAspect( implementation, attributeAspects, attributes, + requiredProviders, requiredAspectProviders, provides, paramAttributes, @@ -166,7 +171,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParams) { builder.propagateAlongAttribute(attributeAspect); } } - + for (Attribute attribute : attributes) { Attribute attr = attribute; // Might be reassigned. if (!aspectParams.getAttribute(attr.getName()).isEmpty()) { @@ -183,6 +188,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParams) { } builder.add(attr); } + builder.requireStarlarkProviderSets(requiredProviders); builder.requireAspectsWithProviders(requiredAspectProviders); ImmutableList.Builder advertisedStarlarkProviders = ImmutableList.builder(); @@ -273,6 +279,7 @@ public boolean equals(Object o) { return Objects.equals(implementation, that.implementation) && Objects.equals(attributeAspects, that.attributeAspects) && Objects.equals(attributes, that.attributes) + && Objects.equals(requiredProviders, that.requiredProviders) && Objects.equals(requiredAspectProviders, that.requiredAspectProviders) && Objects.equals(provides, that.provides) && Objects.equals(paramAttributes, that.paramAttributes) @@ -290,6 +297,7 @@ public int hashCode() { implementation, attributeAspects, attributes, + requiredProviders, requiredAspectProviders, provides, paramAttributes, diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index d42e7fff2117a4..d3b47f4cd0591c 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -627,6 +627,21 @@ public class BuildLanguageOptions extends OptionsBase implements Serializable { + " environment and inputs during execution.") public boolean experimentalShadowedAction; + @Option( + name = "incompatible_top_level_aspects_require_providers", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + help = + "If set to true, the top level aspect will honor its required providers and only run on" + + " top level targets whose rules' advertised providers satisfy the required" + + " providers of the aspect.") + public boolean incompatibleTopLevelAspectsRequireProviders; + /** * An interner to reduce the number of StarlarkSemantics instances. A single Blaze instance should * never accumulate a large number of these and being able to shortcut on object identity makes a @@ -694,6 +709,9 @@ public StarlarkSemantics toStarlarkSemantics() { .set(MAX_COMPUTATION_STEPS, maxComputationSteps) .set(NESTED_SET_DEPTH_LIMIT, nestedSetDepthLimit) .setBool(EXPERIMENTAL_SHADOWED_ACTION, experimentalShadowedAction) + .setBool( + INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS, + incompatibleTopLevelAspectsRequireProviders) .build(); return INTERNER.intern(semantics); } @@ -765,6 +783,8 @@ public StarlarkSemantics toStarlarkSemantics() { public static final String INCOMPATIBLE_VISIBILITY_PRIVATE_ATTRIBUTES_AT_DEFINITION = "-incompatible_visibility_private_attributes_at_definition"; public static final String EXPERIMENTAL_SHADOWED_ACTION = "-experimental_shadowed_action"; + public static final String INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS = + "-incompatible_top_level_aspects_require_providers"; // non-booleans public static final StarlarkSemantics.Key EXPERIMENTAL_BUILTINS_BZL_PATH = diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java index 44ab8a796da506..aead10e5076850 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java @@ -58,12 +58,14 @@ import com.google.devtools.build.lib.packages.NoSuchThingException; import com.google.devtools.build.lib.packages.OutputFile; import com.google.devtools.build.lib.packages.Package; +import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.StarlarkAspect; import com.google.devtools.build.lib.packages.StarlarkAspectClass; import com.google.devtools.build.lib.packages.StarlarkDefinedAspect; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.Type.ConversionException; +import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.profiler.memory.CurrentRuleTracker; import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.skyframe.BzlLoadFunction.BzlLoadFailedException; @@ -78,6 +80,7 @@ import java.util.HashMap; import java.util.Map; import javax.annotation.Nullable; +import net.starlark.java.eval.StarlarkSemantics; /** * The Skyframe function that generates aspects. @@ -314,6 +317,34 @@ public SkyValue compute(SkyKey skyKey, Environment env) baseConfiguredTargetValue.getConfiguredTarget()); } + // If the incompatible flag is set, the top-level aspect should not be applied on top-level + // targets whose rules do not advertise the aspect's required providers. The aspect should not + // also propagate to these targets dependencies. + StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env); + if (starlarkSemantics == null) { + return null; + } + boolean checkRuleAdvertisedProviders = + starlarkSemantics.getBool( + BuildLanguageOptions.INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS); + if (checkRuleAdvertisedProviders) { + Target target = associatedConfiguredTargetAndData.getTarget(); + if (target instanceof Rule) { + if (!aspect + .getDefinition() + .getRequiredProviders() + .isSatisfiedBy(((Rule) target).getRuleClassObject().getAdvertisedProviders())) { + return new AspectValue( + key, + aspect, + target.getLocation(), + ConfiguredAspect.forNonapplicableTarget(), + /*transitivePackagesForPackageRootResolution=*/ NestedSetBuilder + .stableOrder() + .build()); + } + } + } ImmutableList.Builder aspectPathBuilder = ImmutableList.builder(); diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java index b268a76782b5dc..2a82a6ee45632b 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkRuleFunctionsApi.java @@ -416,6 +416,31 @@ StarlarkCallable rule( + "the values restriction. Explicit attributes restrict the " + "aspect to only be used with rules that have attributes of the same " + "name, type, and valid values according to the restriction."), + @Param( + name = "required_providers", + named = true, + defaultValue = "[]", + doc = + "This attribute allows the aspect to limit its propagation to only the targets " + + "whose rules advertise its required providers. The value must be a " + + "list containing either individual providers or lists of providers but not " + + "both. For example, [[FooInfo], [BarInfo], [BazInfo, QuxInfo]] " + + "is a valid value while [FooInfo, BarInfo, [BazInfo, QuxInfo]] " + + "is not valid." + + "" + + "

An unnested list of providers will automatically be converted to a list " + + "containing one list of providers. That is, [FooInfo, BarInfo] " + + "will automatically be converted to [[FooInfo, BarInfo]]." + + "" + + "

To make some rule (e.g. some_rule) targets visible to an " + + "aspect, some_rule must advertise all providers from at least " + + "one of the required providers lists. For example, if the " + + "required_providers of an aspect are " + + "[[FooInfo], [BarInfo], [BazInfo, QuxInfo]], this aspect can " + + "only see some_rule targets if and only if " + + "some_rule provides FooInfo *or* " + + "BarInfo *or* both BazInfo *and* " + + "QuxInfo."), @Param( name = "required_aspect_providers", named = true, @@ -500,6 +525,7 @@ StarlarkAspectApi aspect( StarlarkFunction implementation, Sequence attributeAspects, Object attrs, + Sequence requiredProvidersArg, Sequence requiredAspectProvidersArg, Sequence providesArg, Sequence fragments, diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java index daad2b721f9499..9b163e2455da4e 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkRuleFunctionsApi.java @@ -180,6 +180,7 @@ public StarlarkAspectApi aspect( StarlarkFunction implementation, Sequence attributeAspects, Object attrs, + Sequence requiredProvidersArg, Sequence requiredAspectProvidersArg, Sequence providesArg, Sequence fragments, diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java index 2711dfc1c8b102..741da4564579d3 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java @@ -33,6 +33,8 @@ import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy; import com.google.devtools.build.lib.packages.NativeAspectClass; +import com.google.devtools.build.lib.packages.StarlarkProvider; +import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier; import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; import com.google.devtools.build.lib.util.FileTypeSet; import net.starlark.java.annot.StarlarkBuiltin; @@ -55,6 +57,20 @@ private static final class P3 implements TransitiveInfoProvider {} private static final class P4 implements TransitiveInfoProvider {} + private static final Label FAKE_LABEL = Label.parseAbsoluteUnchecked("//fake/label.bzl"); + + private static final StarlarkProviderIdentifier STARLARK_P1 = + StarlarkProviderIdentifier.forKey(new StarlarkProvider.Key(FAKE_LABEL, "STARLARK_P1")); + + private static final StarlarkProviderIdentifier STARLARK_P2 = + StarlarkProviderIdentifier.forKey(new StarlarkProvider.Key(FAKE_LABEL, "STARLARK_P2")); + + private static final StarlarkProviderIdentifier STARLARK_P3 = + StarlarkProviderIdentifier.forKey(new StarlarkProvider.Key(FAKE_LABEL, "STARLARK_P3")); + + private static final StarlarkProviderIdentifier STARLARK_P4 = + StarlarkProviderIdentifier.forKey(new StarlarkProvider.Key(FAKE_LABEL, "STARLARK_P4")); + /** * A dummy aspect factory. Is there to demonstrate how to define aspects and so that we can test * {@code attributeAspect}. @@ -150,7 +166,7 @@ public void testAttributeAspect_allAttributes() throws Exception { } @Test - public void testRequireProvider_addsToSetOfRequiredProvidersAndNames() throws Exception { + public void testRequireBuiltinProviders_addsToSetOfRequiredProvidersAndNames() throws Exception { AspectDefinition requiresProviders = new AspectDefinition.Builder(TEST_ASPECT_CLASS) .requireProviders(P1.class, P2.class) @@ -176,7 +192,8 @@ public void testRequireProvider_addsToSetOfRequiredProvidersAndNames() throws Ex } @Test - public void testRequireProvider_addsTwoSetsOfRequiredProvidersAndNames() throws Exception { + public void testRequireBuiltinProviders_addsTwoSetsOfRequiredProvidersAndNames() + throws Exception { AspectDefinition requiresProviders = new AspectDefinition.Builder(TEST_ASPECT_CLASS) .requireProviderSets( @@ -202,6 +219,73 @@ public void testRequireProvider_addsTwoSetsOfRequiredProvidersAndNames() throws } + @Test + public void testRequireStarlarkProviders_addsFlatSetOfRequiredProviders() throws Exception { + AspectDefinition requiresProviders = + new AspectDefinition.Builder(TEST_ASPECT_CLASS) + .requireStarlarkProviders(STARLARK_P1, STARLARK_P2) + .build(); + + AdvertisedProviderSet expectedOkSet = + AdvertisedProviderSet.builder() + .addStarlark(STARLARK_P1) + .addStarlark(STARLARK_P2) + .addStarlark(STARLARK_P3) + .build(); + assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(expectedOkSet)).isTrue(); + + AdvertisedProviderSet expectedFailSet = + AdvertisedProviderSet.builder().addStarlark(STARLARK_P1).build(); + assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(expectedFailSet)).isFalse(); + + assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(AdvertisedProviderSet.ANY)) + .isTrue(); + assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(AdvertisedProviderSet.EMPTY)) + .isFalse(); + } + + @Test + public void testRequireStarlarkProviders_addsTwoSetsOfRequiredProviders() throws Exception { + AspectDefinition requiresProviders = + new AspectDefinition.Builder(TEST_ASPECT_CLASS) + .requireStarlarkProviderSets( + ImmutableList.of( + ImmutableSet.of(STARLARK_P1, STARLARK_P2), ImmutableSet.of(STARLARK_P3))) + .build(); + + AdvertisedProviderSet expectedOkSet1 = + AdvertisedProviderSet.builder().addStarlark(STARLARK_P1).addStarlark(STARLARK_P2).build(); + assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(expectedOkSet1)).isTrue(); + + AdvertisedProviderSet expectedOkSet2 = + AdvertisedProviderSet.builder().addStarlark(STARLARK_P3).build(); + assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(expectedOkSet2)).isTrue(); + + AdvertisedProviderSet expectedFailSet = + AdvertisedProviderSet.builder().addStarlark(STARLARK_P4).build(); + assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(expectedFailSet)).isFalse(); + + assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(AdvertisedProviderSet.ANY)) + .isTrue(); + assertThat(requiresProviders.getRequiredProviders().isSatisfiedBy(AdvertisedProviderSet.EMPTY)) + .isFalse(); + } + + @Test + public void testRequireProviders_defaultAcceptsEverything() { + AspectDefinition noRequiredProviders = new AspectDefinition.Builder(TEST_ASPECT_CLASS).build(); + + AdvertisedProviderSet expectedOkSet = + AdvertisedProviderSet.builder().addBuiltin(P4.class).addStarlark(STARLARK_P4).build(); + assertThat(noRequiredProviders.getRequiredProviders().isSatisfiedBy(expectedOkSet)).isTrue(); + + assertThat(noRequiredProviders.getRequiredProviders().isSatisfiedBy(AdvertisedProviderSet.ANY)) + .isTrue(); + assertThat( + noRequiredProviders.getRequiredProviders().isSatisfiedBy(AdvertisedProviderSet.EMPTY)) + .isTrue(); + } + @Test public void testRequireAspectClass_defaultAcceptsNothing() { AspectDefinition noAspects = new AspectDefinition.Builder(TEST_ASPECT_CLASS) diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java index 92800aebec496a..326e56d1ce30dd 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java @@ -50,8 +50,12 @@ import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; +import java.util.ArrayList; +import java.util.List; import net.starlark.java.eval.Sequence; +import net.starlark.java.eval.Starlark; import net.starlark.java.eval.StarlarkInt; +import net.starlark.java.eval.StarlarkList; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -3086,6 +3090,336 @@ public void testAspectActionsDontInheritExecProperties() throws Exception { assertThat(owner.getExecProperties()).isEmpty(); } + @Test + public void testAspectRequiredProviders_defaultNoRequiredProviders() throws Exception { + scratch.file( + "test/defs.bzl", + "prov_a = provider()", + "prov_b = provider()", + "", + "def _my_aspect_impl(target, ctx):", + " targets_labels = [\"//test:defs.bzl%my_aspect({})\".format(target.label)]", + " for dep in ctx.rule.attr.deps:", + " if hasattr(dep, 'target_labels'):", + " targets_labels.extend(dep.target_labels)", + " return struct(target_labels = targets_labels)", + "", + "my_aspect = aspect(", + " implementation = _my_aspect_impl,", + " attr_aspects = ['deps'],", + ")", + "", + "def _rule_without_providers_impl(ctx):", + " s = []", + " for dep in ctx.attr.deps:", + " if hasattr(dep, 'target_labels'):", + " s.extend(dep.target_labels)", + " return struct(rule_deps = s)", + "", + "rule_without_providers = rule(", + " implementation = _rule_without_providers_impl,", + " attrs = {", + " 'deps': attr.label_list(aspects = [my_aspect])", + " },", + ")", + "", + "def _rule_with_providers_impl(ctx):", + " return [prov_a(), prov_b()]", + "", + "rule_with_providers = rule(", + " implementation = _rule_with_providers_impl,", + " attrs = {", + " 'deps': attr.label_list()", + " },", + " provides = [prov_a, prov_b]", + ")", + "", + "rule_with_providers_not_advertised = rule(", + " implementation = _rule_with_providers_impl,", + " attrs = {", + " 'deps': attr.label_list()", + " },", + " provides = []", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'rule_with_providers', 'rule_without_providers',", + " 'rule_with_providers_not_advertised')", + "rule_without_providers(", + " name = 'main',", + " deps = [':target_without_providers', ':target_with_providers',", + " ':target_with_providers_not_advertised'],", + ")", + "rule_without_providers(", + " name = 'target_without_providers',", + ")", + "rule_with_providers(", + " name = 'target_with_providers',", + ")", + "rule_with_providers(", + " name = 'target_with_providers_indeps',", + ")", + "rule_with_providers_not_advertised(", + " name = 'target_with_providers_not_advertised',", + " deps = [':target_with_providers_indeps'],", + ")"); + + AnalysisResult analysisResult = update("//test:main"); + + // my_aspect does not require any providers so it will be applied to all the dependencies of + // main target + List expected = new ArrayList<>(); + expected.add("//test:defs.bzl%my_aspect(//test:target_without_providers)"); + expected.add("//test:defs.bzl%my_aspect(//test:target_with_providers)"); + expected.add("//test:defs.bzl%my_aspect(//test:target_with_providers_not_advertised)"); + expected.add("//test:defs.bzl%my_aspect(//test:target_with_providers_indeps)"); + assertThat(getLabelsToBuild(analysisResult)).containsExactly("//test:main"); + ConfiguredTarget target = analysisResult.getTargetsToBuild().iterator().next(); + Object ruleDepsUnchecked = target.get("rule_deps"); + assertThat(ruleDepsUnchecked).isInstanceOf(StarlarkList.class); + StarlarkList ruleDeps = (StarlarkList) ruleDepsUnchecked; + assertThat(Starlark.toIterable(ruleDeps)).containsExactlyElementsIn(expected); + } + + @Test + public void testAspectRequiredProviders_flatSetOfRequiredProviders() throws Exception { + scratch.file( + "test/defs.bzl", + "prov_a = provider()", + "prov_b = provider()", + "", + "def _my_aspect_impl(target, ctx):", + " targets_labels = [\"//test:defs.bzl%my_aspect({})\".format(target.label)]", + " for dep in ctx.rule.attr.deps:", + " if hasattr(dep, 'target_labels'):", + " targets_labels.extend(dep.target_labels)", + " return struct(target_labels = targets_labels)", + "", + "my_aspect = aspect(", + " implementation = _my_aspect_impl,", + " attr_aspects = ['deps'],", + " required_providers = [prov_a, prov_b],", + ")", + "", + "def _rule_without_providers_impl(ctx):", + " s = []", + " for dep in ctx.attr.deps:", + " if hasattr(dep, 'target_labels'):", + " s.extend(dep.target_labels)", + " return struct(rule_deps = s)", + "", + "rule_without_providers = rule(", + " implementation = _rule_without_providers_impl,", + " attrs = {", + " 'deps': attr.label_list(aspects=[my_aspect])", + " },", + " provides = []", + ")", + "", + "def _rule_with_a_impl(ctx):", + " return [prov_a()]", + "", + "rule_with_a = rule(", + " implementation = _rule_with_a_impl,", + " attrs = {", + " 'deps': attr.label_list()", + " },", + " provides = [prov_a]", + ")", + "", + "def _rule_with_ab_impl(ctx):", + " return [prov_a(), prov_b()]", + "", + "rule_with_ab = rule(", + " implementation = _rule_with_ab_impl,", + " attrs = {", + " 'deps': attr.label_list()", + " },", + " provides = [prov_a, prov_b]", + ")", + "", + "rule_with_ab_not_advertised = rule(", + " implementation = _rule_with_ab_impl,", + " attrs = {", + " 'deps': attr.label_list()", + " },", + " provides = []", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'rule_without_providers', 'rule_with_a', 'rule_with_ab',", + " 'rule_with_ab_not_advertised')", + "rule_without_providers(", + " name = 'main',", + " deps = [':target_without_providers', ':target_with_a', ':target_with_ab',", + " ':target_with_ab_not_advertised'],", + ")", + "rule_without_providers(", + " name = 'target_without_providers',", + ")", + "rule_with_a(", + " name = 'target_with_a',", + " deps = [':target_with_ab_indeps_not_reached']", + ")", + "rule_with_ab(", + " name = 'target_with_ab',", + " deps = [':target_with_ab_indeps_reached']", + ")", + "rule_with_ab(", + " name = 'target_with_ab_indeps_not_reached',", + ")", + "rule_with_ab(", + " name = 'target_with_ab_indeps_reached',", + ")", + "rule_with_ab_not_advertised(", + " name = 'target_with_ab_not_advertised',", + ")"); + + AnalysisResult analysisResult = update("//test:main"); + + // my_aspect will only be applied on target_with_ab and target_with_ab_indeps_reached since + // their rule (rule_with_ab) is the only rule that advertises the aspect required providers. + // However, my_aspect cannot be propagated to target_with_ab_indeps_not_reached because it was + // not applied to its parent (target_with_a) + List expected = new ArrayList<>(); + expected.add("//test:defs.bzl%my_aspect(//test:target_with_ab)"); + expected.add("//test:defs.bzl%my_aspect(//test:target_with_ab_indeps_reached)"); + assertThat(getLabelsToBuild(analysisResult)).containsExactly("//test:main"); + ConfiguredTarget target = analysisResult.getTargetsToBuild().iterator().next(); + Object ruleDepsUnchecked = target.get("rule_deps"); + assertThat(ruleDepsUnchecked).isInstanceOf(StarlarkList.class); + StarlarkList ruleDeps = (StarlarkList) ruleDepsUnchecked; + assertThat(Starlark.toIterable(ruleDeps)).containsExactlyElementsIn(expected); + } + + @Test + public void testAspectRequiredProviders_listOfRequiredProvidersLists() throws Exception { + scratch.file( + "test/defs.bzl", + "prov_a = provider()", + "prov_b = provider()", + "prov_c = provider()", + "", + "def _my_aspect_impl(target, ctx):", + " targets_labels = [\"//test:defs.bzl%my_aspect({})\".format(target.label)]", + " for dep in ctx.rule.attr.deps:", + " if hasattr(dep, 'target_labels'):", + " targets_labels.extend(dep.target_labels)", + " return struct(target_labels = targets_labels)", + "", + "my_aspect = aspect(", + " implementation = _my_aspect_impl,", + " attr_aspects = ['deps'],", + " required_providers = [[prov_a, prov_b], [prov_c]],", + ")", + "", + "def _rule_without_providers_impl(ctx):", + " s = []", + " for dep in ctx.attr.deps:", + " if hasattr(dep, 'target_labels'):", + " s.extend(dep.target_labels)", + " return struct(rule_deps = s)", + "", + "rule_without_providers = rule(", + " implementation = _rule_without_providers_impl,", + " attrs = {", + " 'deps': attr.label_list(aspects=[my_aspect])", + " },", + " provides = []", + ")", + "", + "def _rule_with_a_impl(ctx):", + " return [prov_a()]", + "", + "rule_with_a = rule(", + " implementation = _rule_with_a_impl,", + " attrs = {", + " 'deps': attr.label_list()", + " },", + " provides = [prov_a]", + ")", + "", + "def _rule_with_c_impl(ctx):", + " return [prov_c()]", + "", + "rule_with_c = rule(", + " implementation = _rule_with_c_impl,", + " attrs = {", + " 'deps': attr.label_list()", + " },", + " provides = [prov_c]", + ")", + "", + "def _rule_with_ab_impl(ctx):", + " return [prov_a(), prov_b()]", + "", + "rule_with_ab = rule(", + " implementation = _rule_with_ab_impl,", + " attrs = {", + " 'deps': attr.label_list()", + " },", + " provides = [prov_a, prov_b]", + ")", + "", + "rule_with_ab_not_advertised = rule(", + " implementation = _rule_with_ab_impl,", + " attrs = {", + " 'deps': attr.label_list()", + " },", + " provides = []", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'rule_without_providers', 'rule_with_a', 'rule_with_c',", + " 'rule_with_ab', 'rule_with_ab_not_advertised')", + "rule_without_providers(", + " name = 'main',", + " deps = [':target_without_providers', ':target_with_a', ':target_with_c',", + " ':target_with_ab', 'target_with_ab_not_advertised'],", + ")", + "rule_without_providers(", + " name = 'target_without_providers',", + ")", + "rule_with_a(", + " name = 'target_with_a',", + " deps = [':target_with_c_indeps_not_reached'],", + ")", + "rule_with_c(", + " name = 'target_with_c',", + ")", + "rule_with_c(", + " name = 'target_with_c_indeps_reached',", + ")", + "rule_with_c(", + " name = 'target_with_c_indeps_not_reached',", + ")", + "rule_with_ab(", + " name = 'target_with_ab',", + " deps = [':target_with_c_indeps_reached'],", + ")", + "rule_with_ab_not_advertised(", + " name = 'target_with_ab_not_advertised',", + ")"); + + AnalysisResult analysisResult = update("//test:main"); + + // my_aspect will only be applied on target_with_ab, target_wtih_c and + // target_with_c_indeps_reached because their rules (rule_with_ab and rule_with_c) are the only + // rules advertising the aspect required providers + // However, my_aspect cannot be propagated to target_with_c_indeps_not_reached because it was + // not applied to its parent (target_with_a) + List expected = new ArrayList<>(); + expected.add("//test:defs.bzl%my_aspect(//test:target_with_ab)"); + expected.add("//test:defs.bzl%my_aspect(//test:target_with_c)"); + expected.add("//test:defs.bzl%my_aspect(//test:target_with_c_indeps_reached)"); + assertThat(getLabelsToBuild(analysisResult)).containsExactly("//test:main"); + ConfiguredTarget target = analysisResult.getTargetsToBuild().iterator().next(); + Object ruleDepsUnchecked = target.get("rule_deps"); + assertThat(ruleDepsUnchecked).isInstanceOf(StarlarkList.class); + StarlarkList ruleDeps = (StarlarkList) ruleDepsUnchecked; + assertThat(Starlark.toIterable(ruleDeps)).containsExactlyElementsIn(expected); + } + /** StarlarkAspectTest with "keep going" flag */ @RunWith(JUnit4.class) public static final class WithKeepGoing extends StarlarkDefinedAspectsTest { diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java index 826c37d50e9d20..b43b362558b2f8 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java @@ -1632,6 +1632,91 @@ public void aspectRequiredAspectProvidersDefault() throws Exception { assertThat(requiredProviders.isSatisfiedBy(AdvertisedProviderSet.EMPTY)).isFalse(); } + @Test + public void aspectRequiredProvidersSingle() throws Exception { + evalAndExport( + ev, + "def _impl(target, ctx):", + " pass", + "cc = provider()", + "my_aspect = aspect(_impl, required_providers=['java', cc])"); + StarlarkDefinedAspect myAspect = (StarlarkDefinedAspect) ev.lookup("my_aspect"); + RequiredProviders requiredProviders = + myAspect.getDefinition(AspectParameters.EMPTY).getRequiredProviders(); + + assertThat(requiredProviders.isSatisfiedBy(AdvertisedProviderSet.ANY)).isTrue(); + assertThat(requiredProviders.isSatisfiedBy(AdvertisedProviderSet.EMPTY)).isFalse(); + assertThat( + requiredProviders.isSatisfiedBy( + AdvertisedProviderSet.builder() + .addStarlark(declared("cc")) + .addStarlark("java") + .build())) + .isTrue(); + assertThat( + requiredProviders.isSatisfiedBy( + AdvertisedProviderSet.builder().addStarlark(declared("cc")).build())) + .isFalse(); + } + + @Test + public void aspectRequiredProvidersAlternatives() throws Exception { + evalAndExport( + ev, + "def _impl(target, ctx):", + " pass", + "cc = provider()", + "my_aspect = aspect(_impl, required_providers=[['java'], [cc]])"); + StarlarkDefinedAspect myAspect = (StarlarkDefinedAspect) ev.lookup("my_aspect"); + RequiredProviders requiredProviders = + myAspect.getDefinition(AspectParameters.EMPTY).getRequiredProviders(); + + assertThat(requiredProviders.isSatisfiedBy(AdvertisedProviderSet.ANY)).isTrue(); + assertThat(requiredProviders.isSatisfiedBy(AdvertisedProviderSet.EMPTY)).isFalse(); + assertThat( + requiredProviders.isSatisfiedBy( + AdvertisedProviderSet.builder().addStarlark("java").build())) + .isTrue(); + assertThat( + requiredProviders.isSatisfiedBy( + AdvertisedProviderSet.builder().addStarlark(declared("cc")).build())) + .isTrue(); + assertThat( + requiredProviders.isSatisfiedBy( + AdvertisedProviderSet.builder().addStarlark("prolog").build())) + .isFalse(); + } + + @Test + public void aspectRequiredProvidersEmpty() throws Exception { + evalAndExport( + ev, + "def _impl(target, ctx):", + " pass", + "my_aspect = aspect(_impl, required_providers=[])"); + StarlarkDefinedAspect myAspect = (StarlarkDefinedAspect) ev.lookup("my_aspect"); + RequiredProviders requiredProviders = + myAspect.getDefinition(AspectParameters.EMPTY).getRequiredProviders(); + + assertThat(requiredProviders.isSatisfiedBy(AdvertisedProviderSet.ANY)).isTrue(); + assertThat(requiredProviders.isSatisfiedBy(AdvertisedProviderSet.EMPTY)).isTrue(); + } + + @Test + public void aspectRequiredProvidersDefault() throws Exception { + evalAndExport( + ev, + "def _impl(target, ctx):", // + " pass", + "my_aspect = aspect(_impl)"); + StarlarkDefinedAspect myAspect = (StarlarkDefinedAspect) ev.lookup("my_aspect"); + RequiredProviders requiredProviders = + myAspect.getDefinition(AspectParameters.EMPTY).getRequiredProviders(); + + assertThat(requiredProviders.isSatisfiedBy(AdvertisedProviderSet.ANY)).isTrue(); + assertThat(requiredProviders.isSatisfiedBy(AdvertisedProviderSet.EMPTY)).isTrue(); + } + @Test public void aspectProvides() throws Exception { evalAndExport( diff --git a/src/test/shell/integration/aspect_test.sh b/src/test/shell/integration/aspect_test.sh index 0d960f7b94c9bf..63f0c978e88165 100755 --- a/src/test/shell/integration/aspect_test.sh +++ b/src/test/shell/integration/aspect_test.sh @@ -89,4 +89,356 @@ EOF expect_not_log "IllegalStateException" } +function test_aspect_required_providers_with_toplevel_aspects() { + local package="a" + mkdir -p "${package}" + + cat > "${package}/lib.bzl" < "${package}/BUILD" <"$TEST_log" \ + || fail "Build failed but should have succeeded" + + # Only aspect_a is applied on target_with_a because its "provided" providers + # do not macth aspect_b required providers. + expect_log "aspect_a runs on target //${package}:target_with_a" + expect_not_log "aspect_b runs on target //${package}:target_with_a" + + # Only aspect_a can run on target_with_a_indeps + expect_log "aspect_a runs on target //${package}:target_with_a_indeps" + expect_not_log "aspect_b runs on target //${package}:target_with_a_indeps" + + # Only aspect_b can run on target_with_bc + expect_not_log "aspect_a runs on target //${package}:target_with_bc" + expect_log "aspect_b runs on target //${package}:target_with_bc" + + # using --incompatible_top_level_aspects_require_providers, the top level + # target rule's advertised providers will be checked and only aspect_a will be + # applied on target_with_a and propgated to its dependencies. + bazel build "${package}:target_with_a" \ + --aspects="//${package}:lib.bzl%aspect_a" \ + --aspects="//${package}:lib.bzl%aspect_b" &>"$TEST_log" \ + --incompatible_top_level_aspects_require_providers \ + || fail "Build failed but should have succeeded" + + # Only aspect_a is applied on target_with_a + expect_log "aspect_a runs on target //${package}:target_with_a" + expect_not_log "aspect_b runs on target //${package}:target_with_a" + + # Only aspect_a can run on target_with_a_indeps + expect_log "aspect_a runs on target //${package}:target_with_a_indeps" + expect_not_log "aspect_b runs on target //${package}:target_with_a_indeps" + + # rule_with_bc advertised provides only match the required providers for + # aspect_b, but aspect_b is not propagated from target_with_a + expect_not_log "aspect_a runs on target //${package}:target_with_bc" + expect_not_log "aspect_b runs on target //${package}:target_with_bc" +} + +function test_aspect_required_providers_default_no_required_providers() { + local package="a" + mkdir -p "${package}" + + cat > "${package}/lib.bzl" < "${package}/BUILD" <"$TEST_log" \ + || fail "Build failed but should have succeeded" + + # my_aspect does not require any providers so it will be applied to all the + # dependencies of main target + expect_log "my_aspect runs on target //${package}:target_without_providers" + expect_log "my_aspect runs on target //${package}:target_with_providers" + expect_log "my_aspect runs on target //${package}:target_with_providers_not_advertised" + expect_log "my_aspect runs on target //${package}:target_with_providers_indeps" +} + +function test_aspect_required_providers_flat_set_of_required_providers() { + local package="a" + mkdir -p "${package}" + + cat > "${package}/lib.bzl" < "${package}/BUILD" <"$TEST_log" \ + || fail "Build failed but should have succeeded" + + # my_aspect will only be applied on target_with_ab and + # target_with_ab_indeps_reached since their rule (rule_with_ab) is the only + # rule that advertises the aspect required providers. + expect_log "my_aspect runs on target //${package}:target_with_ab" + expect_log "my_aspect runs on target //${package}:target_with_ab_indeps_reached" + expect_not_log "/^my_aspect runs on target //${package}:target_with_a$/" + expect_not_log "my_aspect runs on target //${package}:target_without_providers" + expect_not_log "my_aspect runs on target //${package}:target_with_ab_not_advertised" + + # my_aspect cannot be propagated to target_with_ab_indeps_not_reached + # because it was not applied to its parent (target_with_a) + expect_not_log "my_aspect runs on target //${package}:target_with_ab_indeps_not_reached" +} + +function test_aspect_required_providers_with_list_of_required_providers_lists() { + local package="a" + mkdir -p "${package}" + + cat > "${package}/lib.bzl" < "${package}/BUILD" <"$TEST_log" \ + || fail "Build failed but should have succeeded" + + # my_aspect will only be applied on target_with_ab, target_wtih_c and + # target_with_c_indeps_reached because their rules (rule_with_ab and + # rule_with_c) are the only rules advertising the aspect required providers + expect_log "my_aspect runs on target //${package}:target_with_ab" + expect_log "my_aspect runs on target //${package}:target_with_c" + expect_log "my_aspect runs on target //${package}:target_with_c_indeps_reached" + expect_not_log "my_aspect runs on target //${package}:target_without_providers" + expect_not_log "/^my_aspect runs on target //${package}:target_with_a$/" + expect_not_log "my_aspect runs on target //${package}:target_with_ab_not_advertised" + + # my_aspect cannot be propagated to target_with_c_indeps_not_reached because it was + # not applied to its parent (target_with_a) + expect_not_log "my_aspect runs on target //${package}:target_with_c_indeps_not_reached" +} + run_suite "Tests for aspects"