Skip to content

Commit

Permalink
Add incompatible_disallow_rule_execution_platform_constraints_allowed…
Browse files Browse the repository at this point in the history
… flag.

Part of #8136 and #8134.

RELNOTES: Adds
incompatible_disallow_rule_execution_platform_constraints_allowed, which
disallows the use of the "execution_platform_constraints_allowed"
attribute when defining new rules.

Closes #8145.

PiperOrigin-RevId: 245405049
  • Loading branch information
katre authored and copybara-github committed Apr 26, 2019
1 parent 9c87e0e commit 5688793
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ public BaseFunction rule(
FuncallExpression ast,
Environment funcallEnv,
StarlarkContext context)
throws EvalException, ConversionException {
throws EvalException {
SkylarkUtils.checkLoadingOrWorkspacePhase(funcallEnv, "rule", ast.getLocation());

BazelStarlarkContext bazelContext = (BazelStarlarkContext) context;
Expand Down Expand Up @@ -398,8 +398,11 @@ public BaseFunction rule(
execCompatibleWith.getContents(String.class, "exec_compatile_with"),
ast.getLocation()));
}

if (executionPlatformConstraintsAllowed) {
builder.executionPlatformConstraintsAllowed(ExecutionPlatformConstraintsAllowed.PER_TARGET);
} else {
builder.executionPlatformConstraintsAllowed(ExecutionPlatformConstraintsAllowed.PER_RULE);
}

return new SkylarkRuleFunction(builder, type, attributes, ast.getLocation());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,20 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
+ "Use for example `cc_library` instead of `native.cc_library`.")
public boolean incompatibleDisallowNativeInBuildFile;

@Option(
name = "incompatible_disallow_rule_execution_platform_constraints_allowed",
defaultValue = "False",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"If set to true, disallow the use of the execution_platform_constraints_allowed "
+ "attribute on rule().")
public boolean incompatibleDisallowRuleExecutionPlatformConstraintsAllowed;

