From c88a9bf674d9d1564c4259dc604ba77ff718973a Mon Sep 17 00:00:00 2001 From: John Cater Date: Wed, 24 Apr 2019 12:39:06 -0400 Subject: [PATCH] Allow all targets to specify exec_compatible_with to add constraints on the execution platform. Remove the ExecutionPlatformConstraintsAllowed enum entirely. Part of work on #8026. --- .../build/lib/analysis/BaseRuleClasses.java | 2 - .../skylark/SkylarkRuleClassFunctions.java | 6 +- .../bazel/rules/sh/BazelShRuleClasses.java | 2 - .../build/lib/packages/RuleClass.java | 69 +--------- .../lib/rules/genrule/GenRuleBaseRule.java | 2 - .../skyframe/ConfiguredTargetFunction.java | 4 +- .../SkylarkRuleFunctionsApi.java | 1 + .../build/lib/packages/RuleClassTest.java | 130 ++++-------------- .../SkylarkRuleClassFunctionsTest.java | 4 +- 9 files changed, 33 insertions(+), 187 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java index d866edcdb91985..e675a621518830 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java @@ -41,7 +41,6 @@ import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; -import com.google.devtools.build.lib.packages.RuleClass.ExecutionPlatformConstraintsAllowed; import com.google.devtools.build.lib.packages.TestSize; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.syntax.Type; @@ -208,7 +207,6 @@ public Object getDefault(AttributeMap rule) { env.getToolsLabel(DEFAULT_COVERAGE_REPORT_GENERATOR_VALUE)))) // The target itself and run_under both run on the same machine. .add(attr(":run_under", LABEL).value(RUN_UNDER).skipPrereqValidatorCheck()) - .executionPlatformConstraintsAllowed(ExecutionPlatformConstraintsAllowed.PER_TARGET) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java index 1c23c1e2ecb24e..c5027ae40259b6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java @@ -59,7 +59,6 @@ import com.google.devtools.build.lib.packages.Provider; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; -import com.google.devtools.build.lib.packages.RuleClass.ExecutionPlatformConstraintsAllowed; import com.google.devtools.build.lib.packages.RuleFactory; import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap; import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException; @@ -220,7 +219,6 @@ public static final RuleClass getTestBaseRule(String toolsRepository) { toolsRepository + BaseRuleClasses.DEFAULT_COVERAGE_REPORT_GENERATOR_VALUE)))) .add(attr(":run_under", LABEL).value(RUN_UNDER)) - .executionPlatformConstraintsAllowed(ExecutionPlatformConstraintsAllowed.PER_TARGET) .build(); } @@ -278,6 +276,7 @@ public BaseFunction rule( SkylarkList toolchains, String doc, SkylarkList providesArg, + // TODO(https://github.com/bazelbuild/bazel/issues/8026): Remove this. Boolean executionPlatformConstraintsAllowed, SkylarkList execCompatibleWith, Object analysisTest, @@ -398,9 +397,6 @@ public BaseFunction rule( execCompatibleWith.getContents(String.class, "exec_compatile_with"), ast.getLocation())); } - if (executionPlatformConstraintsAllowed) { - builder.executionPlatformConstraintsAllowed(ExecutionPlatformConstraintsAllowed.PER_TARGET); - } return new SkylarkRuleFunction(builder, type, attributes, ast.getLocation()); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShRuleClasses.java index 0016b936738a10..955830d2444952 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/BazelShRuleClasses.java @@ -26,7 +26,6 @@ import com.google.devtools.build.lib.packages.PredicateWithMessage; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; -import com.google.devtools.build.lib.packages.RuleClass.ExecutionPlatformConstraintsAllowed; import com.google.devtools.build.lib.util.FileTypeSet; import javax.annotation.Nullable; @@ -72,7 +71,6 @@ All other files required at runtime (whether scripts or data) belong in the .allowedRuleClasses("sh_library") .allowedRuleClassesWithWarning(ALLOWED_RULES_IN_DEPS_WITH_WARNING) .allowedFileTypes()) - .executionPlatformConstraintsAllowed(ExecutionPlatformConstraintsAllowed.PER_TARGET) .build(); } 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 f0be11cb83e203..98ddb8066e2e26 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 @@ -213,44 +213,6 @@ TConfiguredTarget create(TContext ruleContext) public static final class RuleErrorException extends Exception {} } - /** - * Describes in which way a rule implementation allows additional execution platform constraints. - */ - public enum ExecutionPlatformConstraintsAllowed { - /** - * Allows additional execution platform constraints to be added in the rule definition, which - * apply to all targets of that rule. - */ - PER_RULE(1), - /** - * Users are allowed to specify additional execution platform constraints for each target, using - * the 'exec_compatible_with' attribute. This also allows setting constraints in the rule - * definition, like PER_RULE. - */ - PER_TARGET(2); - - private final int priority; - - ExecutionPlatformConstraintsAllowed(int priority) { - this.priority = priority; - } - - public int priority() { - return priority; - } - - public static ExecutionPlatformConstraintsAllowed highestPriority( - ExecutionPlatformConstraintsAllowed first, ExecutionPlatformConstraintsAllowed... rest) { - ExecutionPlatformConstraintsAllowed result = first; - for (ExecutionPlatformConstraintsAllowed value : rest) { - if (result == null || result.priority() < value.priority()) { - result = value; - } - } - return result; - } - } - /** * For Bazel's constraint system: the attribute that declares the set of environments a rule * supports, overriding the defaults for their respective groups. @@ -717,8 +679,6 @@ public enum ThirdPartyLicenseExistencePolicy { private final Map attributes = new LinkedHashMap<>(); private final Set