Don't crash aspect attributes with select().
Aspect attributes take on the value of their corresponding
rule's attributes: https://docs.bazel.build/versions/master/skylark/aspects.html#aspect-definition-1
Aspect attributes don't yet support select(). Make this a proper
error instead of crashing Blaze.
Fixes #5120
PiperOrigin-RevId: 237855536
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 2d02e44..98f0a76 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -2382,13 +2382,25 @@
PredicateWithMessage<Object> allowedValues = attrOfAspect.getAllowedValues();
Object value = attrOfAspect.getDefaultValue(rule);
if (!allowedValues.apply(value)) {
- rule.reportError(
- String.format(
- "%s: invalid value in '%s' attribute: %s",
- rule.getLabel(),
- attrOfAspect.getName(),
- allowedValues.getErrorReason(value)),
- eventHandler);
+ if (RawAttributeMapper.of(rule).isConfigurable(attrOfAspect.getName())) {
+ rule.reportError(
+ String.format(
+ "%s: attribute '%s' has a select() and aspect %s also declares "
+ + "'%s'. Aspect attributes don't currently support select().",
+ rule.getLabel(),
+ attrOfAspect.getName(),
+ aspect.getDefinition().getName(),
+ rule.getLabel()),
+ eventHandler);
+ } else {
+ rule.reportError(
+ String.format(
+ "%s: invalid value in '%s' attribute: %s",
+ rule.getLabel(),
+ attrOfAspect.getName(),
+ allowedValues.getErrorReason(value)),
+ eventHandler);
+ }
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkDefinedAspect.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkDefinedAspect.java
index e8cd11a..e1e1d55 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkDefinedAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkDefinedAspect.java
@@ -204,7 +204,11 @@
getName(), rule.getTargetKind(), param));
}
if (ruleAttr != null && ruleAttr.getType() == aspectAttr.getType()) {
- builder.addAttribute(param, (String) ruleAttrs.get(param, ruleAttr.getType()));
+ // If the attribute has a select() (which aspect attributes don't yet support), the
+ // error gets reported in RuleClass.checkAspectAllowedValues.
+ if (!ruleAttrs.isConfigurable(param)) {
+ builder.addAttribute(param, (String) ruleAttrs.get(param, ruleAttr.getType()));
+ }
}
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
index ae5c76d..0b16cae 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
@@ -1284,6 +1284,42 @@
}
@Test
+ public void aspectParametersDontSupportSelect() throws Exception {
+ scratch.file(
+ "test/aspect.bzl",
+ "def _impl(target, ctx):",
+ " return struct()",
+ "def _rule_impl(ctx):",
+ " return struct()",
+ "MyAspectMismatch = aspect(",
+ " implementation=_impl,",
+ " attrs = { 'my_attr' : attr.string(values=['aaa']) },",
+ ")",
+ "my_rule = rule(",
+ " implementation=_rule_impl,",
+ " attrs = { 'deps' : attr.label_list(aspects=[MyAspectMismatch]),",
+ " 'my_attr' : attr.string() },",
+ ")");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:aspect.bzl', 'my_rule')",
+ "my_rule(name = 'xxx', my_attr = select({'//conditions:default': 'foo'}))");
+
+ reporter.removeHandler(failFastHandler);
+ try {
+ AnalysisResult result = update(ImmutableList.<String>of(), "//test:xxx");
+ assertThat(keepGoing()).isTrue();
+ assertThat(result.hasError()).isTrue();
+ } catch (Exception e) {
+ // expect to fail.
+ }
+ assertContainsEvent(
+ "//test:xxx: attribute 'my_attr' has a select() and aspect "
+ + "//test:aspect.bzl%MyAspectMismatch also declares '//test:xxx'. Aspect attributes "
+ + "don't currently support select().");
+ }
+
+ @Test
public void aspectParametersBadDefault() throws Exception {
scratch.file(
"test/aspect.bzl",
