diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java index 6db9a8993a963c..b7197d0c2579d9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java @@ -310,7 +310,8 @@ public void runShell( Object envUnchecked, Object executionRequirementsUnchecked, Object inputManifestsUnchecked, - Location location) + Location location, + StarlarkSemantics semantics) throws EvalException { context.checkMutable("actions.run_shell"); @@ -338,6 +339,13 @@ public void runShell( } } } else if (commandUnchecked instanceof SkylarkList) { + if (semantics.incompatibleRunShellCommandString()) { + throw new EvalException( + location, + "'command' must be of type string. passing a sequence of strings as 'command'" + + " is deprecated. To temporarily disable this check," + + " set --incompatible_objc_framework_cleanup=false."); + } SkylarkList commandList = (SkylarkList) commandUnchecked; if (argumentList.size() > 0) { throw new EvalException(location, diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java index 5b42fe755e39d1..d9c821e8658593 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java @@ -919,7 +919,8 @@ public Runtime.NoneType action( envUnchecked, executionRequirementsUnchecked, inputManifestsUnchecked, - loc); + loc, + env.getSemantics()); } return Runtime.NONE; } diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java index 2d7691d7cac129..044a42720141a1 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java @@ -503,6 +503,18 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl + "will be available") public boolean incompatibleRemoveNativeMavenJar; + @Option( + name = "incompatible_run_shell_command_string", + 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, the command parameter of actions.run_shell will only accept string") + public boolean incompatibleRunShellCommandString; + /** Used in an integration test to confirm that flags are visible to the interpreter. */ @Option( name = "internal_skylark_flag_test_canary", @@ -625,6 +637,7 @@ public StarlarkSemantics toSkylarkSemantics() { .incompatibleRemapMainRepo(incompatibleRemapMainRepo) .incompatibleRemoveNativeMavenJar(incompatibleRemoveNativeMavenJar) .incompatibleRestrictNamedParams(incompatibleRestrictNamedParams) + .incompatibleRunShellCommandString(incompatibleRunShellCommandString) .incompatibleStringJoinRequiresStrings(incompatibleStringJoinRequiresStrings) .internalSkylarkFlagTestCanary(internalSkylarkFlagTestCanary) .incompatibleDoNotSplitLinkingCmdline(incompatibleDoNotSplitLinkingCmdline) diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkActionFactoryApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkActionFactoryApi.java index 51a113325db428..18618b93ee2f18 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkActionFactoryApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkActionFactoryApi.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.syntax.SkylarkDict; import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; +import com.google.devtools.build.lib.syntax.StarlarkSemantics; /** Module providing functions to create actions. */ @SkylarkModule( @@ -409,7 +410,9 @@ public void run( + "

(Deprecated) If command is a sequence of strings, " + "the first item is the executable to run and the remaining items are its " + "arguments. If this form is used, the arguments parameter must " - + "not be supplied." + + "not be supplied. Note that this form is deprecated and will soon " + + "be removed. It is disabled with `--incompatible_run_shell_command_string`. " + + "Use this flag to verify your code is compatible. " + "" + "

Bazel uses the same shell to execute the command as it does for " + "genrules."), @@ -463,9 +466,10 @@ public void run( "(Experimental) sets the input runfiles metadata; " + "they are typically generated by resolve_command.") }, + useStarlarkSemantics = true, useLocation = true) public void runShell( - SkylarkList outputs, + SkylarkList outputs, Object inputs, Object toolsUnchecked, Object arguments, @@ -476,7 +480,8 @@ public void runShell( Object envUnchecked, Object executionRequirementsUnchecked, Object inputManifestsUnchecked, - Location location) + Location location, + StarlarkSemantics semantics) throws EvalException; @SkylarkCallable( diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java index dcabe4b059ba28..229a0df9834e3d 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java @@ -191,6 +191,8 @@ public boolean flagValue(FlagIdentifier flagIdentifier) { public abstract boolean incompatibleRestrictNamedParams(); + public abstract boolean incompatibleRunShellCommandString(); + public abstract boolean incompatibleStringJoinRequiresStrings(); public abstract boolean internalSkylarkFlagTestCanary(); @@ -264,6 +266,7 @@ public static Builder builderWithDefaults() { .incompatibleObjcFrameworkCleanup(true) .incompatibleRemapMainRepo(false) .incompatibleRemoveNativeMavenJar(false) + .incompatibleRunShellCommandString(false) .incompatibleRestrictNamedParams(false) .incompatibleStringJoinRequiresStrings(true) .internalSkylarkFlagTestCanary(false) @@ -348,6 +351,8 @@ public abstract Builder incompatibleDisallowRuleExecutionPlatformConstraintsAllo public abstract Builder incompatibleRestrictNamedParams(boolean value); + public abstract Builder incompatibleRunShellCommandString(boolean value); + public abstract Builder incompatibleStringJoinRequiresStrings(boolean value); public abstract Builder internalSkylarkFlagTestCanary(boolean value); diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java index 341eea1b7c74e7..72ece5b8c6a095 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java @@ -161,6 +161,7 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E "--incompatible_remap_main_repo=" + rand.nextBoolean(), "--incompatible_remove_native_maven_jar=" + rand.nextBoolean(), "--incompatible_restrict_named_params=" + rand.nextBoolean(), + "--incompatible_run_shell_command_string=" + rand.nextBoolean(), "--incompatible_string_join_requires_strings=" + rand.nextBoolean(), "--incompatible_restrict_string_escapes=" + rand.nextBoolean(), "--internal_skylark_flag_test_canary=" + rand.nextBoolean()); @@ -212,6 +213,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) { .incompatibleRemapMainRepo(rand.nextBoolean()) .incompatibleRemoveNativeMavenJar(rand.nextBoolean()) .incompatibleRestrictNamedParams(rand.nextBoolean()) + .incompatibleRunShellCommandString(rand.nextBoolean()) .incompatibleStringJoinRequiresStrings(rand.nextBoolean()) .incompatibleRestrictStringEscapes(rand.nextBoolean()) .internalSkylarkFlagTestCanary(rand.nextBoolean()) diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java index 556453102ec769..6a7b32953a52cd 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java @@ -2955,6 +2955,26 @@ public void testExecutableNotInRunfiles() throws Exception { assertContainsEvent("exe not included in runfiles"); } + @Test + public void testCommandStringList() throws Exception { + setSkylarkSemanticsOptions("--incompatible_run_shell_command_string"); + scratch.file( + "test/skylark/test_rule.bzl", + "def _my_rule_impl(ctx):", + " exe = ctx.actions.declare_file('exe')", + " ctx.actions.run_shell(outputs=[exe], command=['touch', 'exe'])", + " return []", + "my_rule = rule(implementation = _my_rule_impl)"); + scratch.file( + "test/skylark/BUILD", + "load('//test/skylark:test_rule.bzl', 'my_rule')", + "my_rule(name = 'target')"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//test/skylark:target"); + assertContainsEvent("'command' must be of type string"); + } + /** * Skylark integration test that forces inlining. */