Allow aspects to adveritise providers they provide. -- PiperOrigin-RevId: 146794883 MOS_MIGRATED_REVID=146794883
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java index 11d03fa..e2fbee9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -35,6 +35,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.packages.AdvertisedProviderSet; import com.google.devtools.build.lib.packages.Aspect; import com.google.devtools.build.lib.packages.AspectDescriptor; import com.google.devtools.build.lib.packages.Attribute; @@ -352,7 +353,49 @@ return null; } - return aspectFactory.create(associatedTarget, ruleContext, aspect.getParameters()); + ConfiguredAspect configuredAspect = aspectFactory + .create(associatedTarget, ruleContext, aspect.getParameters()); + validateAdvertisedProviders( + configuredAspect, aspect.getDefinition().getAdvertisedProviders(), + associatedTarget.getTarget(), + env.getEventHandler() + ); + return configuredAspect; + } + + private void validateAdvertisedProviders( + ConfiguredAspect configuredAspect, + AdvertisedProviderSet advertisedProviders, Target target, + EventHandler eventHandler) { + if (advertisedProviders.canHaveAnyProvider()) { + return; + } + for (Class<?> aClass : advertisedProviders.getNativeProviders()) { + if (configuredAspect.getProvider(aClass.asSubclass(TransitiveInfoProvider.class)) == null) { + eventHandler.handle(Event.error( + target.getLocation(), + String.format( + "Aspect '%s', applied to '%s', does not provide advertised provider '%s'", + configuredAspect.getName(), + target.getLabel(), + aClass.getSimpleName() + ))); + } + } + + for (String providerName : advertisedProviders.getSkylarkProviders()) { + SkylarkProviders skylarkProviders = configuredAspect.getProvider(SkylarkProviders.class); + if (skylarkProviders == null || skylarkProviders.getValue(providerName) == null) { + eventHandler.handle(Event.error( + target.getLocation(), + String.format( + "Aspect '%s', applied to '%s', does not provide advertised provider '%s'", + configuredAspect.getName(), + target.getLabel(), + providerName + ))); + } + } } /**
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 55b0616..c7e11c0 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.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; @@ -52,6 +53,7 @@ @Immutable public final class AspectDefinition { private final AspectClass aspectClass; + private final AdvertisedProviderSet advertisedProviders; private final RequiredProviders requiredProviders; private final RequiredProviders requiredProvidersForAspects; private final ImmutableMap<String, Attribute> attributes; @@ -66,14 +68,21 @@ @Nullable private final ImmutableSet<String> restrictToAttributes; @Nullable private final ConfigurationFragmentPolicy configurationFragmentPolicy; + public AdvertisedProviderSet getAdvertisedProviders() { + return advertisedProviders; + } + + private AspectDefinition( AspectClass aspectClass, + AdvertisedProviderSet advertisedProviders, RequiredProviders requiredProviders, RequiredProviders requiredAspectProviders, ImmutableMap<String, Attribute> attributes, @Nullable ImmutableSet<String> restrictToAttributes, @Nullable ConfigurationFragmentPolicy configurationFragmentPolicy) { this.aspectClass = aspectClass; + this.advertisedProviders = advertisedProviders; this.requiredProviders = requiredProviders; this.requiredProvidersForAspects = requiredAspectProviders; @@ -217,6 +226,8 @@ public static final class Builder { private final AspectClass aspectClass; private final Map<String, Attribute> attributes = new LinkedHashMap<>(); + private final AdvertisedProviderSet.Builder advertisedProviders = + AdvertisedProviderSet.builder(); private RequiredProviders.Builder requiredProviders = RequiredProviders.acceptAnyBuilder(); private RequiredProviders.Builder requiredAspectProviders = RequiredProviders.acceptNoneBuilder(); @@ -267,6 +278,29 @@ return this; } + /** + * State that the aspect being built provides given providers. + */ + public Builder advertiseProvider(Class<?>... providers) { + for (Class<?> provider : providers) { + advertisedProviders.addNative(provider); + } + return this; + } + + /** + * State that the aspect being built provides given providers. + */ + public Builder advertiseProvider(ImmutableList<SkylarkProviderIdentifier> providers) { + for (SkylarkProviderIdentifier provider : providers) { + // todo(dslomov,vladmos): support declared providers + Preconditions.checkState(provider.isLegacy()); + advertisedProviders.addSkylark(provider.getLegacyId()); + } + return this; + } + + /** * Declares that this aspect propagates along an {@code attribute} on the target @@ -402,6 +436,7 @@ */ public AspectDefinition build() { return new AspectDefinition(aspectClass, + advertisedProviders.build(), requiredProviders.build(), requiredAspectProviders.build(), ImmutableMap.copyOf(attributes),
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 389c2d0..2288a76 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
@@ -1014,6 +1014,9 @@ return this; } + /** + * Should only be used for deserialization. + */ public Builder<TYPE> aspect(final Aspect aspect) { PredefinedRuleAspect predefinedRuleAspect = new PredefinedRuleAspect(aspect); RuleAspect<?> oldAspect =
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java index c3748fe..76d24e0 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -684,4 +684,29 @@ .containsExactly( "aspect //a:a", "aspect //a:b", "aspect //a:c", "aspect //a:tool", "rule //a:x"); } + + @Test + public void aspectTruthInAdvertisement() throws Exception { + reporter.removeHandler(failFastHandler); // expect errors + setRulesAvailableInTests( + new TestAspects.BaseRule(), + new TestAspects.SimpleRule(), + new TestAspects.FalseAdvertisementAspectRule()); + pkg( + "a", + "simple(name = 's')", + "false_advertisement_aspect(name = 'x', deps = [':s'])" + ); + try { + update("//a:x"); + } catch (ViewCreationFailedException e) { + // expected. + } + assertContainsEvent( + "Aspect 'FalseAdvertisementAspect', applied to '//a:s'," + + " does not provide advertised provider 'RequiredProvider'"); + assertContainsEvent( + "Aspect 'FalseAdvertisementAspect', applied to '//a:s'," + + " does not provide advertised provider 'advertised_provider'"); + } }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java index 3a74a59..0566c43 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
@@ -61,6 +61,7 @@ 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; +import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier; import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.FileTypeSet; @@ -442,6 +443,33 @@ .build(); /** + * An aspect that advertises but fails to provide providers. + */ + public static class FalseAdvertisementAspect extends NativeAspectClass + implements ConfiguredAspectFactory { + + @Override + public AspectDefinition getDefinition(AspectParameters aspectParameters) { + return FALSE_ADVERTISEMENT_DEFINITION; + } + + @Override + public ConfiguredAspect create(ConfiguredTarget base, RuleContext context, + AspectParameters parameters) throws InterruptedException { + return new ConfiguredAspect.Builder(this, parameters, context).build(); + } + } + public static final FalseAdvertisementAspect FALSE_ADVERTISEMENT_ASPECT + = new FalseAdvertisementAspect(); + private static final AspectDefinition FALSE_ADVERTISEMENT_DEFINITION = + new AspectDefinition.Builder(FALSE_ADVERTISEMENT_ASPECT) + .advertiseProvider(RequiredProvider.class) + .advertiseProvider( + ImmutableList.of(SkylarkProviderIdentifier.forLegacy("advertised_provider"))) + .build(); + + + /** * A common base rule for mock rules in this class to reduce boilerplate. * * <p>It has a few common attributes because internal Blaze machinery assumes the presence of @@ -967,6 +995,28 @@ } /** + * Rule with {@link FalseAdvertisementAspect} + */ + public static final class FalseAdvertisementAspectRule implements RuleDefinition { + + @Override + public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) { + return builder + .add(attr("deps", LABEL_LIST).allowedFileTypes().aspect(FALSE_ADVERTISEMENT_ASPECT)) + .build(); + } + + @Override + public Metadata getMetadata() { + return RuleDefinition.Metadata.builder() + .name("false_advertisement_aspect") + .factoryClass(DummyRuleFactory.class) + .ancestors(BaseRule.class) + .build(); + } + } + + /** * Rule with rule class configuration transition. */ public static class RuleClassTransitionRule implements RuleDefinition {