@Option(
name = "incompatible_string_join_requires_strings",
defaultValue = "false",
Expand Down Expand Up @@ -623,6 +637,8 @@ public StarlarkSemantics toSkylarkSemantics() {
.incompatibleDisallowOldOctalNotation(incompatibleDisallowOldOctalNotation)
.incompatibleDisallowOldStyleArgsAdd(incompatibleDisallowOldStyleArgsAdd)
.incompatibleDisallowStructProviderSyntax(incompatibleDisallowStructProviderSyntax)
.incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(
incompatibleDisallowRuleExecutionPlatformConstraintsAllowed)
.incompatibleExpandDirectories(incompatibleExpandDirectories)
.incompatibleNewActionsApi(incompatibleNewActionsApi)
.incompatibleNoAttrLicense(incompatibleNoAttrLicense)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,12 +282,17 @@ public interface SkylarkRuleFunctionsApi<FileApiT extends FileApi> {
named = true,
positional = false,
defaultValue = "False",
disableWithFlag =
FlagIdentifier.INCOMPATIBLE_DISALLOW_RULE_EXECUTION_PLATFORM_CONSTRAINTS_ALLOWED,
valueWhenDisabled = "True",
doc =
"If true, a special attribute named <code>exec_compatible_with</code> of "
+ "label-list type is added, which must not already exist in "
+ "<code>attrs</code>. Targets may use this attribute to specify additional "
+ "constraints on the execution platform beyond those given in the "
+ "<code>exec_compatible_with</code> argument to <code>rule()</code>."),
+ "<code>exec_compatible_with</code> argument to <code>rule()</code>. "
+ "This will be deprecated and removed in the near future, and all rules will "
+ "be able to use <code>exec_compatible_with</code>."),
@Param(
name = "exec_compatible_with",
type = SkylarkList.class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ public enum FlagIdentifier {
INCOMPATIBLE_NO_TARGET_OUTPUT_GROUP(StarlarkSemantics::incompatibleNoTargetOutputGroup),
INCOMPATIBLE_NO_ATTR_LICENSE(StarlarkSemantics::incompatibleNoAttrLicense),
INCOMPATIBLE_OBJC_FRAMEWORK_CLEANUP(StarlarkSemantics::incompatibleObjcFrameworkCleanup),
INCOMPATIBLE_DISALLOW_RULE_EXECUTION_PLATFORM_CONSTRAINTS_ALLOWED(
StarlarkSemantics::incompatibleDisallowRuleExecutionPlatformConstraintsAllowed),
NONE(null);

// Using a Function here makes the enum definitions far cleaner, and, since this is
Expand Down Expand Up @@ -162,6 +164,8 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {

public abstract boolean incompatibleDisallowOldStyleArgsAdd();

public abstract boolean incompatibleDisallowRuleExecutionPlatformConstraintsAllowed();

public abstract boolean incompatibleDisallowStructProviderSyntax();

public abstract boolean incompatibleExpandDirectories();
Expand Down Expand Up @@ -234,6 +238,7 @@ public static Builder builderWithDefaults() {
.incompatibleDisallowNativeInBuildFile(false)
.incompatibleDisallowOldOctalNotation(false)
.incompatibleDisallowOldStyleArgsAdd(true)
.incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(false)
.incompatibleDisallowStructProviderSyntax(false)
.incompatibleExpandDirectories(true)
.incompatibleNewActionsApi(false)
Expand Down Expand Up @@ -304,6 +309,9 @@ public abstract static class Builder {

public abstract Builder incompatibleDisallowNativeInBuildFile(boolean value);

public abstract Builder incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(
boolean value);

public abstract Builder incompatibleDisallowStructProviderSyntax(boolean value);

public abstract Builder incompatibleExpandDirectories(boolean value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
"--incompatible_disallow_old_octal_notation=" + rand.nextBoolean(),
"--incompatible_disallow_old_style_args_add=" + rand.nextBoolean(),
"--incompatible_disallow_struct_provider_syntax=" + rand.nextBoolean(),
"--incompatible_disallow_rule_execution_platform_constraints_allowed=" + rand.nextBoolean(),
"--incompatible_expand_directories=" + rand.nextBoolean(),
"--incompatible_new_actions_api=" + rand.nextBoolean(),
"--incompatible_no_attr_license=" + rand.nextBoolean(),
Expand Down Expand Up @@ -199,6 +200,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
.incompatibleDisallowOldOctalNotation(rand.nextBoolean())
.incompatibleDisallowOldStyleArgsAdd(rand.nextBoolean())
.incompatibleDisallowStructProviderSyntax(rand.nextBoolean())
.incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(rand.nextBoolean())
.incompatibleExpandDirectories(rand.nextBoolean())
.incompatibleNewActionsApi(rand.nextBoolean())
.incompatibleNoAttrLicense(rand.nextBoolean())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ private RuleClass getRuleClass(String name) throws Exception {
}

private void registerDummyUserDefinedFunction() throws Exception {
eval("def impl():\n" + " return 0\n");
eval("def impl():", " pass");
}

@Test
Expand Down Expand Up @@ -1739,7 +1739,14 @@ public void testRuleAddExecutionConstraints() throws Exception {
}

@Test
public void testTargetsCanAddExecutionPlatformConstraints() throws Exception {
public void testTargetsCanAddExecutionPlatformConstraints_enabled() throws Exception {
StarlarkSemantics semantics =
StarlarkSemantics.DEFAULT_SEMANTICS.toBuilder()
.incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(false)
.build();
ev = createEvaluationTestCase(semantics);
ev.initialize();

registerDummyUserDefinedFunction();
scratch.file("test/BUILD", "toolchain_type(name = 'my_toolchain_type')");
evalAndExport(
Expand All @@ -1752,6 +1759,47 @@ public void testTargetsCanAddExecutionPlatformConstraints() throws Exception {
.isEqualTo(ExecutionPlatformConstraintsAllowed.PER_TARGET);
}

@Test
public void testTargetsCanAddExecutionPlatformConstraints_notEnabled() throws Exception {
StarlarkSemantics semantics =
StarlarkSemantics.DEFAULT_SEMANTICS.toBuilder()
.incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(false)
.build();
ev = createEvaluationTestCase(semantics);
ev.initialize();

registerDummyUserDefinedFunction();
scratch.file("test/BUILD", "toolchain_type(name = 'my_toolchain_type')");
evalAndExport(
"r1 = rule(impl, ",
" toolchains=['//test:my_toolchain_type'],",
" execution_platform_constraints_allowed=False,",
")");
RuleClass c = ((SkylarkRuleFunction) lookup("r1")).getRuleClass();
assertThat(c.executionPlatformConstraintsAllowed())
.isEqualTo(ExecutionPlatformConstraintsAllowed.PER_RULE);
}

@Test
public void testTargetsCanAddExecutionPlatformConstraints_disallowed() throws Exception {
StarlarkSemantics semantics =
StarlarkSemantics.DEFAULT_SEMANTICS.toBuilder()
.incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(true)
.build();
ev = createEvaluationTestCase(semantics);
ev.setFailFast(false);
ev.initialize();

registerDummyUserDefinedFunction();
scratch.file("test/BUILD", "toolchain_type(name = 'my_toolchain_type')");
evalAndExport(
"r1 = rule(impl, ",
" toolchains=['//test:my_toolchain_type'],",
" execution_platform_constraints_allowed=True,",
")");
ev.assertContainsError("parameter 'execution_platform_constraints_allowed' is deprecated");
}

@Test
public void testRuleFunctionReturnsNone() throws Exception {
scratch.file("test/rule.bzl",
Expand Down

0 comments on commit 5688793

Please sign in to comment.