Support declared providers for aspects
Fixes #2016
--
PiperOrigin-RevId: 149102037
MOS_MIGRATED_REVID=149102037
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
index 8578e76..48fdc0c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
@@ -31,6 +31,7 @@
import com.google.devtools.build.lib.packages.AspectParameters;
import com.google.devtools.build.lib.packages.ClassObjectConstructor;
import com.google.devtools.build.lib.packages.SkylarkClassObject;
+import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.util.Preconditions;
import java.util.Arrays;
@@ -110,6 +111,8 @@
private final Map<String, NestedSetBuilder<Artifact>> outputGroupBuilders = new TreeMap<>();
private final ImmutableMap.Builder<String, Object> skylarkProviderBuilder =
ImmutableMap.builder();
+ private final ImmutableMap.Builder<SkylarkClassObjectConstructor.Key, SkylarkClassObject>
+ skylarkDeclaredProvidersBuilder = ImmutableMap.builder();
private final RuleContext ruleContext;
private final AspectDescriptor descriptor;
@@ -195,6 +198,19 @@
return this;
}
+ public Builder addSkylarkDeclaredProvider(SkylarkClassObject declaredProvider, Location loc)
+ throws EvalException {
+ ClassObjectConstructor constructor = declaredProvider.getConstructor();
+ SkylarkProviderValidationUtil.validateAndThrowEvalException(
+ constructor.getPrintableName(), declaredProvider, loc);
+ if (!constructor.isExported()) {
+ throw new EvalException(
+ constructor.getLocation(), "All providers must be top level values");
+ }
+ skylarkDeclaredProvidersBuilder.put(constructor.getKey(), declaredProvider);
+ return this;
+ }
+
public ConfiguredAspect build() {
if (!outputGroupBuilders.isEmpty()) {
ImmutableMap.Builder<String, NestedSet<Artifact>> outputGroups = ImmutableMap.builder();
@@ -210,10 +226,10 @@
}
ImmutableMap<String, Object> skylarkProvidersMap = skylarkProviderBuilder.build();
- if (!skylarkProvidersMap.isEmpty()) {
- providers.add(
- new SkylarkProviders(skylarkProvidersMap,
- ImmutableMap.<ClassObjectConstructor.Key, SkylarkClassObject>of()));
+ ImmutableMap<SkylarkClassObjectConstructor.Key, SkylarkClassObject>
+ skylarkDeclaredProvidersMap = skylarkDeclaredProvidersBuilder.build();
+ if (!skylarkProvidersMap.isEmpty() || !skylarkDeclaredProvidersMap.isEmpty()) {
+ providers.add(new SkylarkProviders(skylarkProvidersMap, skylarkDeclaredProvidersMap));
}
addProvider(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
index be75e14..a568329 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
@@ -60,6 +60,11 @@
return skylarkProviders.keySet();
}
+ /** Returns the keys for the declared providers. */
+ public ImmutableCollection<ClassObjectConstructor.Key> getDeclaredProviderKeys() {
+ return declaredProviders.keySet();
+ }
+
/**
* Returns a Skylark provider; "key" must be one from {@link #getKeys()}.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
index 3bf2cdc..809265e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
@@ -94,8 +94,7 @@
&& !(target instanceof Iterable)) {
ruleContext.ruleError(
String.format(
- "Rule should return a return a struct or a list, but got %s",
- SkylarkType.typeOf(target)));
+ "Rule should return a struct or a list, but got %s", SkylarkType.typeOf(target)));
return null;
} else if (!expectFailure.isEmpty()) {
ruleContext.ruleError("Expected failure not found: " + expectFailure);
@@ -246,9 +245,13 @@
} else if (target instanceof Iterable) {
loc = ruleContext.getRule().getRuleClassObject().getConfiguredTargetFunction().getLocation();
for (Object o : (Iterable) target) {
- SkylarkClassObject declaredProvider = SkylarkType.cast(o, SkylarkClassObject.class, loc,
- "A return value of rule implementation function should be "
- + "a sequence of declared providers");
+ SkylarkClassObject declaredProvider =
+ SkylarkType.cast(
+ o,
+ SkylarkClassObject.class,
+ loc,
+ "A return value of a rule implementation function should be "
+ + "a sequence of declared providers");
if (declaredProvider.getConstructor().getKey().equals(
SkylarkRuleContext.getDefaultProvider().getKey())) {
parseProviderKeys(declaredProvider, true, ruleContext, loc, executable,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
index e80a788..bd0900b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
@@ -31,6 +31,7 @@
import com.google.devtools.build.lib.syntax.Environment;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalExceptionWithStackTrace;
+import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.Mutability;
import com.google.devtools.build.lib.syntax.SkylarkType;
import java.util.Map;
@@ -65,7 +66,7 @@
.setGlobals(skylarkAspect.getFuncallEnv().getGlobals())
.setEventHandler(ruleContext.getAnalysisEnvironment().getEventHandler())
.build(); // NB: loading phase functions are not available: this is analysis already,
- // so we do *not* setLoadingPhase().
+ // so we do *not* setLoadingPhase().
Object aspectSkylarkObject;
try {
aspectSkylarkObject =
@@ -79,26 +80,15 @@
if (ruleContext.hasErrors()) {
return null;
- } else if (!(aspectSkylarkObject instanceof SkylarkClassObject)) {
- ruleContext.ruleError("Aspect implementation doesn't return a struct");
+ } else if (!(aspectSkylarkObject instanceof SkylarkClassObject)
+ && !(aspectSkylarkObject instanceof Iterable)) {
+ ruleContext.ruleError(
+ String.format(
+ "Aspect implementation should return a struct or a list, but got %s",
+ SkylarkType.typeOf(aspectSkylarkObject)));
return null;
}
-
- ConfiguredAspect.Builder builder = new ConfiguredAspect.Builder(
- aspectDescriptor, ruleContext);
-
- SkylarkClassObject struct = (SkylarkClassObject) aspectSkylarkObject;
- Location loc = struct.getCreationLoc();
- for (String key : struct.getKeys()) {
- if (key.equals("output_groups")) {
- addOutputGroups(struct.getValue(key), loc, builder);
- } else {
- builder.addSkylarkTransitiveInfo(key, struct.getValue(key), loc);
- }
- }
- ConfiguredAspect configuredAspect = builder.build();
- SkylarkProviderValidationUtil.checkOrphanArtifacts(ruleContext);
- return configuredAspect;
+ return createAspect(aspectSkylarkObject, aspectDescriptor, ruleContext);
} catch (EvalException e) {
addAspectToStackTrace(base, e);
ruleContext.ruleError("\n" + e.print());
@@ -107,6 +97,58 @@
}
}
+ private ConfiguredAspect createAspect(
+ Object aspectSkylarkObject, AspectDescriptor aspectDescriptor, RuleContext ruleContext)
+ throws EvalException {
+
+ ConfiguredAspect.Builder builder = new ConfiguredAspect.Builder(aspectDescriptor, ruleContext);
+
+ if (aspectSkylarkObject instanceof Iterable) {
+ addDeclaredProviders(builder, (Iterable) aspectSkylarkObject);
+ } else {
+ SkylarkClassObject struct = (SkylarkClassObject) aspectSkylarkObject;
+ Location loc = struct.getCreationLoc();
+ for (String key : struct.getKeys()) {
+ if (key.equals("output_groups")) {
+ addOutputGroups(struct.getValue(key), loc, builder);
+ } else if (key.equals("providers")) {
+ Object value = struct.getValue(key);
+ Iterable providers =
+ SkylarkType.cast(
+ value,
+ Iterable.class,
+ loc,
+ "The value for \"providers\" should be a list of declared providers, "
+ + "got %s instead",
+ EvalUtils.getDataTypeName(value, false));
+ addDeclaredProviders(builder, providers);
+ } else {
+ builder.addSkylarkTransitiveInfo(key, struct.getValue(key), loc);
+ }
+ }
+ }
+
+ ConfiguredAspect configuredAspect = builder.build();
+ SkylarkProviderValidationUtil.checkOrphanArtifacts(ruleContext);
+ return configuredAspect;
+ }
+
+ private void addDeclaredProviders(ConfiguredAspect.Builder builder, Iterable aspectSkylarkObject)
+ throws EvalException {
+ for (Object o : aspectSkylarkObject) {
+ Location loc = skylarkAspect.getImplementation().getLocation();
+ SkylarkClassObject declaredProvider =
+ SkylarkType.cast(
+ o,
+ SkylarkClassObject.class,
+ loc,
+ "A return value of an aspect implementation function should be "
+ + "a sequence of declared providers");
+ Location creationLoc = declaredProvider.getCreationLocOrNull();
+ builder.addSkylarkDeclaredProvider(declaredProvider, creationLoc != null ? creationLoc : loc);
+ }
+ }
+
private static void addOutputGroups(Object value, Location loc,
ConfiguredAspect.Builder builder)
throws EvalException {
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java
index 9088a52..b5e951b 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java
@@ -34,6 +34,8 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.packages.AspectDefinition;
import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
+import com.google.devtools.build.lib.packages.ClassObjectConstructor.Key;
+import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor.SkylarkKey;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
import com.google.devtools.build.lib.rules.java.Jvm;
import com.google.devtools.build.lib.skyframe.AspectValue;
@@ -41,6 +43,7 @@
import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import java.util.Arrays;
+import java.util.List;
import javax.annotation.Nullable;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -74,6 +77,54 @@
.containsExactly("//test:aspect.bzl%MyAspect(//test:xxx)");
}
+ @Test
+ public void aspectWithDeclaredProviders() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ "foo = provider()",
+ "bar = provider()",
+ "def _impl(target, ctx):",
+ " return [foo(), bar()]",
+ "MyAspect = aspect(implementation=_impl)");
+ scratch.file("test/BUILD", "java_library(name = 'xxx',)");
+
+ AnalysisResult analysisResult =
+ update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:xxx");
+ assertThat(getLabelsToBuild(analysisResult)).containsExactly("//test:xxx");
+ assertThat(getAspectDescriptions(analysisResult))
+ .containsExactly("//test:aspect.bzl%MyAspect(//test:xxx)");
+
+ List<Key> providers = getDeclaredProviderKeys(analysisResult);
+ assertThat((providers.get(0)))
+ .isEqualTo(new SkylarkKey(Label.parseAbsolute("//test:aspect.bzl"), "foo"));
+ assertThat((providers.get(1)))
+ .isEqualTo(new SkylarkKey(Label.parseAbsolute("//test:aspect.bzl"), "bar"));
+ }
+
+ @Test
+ public void aspectWithDeclaredProvidersInAStruct() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ "foo = provider()",
+ "bar = provider()",
+ "def _impl(target, ctx):",
+ " return struct(foobar='foobar', providers=[foo(), bar()])",
+ "MyAspect = aspect(implementation=_impl)");
+ scratch.file("test/BUILD", "java_library(name = 'xxx',)");
+
+ AnalysisResult analysisResult =
+ update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:xxx");
+ assertThat(getLabelsToBuild(analysisResult)).containsExactly("//test:xxx");
+ assertThat(getAspectDescriptions(analysisResult))
+ .containsExactly("//test:aspect.bzl%MyAspect(//test:xxx)");
+
+ List<Key> providers = getDeclaredProviderKeys(analysisResult);
+ assertThat((providers.get(0)))
+ .isEqualTo(new SkylarkKey(Label.parseAbsolute("//test:aspect.bzl"), "foo"));
+ assertThat((providers.get(1)))
+ .isEqualTo(new SkylarkKey(Label.parseAbsolute("//test:aspect.bzl"), "bar"));
+ }
+
private Iterable<String> getAspectDescriptions(AnalysisResult analysisResult) {
return transform(
analysisResult.getAspects(),
@@ -89,6 +140,25 @@
});
}
+ private List<Key> getDeclaredProviderKeys(AnalysisResult analysisResult) {
+ return transform(
+ analysisResult.getAspects(),
+ new Function<AspectValue, List<Key>>() {
+ @Nullable
+ @Override
+ public List<Key> apply(AspectValue aspectValue) {
+ return aspectValue
+ .getConfiguredAspect()
+ .getProviders()
+ .getProvider(SkylarkProviders.class)
+ .getDeclaredProviderKeys()
+ .asList();
+ }
+ })
+ .iterator()
+ .next(); // Assume there's only one aspect
+ }
+
@Test
public void aspectCommandLineLabel() throws Exception {
scratch.file(
@@ -585,7 +655,7 @@
} catch (ViewCreationFailedException e) {
// expect to fail.
}
- assertContainsEvent("Aspect implementation doesn't return a struct");
+ assertContainsEvent("Aspect implementation should return a struct or a list, but got int");
}
@Test
