diff options
3 files changed, 2 insertions, 49 deletions
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 75fd947223..fba4c92a65 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 @@ public class RuleClass { "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 @@ public class RuleClass { 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 bc9d068414..45d332cc48 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.BOOLEAN; 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 @@ public class RuleClassTest extends PackageLoadingTestCase { assertThat(childRuleClass.executionPlatformConstraintsAllowed()) .isEqualTo(ExecutionPlatformConstraintsAllowed.PER_RULE); - assertThat(childRuleClass.hasAttr("exec_compatible_with", LABEL_LIST)).isFalse(); } @Test @@ -1213,19 +1211,4 @@ public class RuleClassTest extends PackageLoadingTestCase { .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 5a68334bba..23d9142131 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 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase { } @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):", |