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.
*/
