Implement Skylark aspects originating from Skylark rules.
--
MOS_MIGRATED_REVID=108777120
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 c2b3eea..9898294 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
@@ -725,11 +725,36 @@
*/
public <T extends NativeAspectFactory> Builder<TYPE> aspect(
Class<T> aspect, Function<Rule, AspectParameters> evaluator) {
- this.aspects.add(new RuleAspect(new NativeAspectClass<T>(aspect), evaluator));
+ return this.aspect(new NativeAspectClass<T>(aspect), evaluator);
+ }
+
+ /**
+ * Asserts that a particular parameterized aspect probably needs to be computed for all direct
+ * dependencies through this attribute.
+ *
+ * @param evaluator function that extracts aspect parameters from rule.
+ */
+ public Builder<TYPE> aspect(AspectClass aspect, Function<Rule, AspectParameters> evaluator) {
+ this.aspects.add(new RuleAspect(aspect, evaluator));
return this;
}
/**
+ * Asserts that a particular parameterized aspect probably needs to be computed for all direct
+ * dependencies through this attribute.
+ */
+ public Builder<TYPE> aspect(AspectClass aspect) {
+ Function<Rule, AspectParameters> noParameters =
+ new Function<Rule, AspectParameters>() {
+ @Override
+ public AspectParameters apply(Rule input) {
+ return AspectParameters.EMPTY;
+ }
+ };
+ return this.aspect(aspect, noParameters);
+ }
+
+ /**
* Sets the predicate-like edge validity checker.
*/
public Builder<TYPE> validityPredicate(ValidityPredicate validityPredicate) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
index f445eb66..58b9386 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.rules;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Attribute;
@@ -21,6 +23,7 @@
import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
import com.google.devtools.build.lib.packages.Attribute.SkylarkLateBound;
import com.google.devtools.build.lib.packages.BuildType;
+import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspect;
import com.google.devtools.build.lib.syntax.BuiltinFunction;
import com.google.devtools.build.lib.syntax.Environment;
import com.google.devtools.build.lib.syntax.EvalException;
@@ -68,6 +71,10 @@
"which rule targets (name of the classes) are allowed. This is deprecated (kept only for "
+ "compatiblity), use providers instead.";
+ private static final String ASPECTS_ARG = "aspects";
+ private static final String ASPECT_ARG_DOC =
+ "aspects that should be applied to dependencies specified by this attribute";
+
private static final String CONFIGURATION_ARG = "cfg";
private static final String CONFIGURATION_DOC =
"configuration of the attribute. " + "For example, use DATA_CFG or HOST_CFG.";
@@ -184,7 +191,7 @@
return builder;
}
- private static Descriptor createAttribute(
+ private static Descriptor createAttrDescriptor(
Map<String, Object> kwargs, Type<?> type, FuncallExpression ast, Environment env)
throws EvalException {
try {
@@ -230,7 +237,7 @@
throws EvalException {
// TODO(bazel-team): Replace literal strings with constants.
env.checkLoadingPhase("attr.int", ast.getLocation());
- return createAttribute(
+ return createAttrDescriptor(
EvalUtils.optionMap(
DEFAULT_ARG, defaultInt, MANDATORY_ARG, mandatory, VALUES_ARG, values),
Type.INTEGER,
@@ -274,7 +281,7 @@
Environment env)
throws EvalException {
env.checkLoadingPhase("attr.string", ast.getLocation());
- return createAttribute(
+ return createAttrDescriptor(
EvalUtils.optionMap(
DEFAULT_ARG, defaultString, MANDATORY_ARG, mandatory, VALUES_ARG, values),
Type.STRING,
@@ -362,7 +369,7 @@
Environment env)
throws EvalException {
env.checkLoadingPhase("attr.label", ast.getLocation());
- return createAttribute(
+ return createAttrDescriptor(
EvalUtils.optionMap(
DEFAULT_ARG,
defaultO,
@@ -417,7 +424,7 @@
Environment env)
throws EvalException {
env.checkLoadingPhase("attr.string_list", ast.getLocation());
- return createAttribute(
+ return createAttrDescriptor(
EvalUtils.optionMap(
DEFAULT_ARG, defaultList, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
Type.STRING_LIST,
@@ -457,7 +464,7 @@
Environment env)
throws EvalException {
env.checkLoadingPhase("attr.int_list", ast.getLocation());
- return createAttribute(
+ return createAttrDescriptor(
EvalUtils.optionMap(
DEFAULT_ARG, defaultList, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
Type.INTEGER_LIST,
@@ -522,6 +529,13 @@
noneable = true,
defaultValue = "None",
doc = CONFIGURATION_DOC
+ ),
+ @Param(
+ name = ASPECTS_ARG,
+ type = SkylarkList.class,
+ generic1 = SkylarkAspect.class,
+ defaultValue = "[]",
+ doc = ASPECT_ARG_DOC
)
},
useAst = true,
@@ -538,31 +552,48 @@
Boolean mandatory,
Boolean nonEmpty,
Object cfg,
+ SkylarkList aspects,
FuncallExpression ast,
Environment env)
throws EvalException {
env.checkLoadingPhase("attr.label_list", ast.getLocation());
- return createAttribute(
- EvalUtils.optionMap(
- DEFAULT_ARG,
- defaultList,
- ALLOW_FILES_ARG,
- allowFiles,
- ALLOW_RULES_ARG,
- allowRules,
- PROVIDERS_ARG,
- providers,
- FLAGS_ARG,
- flags,
- MANDATORY_ARG,
- mandatory,
- NON_EMPTY_ARG,
- nonEmpty,
- CONFIGURATION_ARG,
- cfg),
- BuildType.LABEL_LIST,
- ast,
- env);
+ ImmutableMap<String, Object> kwargs = EvalUtils.optionMap(
+ DEFAULT_ARG, defaultList,
+ ALLOW_FILES_ARG,
+ allowFiles,
+ ALLOW_RULES_ARG,
+ allowRules,
+ PROVIDERS_ARG,
+ providers,
+ FLAGS_ARG,
+ flags,
+ MANDATORY_ARG,
+ mandatory,
+ NON_EMPTY_ARG,
+ nonEmpty,
+ CONFIGURATION_ARG,
+ cfg);
+ try {
+ Attribute.Builder<?> attribute = createAttribute(
+ BuildType.LABEL_LIST, kwargs, ast, env);
+
+ ImmutableList<SkylarkAspect> skylarkAspects = getSkylarkAspects(aspects);
+ return new Descriptor(attribute, skylarkAspects);
+ } catch (ConversionException e) {
+ throw new EvalException(ast.getLocation(), e.getMessage());
+ }
+ }
+
+ protected ImmutableList<SkylarkAspect> getSkylarkAspects(SkylarkList aspects)
+ throws ConversionException {
+ ImmutableList.Builder<SkylarkAspect> aspectBuilder = ImmutableList.builder();
+ for (Object aspect : aspects) {
+ if (!(aspect instanceof SkylarkAspect)) {
+ throw new ConversionException("Expected a list of aspects for 'aspects'");
+ }
+ aspectBuilder.add((SkylarkAspect) aspect);
+ }
+ return aspectBuilder.build();
}
};
@@ -585,7 +616,7 @@
Boolean defaultO, Boolean mandatory, FuncallExpression ast, Environment env)
throws EvalException {
env.checkLoadingPhase("attr.bool", ast.getLocation());
- return createAttribute(
+ return createAttrDescriptor(
EvalUtils.optionMap(DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
Type.BOOLEAN,
ast,
@@ -621,7 +652,7 @@
Object defaultO, Boolean mandatory, FuncallExpression ast, Environment env)
throws EvalException {
env.checkLoadingPhase("attr.output", ast.getLocation());
- return createAttribute(
+ return createAttrDescriptor(
EvalUtils.optionMap(DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
BuildType.OUTPUT,
ast,
@@ -662,7 +693,7 @@
Environment env)
throws EvalException {
env.checkLoadingPhase("attr.output_list", ast.getLocation());
- return createAttribute(
+ return createAttrDescriptor(
EvalUtils.optionMap(
DEFAULT_ARG, defaultList, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
BuildType.OUTPUT_LIST,
@@ -698,7 +729,7 @@
Environment env)
throws EvalException {
env.checkLoadingPhase("attr.string_dict", ast.getLocation());
- return createAttribute(
+ return createAttrDescriptor(
EvalUtils.optionMap(
DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
Type.STRING_DICT,
@@ -734,7 +765,7 @@
Environment env)
throws EvalException {
env.checkLoadingPhase("attr.string_list_dict", ast.getLocation());
- return createAttribute(
+ return createAttrDescriptor(
EvalUtils.optionMap(
DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
Type.STRING_LIST_DICT,
@@ -764,7 +795,7 @@
Object defaultO, Boolean mandatory, FuncallExpression ast, Environment env)
throws EvalException {
env.checkLoadingPhase("attr.license", ast.getLocation());
- return createAttribute(
+ return createAttrDescriptor(
EvalUtils.optionMap(DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
BuildType.LICENSE,
ast,
@@ -777,14 +808,24 @@
*/
public static final class Descriptor {
private final Attribute.Builder<?> attributeBuilder;
+ private final ImmutableList<SkylarkAspect> aspects;
public Descriptor(Attribute.Builder<?> attributeBuilder) {
+ this(attributeBuilder, ImmutableList.<SkylarkAspect>of());
+ }
+
+ public Descriptor(Attribute.Builder<?> attributeBuilder, ImmutableList<SkylarkAspect> aspects) {
this.attributeBuilder = attributeBuilder;
+ this.aspects = aspects;
}
public Attribute.Builder<?> getAttributeBuilder() {
return attributeBuilder;
}
+
+ public ImmutableList<SkylarkAspect> getAspects() {
+ return aspects;
+ }
}
static {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
index 63b6307..df27c88 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
@@ -89,6 +89,7 @@
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
+import java.util.Set;
import java.util.concurrent.ExecutionException;
/**
@@ -423,8 +424,17 @@
+ "', test rule class names must end with '_test' and other rule classes must not");
}
for (Pair<String, SkylarkAttr.Descriptor> attribute : attributes) {
+ SkylarkAttr.Descriptor descriptor = attribute.getSecond();
+ Attribute.Builder<?> attributeBuilder = descriptor.getAttributeBuilder();
+ for (SkylarkAspect skylarkAspect : descriptor.getAspects()) {
+ if (!skylarkAspect.isExported()) {
+ throw new EvalException(definitionLocation,
+ "All aspects applied to rule dependencies must be top-level values");
+ }
+ attributeBuilder.aspect(new SkylarkAspectClass(skylarkAspect));
+ }
addAttribute(definitionLocation, builder,
- attribute.getSecond().getAttributeBuilder().build(attribute.getFirst()));
+ descriptor.getAttributeBuilder().build(attribute.getFirst()));
}
this.ruleClass = builder.build(ruleClassName);
@@ -441,7 +451,25 @@
public static void exportRuleFunctionsAndAspects(Environment env, Label skylarkLabel)
throws EvalException {
- for (String name : env.getGlobals().getDirectVariableNames()) {
+ Set<String> globalNames = env.getGlobals().getDirectVariableNames();
+
+ // Export aspects first since rules can depend on aspects.
+ for (String name : globalNames) {
+ Object value;
+ try {
+ value = env.lookup(name);
+ } catch (NoSuchVariableException e) {
+ throw new AssertionError(e);
+ }
+ if (value instanceof SkylarkAspect) {
+ SkylarkAspect skylarkAspect = (SkylarkAspect) value;
+ if (!skylarkAspect.isExported()) {
+ skylarkAspect.export(skylarkLabel, name);
+ }
+ }
+ }
+
+ for (String name : globalNames) {
try {
Object value = env.lookup(name);
if (value instanceof RuleFunction) {
@@ -450,12 +478,6 @@
function.export(skylarkLabel, name);
}
}
- if (value instanceof SkylarkAspect) {
- SkylarkAspect skylarkAspect = (SkylarkAspect) value;
- if (!skylarkAspect.isExported()) {
- skylarkAspect.export(skylarkLabel, name);
- }
- }
} catch (NoSuchVariableException e) {
throw new AssertionError(e);
}
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 b12ccd0..b3b6bc9 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
@@ -141,6 +141,80 @@
.containsExactly("//test:xxx", "//test:yyy");
}
+
+ public void testAspectsFromSkylarkRules() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ "def _impl(target, ctx):",
+ " s = set([target.label])",
+ " for i in ctx.attr.deps:",
+ " s += i.target_labels",
+ " return struct(target_labels = s)",
+ "def _rule_impl(ctx):",
+ " s = set([])",
+ " for i in ctx.attr.attr:",
+ " s += i.target_labels",
+ " return struct(rule_deps = s)",
+ "",
+ "MyAspect = aspect(",
+ " implementation=_impl,",
+ " attr_aspects=['deps'],",
+ ")",
+ "my_rule = rule(",
+ " implementation=_rule_impl,",
+ " attrs = { 'attr' : ",
+ " attr.label_list(mandatory=True, allow_files=True, aspects = [MyAspect]) },",
+ ")");
+
+ scratch.file(
+ "test/BUILD",
+ "load('/test/aspect', 'my_rule')",
+ "java_library(",
+ " name = 'yyy',",
+ ")",
+ "my_rule(",
+ " name = 'xxx',",
+ " attr = [':yyy'],",
+ ")");
+
+ AnalysisResult analysisResult =
+ update(
+ ImmutableList.of("//test:xxx"),
+ ImmutableList.<String>of(),
+ false,
+ LOADING_PHASE_THREADS,
+ true,
+ new EventBus());
+ assertThat(
+ transform(
+ analysisResult.getTargetsToBuild(),
+ new Function<ConfiguredTarget, String>() {
+ @Nullable
+ @Override
+ public String apply(ConfiguredTarget configuredTarget) {
+ return configuredTarget.getLabel().toString();
+ }
+ }))
+ .containsExactly("//test:xxx");
+ ConfiguredTarget target = analysisResult.getTargetsToBuild().iterator().next();
+ SkylarkProviders skylarkProviders = target.getProvider(SkylarkProviders.class);
+ assertThat(skylarkProviders).isNotNull();
+ Object names = skylarkProviders.getValue("rule_deps");
+ assertThat(names).isInstanceOf(SkylarkNestedSet.class);
+ assertThat(
+ transform(
+ (SkylarkNestedSet) names,
+ new Function<Object, String>() {
+ @Nullable
+ @Override
+ public String apply(Object o) {
+ assertThat(o).isInstanceOf(Label.class);
+ return o.toString();
+ }
+ }))
+ .containsExactly("//test:yyy");
+ }
+
public void testAspectFailingExecution() throws Exception {
scratch.file(
"test/aspect.bzl",
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index bca93ac..83ea26a 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -178,6 +178,30 @@
}
@Test
+ public void testLabelListWithAspects() throws Exception {
+ SkylarkAttr.Descriptor attr =
+ (SkylarkAttr.Descriptor) evalRuleClassCode(
+ "def _impl(target, ctx):",
+ " pass",
+ "my_aspect = aspect(implementation = _impl)",
+ "attr.label_list(aspects = [my_aspect])");
+ Object aspect = ev.lookup("my_aspect");
+ assertThat(aspect).isNotNull();
+ assertThat(attr.getAspects()).containsExactly(aspect);
+ }
+
+ @Test
+ public void testLabelListWithAspectsError() throws Exception {
+ checkErrorContains(
+ "Expected a list of aspects for 'aspects'",
+ "def _impl(target, ctx):",
+ " pass",
+ "my_aspect = aspect(implementation = _impl)",
+ "attr.label_list(aspects = [my_aspect, 123])"
+ );
+ }
+
+ @Test
public void testNonLabelAttrWithProviders() throws Exception {
checkErrorContains(
"unexpected keyword 'providers' in call to string", "attr.string(providers = ['a'])");
