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 {
