Require the "command" param of run_shell is a string This constitutes an incompatible change guarded by flag --incompatible_run_shell_command_string. Progress toward #5903. RELNOTES: The `command` parameter of the `actions.run_shell()` function will be restricted to only accept strings (and not string sequences). This check is attached to flag `--incompatible_run_shell_command_string`. One may migrate by using the `arguments` parameter of `actions.run()` instead. See https://github.com/bazelbuild/bazel/issues/5903 for more info. PiperOrigin-RevId: 253122136
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 6db9a89..b7197d0 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 @@ Object envUnchecked, Object executionRequirementsUnchecked, Object inputManifestsUnchecked, - Location location) + Location location, + StarlarkSemantics semantics) throws EvalException { context.checkMutable("actions.run_shell"); @@ -338,6 +339,13 @@ } } } 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 5b42fe7..d9c821e 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 @@ 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 2d7691d..044a427 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 @@ + "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 @@ .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 51a113325..18618b9 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 @@ + "<p><b>(Deprecated)</b> If <code>command</code> 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 <code>arguments</code> parameter must " - + "not be supplied." + + "not be supplied. <i>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. </i>" + "" + "<p>Bazel uses the same shell to execute the command as it does for " + "genrules."), @@ -463,9 +466,10 @@ "(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 @@ 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 dcabe4b..229a0df 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 abstract boolean incompatibleRestrictNamedParams(); + public abstract boolean incompatibleRunShellCommandString(); + public abstract boolean incompatibleStringJoinRequiresStrings(); public abstract boolean internalSkylarkFlagTestCanary(); @@ -264,6 +266,7 @@ .incompatibleObjcFrameworkCleanup(true) .incompatibleRemapMainRepo(false) .incompatibleRemoveNativeMavenJar(false) + .incompatibleRunShellCommandString(false) .incompatibleRestrictNamedParams(false) .incompatibleStringJoinRequiresStrings(true) .internalSkylarkFlagTestCanary(false) @@ -348,6 +351,8 @@ 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 341eea1..72ece5b 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 @@ "--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 @@ .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 5564531..6a7b329 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 @@ 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. */