Be more permissive when dealing with rules which have
ExecutionPlatformConstraintsAllowed set to PER_TARGET and also have an
alread-existing exec_compatible_with attribute.
PiperOrigin-RevId: 201238805
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 75fd947..fba4c92 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
@@ -703,13 +703,6 @@
"Attribute %s is inherited multiple times in %s ruleclass",
attrName,
name);
- if (attrName.equals("exec_compatible_with")
- && parent.executionPlatformConstraintsAllowed
- == ExecutionPlatformConstraintsAllowed.PER_TARGET) {
- // This attribute should not be inherited because executionPlatformConstraintsAllowed is
- // not inherited.
- continue;
- }
attributes.put(attrName, attribute);
}
@@ -762,13 +755,8 @@
if (type == RuleClassType.PLACEHOLDER) {
Preconditions.checkNotNull(ruleDefinitionEnvironmentHashCode, this.name);
}
- if (executionPlatformConstraintsAllowed == ExecutionPlatformConstraintsAllowed.PER_TARGET) {
- // Only rules that allow per target execution constraints need this attribute.
- Preconditions.checkState(
- !this.attributes.containsKey("exec_compatible_with"),
- "Rule %s should not already define the attribute \"exec_compatible_with\""
- + " because executionPlatformConstraintsAllowed is set to PER_TARGET",
- key);
+ if (executionPlatformConstraintsAllowed == ExecutionPlatformConstraintsAllowed.PER_TARGET
+ && !this.contains("exec_compatible_with")) {
this.add(
attr("exec_compatible_with", BuildType.LABEL_LIST)
.allowedFileTypes()
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
index bc9d068..45d332c 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
@@ -25,7 +25,6 @@
import static com.google.devtools.build.lib.syntax.Type.INTEGER;
import static com.google.devtools.build.lib.syntax.Type.STRING;
import static com.google.devtools.build.lib.syntax.Type.STRING_LIST;
-import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
import static org.junit.Assert.fail;
import com.google.common.base.Function;
@@ -1191,7 +1190,6 @@
assertThat(childRuleClass.executionPlatformConstraintsAllowed())
.isEqualTo(ExecutionPlatformConstraintsAllowed.PER_RULE);
- assertThat(childRuleClass.hasAttr("exec_compatible_with", LABEL_LIST)).isFalse();
}
@Test
@@ -1213,19 +1211,4 @@
.isEqualTo(ExecutionPlatformConstraintsAllowed.PER_TARGET);
assertThat(childRuleClass.hasAttr("exec_compatible_with", LABEL_LIST)).isTrue();
}
-
- @Test
- public void testExecutionPlatformConstraints_extraExecCompatibleWithAttribute() {
- RuleClass.Builder ruleClassBuilder =
- new RuleClass.Builder("ruleClass", RuleClassType.NORMAL, false)
- .factory(DUMMY_CONFIGURED_TARGET_FACTORY)
- .add(attr("tags", STRING_LIST));
-
- ruleClassBuilder.add(
- attr("exec_compatible_with", LABEL_LIST).allowedFileTypes().value(ImmutableList.of()));
- ruleClassBuilder.executionPlatformConstraintsAllowed(
- ExecutionPlatformConstraintsAllowed.PER_TARGET);
-
- assertThrows(IllegalStateException.class, () -> ruleClassBuilder.build());
- }
}
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 5a68334..23d9142 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
@@ -1655,24 +1655,6 @@
}
@Test
- public void testTargetsCanAddExecutionPlatformConstraints_attrAlreadyDefined() throws Exception {
- registerDummyUserDefinedFunction();
- scratch.file("test/BUILD", "toolchain_type(name = 'my_toolchain_type')");
- ev.setFailFast(false);
- evalAndExport(
- "r1 = rule(impl, ",
- " attrs = {",
- " 'exec_compatible_with': attr.label_list(),",
- " },",
- " toolchains=['//test:my_toolchain_type'],",
- " execution_platform_constraints_allowed=True,",
- ")");
- ev.assertContainsError(
- "Rule //fake/label.bzl:label.bzl%r1 should not already"
- + " define the attribute \"exec_compatible_with\"");
- }
-
- @Test
public void testRuleFunctionReturnsNone() throws Exception {
scratch.file("test/rule.bzl",
"def _impl(ctx):",
