Aspects propagating other aspects This CL adds a new parameter to the `aspect()` API called `requires` guarded by `experimental_required_aspects` flag. Through this parameter we can specify a list of aspects needed by the aspect being defined to be propagated before it to the targets it operates on. Aspects can only require Starlark defined aspects. PiperOrigin-RevId: 377473191
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java index 0cbc338..d0d622f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectCollection.java
@@ -236,8 +236,9 @@ // Calculate all needed aspects. Already discovered aspects are in key set of deps. // 1) Start from the end of the path. The aspect only sees other aspects that are // before it - // 2) Otherwise, check whether 'aspect' is visible to any already seen aspects. If it is visible - // to 'depAspect', add the 'aspect' to a list of aspects visible to 'depAspect'. + // 2) Otherwise, check whether 'aspect' is visible to or required by any already seen aspects. + // If it is visible to 'depAspect' or explicitly required by it, add the 'aspect' to a list of + // aspects visible to 'depAspect'. // At the end of this algorithm, key set of 'deps' contains the original aspect list in reverse // (since we iterate the original list in reverse). // @@ -246,8 +247,11 @@ ImmutableList.copyOf(aspectMap.entrySet()).reverse()) { for (AspectDescriptor depAspectDescriptor : deps.keySet()) { Aspect depAspect = aspectMap.get(depAspectDescriptor); - if (depAspect.getDefinition().getRequiredProvidersForAspects() - .isSatisfiedBy(aspect.getValue().getDefinition().getAdvertisedProviders())) { + if (depAspect + .getDefinition() + .getRequiredProvidersForAspects() + .isSatisfiedBy(aspect.getValue().getDefinition().getAdvertisedProviders()) + || depAspect.getDefinition().requires(aspect.getValue())) { deps.get(depAspectDescriptor).add(aspect.getKey()); } } @@ -287,12 +291,12 @@ } /** - * Detect inconsistent duplicate occurrence of an aspect on the path. - * There is a previous occurrence of {@code aspect} in {@code seenAspects}. + * Detect inconsistent duplicate occurrence of an aspect on the path. There is a previous + * occurrence of {@code aspect} in {@code seenAspects}. * - * If in between that previous occurrence and the newly discovered occurrence - * there is an aspect that is visible to {@code aspect}, then the second occurrence - * is inconsistent - the set of aspects it sees is different from the first one. + * <p>If in between that previous occurrence and the newly discovered occurrence there is an + * aspect that is visible to or required by {@code aspect}, then the second occurrence is + * inconsistent - the set of aspects it sees is different from the first one. */ private static void validateDuplicateAspect(Aspect aspect, ArrayList<Aspect> seenAspects) throws AspectCycleOnPathException { @@ -303,8 +307,11 @@ return; } - if (aspect.getDefinition().getRequiredProvidersForAspects() - .isSatisfiedBy(seenAspect.getDefinition().getAdvertisedProviders())) { + if (aspect + .getDefinition() + .getRequiredProvidersForAspects() + .isSatisfiedBy(seenAspect.getDefinition().getAdvertisedProviders()) + || aspect.getDefinition().requires(seenAspect)) { throw new AspectCycleOnPathException(aspect.getDescriptor(), seenAspect.getDescriptor()); } }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java index 04db273..8ce6e4d 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
@@ -37,6 +37,7 @@ import com.google.devtools.build.lib.causes.Cause; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.packages.AdvertisedProviderSet; import com.google.devtools.build.lib.packages.Aspect; import com.google.devtools.build.lib.packages.AspectClass; import com.google.devtools.build.lib.packages.Attribute; @@ -719,7 +720,8 @@ continue; } - if (aspect.getDefinition().propagateAlong(attributeName)) { + if (aspect.getDefinition().propagateAlong(attributeName) + || aspect.getDescriptor().inheritedPropagateAlong(attributeName)) { filteredAspectPath.add(aspect); } } @@ -773,12 +775,14 @@ Rule toRule = (Rule) toTarget; ImmutableList.Builder<Aspect> filteredAspectPath = ImmutableList.builder(); - + AdvertisedProviderSet advertisedProviders = + toRule.getRuleClassObject().getAdvertisedProviders(); for (Aspect aspect : aspects) { - if (aspect - .getDefinition() - .getRequiredProviders() - .isSatisfiedBy(toRule.getRuleClassObject().getAdvertisedProviders())) { + if (aspect.getDefinition().getRequiredProviders().isSatisfiedBy(advertisedProviders) + || aspect + .getDescriptor() + .getInheritedRequiredProviders() + .isSatisfiedBy(advertisedProviders)) { filteredAspectPath.add(aspect); } }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java index db1048f..5593d21 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java
@@ -256,7 +256,14 @@ if (containsNonNoneKey(arguments, ASPECTS_ARG)) { Object obj = arguments.get(ASPECTS_ARG); for (StarlarkAspect aspect : Sequence.cast(obj, StarlarkAspect.class, "aspects")) { - aspect.attachToAttribute(builder); + aspect.attachToAttribute( + /** baseAspectName= */ + null, + builder, + /** inheritedRequiredProviders= */ + ImmutableList.of(), + /** inheritedAttributeAspects= */ + ImmutableList.of()); } }
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 1e1e2ff..65a5f4e 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
@@ -527,6 +527,7 @@ Sequence<?> requiredProvidersArg, Sequence<?> requiredAspectProvidersArg, Sequence<?> providesArg, + Sequence<?> requiredAspects, Sequence<?> fragments, Sequence<?> hostFragments, Sequence<?> toolchains, @@ -619,6 +620,7 @@ requiredAspectProvidersArg, "required_aspect_providers"), StarlarkAttrModule.getStarlarkProviderIdentifiers(providesArg), requiredParams.build(), + ImmutableSet.copyOf(Sequence.cast(requiredAspects, StarlarkAspect.class, "requires")), ImmutableSet.copyOf(Sequence.cast(fragments, String.class, "fragments")), HostTransition.INSTANCE, ImmutableSet.copyOf(Sequence.cast(hostFragments, String.class, "host_fragments")),
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java index 716f6b8..c83d2f6 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Aspect.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Aspect.java
@@ -17,6 +17,7 @@ import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.skyframe.serialization.DeserializationContext; import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec; @@ -71,6 +72,36 @@ this.aspectDefinition = Preconditions.checkNotNull(aspectDefinition); } + private Aspect( + AspectClass aspectClass, + AspectDefinition aspectDefinition, + AspectParameters parameters, + RequiredProviders inheritedRequiredProviders, + ImmutableSet<String> inheritedAttributeAspects) { + this.aspectDescriptor = + new AspectDescriptor( + Preconditions.checkNotNull(aspectClass), + Preconditions.checkNotNull(parameters), + Preconditions.checkNotNull(inheritedRequiredProviders), + inheritedAttributeAspects); + this.aspectDefinition = Preconditions.checkNotNull(aspectDefinition); + } + + public static Aspect forNative( + NativeAspectClass nativeAspectClass, + AspectParameters parameters, + RequiredProviders inheritedRequiredProviders, + ImmutableSet<String> inheritedAttributeAspects) { + AspectDefinition definition = + definitionCache.getUnchecked(nativeAspectClass).getUnchecked(parameters); + return new Aspect( + nativeAspectClass, + definition, + parameters, + inheritedRequiredProviders, + inheritedAttributeAspects); + } + public static Aspect forNative( NativeAspectClass nativeAspectClass, AspectParameters parameters) { AspectDefinition definition = @@ -85,8 +116,15 @@ public static Aspect forStarlark( StarlarkAspectClass starlarkAspectClass, AspectDefinition aspectDefinition, - AspectParameters parameters) { - return new Aspect(starlarkAspectClass, aspectDefinition, parameters); + AspectParameters parameters, + RequiredProviders inheritedRequiredProviders, + ImmutableSet<String> inheritedAttributeAspects) { + return new Aspect( + starlarkAspectClass, + aspectDefinition, + parameters, + inheritedRequiredProviders, + inheritedAttributeAspects); } /** @@ -153,7 +191,9 @@ return forStarlark( (StarlarkAspectClass) aspectDescriptor.getAspectClass(), aspectDefinition, - aspectDescriptor.getParameters()); + aspectDescriptor.getParameters(), + aspectDescriptor.getInheritedRequiredProviders(), + aspectDescriptor.getInheritedAttributeAspects()); } } }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java b/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java index c833f14..bf90746 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AspectClass.java
@@ -53,7 +53,8 @@ * {@link AspectClass} * | * V - * {@code AspectDescriptor} <- {@link AspectParameters} + * {@code AspectDescriptor} <- {@link AspectParameters}, {@code inheritedRequiredProviders}, + * \ {@code inheritedAttributeAspects} * \ * V * {@link Aspect} <- {@link AspectDefinition} (might require loading Starlark files) @@ -69,8 +70,9 @@ * label of .bzl file + symbol name. * <li>{@link AspectParameters} is a (key,value) pair list that can be used to parameterize aspect * classes - * <li>{@link AspectDescriptor} is a pair of {@code AspectClass} and {@link AspectParameters}. It - * uniquely identifies the aspect and can be used in SkyKeys. + * <li>{@link AspectDescriptor} is a wrapper for {@code AspectClass}, {@link AspectParameters}, + * {@code inheritedRequiredProviders} and {@code inheritedAttributeAspects}. It uniquely + * identifies the aspect and can be used in SkyKeys. * <li>{@link AspectDefinition} is a class encapsulating the aspect definition (what attributes * aspoect has, and along which dependencies does it propagate. * <li>{@link Aspect} is a fully instantiated instance of an Aspect after it is loaded. Getting an @@ -82,9 +84,10 @@ * </ul> * * {@link AspectDescriptor}, or in general, a tuple of ({@link AspectClass}, {@link - * AspectParameters}) is an identifier that should be used in SkyKeys or in other contexts that need - * equality for aspects. See also {@link com.google.devtools.build.lib.skyframe.AspectFunction} for - * details on Skyframe treatment of Aspects. + * AspectParameters}), {@code inheritedRequiredProviders} and {@code inheritedAttributeAspects} is + * an identifier that should be used in SkyKeys or in other contexts that need equality for aspects. + * See also {@link com.google.devtools.build.lib.skyframe.AspectFunction} for details on Skyframe + * treatment of Aspects. * * @see com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory * @see com.google.devtools.build.lib.skyframe.AspectFunction
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 bf5c3e0..1298594 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
@@ -14,6 +14,7 @@ package com.google.devtools.build.lib.packages; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -91,6 +92,8 @@ */ private final BiPredicate<Object, String> propagateViaAttribute; + private final ImmutableSet<AspectClass> requiredAspectClasses; + public AdvertisedProviderSet getAdvertisedProviders() { return advertisedProviders; } @@ -108,7 +111,8 @@ @Nullable ConfigurationFragmentPolicy configurationFragmentPolicy, boolean applyToFiles, boolean applyToGeneratingRules, - BiPredicate<Object, String> propagateViaAttribute) { + BiPredicate<Object, String> propagateViaAttribute, + ImmutableSet<AspectClass> requiredAspectClasses) { this.aspectClass = aspectClass; this.advertisedProviders = advertisedProviders; this.requiredProviders = requiredProviders; @@ -122,6 +126,7 @@ this.applyToFiles = applyToFiles; this.applyToGeneratingRules = applyToGeneratingRules; this.propagateViaAttribute = propagateViaAttribute; + this.requiredAspectClasses = requiredAspectClasses; } public String getName() { @@ -169,7 +174,7 @@ return requiredProvidersForAspects; } - /** Returns the set of required aspects for a given attribute. */ + /** Returns whether the aspect propagates along the give {@code attributeName} or not. */ public boolean propagateAlong(String attributeName) { if (restrictToAttributes != null) { return restrictToAttributes.contains(attributeName); @@ -177,6 +182,12 @@ return true; } + /** Returns the set of attributes along which the aspect propagates. */ + @VisibleForTesting + public ImmutableSet<String> getRestrictToAttributes() { + return restrictToAttributes; + } + /** Returns the set of configuration fragments required by this Aspect. */ public ConfigurationFragmentPolicy getConfigurationFragmentPolicy() { return configurationFragmentPolicy; @@ -212,6 +223,11 @@ return aspect.getDefinition().getRequiredProviders().isSatisfiedBy(advertisedProviderSet); } + /** Checks if the given {@code maybeRequiredAspect} is required by this aspect definition */ + public boolean requires(Aspect maybeRequiredAspect) { + return requiredAspectClasses.contains(maybeRequiredAspect.getAspectClass()); + } + @Nullable private static Label maybeGetRepositoryRelativeLabel(Rule from, @Nullable Label label) { return label == null ? null : from.getLabel().resolveRepositoryRelative(label); @@ -273,6 +289,7 @@ private boolean applyToGeneratingRules = false; private final List<Label> requiredToolchains = new ArrayList<>(); private boolean useToolchainTransition = false; + private ImmutableSet<AspectClass> requiredAspectClasses = ImmutableSet.of(); public Builder(AspectClass aspectClass) { this.aspectClass = aspectClass; @@ -323,6 +340,15 @@ } /** + * Asserts that this aspect requires a list of aspects to be applied before it on the configured + * target. + */ + public Builder requiredAspectClasses(ImmutableSet<AspectClass> requiredAspectClasses) { + this.requiredAspectClasses = requiredAspectClasses; + return this; + } + + /** * Optional predicate to conditionally propagate down an attribute based on the {@link * BuildConfiguration}. * @@ -605,7 +631,8 @@ configurationFragmentPolicy.build(), applyToFiles, applyToGeneratingRules, - propagateViaAttribute); + propagateViaAttribute, + requiredAspectClasses); } } }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectDescriptor.java b/src/main/java/com/google/devtools/build/lib/packages/AspectDescriptor.java index 947b8d5..aac0539 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AspectDescriptor.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AspectDescriptor.java
@@ -15,13 +15,16 @@ package com.google.devtools.build.lib.packages; import com.google.common.collect.ImmutableMultimap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.protobuf.TextFormat; import java.util.Map; import java.util.Objects; +import javax.annotation.Nullable; /** - * A pair of {@link AspectClass} and {@link AspectParameters}. + * A wrapper for {@link AspectClass}, {@link AspectParameters}, {@code inheritedRequiredProviders} + * and {@code inheritedAttributeAspects} * * <p>Used for dependency resolution. */ @@ -30,13 +33,42 @@ private final AspectClass aspectClass; private final AspectParameters aspectParameters; - public AspectDescriptor(AspectClass aspectClass, AspectParameters aspectParameters) { + /** + * Inherited required providers to enable aspects required by other aspects to be propagated along + * with their main aspect until they can be applied. + */ + private final RequiredProviders inheritedRequiredProviders; + /** + * Inherited required providers to enable aspects required by other aspects to be propagated along + * with their main aspect based on its propgation attributes. + */ + @Nullable private final ImmutableSet<String> inheritedAttributeAspects; + + public AspectDescriptor( + AspectClass aspectClass, + AspectParameters aspectParameters, + RequiredProviders inheritedRequiredProviders, + ImmutableSet<String> inheritedAttributeAspects) { this.aspectClass = aspectClass; this.aspectParameters = aspectParameters; + this.inheritedRequiredProviders = inheritedRequiredProviders; + this.inheritedAttributeAspects = inheritedAttributeAspects; + } + + public AspectDescriptor(AspectClass aspectClass, AspectParameters aspectParameters) { + this( + aspectClass, + aspectParameters, + /*inheritedRequiredProviders=*/ RequiredProviders.acceptNoneBuilder().build(), + /*inheritedAttributeAspects=*/ ImmutableSet.of()); } public AspectDescriptor(AspectClass aspectClass) { - this(aspectClass, AspectParameters.EMPTY); + this( + aspectClass, + AspectParameters.EMPTY, + /*inheritedRequiredProviders=*/ RequiredProviders.acceptNoneBuilder().build(), + /*inheritedAttributeAspects=*/ ImmutableSet.of()); } public AspectClass getAspectClass() { @@ -47,9 +79,26 @@ return aspectParameters; } + public RequiredProviders getInheritedRequiredProviders() { + return inheritedRequiredProviders; + } + + @Nullable + public ImmutableSet<String> getInheritedAttributeAspects() { + return inheritedAttributeAspects; + } + + public boolean inheritedPropagateAlong(String attributeName) { + if (inheritedAttributeAspects != null) { + return inheritedAttributeAspects.contains(attributeName); + } + return true; + } + @Override public int hashCode() { - return Objects.hash(aspectClass, aspectParameters); + return Objects.hash( + aspectClass, aspectParameters, inheritedRequiredProviders, inheritedAttributeAspects); } @Override @@ -64,7 +113,9 @@ AspectDescriptor that = (AspectDescriptor) obj; return Objects.equals(aspectClass, that.aspectClass) - && Objects.equals(aspectParameters, that.aspectParameters); + && Objects.equals(aspectParameters, that.aspectParameters) + && Objects.equals(inheritedRequiredProviders, that.inheritedRequiredProviders) + && Objects.equals(inheritedAttributeAspects, that.inheritedAttributeAspects); } @Override @@ -79,26 +130,37 @@ * parseable. */ public String getDescription() { - if (aspectParameters.isEmpty()) { - return aspectClass.getName(); + StringBuilder builder = new StringBuilder(aspectClass.getName()); + if (!aspectParameters.isEmpty()) { + builder.append('['); + ImmutableMultimap<String, String> attributes = aspectParameters.getAttributes(); + boolean first = true; + for (Map.Entry<String, String> attribute : attributes.entries()) { + if (!first) { + builder.append(','); + } else { + first = false; + } + builder.append(attribute.getKey()); + builder.append("=\""); + builder.append(TextFormat.escapeDoubleQuotesAndBackslashes(attribute.getValue())); + builder.append("\""); + } + builder.append(']'); } - StringBuilder builder = new StringBuilder(aspectClass.getName()); - builder.append('['); - ImmutableMultimap<String, String> attributes = aspectParameters.getAttributes(); - boolean first = true; - for (Map.Entry<String, String> attribute : attributes.entries()) { - if (!first) { - builder.append(','); - } else { - first = false; - } - builder.append(attribute.getKey()); - builder.append("=\""); - builder.append(TextFormat.escapeDoubleQuotesAndBackslashes(attribute.getValue())); - builder.append("\""); + if (inheritedAttributeAspects == null) { + builder.append("[*]"); + } else if (!inheritedAttributeAspects.isEmpty()) { + builder.append(inheritedAttributeAspects); } - builder.append(']'); + + if (!inheritedRequiredProviders.equals(RequiredProviders.acceptNoneBuilder().build())) { + builder.append('['); + builder.append(inheritedRequiredProviders); + builder.append(']'); + } + return builder.toString(); } }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java index d7f4a0b..29149fe 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
@@ -78,12 +78,40 @@ /** Wraps the information necessary to construct an Aspect. */ @VisibleForSerialization abstract static class RuleAspect<C extends AspectClass> { + private static final ImmutableList<String> ALL_ATTR_ASPECTS = ImmutableList.of("*"); + protected final C aspectClass; protected final Function<Rule, AspectParameters> parametersExtractor; + protected String baseAspectName; + protected ImmutableList.Builder<ImmutableSet<StarlarkProviderIdentifier>> + inheritedRequiredProviders; + protected ImmutableList.Builder<String> inheritedAttributeAspects; + protected boolean inheritedAllProviders = false; + protected boolean inheritedAllAttributes = false; + private RuleAspect(C aspectClass, Function<Rule, AspectParameters> parametersExtractor) { this.aspectClass = aspectClass; this.parametersExtractor = parametersExtractor; + this.inheritedRequiredProviders = ImmutableList.builder(); + this.inheritedAttributeAspects = ImmutableList.builder(); + } + + private RuleAspect( + C aspectClass, + Function<Rule, AspectParameters> parametersExtractor, + String baseAspectName, + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProviders, + ImmutableList<String> inheritedAttributeAspects) { + this.aspectClass = aspectClass; + this.parametersExtractor = parametersExtractor; + this.baseAspectName = baseAspectName; + this.inheritedRequiredProviders = ImmutableList.builder(); + this.inheritedAttributeAspects = ImmutableList.builder(); + if (baseAspectName != null) { + updateInheritedRequiredProviders(inheritedRequiredProviders); + updateInheritedAttributeAspects(inheritedAttributeAspects); + } } public String getName() { @@ -99,6 +127,70 @@ public C getAspectClass() { return aspectClass; } + + protected void updateInheritedRequiredProviders( + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredProviders) { + if (!inheritedAllProviders && !requiredProviders.isEmpty()) { + inheritedRequiredProviders.addAll(requiredProviders); + } else { + inheritedAllProviders = true; + } + } + + protected void updateInheritedAttributeAspects(ImmutableList<String> attributeAspects) { + if (!inheritedAllAttributes && !ALL_ATTR_ASPECTS.equals(attributeAspects)) { + inheritedAttributeAspects.addAll(attributeAspects); + } else { + inheritedAllAttributes = true; + } + } + + protected RequiredProviders buildInheritedRequiredProviders() { + if (baseAspectName == null) { + return RequiredProviders.acceptNoneBuilder().build(); + } else if (inheritedAllProviders) { + return RequiredProviders.acceptAnyBuilder().build(); + } else { + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProvidersList = + inheritedRequiredProviders.build(); + RequiredProviders.Builder inheritedRequiredProvidersBuilder = + RequiredProviders.acceptAnyBuilder(); + for (ImmutableSet<StarlarkProviderIdentifier> providerSet : + inheritedRequiredProvidersList) { + if (!providerSet.isEmpty()) { + inheritedRequiredProvidersBuilder.addStarlarkSet(providerSet); + } + } + return inheritedRequiredProvidersBuilder.build(); + } + } + + @Nullable + protected ImmutableSet<String> buildInheritedAttributeAspects() { + if (baseAspectName == null) { + return ImmutableSet.of(); + } else if (inheritedAllAttributes) { + return null; + } else { + return ImmutableSet.<String>copyOf(inheritedAttributeAspects.build()); + } + } + + @VisibleForSerialization + public ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> + getInheritedRequiredProvidersList() { + return inheritedRequiredProviders.build(); + } + + @VisibleForSerialization + public ImmutableList<String> getInheritedAttributeAspectsList() { + return inheritedAttributeAspects.build(); + } + + @VisibleForSerialization + public String getBaseAspectName() { + return baseAspectName; + } } private static class NativeRuleAspect extends RuleAspect<NativeAspectClass> { @@ -107,10 +199,30 @@ super(aspectClass, parametersExtractor); } + NativeRuleAspect( + NativeAspectClass aspectClass, + Function<Rule, AspectParameters> parametersExtractor, + String baseAspectName, + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProvidersList, + ImmutableList<String> inheritedAttributeAspectsList) { + super( + aspectClass, + parametersExtractor, + baseAspectName, + inheritedRequiredProvidersList, + inheritedAttributeAspectsList); + } + @Override public Aspect getAspect(Rule rule) { AspectParameters params = parametersExtractor.apply(rule); - return params == null ? null : Aspect.forNative(aspectClass, params); + return params == null + ? null + : Aspect.forNative( + aspectClass, + params, + buildInheritedRequiredProviders(), + buildInheritedAttributeAspects()); } } @@ -120,8 +232,17 @@ private final StarlarkDefinedAspect aspect; @VisibleForSerialization - StarlarkRuleAspect(StarlarkDefinedAspect aspect) { - super(aspect.getAspectClass(), aspect.getDefaultParametersExtractor()); + StarlarkRuleAspect( + StarlarkDefinedAspect aspect, + String baseAspectName, + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProvidersList, + ImmutableList<String> inheritedAttributeAspectsList) { + super( + aspect.getAspectClass(), + aspect.getDefaultParametersExtractor(), + baseAspectName, + inheritedRequiredProvidersList, + inheritedAttributeAspectsList); this.aspect = aspect; } @@ -133,7 +254,12 @@ @Override public Aspect getAspect(Rule rule) { AspectParameters parameters = parametersExtractor.apply(rule); - return Aspect.forStarlark(aspectClass, aspect.getDefinition(parameters), parameters); + return Aspect.forStarlark( + aspectClass, + aspect.getDefinition(parameters), + parameters, + buildInheritedRequiredProviders(), + buildInheritedAttributeAspects()); } } @@ -993,6 +1119,9 @@ return this; } + @AutoCodec @AutoCodec.VisibleForSerialization + static final Function<Rule, AspectParameters> EMPTY_FUNCTION = input -> AspectParameters.EMPTY; + /** * Asserts that a particular parameterized aspect probably needs to be computed for all direct * dependencies through this attribute. @@ -1019,14 +1148,51 @@ return this.aspect(aspect, EMPTY_FUNCTION); } - @AutoCodec @AutoCodec.VisibleForSerialization - static final Function<Rule, AspectParameters> EMPTY_FUNCTION = input -> AspectParameters.EMPTY; + public Builder<TYPE> aspect( + StarlarkDefinedAspect starlarkAspect, + String baseAspectName, + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProviders, + ImmutableList<String> inheritedAttributeAspects) + throws EvalException { + boolean needsToAdd = + checkAndUpdateExistingAspects( + starlarkAspect.getName(), + baseAspectName, + inheritedRequiredProviders, + inheritedAttributeAspects); + if (needsToAdd) { + StarlarkRuleAspect starlarkRuleAspect = + new StarlarkRuleAspect( + starlarkAspect, + baseAspectName, + inheritedRequiredProviders, + inheritedAttributeAspects); + this.aspects.put(starlarkAspect.getName(), starlarkRuleAspect); + } + return this; + } - public Builder<TYPE> aspect(StarlarkDefinedAspect starlarkAspect) throws EvalException { - StarlarkRuleAspect starlarkRuleAspect = new StarlarkRuleAspect(starlarkAspect); - RuleAspect<?> oldAspect = this.aspects.put(starlarkAspect.getName(), starlarkRuleAspect); - if (oldAspect != null) { - throw Starlark.errorf("aspect %s added more than once", starlarkAspect.getName()); + public Builder<TYPE> aspect( + NativeAspectClass nativeAspect, + String baseAspectName, + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProviders, + ImmutableList<String> inheritedAttributeAspects) + throws EvalException { + boolean needsToAdd = + checkAndUpdateExistingAspects( + nativeAspect.getName(), + baseAspectName, + inheritedRequiredProviders, + inheritedAttributeAspects); + if (needsToAdd) { + NativeRuleAspect nativeRuleAspect = + new NativeRuleAspect( + nativeAspect, + EMPTY_FUNCTION, + baseAspectName, + inheritedRequiredProviders, + inheritedAttributeAspects); + this.aspects.put(nativeAspect.getName(), nativeRuleAspect); } return this; } @@ -1043,6 +1209,40 @@ return this; } + private boolean checkAndUpdateExistingAspects( + String aspectName, + String baseAspectName, + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProviders, + ImmutableList<String> inheritedAttributeAspects) + throws EvalException { + + RuleAspect<?> oldAspect = this.aspects.get(aspectName); + + if (oldAspect != null) { + // If the aspect to be added is required by another aspect, i.e. {@code baseAspectName} is + // not null, then we need to update its inherited required providers and propgation + // attributes. + if (baseAspectName != null) { + oldAspect.baseAspectName = baseAspectName; + oldAspect.updateInheritedRequiredProviders(inheritedRequiredProviders); + oldAspect.updateInheritedAttributeAspects(inheritedAttributeAspects); + return false; // no need to add the new aspect + } else { + // If the aspect to be added is not required by another aspect, then we + // should throw an error + String oldAspectBaseAspectName = oldAspect.baseAspectName; + if (oldAspectBaseAspectName != null) { + throw Starlark.errorf( + "aspect %s was added before as a required aspect of aspect %s", + oldAspect.getName(), oldAspectBaseAspectName); + } + throw Starlark.errorf("aspect %s added more than once", oldAspect.getName()); + } + } + + return true; // we need to add the new aspect + } + /** Sets the predicate-like edge validity checker. */ public Builder<TYPE> validityPredicate(ValidityPredicate validityPredicate) { propertyFlags.add(PropertyFlag.STRICT_LABEL_CHECKING);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkAspect.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkAspect.java index 7c65d9b..f3efc0a 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkAspect.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkAspect.java
@@ -14,6 +14,7 @@ package com.google.devtools.build.lib.packages; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.starlarkbuildapi.StarlarkAspectApi; import net.starlark.java.eval.EvalException; @@ -22,12 +23,30 @@ public interface StarlarkAspect extends StarlarkAspectApi { /** - * Attaches this aspect to an attribute. + * Attaches this aspect and its required aspects to the given builder. * - * @param attrBuilder the builder of the attribute to add this aspect to + * <p>Also pass the list of required_providers of the base aspect to its required aspects to + * ensure that they will be propgataed to the same targets. But whether the required aspects will + * run on these targets or not depends on their required providers. + * + * <p>The list of attr_aspects of the base aspects is also passed to its required aspects to + * ensure that they will be propagated with it along the same attributes. + * + * @param baseAspectName is the name of the base aspect requiring this aspect, can be {@code null} + * if the aspect is directly listed in the attribute aspects list + * @param builder is the builder of the attribute to add this aspect to + * @param inheritedRequiredProviders is the list of required providers inherited from the aspect + * parent aspects + * @param inheritedAttributeAspects is the list of attribute aspects inherited from the aspect + * parent aspects * @throws EvalException if this aspect cannot be successfully applied to the given attribute */ - void attachToAttribute(Attribute.Builder<?> attrBuilder) throws EvalException; + void attachToAttribute( + String baseAspectName, + Attribute.Builder<?> builder, + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProviders, + ImmutableList<String> inheritedAttributeAspects) + throws EvalException; /** Returns the aspect class for this aspect. */ AspectClass getAspectClass();
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 959d6ec..de01997 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
@@ -23,8 +23,6 @@ import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization; -import java.util.Arrays; -import java.util.List; import java.util.Objects; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Printer; @@ -41,6 +39,7 @@ private final ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredAspectProviders; private final ImmutableSet<StarlarkProviderIdentifier> provides; private final ImmutableSet<String> paramAttributes; + private final ImmutableSet<StarlarkAspect> requiredAspects; private final ImmutableSet<String> fragments; private final ConfigurationTransition hostTransition; private final ImmutableSet<String> hostFragments; @@ -58,6 +57,7 @@ ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredAspectProviders, ImmutableSet<StarlarkProviderIdentifier> provides, ImmutableSet<String> paramAttributes, + ImmutableSet<StarlarkAspect> requiredAspects, ImmutableSet<String> fragments, // The host transition is in lib.analysis, so we can't reference it directly here. ConfigurationTransition hostTransition, @@ -72,6 +72,7 @@ this.requiredAspectProviders = requiredAspectProviders; this.provides = provides; this.paramAttributes = paramAttributes; + this.requiredAspects = requiredAspects; this.fragments = fragments; this.hostTransition = hostTransition; this.hostFragments = hostFragments; @@ -91,6 +92,7 @@ ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> requiredAspectProviders, ImmutableSet<StarlarkProviderIdentifier> provides, ImmutableSet<String> paramAttributes, + ImmutableSet<StarlarkAspect> requiredAspects, ImmutableSet<String> fragments, // The host transition is in lib.analysis, so we can't reference it directly here. ConfigurationTransition hostTransition, @@ -107,6 +109,7 @@ requiredAspectProviders, provides, paramAttributes, + requiredAspects, fragments, hostTransition, hostFragments, @@ -160,11 +163,11 @@ this.aspectClass = new StarlarkAspectClass(extensionLabel, name); } - private static final List<String> allAttrAspects = Arrays.asList("*"); + private static final ImmutableList<String> ALL_ATTR_ASPECTS = ImmutableList.of("*"); public AspectDefinition getDefinition(AspectParameters aspectParams) { AspectDefinition.Builder builder = new AspectDefinition.Builder(aspectClass); - if (allAttrAspects.equals(attributeAspects)) { + if (ALL_ATTR_ASPECTS.equals(attributeAspects)) { builder.propagateAlongAllAttributes(); } else { for (String attributeAspect : attributeAspects) { @@ -201,6 +204,11 @@ builder.addRequiredToolchains(requiredToolchains); builder.useToolchainTransition(useToolchainTransition); builder.applyToGeneratingRules(applyToGeneratingRules); + ImmutableSet.Builder<AspectClass> requiredAspectsClasses = ImmutableSet.builder(); + for (StarlarkAspect requiredAspect : requiredAspects) { + requiredAspectsClasses.add(requiredAspect.getAspectClass()); + } + builder.requiredAspectClasses(requiredAspectsClasses.build()); return builder.build(); } @@ -259,12 +267,53 @@ } @Override - public void attachToAttribute(Attribute.Builder<?> attrBuilder) throws EvalException { - if (!isExported()) { + public void attachToAttribute( + String baseAspectName, + Attribute.Builder<?> builder, + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProviders, + ImmutableList<String> inheritedAttributeAspects) + throws EvalException { + if (!this.isExported()) { throw Starlark.errorf( "Aspects should be top-level values in extension files that define them."); } - attrBuilder.aspect(this); + + ImmutableList.Builder<ImmutableSet<StarlarkProviderIdentifier>> + requiredAspectInheritedRequiredProviders = ImmutableList.builder(); + ImmutableList.Builder<String> requiredAspectInheritedAttributeAspects = ImmutableList.builder(); + if (baseAspectName == null) { + requiredAspectInheritedRequiredProviders.addAll(this.requiredProviders); + requiredAspectInheritedAttributeAspects.addAll(this.attributeAspects); + } else { + if (!requiredProviders.isEmpty() && !inheritedRequiredProviders.isEmpty()) { + requiredAspectInheritedRequiredProviders.addAll(inheritedRequiredProviders); + requiredAspectInheritedRequiredProviders.addAll(requiredProviders); + } + if (!ALL_ATTR_ASPECTS.equals(inheritedAttributeAspects) + && !ALL_ATTR_ASPECTS.equals(attributeAspects)) { + requiredAspectInheritedAttributeAspects.addAll(inheritedAttributeAspects); + requiredAspectInheritedAttributeAspects.addAll(attributeAspects); + } else { + requiredAspectInheritedAttributeAspects.add("*"); + } + } + + for (StarlarkAspect requiredAspect : requiredAspects) { + requiredAspect.attachToAttribute( + this.getName(), + builder, + requiredAspectInheritedRequiredProviders.build(), + requiredAspectInheritedAttributeAspects.build()); + } + builder.aspect(this, baseAspectName, inheritedRequiredProviders, inheritedAttributeAspects); + } + + public ImmutableSet<StarlarkAspect> getRequiredAspects() { + return requiredAspects; + } + + public ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> getRequiredProviders() { + return requiredProviders; } @Override @@ -283,6 +332,7 @@ && Objects.equals(requiredAspectProviders, that.requiredAspectProviders) && Objects.equals(provides, that.provides) && Objects.equals(paramAttributes, that.paramAttributes) + && Objects.equals(requiredAspects, that.requiredAspects) && Objects.equals(fragments, that.fragments) && Objects.equals(hostTransition, that.hostTransition) && Objects.equals(hostFragments, that.hostFragments) @@ -301,6 +351,7 @@ requiredAspectProviders, provides, paramAttributes, + requiredAspects, fragments, hostTransition, hostFragments,
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeAspect.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeAspect.java index 6d0fe86..14cf33e 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeAspect.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeAspect.java
@@ -14,7 +14,9 @@ package com.google.devtools.build.lib.packages; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Printer; /** A natively-defined aspect that is may be referenced by Starlark attribute definitions. */ @@ -25,8 +27,13 @@ } @Override - public void attachToAttribute(Attribute.Builder<?> attrBuilder) { - attrBuilder.aspect(this); + public void attachToAttribute( + String baseAspectName, + Attribute.Builder<?> builder, + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProviders, + ImmutableList<String> inheritedAttributeAspects) + throws EvalException { + builder.aspect(this, baseAspectName, inheritedRequiredProviders, inheritedAttributeAspects); } @Override
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 d244a60..60262c1 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
@@ -630,6 +630,17 @@ + " providers of the aspect.") public boolean incompatibleTopLevelAspectsRequireProviders; + @Option( + name = "experimental_required_aspects", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + metadataTags = {OptionMetadataTag.EXPERIMENTAL}, + help = + "If set to true, allows created aspect to require a list of aspects to be propagated" + + " before it.") + public boolean experimentalRequiredAspects; + /** * 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 @@ -699,6 +710,7 @@ .setBool( INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS, incompatibleTopLevelAspectsRequireProviders) + .setBool(EXPERIMENTAL_REQUIRED_ASPECTS, experimentalRequiredAspects) .build(); return INTERNER.intern(semantics); } @@ -771,6 +783,7 @@ "-incompatible_visibility_private_attributes_at_definition"; public static final String INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS = "-incompatible_top_level_aspects_require_providers"; + public static final String EXPERIMENTAL_REQUIRED_ASPECTS = "-experimental_required_aspects"; // non-booleans public static final StarlarkSemantics.Key<String> 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 62b456b..4add5da 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
@@ -219,7 +219,9 @@ Aspect.forStarlark( starlarkAspect.getAspectClass(), starlarkAspect.getDefinition(key.getParameters()), - key.getParameters()); + key.getParameters(), + key.getInheritedRequiredProviders(), + key.getInheritedAttributeAspects()); } else { throw new IllegalStateException(); }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java index 1d73b1c..f6ecb9c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java
@@ -16,6 +16,7 @@ import com.google.common.base.MoreObjects; import com.google.common.base.Objects; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Interner; import com.google.devtools.build.lib.actions.ActionLookupKey; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; @@ -24,6 +25,7 @@ import com.google.devtools.build.lib.packages.AspectClass; import com.google.devtools.build.lib.packages.AspectDescriptor; import com.google.devtools.build.lib.packages.AspectParameters; +import com.google.devtools.build.lib.packages.RequiredProviders; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.skyframe.SkyFunctionName; import javax.annotation.Nullable; @@ -158,6 +160,15 @@ return aspectDescriptor.getParameters(); } + public RequiredProviders getInheritedRequiredProviders() { + return aspectDescriptor.getInheritedRequiredProviders(); + } + + @Nullable + public ImmutableSet<String> getInheritedAttributeAspects() { + return aspectDescriptor.getInheritedAttributeAspects(); + } + public AspectDescriptor getAspectDescriptor() { return aspectDescriptor; }
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 9a8467f..d7b32f5 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
@@ -466,6 +466,14 @@ + "<code>BazInfo</code> *and* <code>QuxInfo</code>."), @Param(name = "provides", named = true, defaultValue = "[]", doc = PROVIDES_DOC), @Param( + name = "requires", + allowedTypes = {@ParamType(type = Sequence.class, generic1 = StarlarkAspectApi.class)}, + named = true, + enableOnlyWithFlag = BuildLanguageOptions.EXPERIMENTAL_REQUIRED_ASPECTS, + defaultValue = "[]", + valueWhenDisabled = "[]", + doc = "(Experimental) List of aspects required to be propagated before this aspect."), + @Param( name = "fragments", allowedTypes = {@ParamType(type = Sequence.class, generic1 = String.class)}, named = true, @@ -529,6 +537,7 @@ Sequence<?> requiredProvidersArg, Sequence<?> requiredAspectProvidersArg, Sequence<?> providesArg, + Sequence<?> requiredAspects, Sequence<?> fragments, Sequence<?> hostFragments, Sequence<?> toolchains,
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 9b163e2..9ad4433 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
@@ -183,6 +183,7 @@ Sequence<?> requiredProvidersArg, Sequence<?> requiredAspectProvidersArg, Sequence<?> providesArg, + Sequence<?> requiredAspects, Sequence<?> fragments, Sequence<?> hostFragments, Sequence<?> toolchains,
diff --git a/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java b/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java index 75b4d8e..a4db7e1 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java
@@ -23,7 +23,9 @@ import static org.junit.Assert.assertThrows; import com.google.common.base.Predicates; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.BuildOptionsView; import com.google.devtools.build.lib.analysis.config.HostTransition; @@ -319,4 +321,229 @@ .build()); assertThat(e).hasMessageThat().contains("may not contain the same rule classes"); } + + 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")); + + @Test + public void testAttrRequiredAspects_inheritAttrAspects() throws Exception { + ImmutableList<String> inheritedAttributeAspects1 = ImmutableList.of("attr1", "attr2"); + ImmutableList<String> inheritedAttributeAspects2 = ImmutableList.of("attr3", "attr2"); + + Attribute attr = + attr("x", LABEL) + .aspect( + TestAspects.SIMPLE_ASPECT, + "base_aspect_1", + /** inheritedRequiredProviders= */ + ImmutableList.of(), + inheritedAttributeAspects1) + .aspect( + TestAspects.SIMPLE_ASPECT, + "base_aspect_2", + /** inheritedRequiredProviders= */ + ImmutableList.of(), + inheritedAttributeAspects2) + .allowedFileTypes() + .build(); + + ImmutableList<Aspect> aspects = attr.getAspects(null); + assertThat(aspects).hasSize(1); + AspectDescriptor aspectDescriptor = aspects.get(0).getDescriptor(); + assertThat(aspectDescriptor.getInheritedAttributeAspects()) + .containsExactly("attr1", "attr2", "attr3"); + } + + @Test + public void testAttrRequiredAspects_inheritRequiredProviders() throws Exception { + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProviders1 = + ImmutableList.of(ImmutableSet.of(STARLARK_P1), ImmutableSet.of(STARLARK_P2, STARLARK_P3)); + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProviders2 = + ImmutableList.of(ImmutableSet.of(STARLARK_P4), ImmutableSet.of(STARLARK_P2, STARLARK_P3)); + + Attribute attr = + attr("x", LABEL) + .aspect( + TestAspects.SIMPLE_ASPECT, + "base_aspect_1", + inheritedRequiredProviders1, + /** inheritedAttributeAspects= */ + ImmutableList.of()) + .aspect( + TestAspects.SIMPLE_ASPECT, + "base_aspect_2", + inheritedRequiredProviders2, + /** inheritedAttributeAspects= */ + ImmutableList.of()) + .allowedFileTypes() + .build(); + + ImmutableList<Aspect> aspects = attr.getAspects(null); + assertThat(aspects).hasSize(1); + + RequiredProviders actualInheritedRequiredProviders = + aspects.get(0).getDescriptor().getInheritedRequiredProviders(); + AdvertisedProviderSet expectedOkSet1 = + AdvertisedProviderSet.builder().addStarlark(STARLARK_P1).build(); + assertThat(actualInheritedRequiredProviders.isSatisfiedBy(expectedOkSet1)).isTrue(); + + AdvertisedProviderSet expectedOkSet2 = + AdvertisedProviderSet.builder().addStarlark(STARLARK_P4).build(); + assertThat(actualInheritedRequiredProviders.isSatisfiedBy(expectedOkSet2)).isTrue(); + + AdvertisedProviderSet expectedOkSet3 = + AdvertisedProviderSet.builder().addStarlark(STARLARK_P2).addStarlark(STARLARK_P3).build(); + assertThat(actualInheritedRequiredProviders.isSatisfiedBy(expectedOkSet3)).isTrue(); + + assertThat(actualInheritedRequiredProviders.isSatisfiedBy(AdvertisedProviderSet.ANY)).isTrue(); + assertThat(actualInheritedRequiredProviders.isSatisfiedBy(AdvertisedProviderSet.EMPTY)) + .isFalse(); + } + + @Test + public void testAttrRequiredAspects_aspectAlreadyExists_inheritAttrAspects() throws Exception { + ImmutableList<String> inheritedAttributeAspects = ImmutableList.of("attr1", "attr2"); + + Attribute attr = + attr("x", LABEL) + .aspect(TestAspects.SIMPLE_ASPECT) + .aspect( + TestAspects.SIMPLE_ASPECT, + "base_aspect", + /** inheritedRequiredProviders = */ + ImmutableList.of(), + inheritedAttributeAspects) + .allowedFileTypes() + .build(); + + ImmutableList<Aspect> aspects = attr.getAspects(null); + assertThat(aspects).hasSize(1); + AspectDescriptor aspectDescriptor = aspects.get(0).getDescriptor(); + assertThat(aspectDescriptor.getInheritedAttributeAspects()).containsExactly("attr1", "attr2"); + } + + @Test + public void testAttrRequiredAspects_aspectAlreadyExists_inheritRequiredProviders() + throws Exception { + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProviders = + ImmutableList.of(ImmutableSet.of(STARLARK_P1), ImmutableSet.of(STARLARK_P2, STARLARK_P3)); + + Attribute attr = + attr("x", LABEL) + .aspect(TestAspects.SIMPLE_ASPECT) + .aspect( + TestAspects.SIMPLE_ASPECT, + "base_aspect", + inheritedRequiredProviders, + /** inheritedAttributeAspects= */ + ImmutableList.of()) + .allowedFileTypes() + .build(); + + ImmutableList<Aspect> aspects = attr.getAspects(null); + assertThat(aspects).hasSize(1); + + RequiredProviders actualInheritedRequiredProviders = + aspects.get(0).getDescriptor().getInheritedRequiredProviders(); + AdvertisedProviderSet expectedOkSet1 = + AdvertisedProviderSet.builder().addStarlark(STARLARK_P1).build(); + assertThat(actualInheritedRequiredProviders.isSatisfiedBy(expectedOkSet1)).isTrue(); + + AdvertisedProviderSet expectedOkSet2 = + AdvertisedProviderSet.builder().addStarlark(STARLARK_P4).build(); + assertThat(actualInheritedRequiredProviders.isSatisfiedBy(expectedOkSet2)).isFalse(); + + AdvertisedProviderSet expectedOkSet3 = + AdvertisedProviderSet.builder().addStarlark(STARLARK_P2).addStarlark(STARLARK_P3).build(); + assertThat(actualInheritedRequiredProviders.isSatisfiedBy(expectedOkSet3)).isTrue(); + + assertThat(actualInheritedRequiredProviders.isSatisfiedBy(AdvertisedProviderSet.ANY)).isTrue(); + assertThat(actualInheritedRequiredProviders.isSatisfiedBy(AdvertisedProviderSet.EMPTY)) + .isFalse(); + } + + @Test + public void testAttrRequiredAspects_inheritAllAttrAspects() throws Exception { + ImmutableList<String> inheritedAttributeAspects1 = ImmutableList.of("attr1", "attr2"); + ImmutableList<String> inheritedAttributeAspects2 = ImmutableList.of("*"); + + Attribute attr = + attr("x", LABEL) + .aspect(TestAspects.SIMPLE_ASPECT) + .aspect( + TestAspects.SIMPLE_ASPECT, + "base_aspect_1", + /** inheritedRequiredProviders = */ + ImmutableList.of(), + inheritedAttributeAspects1) + .aspect( + TestAspects.SIMPLE_ASPECT, + "base_aspect_2", + /** inheritedRequiredProviders = */ + ImmutableList.of(), + inheritedAttributeAspects2) + .allowedFileTypes() + .build(); + + ImmutableList<Aspect> aspects = attr.getAspects(null); + assertThat(aspects).hasSize(1); + AspectDescriptor aspectDescriptor = aspects.get(0).getDescriptor(); + assertThat(aspectDescriptor.getInheritedAttributeAspects()).isNull(); + } + + @Test + public void testAttrRequiredAspects_inheritAllRequiredProviders() throws Exception { + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProviders1 = + ImmutableList.of(); + ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> inheritedRequiredProviders2 = + ImmutableList.of(ImmutableSet.of(STARLARK_P4), ImmutableSet.of(STARLARK_P2, STARLARK_P3)); + + Attribute attr = + attr("x", LABEL) + .aspect(TestAspects.SIMPLE_ASPECT) + .aspect( + TestAspects.SIMPLE_ASPECT, + "base_aspect_1", + inheritedRequiredProviders1, + /** inheritedAttributeAspects= */ + ImmutableList.of()) + .aspect( + TestAspects.SIMPLE_ASPECT, + "base_aspect_2", + inheritedRequiredProviders2, + /** inheritedAttributeAspects= */ + ImmutableList.of()) + .allowedFileTypes() + .build(); + + ImmutableList<Aspect> aspects = attr.getAspects(null); + assertThat(aspects).hasSize(1); + AspectDescriptor aspectDescriptor = aspects.get(0).getDescriptor(); + assertThat(aspectDescriptor.getInheritedRequiredProviders()) + .isEqualTo(RequiredProviders.acceptAnyBuilder().build()); + } + + @Test + public void testAttrRequiredAspects_defaultInheritedRequiredProvidersAndAttrAspects() + throws Exception { + Attribute attr = attr("x", LABEL).aspect(TestAspects.SIMPLE_ASPECT).allowedFileTypes().build(); + + ImmutableList<Aspect> aspects = attr.getAspects(null); + assertThat(aspects).hasSize(1); + AspectDescriptor aspectDescriptor = aspects.get(0).getDescriptor(); + assertThat(aspectDescriptor.getInheritedAttributeAspects()).isEmpty(); + assertThat(aspectDescriptor.getInheritedRequiredProviders()) + .isEqualTo(RequiredProviders.acceptNoneBuilder().build()); + } }
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 326e56d..b815308 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
@@ -2298,6 +2298,82 @@ + "(when propagating to //test:r1)"); } + @Test + public void aspectRequiresAspectInconsistentVisibility() throws Exception { + scratch.file( + "test/aspect.bzl", + "def _aspect_impl(target,ctx):", + " return struct()", + "a1 = aspect(_aspect_impl, attr_aspects = ['dep'])", + "a2 = aspect(_aspect_impl, attr_aspects = ['dep'], requires = [a1])", + "def _rule_impl(ctx):", + " pass", + "r1 = rule(_rule_impl, attrs = { 'dep' : attr.label(aspects = [a1])})", + "r2 = rule(_rule_impl, attrs = { 'dep' : attr.label(aspects = [a2])})"); + scratch.file( + "test/BUILD", + "load(':aspect.bzl', 'r1', 'r2')", + "r1(name = 'r0')", + "r1(name = 'r1', dep = ':r0')", + "r2(name = 'r2', dep = ':r1')", + "r1(name = 'r1_1', dep = ':r2')", + "r2(name = 'r2_1', dep = ':r1_1')"); + reporter.removeHandler(failFastHandler); + useConfiguration("--experimental_required_aspects"); + + // The call to `update` does not throw an exception when "--keep_going" is passed in the + // WithKeepGoing test suite. Otherwise, it throws ViewCreationFailedException. + if (keepGoing()) { + AnalysisResult result = update("//test:r2_1"); + assertThat(result.hasError()).isTrue(); + } else { + assertThrows(ViewCreationFailedException.class, () -> update("//test:r2_1")); + } + assertContainsEvent( + "ERROR /workspace/test/BUILD:3:3: Aspect //test:aspect.bzl%a2 is" + + " applied twice, both before and after aspect //test:aspect.bzl%a1 " + + "(when propagating to //test:r1)"); + } + + @Test + public void aspectRequiresAspectInconsistentVisibilityIndirect() throws Exception { + scratch.file( + "test/aspect.bzl", + "def _aspect_impl(target,ctx):", + " return struct()", + "a1 = aspect(_aspect_impl, attr_aspects = ['dep'])", + "a2 = aspect(_aspect_impl, attr_aspects = ['dep'], requires = [a1])", + "def _rule_impl(ctx):", + " pass", + "r1 = rule(_rule_impl, attrs = { 'dep' : attr.label(aspects = [a1])})", + "r2 = rule(_rule_impl, attrs = { 'dep' : attr.label(aspects = [a2])})", + "r0 = rule(_rule_impl, attrs = { 'dep' : attr.label()})"); + scratch.file( + "test/BUILD", + "load(':aspect.bzl', 'r0', 'r1', 'r2')", + "r0(name = 'r0')", + "r1(name = 'r1', dep = ':r0')", + "r2(name = 'r2', dep = ':r1')", + "r1(name = 'r1_1', dep = ':r2')", + "r2(name = 'r2_1', dep = ':r1_1')", + "r0(name = 'r0_1', dep = ':r2_1')"); + reporter.removeHandler(failFastHandler); + useConfiguration("--experimental_required_aspects"); + + // The call to `update` does not throw an exception when "--keep_going" is passed in the + // WithKeepGoing test suite. Otherwise, it throws ViewCreationFailedException. + if (keepGoing()) { + AnalysisResult result = update("//test:r0_1"); + assertThat(result.hasError()).isTrue(); + } else { + assertThrows(ViewCreationFailedException.class, () -> update("//test:r0_1")); + } + assertContainsEvent( + "ERROR /workspace/test/BUILD:3:3: Aspect //test:aspect.bzl%a2 is" + + " applied twice, both before and after aspect //test:aspect.bzl%a1 " + + "(when propagating to //test:r1)"); + } + /** * Aspect a3 sees aspect a2, aspect a2 sees aspect a1, but a3 does not see a1. All three aspects * should still propagate together. @@ -3420,6 +3496,704 @@ assertThat(Starlark.toIterable(ruleDeps)).containsExactlyElementsIn(expected); } + @Test + public void testAspectRequiresAspect_aspectsParameters() throws Exception { + scratch.file( + "test/defs.bzl", + "RequiredAspectProv = provider()", + "BaseAspectProv = provider()", + "", + "def _required_aspect_impl(target, ctx):", + " p1_val = 'In required_aspect, p1 = {} on target {}'.format(ctx.attr.p1, target.label)", + " p2_val = 'invalid value'", + " if not hasattr(ctx.attr, 'p2'):", + " p2_val = 'In required_aspect, p2 not found on target {}'.format(target.label)", + " return [RequiredAspectProv(p1_val = p1_val, p2_val = p2_val)]", + "required_aspect = aspect(", + " implementation = _required_aspect_impl,", + " attr_aspects = ['dep'],", + " attrs = {'p1' : attr.string(values = ['p1_v1', 'p1_v2'])}", + ")", + "", + "def _base_aspect_impl(target, ctx):", + " p2_val = 'In base_aspect, p2 = {} on target {}'.format(ctx.attr.p2, target.label)", + " p1_val = 'invalid value'", + " if not hasattr(ctx.attr, 'p1'):", + " p1_val = 'In base_aspect, p1 not found on target {}'.format(target.label)", + " return [BaseAspectProv(p1_val = p1_val, p2_val = p2_val)]", + "base_aspect = aspect(", + " implementation = _base_aspect_impl,", + " attr_aspects = ['dep'],", + " attrs = {'p2' : attr.string(values = ['p2_v1', 'p2_v2'])},", + " requires = [required_aspect],", + ")", + "", + "def _main_rule_impl(ctx):", + " return [ctx.attr.dep[RequiredAspectProv], ctx.attr.dep[BaseAspectProv]]", + "def _dep_rule_impl(ctx):", + " pass", + "", + "main_rule = rule(", + " implementation = _main_rule_impl,", + " attrs = {", + " 'dep': attr.label(aspects=[base_aspect]),", + " 'p1' : attr.string(values = ['p1_v1', 'p1_v2']),", + " 'p2' : attr.string(values = ['p2_v1', 'p2_v2'])", + " },", + ")", + "", + "dep_rule = rule(", + " implementation = _dep_rule_impl,", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'main_rule', 'dep_rule')", + "main_rule(", + " name = 'main',", + " dep = ':dep_target',", + " p1 = 'p1_v1',", + " p2 = 'p2_v1'", + ")", + "dep_rule(", + " name = 'dep_target',", + ")"); + + useConfiguration("--experimental_required_aspects"); + AnalysisResult analysisResult = update("//test:main"); + + // Both base_aspect and required_aspect can get their parameters values from the base rule + ConfiguredTarget configuredTarget = + Iterables.getOnlyElement(analysisResult.getTargetsToBuild()); + StarlarkProvider.Key requiredAspectProv = + new StarlarkProvider.Key( + Label.parseAbsolute("//test:defs.bzl", ImmutableMap.of()), "RequiredAspectProv"); + StructImpl requiredAspectProvider = (StructImpl) configuredTarget.get(requiredAspectProv); + assertThat(requiredAspectProvider.getValue("p1_val")) + .isEqualTo("In required_aspect, p1 = p1_v1 on target //test:dep_target"); + assertThat(requiredAspectProvider.getValue("p2_val")) + .isEqualTo("In required_aspect, p2 not found on target //test:dep_target"); + + StarlarkProvider.Key baseAspectProv = + new StarlarkProvider.Key( + Label.parseAbsolute("//test:defs.bzl", ImmutableMap.of()), "BaseAspectProv"); + StructImpl baseAspectProvider = (StructImpl) configuredTarget.get(baseAspectProv); + assertThat(baseAspectProvider.getValue("p1_val")) + .isEqualTo("In base_aspect, p1 not found on target //test:dep_target"); + assertThat(baseAspectProvider.getValue("p2_val")) + .isEqualTo("In base_aspect, p2 = p2_v1 on target //test:dep_target"); + } + + @Test + public void testAspectRequiresAspect_ruleAttributes() throws Exception { + scratch.file( + "test/defs.bzl", + "RequiredAspectProv = provider()", + "BaseAspectProv = provider()", + "", + "def _required_aspect_impl(target, ctx):", + " p_val = 'In required_aspect, p = {} on target {}'.format(ctx.rule.attr.p, target.label)", + " return [RequiredAspectProv(p_val = p_val)]", + "required_aspect = aspect(", + " implementation = _required_aspect_impl,", + " attr_aspects = ['dep'],", + ")", + "", + "def _base_aspect_impl(target, ctx):", + " p_val = 'In base_aspect, p = {} on target {}'.format(ctx.rule.attr.p, target.label)", + " return [BaseAspectProv(p_val = p_val)]", + "base_aspect = aspect(", + " implementation = _base_aspect_impl,", + " attr_aspects = ['dep'],", + " requires = [required_aspect],", + ")", + "", + "def _main_rule_impl(ctx):", + " return [ctx.attr.dep[RequiredAspectProv], ctx.attr.dep[BaseAspectProv]]", + "def _dep_rule_impl(ctx):", + " pass", + "", + "main_rule = rule(", + " implementation = _main_rule_impl,", + " attrs = {", + " 'dep': attr.label(aspects=[base_aspect]),", + " },", + ")", + "", + "dep_rule = rule(", + " implementation = _dep_rule_impl,", + " attrs = {", + " 'p' : attr.string(values = ['p_v1', 'p_v2']),", + " },", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'main_rule', 'dep_rule')", + "main_rule(", + " name = 'main',", + " dep = ':dep_target',", + ")", + "dep_rule(", + " name = 'dep_target',", + " p = 'p_v2',", + ")"); + + useConfiguration("--experimental_required_aspects"); + AnalysisResult analysisResult = update("//test:main"); + + // Both base_aspect and required_aspect can see the attributes of the rule they run on + ConfiguredTarget configuredTarget = + Iterables.getOnlyElement(analysisResult.getTargetsToBuild()); + StarlarkProvider.Key requiredAspectProv = + new StarlarkProvider.Key( + Label.parseAbsolute("//test:defs.bzl", ImmutableMap.of()), "RequiredAspectProv"); + StructImpl requiredAspectProvider = (StructImpl) configuredTarget.get(requiredAspectProv); + assertThat(requiredAspectProvider.getValue("p_val")) + .isEqualTo("In required_aspect, p = p_v2 on target //test:dep_target"); + + StarlarkProvider.Key baseAspectProv = + new StarlarkProvider.Key( + Label.parseAbsolute("//test:defs.bzl", ImmutableMap.of()), "BaseAspectProv"); + StructImpl baseAspectProvider = (StructImpl) configuredTarget.get(baseAspectProv); + assertThat(baseAspectProvider.getValue("p_val")) + .isEqualTo("In base_aspect, p = p_v2 on target //test:dep_target"); + } + + @Test + public void testAspectRequiresAspect_inheritPropagationAttributes() throws Exception { + // base_aspect propagates over base_dep attribute and requires first_required_aspect which + // propagates + // over first_dep attribute and requires second_required aspect which propagates over second_dep + // attribute + scratch.file( + "test/defs.bzl", + "BaseAspectProv = provider()", + "FirstRequiredAspectProv = provider()", + "SecondRequiredAspectProv = provider()", + "", + "def _second_required_aspect_impl(target, ctx):", + " result = []", + " if getattr(ctx.rule.attr, 'second_dep'):", + " result += getattr(ctx.rule.attr, 'second_dep')[SecondRequiredAspectProv].result", + " result += ['second_required_aspect run on target {}'.format(target.label)]", + " return [SecondRequiredAspectProv(result = result)]", + "second_required_aspect = aspect(", + " implementation = _second_required_aspect_impl,", + " attr_aspects = ['second_dep'],", + ")", + "", + "def _first_required_aspect_impl(target, ctx):", + " result = []", + " result += target[SecondRequiredAspectProv].result", + " if getattr(ctx.rule.attr, 'first_dep'):", + " result += getattr(ctx.rule.attr, 'first_dep')[FirstRequiredAspectProv].result", + " result += ['first_required_aspect run on target {}'.format(target.label)]", + " return [FirstRequiredAspectProv(result = result)]", + "first_required_aspect = aspect(", + " implementation = _first_required_aspect_impl,", + " attr_aspects = ['first_dep'],", + " requires = [second_required_aspect],", + ")", + "", + "def _base_aspect_impl(target, ctx):", + " result = []", + " result += target[FirstRequiredAspectProv].result", + " if getattr(ctx.rule.attr, 'base_dep'):", + " result += getattr(ctx.rule.attr, 'base_dep')[BaseAspectProv].result", + " result += ['base_aspect run on target {}'.format(target.label)]", + " return [BaseAspectProv(result = result)]", + "base_aspect = aspect(", + " implementation = _base_aspect_impl,", + " attr_aspects = ['base_dep'],", + " requires = [first_required_aspect],", + ")", + "", + "def _main_rule_impl(ctx):", + " return [ctx.attr.dep[BaseAspectProv]]", + "def _dep_rule_impl(ctx):", + " pass", + "", + "main_rule = rule(", + " implementation = _main_rule_impl,", + " attrs = {", + " 'dep': attr.label(aspects=[base_aspect]),", + " },", + ")", + "", + "dep_rule = rule(", + " implementation = _dep_rule_impl,", + " attrs = {", + " 'base_dep': attr.label(),", + " 'first_dep': attr.label(),", + " 'second_dep': attr.label()", + " },", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'main_rule', 'dep_rule')", + "main_rule(", + " name = 'main',", + " dep = ':dep_target',", + ")", + "dep_rule(", + " name = 'dep_target',", + " base_dep = ':base_dep_target',", + " first_dep = ':first_dep_target',", + " second_dep = ':second_dep_target',", + ")", + "dep_rule(", + " name = 'base_dep_target',", + ")", + "dep_rule(", + " name = 'first_dep_target',", + ")", + "dep_rule(", + " name = 'second_dep_target',", + ")"); + + useConfiguration("--experimental_required_aspects"); + AnalysisResult analysisResult = update("//test:main"); + + // base_aspect should propagate only along its attr_aspects: 'base_dep' + // first_required_aspect should propagate along 'base_dep' and 'first_dep' + // second_required_aspect should propagate along 'base_dep', 'first_dep' and `second_dep` + ConfiguredTarget configuredTarget = + Iterables.getOnlyElement(analysisResult.getTargetsToBuild()); + StarlarkProvider.Key baseAspectProv = + new StarlarkProvider.Key( + Label.parseAbsolute("//test:defs.bzl", ImmutableMap.of()), "BaseAspectProv"); + StructImpl baseAspectProvider = (StructImpl) configuredTarget.get(baseAspectProv); + assertThat((Sequence<?>) baseAspectProvider.getValue("result")) + .containsExactly( + "second_required_aspect run on target //test:second_dep_target", + "second_required_aspect run on target //test:dep_target", + "second_required_aspect run on target //test:first_dep_target", + "first_required_aspect run on target //test:first_dep_target", + "first_required_aspect run on target //test:dep_target", + "second_required_aspect run on target //test:base_dep_target", + "first_required_aspect run on target //test:base_dep_target", + "base_aspect run on target //test:base_dep_target", + "base_aspect run on target //test:dep_target"); + } + + @Test + public void testAspectRequiresAspect_inheritRequiredProviders() throws Exception { + // aspect_a requires provider Prov_A and requires aspect_b which requires + // provider Prov_B and requires aspect_c which requires provider Prov_C + scratch.file( + "test/defs.bzl", + "Prov_A = provider()", + "Prov_B = provider()", + "Prov_C = provider()", + "", + "CollectorProv = provider()", + "", + "def _aspect_c_impl(target, ctx):", + " collector_result = ['aspect_c run on target {} and value of Prov_C =" + + " {}'.format(target.label, target[Prov_C].val)]", + " return [CollectorProv(result = collector_result)]", + "aspect_c = aspect(", + " implementation = _aspect_c_impl,", + " required_providers = [Prov_C],", + ")", + "", + "def _aspect_b_impl(target, ctx):", + " collector_result = []", + " collector_result += ctx.rule.attr.dep[CollectorProv].result", + " collector_result += ['aspect_b run on target {} and value of Prov_B =" + + " {}'.format(target.label, target[Prov_B].val)]", + " return [ CollectorProv(result = collector_result)]", + "aspect_b = aspect(", + " implementation = _aspect_b_impl,", + " required_providers = [Prov_B],", + " requires = [aspect_c],", + ")", + "", + "def _aspect_a_impl(target, ctx):", + " collector_result = []", + " collector_result += ctx.rule.attr.dep[CollectorProv].result", + " collector_result += ['aspect_a run on target {} and value of Prov_A =" + + " {}'.format(target.label, target[Prov_A].val)]", + " return [CollectorProv(result = collector_result)]", + "aspect_a = aspect(", + " implementation = _aspect_a_impl,", + " attr_aspects = ['dep'],", + " required_providers = [Prov_A],", + " requires = [aspect_b],", + ")", + "", + "def _main_rule_impl(ctx):", + " return [ctx.attr.dep[CollectorProv]]", + "main_rule = rule(", + " implementation = _main_rule_impl,", + " attrs = {", + " 'dep': attr.label(aspects = [aspect_a]),", + " },", + ")", + "", + "def _rule_with_prov_a_impl(ctx):", + " return [Prov_A(val='val_a')]", + "rule_with_prov_a = rule(", + " implementation = _rule_with_prov_a_impl,", + " attrs = {", + " 'dep': attr.label(),", + " },", + " provides = [Prov_A]", + ")", + "", + "def _rule_with_prov_b_impl(ctx):", + " return [Prov_B(val = 'val_b')]", + "rule_with_prov_b = rule(", + " implementation = _rule_with_prov_b_impl,", + " attrs = {", + " 'dep': attr.label(),", + " },", + " provides = [Prov_B]", + ")", + "", + "def _rule_with_prov_c_impl(ctx):", + " return [Prov_C(val = 'val_c')]", + "rule_with_prov_c = rule(", + " implementation = _rule_with_prov_c_impl,", + " provides = [Prov_C]", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'main_rule', 'rule_with_prov_a', 'rule_with_prov_b'," + + " 'rule_with_prov_c')", + "main_rule(", + " name = 'main',", + " dep = ':target_with_prov_a',", + ")", + "rule_with_prov_a(", + " name = 'target_with_prov_a',", + " dep = ':target_with_prov_b'", + ")", + "rule_with_prov_b(", + " name = 'target_with_prov_b',", + " dep = ':target_with_prov_c'", + ")", + "rule_with_prov_c(", + " name = 'target_with_prov_c'", + ")"); + + useConfiguration("--experimental_required_aspects"); + AnalysisResult analysisResult = update("//test:main"); + + // aspect_a should only run on target_with_prov_a, aspect_b should only run on + // target_with_prov_b and aspect_c + // should only run on target_with_prov_c. + // aspect_c will reach target target_with_prov_c because it inherits the required_providers of + // aspect_c otherwise it + // would have stopped propagating after target_with_prov_b + ConfiguredTarget configuredTarget = + Iterables.getOnlyElement(analysisResult.getTargetsToBuild()); + StarlarkProvider.Key collectorProv = + new StarlarkProvider.Key( + Label.parseAbsolute("//test:defs.bzl", ImmutableMap.of()), "CollectorProv"); + StructImpl collectorProvider = (StructImpl) configuredTarget.get(collectorProv); + assertThat((Sequence<?>) collectorProvider.getValue("result")) + .containsExactly( + "aspect_c run on target //test:target_with_prov_c and value of Prov_C = val_c", + "aspect_b run on target //test:target_with_prov_b and value of Prov_B = val_b", + "aspect_a run on target //test:target_with_prov_a and value of Prov_A = val_a") + .inOrder(); + } + + @Test + public void testAspectRequiresAspect_inspectRequiredAspectActions() throws Exception { + scratch.file( + "test/defs.bzl", + "def _required_aspect_impl(target, ctx):", + " f = ctx.actions.declare_file('dummy.txt')", + " ctx.actions.run_shell(outputs = [f], command='echo xxx > $(location f)',", + " mnemonic='RequiredAspectAction')", + " return struct()", + "required_aspect = aspect(", + " implementation = _required_aspect_impl,", + ")", + "", + "def _base_aspect_impl(target, ctx):", + " required_aspect_action = None", + " for action in target.actions:", + " if action.mnemonic == 'RequiredAspectAction':", + " required_aspect_action = action", + " if required_aspect_action:", + " return struct(result = 'base_aspect can see required_aspect action')", + " else:", + " return struct(result = 'base_aspect cannot see required_aspect action')", + "base_aspect = aspect(", + " implementation = _base_aspect_impl,", + " attr_aspects = ['dep'],", + " requires = [required_aspect]", + ")", + "", + "def _main_rule_impl(ctx):", + " return struct(result = ctx.attr.dep.result)", + "main_rule = rule(", + " implementation = _main_rule_impl,", + " attrs = {", + " 'dep': attr.label(aspects = [base_aspect]),", + " },", + ")", + "", + "def _dep_rule_impl(ctx):", + " pass", + "dep_rule = rule(", + " implementation = _dep_rule_impl,", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'main_rule', 'dep_rule')", + "main_rule(", + " name = 'main',", + " dep = ':dep_target',", + ")", + "dep_rule(", + " name = 'dep_target',", + ")"); + + useConfiguration("--experimental_required_aspects"); + AnalysisResult analysisResult = update("//test:main"); + + ConfiguredTarget configuredTarget = + Iterables.getOnlyElement(analysisResult.getTargetsToBuild()); + String result = (String) configuredTarget.get("result"); + assertThat(result).isEqualTo("base_aspect can see required_aspect action"); + } + + @Test + public void testAspectRequiresAspect_inspectRequiredAspectGeneratedFiles() throws Exception { + scratch.file( + "test/defs.bzl", + "def _required_aspect_impl(target, ctx):", + " file = ctx.actions.declare_file('required_aspect_file')", + " ctx.actions.write(file, 'data')", + " return [OutputGroupInfo(out = [file])]", + "required_aspect = aspect(", + " implementation = _required_aspect_impl,", + ")", + "", + "def _base_aspect_impl(target, ctx):", + " files = ['base_aspect can see file ' + f.path.split('/')[-1] for f in" + + " target[OutputGroupInfo].out.to_list()]", + " return struct(my_files = files)", + "base_aspect = aspect(", + " implementation = _base_aspect_impl,", + " attr_aspects = ['dep'],", + " requires = [required_aspect]", + ")", + "", + "def _main_rule_impl(ctx):", + " return struct(my_files = ctx.attr.dep.my_files)", + "main_rule = rule(", + " implementation = _main_rule_impl,", + " attrs = {", + " 'dep': attr.label(aspects = [base_aspect]),", + " },", + ")", + "", + "def _dep_rule_impl(ctx):", + " pass", + "dep_rule = rule(", + " implementation = _dep_rule_impl,", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'main_rule', 'dep_rule')", + "main_rule(", + " name = 'main',", + " dep = ':dep_target',", + ")", + "dep_rule(", + " name = 'dep_target',", + ")"); + + useConfiguration("--experimental_required_aspects"); + AnalysisResult analysisResult = update("//test:main"); + + ConfiguredTarget configuredTarget = + Iterables.getOnlyElement(analysisResult.getTargetsToBuild()); + StarlarkList<?> files = (StarlarkList) configuredTarget.get("my_files"); + assertThat(Starlark.toIterable(files)) + .containsExactly("base_aspect can see file required_aspect_file"); + } + + @Test + public void testAspectRequiresAspect_withRequiredAspectProvidersSatisfied() throws Exception { + scratch.file( + "test/defs.bzl", + "prov_a = provider()", + "prov_b = provider()", + "prov_b_forwarded = provider()", + "", + "def _aspect_b_impl(target, ctx):", + " result = 'aspect_b on target {} '.format(target.label)", + " if prov_b in target:", + " result += 'found prov_b = {}'.format(target[prov_b].val)", + " return struct(aspect_b_result = result,", + " providers = [prov_b_forwarded(val = target[prov_b].val)])", + " else:", + " result += 'cannot find prov_b'", + " return struct(aspect_b_result = result)", + "aspect_b = aspect(", + " implementation = _aspect_b_impl,", + " required_aspect_providers = [prov_b]", + ")", + "", + "def _aspect_a_impl(target, ctx):", + " result = 'aspect_a on target {} '.format(target.label)", + " if prov_a in target:", + " result += 'found prov_a = {}'.format(target[prov_a].val)", + " else:", + " result += 'cannot find prov_a'", + " if prov_b_forwarded in target:", + " result += ' and found prov_b = {}'.format(target[prov_b_forwarded].val)", + " else:", + " result += ' but cannot find prov_b'", + " return struct(aspect_a_result = result)", + "", + "aspect_a = aspect(", + " implementation = _aspect_a_impl,", + " required_aspect_providers = [prov_a],", + " attr_aspects = ['dep'],", + " requires = [aspect_b]", + ")", + "", + "def _aspect_with_prov_a_impl(target, ctx):", + " return [prov_a(val = 'a1')]", + "aspect_with_prov_a = aspect(", + " implementation = _aspect_with_prov_a_impl,", + " provides = [prov_a],", + " attr_aspects = ['dep'],", + ")", + "", + "def _aspect_with_prov_b_impl(target, ctx):", + " return [prov_b(val = 'b1')]", + "aspect_with_prov_b = aspect(", + " implementation = _aspect_with_prov_b_impl,", + " provides = [prov_b],", + " attr_aspects = ['dep'],", + ")", + "", + "def _main_rule_impl(ctx):", + " return struct(aspect_a_result = ctx.attr.dep.aspect_a_result,", + " aspect_b_result = ctx.attr.dep.aspect_b_result)", + "main_rule = rule(", + " implementation = _main_rule_impl,", + " attrs = {", + " 'dep': attr.label(aspects = [aspect_with_prov_a, aspect_with_prov_b, aspect_a]),", + " },", + ")", + "", + "def _dep_rule_impl(ctx):", + " pass", + "dep_rule = rule(", + " implementation = _dep_rule_impl,", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'main_rule', 'dep_rule')", + "main_rule(", + " name = 'main',", + " dep = ':dep_target',", + ")", + "dep_rule(", + " name = 'dep_target',", + ")"); + + useConfiguration("--experimental_required_aspects"); + AnalysisResult analysisResult = update("//test:main"); + + ConfiguredTarget configuredTarget = + Iterables.getOnlyElement(analysisResult.getTargetsToBuild()); + String aspectAResult = (String) configuredTarget.get("aspect_a_result"); + assertThat(aspectAResult) + .isEqualTo("aspect_a on target //test:dep_target found prov_a = a1 and found prov_b = b1"); + + String aspectBResult = (String) configuredTarget.get("aspect_b_result"); + assertThat(aspectBResult).isEqualTo("aspect_b on target //test:dep_target found prov_b = b1"); + } + + @Test + public void testAspectRequiresAspect_withRequiredAspectProvidersNotFound() throws Exception { + scratch.file( + "test/defs.bzl", + "prov_a = provider()", + "prov_b = provider()", + "", + "def _aspect_b_impl(target, ctx):", + " result = 'aspect_b on target {} '.format(target.label)", + " if prov_b in target:", + " result += 'found prov_b = {}'.format(target[prov_b].val)", + " else:", + " result += 'cannot find prov_b'", + " return struct(aspect_b_result = result)", + "aspect_b = aspect(", + " implementation = _aspect_b_impl,", + " required_aspect_providers = [prov_b]", + ")", + "", + "def _aspect_a_impl(target, ctx):", + " result = 'aspect_a on target {} '.format(target.label)", + " if prov_a in target:", + " result += 'found prov_a = {}'.format(target[prov_a].val)", + " else:", + " result += 'cannot find prov_a'", + " return struct(aspect_a_result = result)", + "", + "aspect_a = aspect(", + " implementation = _aspect_a_impl,", + " required_aspect_providers = [prov_a],", + " attr_aspects = ['dep'],", + " requires = [aspect_b]", + ")", + "", + "def _aspect_with_prov_a_impl(target, ctx):", + " return [prov_a(val = 'a1')]", + "aspect_with_prov_a = aspect(", + " implementation = _aspect_with_prov_a_impl,", + " provides = [prov_a],", + " attr_aspects = ['dep'],", + ")", + "", + "def _main_rule_impl(ctx):", + " return struct(aspect_a_result = ctx.attr.dep.aspect_a_result,", + " aspect_b_result = ctx.attr.dep.aspect_b_result)", + "main_rule = rule(", + " implementation = _main_rule_impl,", + " attrs = {", + " 'dep': attr.label(aspects = [aspect_with_prov_a, aspect_a]),", + " },", + ")", + "", + "def _dep_rule_impl(ctx):", + " pass", + "dep_rule = rule(", + " implementation = _dep_rule_impl,", + ")"); + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'main_rule', 'dep_rule')", + "main_rule(", + " name = 'main',", + " dep = ':dep_target',", + ")", + "dep_rule(", + " name = 'dep_target',", + ")"); + + useConfiguration("--experimental_required_aspects"); + AnalysisResult analysisResult = update("//test:main"); + + ConfiguredTarget configuredTarget = + Iterables.getOnlyElement(analysisResult.getTargetsToBuild()); + String aspectAResult = (String) configuredTarget.get("aspect_a_result"); + assertThat(aspectAResult).isEqualTo("aspect_a on target //test:dep_target found prov_a = a1"); + + String aspectBResult = (String) configuredTarget.get("aspect_b_result"); + assertThat(aspectBResult).isEqualTo("aspect_b on target //test:dep_target cannot find prov_b"); + } + /** 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 b6c063d..d408add 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
@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.config.transitions.NoTransition; import com.google.devtools.build.lib.analysis.starlark.StarlarkAttrModule; import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleClassFunctions.StarlarkRuleFunction; @@ -33,6 +34,8 @@ import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.NullEventHandler; import com.google.devtools.build.lib.packages.AdvertisedProviderSet; +import com.google.devtools.build.lib.packages.Aspect; +import com.google.devtools.build.lib.packages.AspectClass; import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.BuildType; @@ -40,6 +43,7 @@ import com.google.devtools.build.lib.packages.ImplicitOutputsFunction; import com.google.devtools.build.lib.packages.PredicateWithMessage; import com.google.devtools.build.lib.packages.RequiredProviders; +import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; import com.google.devtools.build.lib.packages.StarlarkAspectClass; @@ -54,6 +58,8 @@ import com.google.devtools.build.lib.starlark.util.BazelEvaluationTestCase; import com.google.devtools.build.lib.testutil.MoreAsserts; import com.google.devtools.build.lib.util.FileTypeSet; +import java.util.Arrays; +import java.util.List; import javax.annotation.Nullable; import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; @@ -68,7 +74,6 @@ import net.starlark.java.syntax.Program; import net.starlark.java.syntax.StarlarkFile; import org.junit.Before; -import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; @@ -90,7 +95,7 @@ ev.setSemantics(options); // for StarlarkThread } - @Rule public ExpectedException thrown = ExpectedException.none(); + @org.junit.Rule public ExpectedException thrown = ExpectedException.none(); @Before public final void createBuildFile() throws Exception { @@ -407,6 +412,881 @@ } @Test + public void testAttrWithAspectRequiringAspects_stackOfRequiredAspects() throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + + evalAndExport( + ev, + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl)", + "aspect_b = aspect(implementation = _impl, requires = [aspect_c])", + "aspect_a = aspect(implementation = _impl, requires = [aspect_b])", + "a = attr.label_list(aspects = [aspect_a])"); + StarlarkAttrModule.Descriptor attr = (StarlarkAttrModule.Descriptor) ev.lookup("a"); + + StarlarkDefinedAspect aspectA = (StarlarkDefinedAspect) ev.lookup("aspect_a"); + assertThat(aspectA).isNotNull(); + StarlarkDefinedAspect aspectB = (StarlarkDefinedAspect) ev.lookup("aspect_b"); + assertThat(aspectB).isNotNull(); + StarlarkDefinedAspect aspectC = (StarlarkDefinedAspect) ev.lookup("aspect_c"); + assertThat(aspectC).isNotNull(); + List<AspectClass> expectedAspects = + Arrays.asList(aspectA.getAspectClass(), aspectB.getAspectClass(), aspectC.getAspectClass()); + assertThat(attr.build("xxx").getAspectClasses()).containsExactlyElementsIn(expectedAspects); + } + + @Test + public void testAttrWithAspectRequiringAspects_aspectRequiredByMultipleAspects() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + + evalAndExport( + ev, + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl)", + "aspect_b = aspect(implementation = _impl, requires = [aspect_c])", + "aspect_a = aspect(implementation = _impl, requires = [aspect_c])", + "a = attr.label_list(aspects = [aspect_a, aspect_b])"); + StarlarkAttrModule.Descriptor attr = (StarlarkAttrModule.Descriptor) ev.lookup("a"); + + StarlarkDefinedAspect aspectA = (StarlarkDefinedAspect) ev.lookup("aspect_a"); + assertThat(aspectA).isNotNull(); + StarlarkDefinedAspect aspectB = (StarlarkDefinedAspect) ev.lookup("aspect_b"); + assertThat(aspectB).isNotNull(); + StarlarkDefinedAspect aspectC = (StarlarkDefinedAspect) ev.lookup("aspect_c"); + assertThat(aspectC).isNotNull(); + List<AspectClass> expectedAspects = + Arrays.asList(aspectA.getAspectClass(), aspectB.getAspectClass(), aspectC.getAspectClass()); + assertThat(attr.build("xxx").getAspectClasses()).containsExactlyElementsIn(expectedAspects); + } + + @Test + public void testAttrWithAspectRequiringAspects_aspectRequiredByMultipleAspects2() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + + evalAndExport( + ev, + "def _impl(target, ctx):", + " pass", + "aspect_d = aspect(implementation = _impl)", + "aspect_c = aspect(implementation = _impl, requires = [aspect_d])", + "aspect_b = aspect(implementation = _impl, requires = [aspect_d])", + "aspect_a = aspect(implementation = _impl, requires = [aspect_b, aspect_c])", + "a = attr.label_list(aspects = [aspect_a])"); + StarlarkAttrModule.Descriptor attr = (StarlarkAttrModule.Descriptor) ev.lookup("a"); + + StarlarkDefinedAspect aspectA = (StarlarkDefinedAspect) ev.lookup("aspect_a"); + assertThat(aspectA).isNotNull(); + StarlarkDefinedAspect aspectB = (StarlarkDefinedAspect) ev.lookup("aspect_b"); + assertThat(aspectB).isNotNull(); + StarlarkDefinedAspect aspectC = (StarlarkDefinedAspect) ev.lookup("aspect_c"); + assertThat(aspectC).isNotNull(); + StarlarkDefinedAspect aspectD = (StarlarkDefinedAspect) ev.lookup("aspect_d"); + assertThat(aspectD).isNotNull(); + List<AspectClass> expectedAspects = + Arrays.asList( + aspectA.getAspectClass(), + aspectB.getAspectClass(), + aspectC.getAspectClass(), + aspectD.getAspectClass()); + assertThat(attr.build("xxx").getAspectClasses()).containsExactlyElementsIn(expectedAspects); + } + + @Test + public void testAttrWithAspectRequiringAspects_requireExistingAspect_passed() throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + + evalAndExport( + ev, + "def _impl(target, ctx):", + " pass", + "aspect_b = aspect(implementation = _impl)", + "aspect_a = aspect(implementation = _impl, requires = [aspect_b])", + "a = attr.label_list(aspects = [aspect_b, aspect_a])"); + StarlarkAttrModule.Descriptor attr = (StarlarkAttrModule.Descriptor) ev.lookup("a"); + + StarlarkDefinedAspect aspectA = (StarlarkDefinedAspect) ev.lookup("aspect_a"); + assertThat(aspectA).isNotNull(); + StarlarkDefinedAspect aspectB = (StarlarkDefinedAspect) ev.lookup("aspect_b"); + assertThat(aspectB).isNotNull(); + List<AspectClass> expectedAspects = + Arrays.asList(aspectA.getAspectClass(), aspectB.getAspectClass()); + assertThat(attr.build("xxx").getAspectClasses()).containsExactlyElementsIn(expectedAspects); + } + + @Test + public void testAttrWithAspectRequiringAspects_requireExistingAspect_failed() throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + ev.setFailFast(false); + + evalAndExport( + ev, + "def _impl(target, ctx):", + " pass", + "aspect_b = aspect(implementation = _impl)", + "aspect_a = aspect(implementation = _impl, requires = [aspect_b])", + "attr.label_list(aspects = [aspect_a, aspect_b])"); + + ev.assertContainsError( + String.format( + "aspect %s%%aspect_b was added before as a required aspect of aspect %s%%aspect_a", + FAKE_LABEL, FAKE_LABEL)); + } + + @Test + public void testAttrWithAspectRequiringAspects_inheritDefaultValues() throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "def _impl(target, ctx):", + " pass", + "aspect_b = aspect(implementation = _impl)", + "aspect_a = aspect(implementation = _impl, requires = [aspect_b])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl, attrs={'a': attr.label_list(aspects = [aspect_a])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute a = rule.getRuleClassObject().getAttributeByName("a"); + ImmutableList<Aspect> aspects = a.getAspects(rule); + // aspect_b inherits the required providers and propagation attributes from aspect_a + Aspect aspectB = aspects.get(0); + assertThat(aspectB.getDescriptor().getInheritedRequiredProviders()) + .isEqualTo(RequiredProviders.acceptAnyBuilder().build()); + assertThat(aspectB.getDescriptor().getInheritedAttributeAspects()).isEmpty(); + + // aspect_a is not required by any other aspect so its does not inherit anything + Aspect aspectA = aspects.get(1); + assertThat(aspectA.getDescriptor().getInheritedRequiredProviders()) + .isEqualTo(RequiredProviders.acceptNoneBuilder().build()); + assertThat(aspectA.getDescriptor().getInheritedAttributeAspects()).isEmpty(); + } + + @Test + public void testAttrWithAspectRequiringAspects_inheritAttrAspectsFromSingleAspect() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "cc = provider()", + "def _impl(target, ctx):", + " pass", + "aspect_b = aspect(implementation = _impl,", + " attr_aspects = ['extra_deps'])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_b],", + " attr_aspects = ['deps'])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl,", + " attrs = {'a': attr.label_list(aspects = [aspect_a])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute a = rule.getRuleClassObject().getAttributeByName("a"); + ImmutableList<Aspect> aspects = a.getAspects(rule); + Aspect aspectB = aspects.get(0); + assertThat(aspectB.getDescriptor().getInheritedAttributeAspects()).containsExactly("deps"); + assertThat(aspectB.getDefinition().getRestrictToAttributes()).containsExactly("extra_deps"); + } + + @Test + public void testAttrWithAspectRequiringAspects_inheritRequiredProvidersFromSingleAspect() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "cc = provider()", + "def _impl(target, ctx):", + " pass", + "aspect_b = aspect(implementation = _impl,", + " required_providers= ['go'])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_b],", + " required_providers=[['java', cc], ['python']])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl,", + " attrs = {'a': attr.label_list(aspects = [aspect_a])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute a = rule.getRuleClassObject().getAttributeByName("a"); + ImmutableList<Aspect> aspects = a.getAspects(rule); + Aspect aspectB = aspects.get(0); + assertThat(aspectB.getDescriptor().getInheritedRequiredProviders().getDescription()) + .isEqualTo("['java', 'cc'] or 'python'"); + assertThat(aspectB.getDefinition().getRequiredProviders().getDescription()).isEqualTo("'go'"); + } + + @Test + public void testAttrWithAspectRequiringExistingAspect_inheritAttrAspectsFromSingleAspect() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "cc = provider()", + "def _impl(target, ctx):", + " pass", + "aspect_b = aspect(implementation = _impl)", + "aspect_a = aspect(implementation = _impl, requires = [aspect_b],", + " attr_aspects = ['deps'])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl,", + " attrs={'a': attr.label_list(aspects = [aspect_b, aspect_a])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute a = rule.getRuleClassObject().getAttributeByName("a"); + ImmutableList<Aspect> aspects = a.getAspects(rule); + Aspect aspectB = aspects.get(0); + assertThat(aspectB.getDescriptor().getInheritedAttributeAspects()).containsExactly("deps"); + } + + @Test + public void testAttrWithAspectRequiringExistingAspect_inheritRequiredProvidersFromSingleAspect() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "cc = provider()", + "def _impl(target, ctx):", + " pass", + "aspect_b = aspect(implementation = _impl)", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_b],", + " required_providers=[['java', cc], ['python']])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl, attrs={'a': attr.label_list(aspects = [aspect_b, aspect_a])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute a = rule.getRuleClassObject().getAttributeByName("a"); + ImmutableList<Aspect> aspects = a.getAspects(rule); + Aspect aspectB = aspects.get(0); + assertThat(aspectB.getDescriptor().getInheritedRequiredProviders().getDescription()) + .isEqualTo("['java', 'cc'] or 'python'"); + } + + @Test + public void testAttrWithAspectRequiringAspects_inheritAttrAspectsFromMultipleAspects() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "cc = provider()", + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl)", + "aspect_b = aspect(implementation = _impl,", + " requires = [aspect_c],", + " attr_aspects = ['extra_deps'])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_c],", + " attr_aspects = ['deps'])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl,", + " attrs={'a': attr.label_list(aspects = [aspect_a, aspect_b])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute a = rule.getRuleClassObject().getAttributeByName("a"); + ImmutableList<Aspect> aspects = a.getAspects(rule); + Aspect aspectC = aspects.get(0); + assertThat(aspectC.getDescriptor().getInheritedAttributeAspects()) + .containsExactly("deps", "extra_deps"); + } + + @Test + public void testAttrWithAspectRequiringAspects_inheritRequiredProvidersFromMultipleAspects() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "cc = provider()", + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl)", + "aspect_b = aspect(implementation = _impl,", + " requires = [aspect_c],", + " required_providers=['go'])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_c],", + " required_providers=[['java', cc], ['python']])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl,", + " attrs={'a': attr.label_list(aspects = [aspect_a, aspect_b])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute a = rule.getRuleClassObject().getAttributeByName("a"); + ImmutableList<Aspect> aspects = a.getAspects(rule); + Aspect aspectC = aspects.get(0); + assertThat(aspectC.getDescriptor().getInheritedRequiredProviders().getDescription()) + .isEqualTo("['java', 'cc'] or 'python' or 'go'"); + } + + @Test + public void testAttrWithAspectRequiringAspects_inheritAllAttrAspects() throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "cc = provider()", + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl)", + "aspect_b = aspect(implementation = _impl,", + " requires = [aspect_c],", + " attr_aspects = ['extra_deps'])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_c],", + " attr_aspects = ['*'])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl, attrs={'a': attr.label_list(aspects = [aspect_a, aspect_b])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute a = rule.getRuleClassObject().getAttributeByName("a"); + ImmutableList<Aspect> aspects = a.getAspects(rule); + Aspect aspectC = aspects.get(0); + assertThat(aspectC.getDescriptor().getInheritedAttributeAspects()) + .isNull(); // propagate along all attributes '*' + } + + @Test + public void testAttrWithAspectRequiringAspects_inheritAllRequiredProviders() throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "cc = provider()", + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl)", + "aspect_b = aspect(implementation = _impl,", + " requires = [aspect_c],", + " required_providers = [])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_c],", + " required_providers=[['java', cc], ['python']])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl, attrs={'a': attr.label_list(aspects = [aspect_a, aspect_b])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute a = rule.getRuleClassObject().getAttributeByName("a"); + ImmutableList<Aspect> aspects = a.getAspects(rule); + Aspect aspectC = aspects.get(0); + assertThat(aspectC.getDescriptor().getInheritedRequiredProviders()) + .isEqualTo(RequiredProviders.acceptAnyBuilder().build()); + } + + @Test + public void testAttrWithAspectRequiringAspects_inheritAttrAspectsFromAspectsStack() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "cc = provider()", + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl)", + "aspect_b = aspect(implementation = _impl,", + " requires = [aspect_c],", + " attr_aspects = ['extra_deps'])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_b],", + " attr_aspects = ['deps'])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl, attrs={'a': attr.label_list(aspects = [aspect_a])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute a = rule.getRuleClassObject().getAttributeByName("a"); + ImmutableList<Aspect> aspects = a.getAspects(rule); + Aspect aspectC = aspects.get(0); + assertThat(aspectC.getDescriptor().getInheritedAttributeAspects()) + .containsExactly("deps", "extra_deps"); + } + + @Test + public void testAttrWithAspectRequiringAspects_inheritRequiredProvidersFromAspectsStack() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "cc = provider()", + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl)", + "aspect_b = aspect(implementation = _impl,", + " requires = [aspect_c],", + " required_providers=['go'])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_b],", + " required_providers=[['java', cc], ['python']])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl, attrs={'a': attr.label_list(aspects = [aspect_a])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute a = rule.getRuleClassObject().getAttributeByName("a"); + ImmutableList<Aspect> aspects = a.getAspects(rule); + Aspect aspectC = aspects.get(0); + assertThat(aspectC.getDescriptor().getInheritedRequiredProviders().getDescription()) + .isEqualTo("['java', 'cc'] or 'python' or 'go'"); + } + + @Test + public void testAttrWithAspectRequiringAspects_inheritAllAttrAspectsFromAspectsStack() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "cc = provider()", + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl)", + "aspect_b = aspect(implementation = _impl,", + " requires = [aspect_c],", + " attr_aspects = ['*'])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_b],", + " attr_aspects = ['deps'])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl, attrs={'a': attr.label_list(aspects = [aspect_a])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute a = rule.getRuleClassObject().getAttributeByName("a"); + ImmutableList<Aspect> aspects = a.getAspects(rule); + Aspect aspectC = aspects.get(0); + assertThat(aspectC.getDescriptor().getInheritedAttributeAspects()) + .isNull(); // propagate along all attributes '*' + } + + @Test + public void testAttrWithAspectRequiringAspects_inheritAllRequiredProvidersFromAspectsStack() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "cc = provider()", + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl)", + "aspect_b = aspect(implementation = _impl,", + " requires = [aspect_c],", + " required_providers=[cc])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_b],", + " required_providers=[])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl, attrs={'a': attr.label_list(aspects = [aspect_a])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute a = rule.getRuleClassObject().getAttributeByName("a"); + ImmutableList<Aspect> aspects = a.getAspects(rule); + Aspect aspectC = aspects.get(0); + assertThat(aspectC.getDescriptor().getInheritedRequiredProviders()) + .isEqualTo(RequiredProviders.acceptAnyBuilder().build()); + } + + /** + * An aspect required by different aspects in different attributes inherits the correct + * propagation attributes. + */ + @Test + public void testAttrWithAspectRequiringAspects_aspectRequiredInMultipleAttrs_inheritAttrAspects() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl)", + "aspect_b = aspect(implementation = _impl,", + " requires = [aspect_c],", + " attr_aspects = ['deps'])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_c],", + " attr_aspects = ['extra_deps'])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl,", + " attrs={'attr1': attr.label_list(aspects = [aspect_a]),", + " 'attr2': attr.label_list(aspects = [aspect_b]),", + " 'attr3': attr.label_list(aspects = [aspect_c])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute attr1 = rule.getRuleClassObject().getAttributeByName("attr1"); + ImmutableList<Aspect> aspects = attr1.getAspects(rule); + Aspect aspectC = aspects.get(0); + assertThat(aspectC.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_c"); + assertThat(aspectC.getDescriptor().getInheritedAttributeAspects()) + .containsExactly("extra_deps"); + + Attribute attr2 = rule.getRuleClassObject().getAttributeByName("attr2"); + aspects = attr2.getAspects(rule); + aspectC = aspects.get(0); + assertThat(aspectC.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_c"); + assertThat(aspectC.getDescriptor().getInheritedAttributeAspects()).containsExactly("deps"); + + Attribute attr3 = rule.getRuleClassObject().getAttributeByName("attr3"); + aspects = attr3.getAspects(rule); + aspectC = aspects.get(0); + assertThat(aspectC.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_c"); + assertThat(aspectC.getDescriptor().getInheritedAttributeAspects()).isEmpty(); + } + + /** + * An aspect required by different aspects in different attributes inherits the correct required + * providers. + */ + @Test + public void + testAttrWithAspectRequiringAspects_aspectRequiredInMultipleAttrs_inheritRequiredProviders() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl)", + "aspect_b = aspect(implementation = _impl,", + " requires = [aspect_c],", + " required_providers=['prov1', 'prov2'])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_c],", + " required_providers=[['prov3', 'prov4'], ['prov2']])", + "def impl(ctx):", + " return None", + "my_rule = rule(impl,", + " attrs={'attr1': attr.label_list(aspects = [aspect_a]),", + " 'attr2': attr.label_list(aspects = [aspect_b]),", + " 'attr3': attr.label_list(aspects = [aspect_c])})"); + scratch.file("BUILD", "load(':lib.bzl', 'my_rule')", "my_rule(name = 'main')"); + + RuleContext ruleContext = createRuleContext("//:main").getRuleContext(); + + Rule rule = ruleContext.getRule(); + Attribute attr1 = rule.getRuleClassObject().getAttributeByName("attr1"); + ImmutableList<Aspect> aspects = attr1.getAspects(rule); + Aspect aspectC = aspects.get(0); + assertThat(aspectC.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_c"); + assertThat(aspectC.getDescriptor().getInheritedRequiredProviders().getDescription()) + .isEqualTo("['prov3', 'prov4'] or 'prov2'"); + + Attribute attr2 = rule.getRuleClassObject().getAttributeByName("attr2"); + aspects = attr2.getAspects(rule); + aspectC = aspects.get(0); + assertThat(aspectC.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_c"); + assertThat(aspectC.getDescriptor().getInheritedRequiredProviders().getDescription()) + .isEqualTo("['prov1', 'prov2']"); + + Attribute attr3 = rule.getRuleClassObject().getAttributeByName("attr3"); + aspects = attr3.getAspects(rule); + aspectC = aspects.get(0); + assertThat(aspectC.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_c"); + assertThat(aspectC.getDescriptor().getInheritedRequiredProviders()) + .isEqualTo(RequiredProviders.acceptNoneBuilder().build()); + } + + /** + * An aspect required by different aspects in different rules inherits the correct propagation + * attributes. + */ + @Test + public void testAttrWithAspectRequiringAspects_aspectRequiredInMultipleRules_inheritAttrAspects() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl)", + "aspect_b = aspect(implementation = _impl,", + " requires = [aspect_c],", + " attr_aspects = ['deps'])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_c],", + " attr_aspects = ['extra_deps'])", + "def impl(ctx):", + " return None", + "rule_1 = rule(impl,", + " attrs={'attr': attr.label_list(aspects = [aspect_a])})", + "rule_2 = rule(impl,", + " attrs={'attr': attr.label_list(aspects = [aspect_b])})"); + scratch.file( + "BUILD", + "load(':lib.bzl', 'rule_1', 'rule_2')", + "rule_1(name = 't1')", + "rule_2(name = 't2')"); + + RuleContext ruleContext1 = createRuleContext("//:t1").getRuleContext(); + RuleContext ruleContext2 = createRuleContext("//:t2").getRuleContext(); + + Rule rule = ruleContext1.getRule(); + Attribute attr = rule.getRuleClassObject().getAttributeByName("attr"); + ImmutableList<Aspect> aspects = attr.getAspects(rule); + Aspect aspectC = aspects.get(0); + assertThat(aspectC.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_c"); + assertThat(aspectC.getDescriptor().getInheritedAttributeAspects()) + .containsExactly("extra_deps"); + + rule = ruleContext2.getRule(); + attr = rule.getRuleClassObject().getAttributeByName("attr"); + aspects = attr.getAspects(rule); + aspectC = aspects.get(0); + assertThat(aspectC.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_c"); + assertThat(aspectC.getDescriptor().getInheritedAttributeAspects()).containsExactly("deps"); + } + + /** + * An aspect required by different aspects in different rules inherits the correct required + * providers. + */ + @Test + public void + testAttrWithAspectRequiringAspects_aspectRequiredInMultipleRules_inheritRequiredProviders() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl)", + "aspect_b = aspect(implementation = _impl,", + " requires = [aspect_c],", + " required_providers=['prov1', 'prov2'])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_c],", + " required_providers=[['prov3', 'prov4'], ['prov2']])", + "def impl(ctx):", + " return None", + "rule_1 = rule(impl,", + " attrs={'attr': attr.label_list(aspects = [aspect_a])})", + "rule_2 = rule(impl,", + " attrs={'attr': attr.label_list(aspects = [aspect_b])})"); + scratch.file( + "BUILD", + "load(':lib.bzl', 'rule_1', 'rule_2')", + "rule_1(name = 't1')", + "rule_2(name = 't2')"); + + RuleContext ruleContext1 = createRuleContext("//:t1").getRuleContext(); + RuleContext ruleContext2 = createRuleContext("//:t2").getRuleContext(); + + Rule rule = ruleContext1.getRule(); + Attribute attr = rule.getRuleClassObject().getAttributeByName("attr"); + ImmutableList<Aspect> aspects = attr.getAspects(rule); + Aspect aspectC = aspects.get(0); + assertThat(aspectC.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_c"); + assertThat(aspectC.getDescriptor().getInheritedRequiredProviders().getDescription()) + .isEqualTo("['prov3', 'prov4'] or 'prov2'"); + + rule = ruleContext2.getRule(); + attr = rule.getRuleClassObject().getAttributeByName("attr"); + aspects = attr.getAspects(rule); + aspectC = aspects.get(0); + assertThat(aspectC.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_c"); + assertThat(aspectC.getDescriptor().getInheritedRequiredProviders().getDescription()) + .isEqualTo("['prov1', 'prov2']"); + } + + /** + * An aspect required by different aspects in different rules gets the correct parameters values + * from its base rule. + */ + @Test + public void testAttrWithAspectRequiringAspects_aspectRequiredInMultipleRules_getParametersValues() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "def _impl(target, ctx):", + " pass", + "aspect_c = aspect(implementation = _impl,", + " attrs = {'param': attr.string(values = ['v1', 'v2', 'v3'])})", + "aspect_b = aspect(implementation = _impl,", + " requires = [aspect_c])", + "aspect_a = aspect(implementation = _impl,", + " requires = [aspect_c])", + "def impl(ctx):", + " return None", + "rule_1 = rule(impl,", + " attrs={'attr': attr.label_list(aspects = [aspect_a]),", + " 'param': attr.string()})", + "rule_2 = rule(impl,", + " attrs={'attr': attr.label_list(aspects = [aspect_b]),", + " 'param': attr.string()})"); + scratch.file( + "BUILD", + "load(':lib.bzl', 'rule_1', 'rule_2')", + "rule_1(name = 't1', param = 'v1')", + "rule_2(name = 't2', param = 'v2')"); + + RuleContext ruleContext1 = createRuleContext("//:t1").getRuleContext(); + RuleContext ruleContext2 = createRuleContext("//:t2").getRuleContext(); + + Rule rule = ruleContext1.getRule(); + Attribute attr = rule.getRuleClassObject().getAttributeByName("attr"); + ImmutableList<Aspect> aspects = attr.getAspects(rule); + Aspect aspectC = aspects.get(0); + assertThat(aspectC.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_c"); + assertThat(aspectC.getDefinition().getAttributes().get("param").getDefaultValueUnchecked()) + .isEqualTo("v1"); + + rule = ruleContext2.getRule(); + attr = rule.getRuleClassObject().getAttributeByName("attr"); + aspects = attr.getAspects(rule); + aspectC = aspects.get(0); + assertThat(aspectC.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_c"); + assertThat(aspectC.getDefinition().getAttributes().get("param").getDefaultValueUnchecked()) + .isEqualTo("v2"); + } + + /** + * Using the same attribute in multiple rules should not change the required aspect inherited + * propagation attributes. + */ + @Test + public void testAttrWithAspectRequiringAspects_sameAttrInMultipleRules_inheritSameAttrAspects() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "def _impl(target, ctx):", + " pass", + "aspect_b = aspect(implementation = _impl)", + "aspect_a = aspect(implementation = _impl, requires = [aspect_b],", + " attr_aspects = ['extra_deps'])", + "my_attr = attr.label_list(aspects = [aspect_a])", + "def impl(ctx):", + " return None", + "rule_1 = rule(impl,", + " attrs={'attr': my_attr})", + "rule_2 = rule(impl,", + " attrs={'attr': my_attr})"); + scratch.file( + "BUILD", + "load(':lib.bzl', 'rule_1', 'rule_2')", + "rule_1(name = 't1')", + "rule_2(name = 't2')"); + + RuleContext ruleContext1 = createRuleContext("//:t1").getRuleContext(); + RuleContext ruleContext2 = createRuleContext("//:t2").getRuleContext(); + + Rule rule = ruleContext1.getRule(); + Attribute attr = rule.getRuleClassObject().getAttributeByName("attr"); + ImmutableList<Aspect> aspects = attr.getAspects(rule); + Aspect aspectB = aspects.get(0); + assertThat(aspectB.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_b"); + assertThat(aspectB.getDescriptor().getInheritedAttributeAspects()) + .containsExactly("extra_deps"); + + rule = ruleContext2.getRule(); + attr = rule.getRuleClassObject().getAttributeByName("attr"); + aspects = attr.getAspects(rule); + aspectB = aspects.get(0); + assertThat(aspectB.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_b"); + assertThat(aspectB.getDescriptor().getInheritedAttributeAspects()) + .containsExactly("extra_deps"); + } + + /** + * Using the same attribute in multiple rules should not change the required aspect inherited + * required providers. + */ + @Test + public void + testAttrWithAspectRequiringAspects_sameAttrInMultipleRules_inheritSameRequiredProviders() + throws Exception { + setBuildLanguageOptions("--experimental_required_aspects=true"); + scratch.file( + "lib.bzl", + "def _impl(target, ctx):", + " pass", + "aspect_b = aspect(implementation = _impl)", + "aspect_a = aspect(implementation = _impl, requires = [aspect_b],", + " required_providers=[['prov3', 'prov4'], ['prov2']])", + "my_attr = attr.label_list(aspects = [aspect_a])", + "def impl(ctx):", + " return None", + "rule_1 = rule(impl,", + " attrs={'attr': my_attr})", + "rule_2 = rule(impl,", + " attrs={'attr': my_attr})"); + scratch.file( + "BUILD", + "load(':lib.bzl', 'rule_1', 'rule_2')", + "rule_1(name = 't1')", + "rule_2(name = 't2')"); + + RuleContext ruleContext1 = createRuleContext("//:t1").getRuleContext(); + RuleContext ruleContext2 = createRuleContext("//:t2").getRuleContext(); + + Rule rule = ruleContext1.getRule(); + Attribute attr = rule.getRuleClassObject().getAttributeByName("attr"); + ImmutableList<Aspect> aspects = attr.getAspects(rule); + Aspect aspectB = aspects.get(0); + assertThat(aspectB.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_b"); + assertThat(aspectB.getDescriptor().getInheritedRequiredProviders().getDescription()) + .isEqualTo("['prov3', 'prov4'] or 'prov2'"); + + rule = ruleContext2.getRule(); + attr = rule.getRuleClassObject().getAttributeByName("attr"); + aspects = attr.getAspects(rule); + aspectB = aspects.get(0); + assertThat(aspectB.getAspectClass().getName()).isEqualTo("//:lib.bzl%aspect_b"); + assertThat(aspectB.getDescriptor().getInheritedRequiredProviders().getDescription()) + .isEqualTo("['prov3', 'prov4'] or 'prov2'"); + } + + @Test public void testAspectExtraDeps() throws Exception { evalAndExport( ev,
diff --git a/src/test/shell/integration/aspect_test.sh b/src/test/shell/integration/aspect_test.sh index 63f0c97..12dd171 100755 --- a/src/test/shell/integration/aspect_test.sh +++ b/src/test/shell/integration/aspect_test.sh
@@ -441,4 +441,164 @@ expect_not_log "my_aspect runs on target //${package}:target_with_c_indeps_not_reached" } +function test_aspects_propagating_other_aspects() { + local package="a" + mkdir -p "${package}" + + cat > "${package}/lib.bzl" <<EOF +prov_a = provider() + +def _required_aspect_impl(target, ctx): + print("required_aspect runs on target {}".format(target.label)) + return [] + +required_aspect = aspect(implementation = _required_aspect_impl, + required_providers = [prov_a]) + +def _base_aspect_impl(target, ctx): + print("base_aspect runs on target {}".format(target.label)) + return [] + +base_aspect = aspect(implementation = _base_aspect_impl, + attr_aspects = ['dep'], + requires = [required_aspect]) + +def _rule_impl(ctx): + pass + +base_rule = rule(implementation = _rule_impl, + attrs = {'dep': attr.label(aspects=[base_aspect])}) + +dep_rule_without_providers = rule(implementation = _rule_impl, + attrs = {'dep': attr.label()}) + +def _dep_rule_with_prov_a_impl(ctx): + return [prov_a()] + +dep_rule_with_prov_a = rule(implementation = _dep_rule_with_prov_a_impl, + provides = [prov_a]) + +EOF + + cat > "${package}/BUILD" <<EOF +load('//${package}:lib.bzl', 'base_rule', 'dep_rule_without_providers', + 'dep_rule_with_prov_a') + +base_rule( + name = 'main', + dep = ':dep_target_without_providers', +) + +dep_rule_without_providers( + name = 'dep_target_without_providers', + dep = ':dep_target_without_providers_1' +) + +dep_rule_without_providers( + name = 'dep_target_without_providers_1', + dep = ':dep_target_with_prov_a' +) + +dep_rule_with_prov_a( + name = 'dep_target_with_prov_a', +) + +EOF + + bazel build "${package}:main" \ + --experimental_required_aspects &>"$TEST_log" \ + || fail "Build failed but should have succeeded" + + # base_aspect will run on dep_target_without_providers, + # dep_target_without_providers_1 and dep_target_with_prov_a but + # required_aspect will only run on dep_target_with_prov_a because + # it satisfies its required providers + expect_log "base_aspect runs on target //${package}:dep_target_with_prov_a" + expect_log "base_aspect runs on target //${package}:dep_target_without_providers_1" + expect_log "base_aspect runs on target //${package}:dep_target_without_providers" + expect_log "required_aspect runs on target //${package}:dep_target_with_prov_a" + expect_not_log "required_aspect runs on target //${package}:dep_target_without_providers_1" + expect_not_log "required_aspect runs on target //${package}:dep_target_without_providers/" +} + +function test_aspects_propagating_other_aspects_stack_of_required_aspects() { + local package="pkg" + mkdir -p "${package}" + + cat > "${package}/lib.bzl" <<EOF + +def _aspect_a_impl(target, ctx): + print("Aspect 'a' applied on target: {}".format(target.label)) + return [] + +def _aspect_b_impl(target, ctx): + print("Aspect 'b' applied on target: {}".format(target.label)) + return [] + +def _aspect_c_impl(target, ctx): + print("Aspect 'c' applied on target: {}".format(target.label)) + return [] + +def r_impl(ctx): + return [] + +aspect_c = aspect(implementation = _aspect_c_impl, + attr_aspects = ["deps"]) + +aspect_b = aspect(implementation = _aspect_b_impl, + attr_aspects = ["deps"], requires = [aspect_c]) + +aspect_a = aspect(implementation = _aspect_a_impl, + attr_aspects = ["deps"], requires = [aspect_b]) + +rule_r = rule( + implementation = r_impl, + attrs = { + "deps": attr.label_list(aspects = [aspect_a]), + } +) + +EOF + +echo "inline int x() { return 42; }" > "${package}/x.h" + cat > "${package}/t.cc" <<EOF +#include "${package}/x.h" + +int a() { return x(); } +EOF + cat > "${package}/BUILD" <<EOF +load("//${package}:lib.bzl", "rule_r") + +cc_library( + name = "x", + hdrs = ["x.h"], +) + +cc_library( + name = "t", + srcs = ["t.cc"], + deps = [":x"], +) + +rule_r( + name = "test", + deps = [":t"], +) +EOF + + bazel build "${package}:test" \ + --experimental_required_aspects &>"$TEST_log" \ + || fail "Build failed but should have succeeded" + + # Check that aspects: aspect_a, aspect_b, aspect_c were propagated to the + # dependencies of target test: t and x when only aspect_a is specified + # in the rule definition + expect_log_once "Aspect 'c' applied on target: //${package}:x" + expect_log_once "Aspect 'c' applied on target: //${package}:t" + expect_log_once "Aspect 'b' applied on target: //${package}:x" + expect_log_once "Aspect 'b' applied on target: //${package}:t" + expect_log_once "Aspect 'a' applied on target: //${package}:x" + expect_log_once "Aspect 'a' applied on target: //${package}:t" +} + run_suite "Tests for aspects"