From 213043a1b9fa5053fae720220f28ebc604283079 Mon Sep 17 00:00:00 2001 From: laurentlb Date: Thu, 29 Oct 2020 04:26:32 -0700 Subject: [PATCH] Enable incompatible_run_shell_command_string by default Fixes /~https://github.com/bazelbuild/bazel/issues/5903 RELNOTES: --incompatible_run_shell_command_string is enabled by default (#5903) PiperOrigin-RevId: 339641875 --- .../analysis/starlark/StarlarkActionFactory.java | 2 +- .../packages/semantics/BuildLanguageOptions.java | 4 ++-- .../StarlarkRuleImplementationFunctionsTest.java | 4 +++- src/test/shell/bazel/external_path_test.sh | 15 +++++++++------ src/test/shell/bazel/starlark_rule_test.sh | 4 ++-- .../shell/bazel/tags_propagation_starlark_test.sh | 2 +- .../implicit_dependency_reporting_test.sh | 5 +++-- 7 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java index 39efcf36bacdaa..31266ec9550829 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java @@ -454,7 +454,7 @@ public void runShell( throw Starlark.errorf( "'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."); + + " set --incompatible_run_shell_command_string=false."); } Sequence commandList = (Sequence) commandUnchecked; if (argumentList.size() > 0) { diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index 863de5fcfa16b0..294f98a4bf95e8 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -462,7 +462,7 @@ public class BuildLanguageOptions extends OptionsBase implements Serializable { @Option( name = "incompatible_run_shell_command_string", - defaultValue = "false", + defaultValue = "true", documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS}, metadataTags = { @@ -720,7 +720,7 @@ public StarlarkSemantics toStarlarkSemantics() { public static final String INCOMPATIBLE_RESTRICT_STRING_ESCAPES = "-incompatible_restrict_string_escapes"; public static final String INCOMPATIBLE_RUN_SHELL_COMMAND_STRING = - "-incompatible_run_shell_command_string"; + "+incompatible_run_shell_command_string"; public static final String INCOMPATIBLE_STRUCT_HAS_NO_METHODS = "-incompatible_struct_has_no_methods"; public static final String INCOMPATIBLE_USE_CC_CONFIGURE_FROM_RULES_CC = diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java index 66cd2124d38ff9..0bb7c7fc0d4fed 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java @@ -345,6 +345,7 @@ public void testCreateSpawnActionArgumentsBadExecutable() throws Exception { @Test public void testCreateSpawnActionShellCommandList() throws Exception { + ev.setSemantics("--incompatible_run_shell_command_string=false"); StarlarkRuleContext ruleContext = createRuleContext("//foo:foo"); setRuleContext(ruleContext); ev.exec( @@ -444,6 +445,7 @@ public void testCreateSpawnActionBadGenericArg() throws Exception { @Test public void testRunShellArgumentsWithCommandSequence() throws Exception { + ev.setSemantics("--incompatible_run_shell_command_string=false"); setRuleContext(createRuleContext("//foo:foo")); ev.checkEvalErrorContains( "'arguments' must be empty if 'command' is a sequence of strings", @@ -3047,7 +3049,7 @@ public void starlarkCustomCommandLineKeyComputation_inconsequentialChangeToStarl "def _path(f): return f.path", "args.add_all([directory], map_each=_path)"); - ev.setSemantics("--incompatible_run_shell_command_string"); + ev.setSemantics("--incompatible_run_shell_command_string=false"); // setBuildLanguageOptions reinitializes the thread -- set the ruleContext on the new one. setRuleContext(createRuleContext("//foo:foo")); diff --git a/src/test/shell/bazel/external_path_test.sh b/src/test/shell/bazel/external_path_test.sh index c17c233a7042d5..3720c6de1a587b 100755 --- a/src/test/shell/bazel/external_path_test.sh +++ b/src/test/shell/bazel/external_path_test.sh @@ -338,10 +338,11 @@ EOF cat > rule/to_upper.bzl <<'EOF' def _to_upper_impl(ctx): output = ctx.actions.declare_file(ctx.label.name + ".txt") - ctx.actions.run_shell( + ctx.actions.run( inputs = ctx.files.src + ctx.files._toupper_sh, outputs = [output], - command = ["/bin/sh"] + [f.path for f in ctx.files._toupper_sh] \ + executable = "/bin/sh", + arguments = [f.path for f in ctx.files._toupper_sh] + [f.path for f in ctx.files.src] + [output.path], use_default_shell_env = True, mnemonic = "ToUpper", @@ -490,10 +491,11 @@ EOF cat > rule/to_html.bzl <<'EOF' def _to_html_impl(ctx): output = ctx.actions.declare_file(ctx.label.name + ".html") - ctx.actions.run_shell( + ctx.actions.run( inputs = ctx.files.src + ctx.files._to_html + ctx.files._preamb + ctx.files._postamb, outputs = [output], - command = ["/bin/sh"] + [f.path for f in ctx.files._to_html] \ + executable = "/bin/sh", + arguments = [f.path for f in ctx.files._to_html] + [f.path for f in ctx.files.src] + [output.path], use_default_shell_env = True, mnemonic = "ToHtml", @@ -643,10 +645,11 @@ EOF cat > rule/add_preamb.bzl <<'EOF' def _add_preamb_impl(ctx): output = ctx.actions.declare_file(ctx.label.name + ".txt") - ctx.actions.run_shell( + ctx.actions.run( inputs = ctx.files.src + ctx.files._add_preamb + ctx.files._preamb, outputs = [output], - command = ["/bin/sh"] + [f.path for f in ctx.files._add_preamb] \ + executable = "/bin/sh", + arguments = [f.path for f in ctx.files._add_preamb] \ + [f.path for f in ctx.files.src] + [output.path], use_default_shell_env = True, mnemonic = "AddPreamb", diff --git a/src/test/shell/bazel/starlark_rule_test.sh b/src/test/shell/bazel/starlark_rule_test.sh index 2fc0bcfa3e0697..0145c3eadc6a27 100755 --- a/src/test/shell/bazel/starlark_rule_test.sh +++ b/src/test/shell/bazel/starlark_rule_test.sh @@ -36,7 +36,7 @@ EOF cat << 'EOF' >> test/starlark.bzl def _test_impl(ctx): ctx.actions.run_shell(outputs = [ctx.outputs.out], - command = ["touch", ctx.outputs.out.path]) + command = "touch " + ctx.outputs.out.path) files_to_build = depset([ctx.outputs.out]) return DefaultInfo( files = files_to_build, @@ -69,7 +69,7 @@ EOF cat << 'EOF' >> test/starlark.bzl def _test_impl(ctx): ctx.actions.run_shell(outputs = [ctx.outputs.out], - command = ["not_a_command", ctx.outputs.out.path]) + command = "not_a_command") files_to_build = depset([ctx.outputs.out]) return DefaultInfo( files = files_to_build, diff --git a/src/test/shell/bazel/tags_propagation_starlark_test.sh b/src/test/shell/bazel/tags_propagation_starlark_test.sh index cee8cb9820a45b..576fb4e435a0be 100755 --- a/src/test/shell/bazel/tags_propagation_starlark_test.sh +++ b/src/test/shell/bazel/tags_propagation_starlark_test.sh @@ -39,7 +39,7 @@ EOF cat << 'EOF' >> test/starlark.bzl def _test_impl(ctx): ctx.actions.run_shell(outputs = [ctx.outputs.out], - command = ["touch", ctx.outputs.out.path]) + command = "touch" + ctx.outputs.out.path) files_to_build = depset([ctx.outputs.out]) return DefaultInfo( files = files_to_build, diff --git a/src/test/shell/integration/implicit_dependency_reporting_test.sh b/src/test/shell/integration/implicit_dependency_reporting_test.sh index da91e9b44ee694..9f2c8932c43c5c 100755 --- a/src/test/shell/integration/implicit_dependency_reporting_test.sh +++ b/src/test/shell/integration/implicit_dependency_reporting_test.sh @@ -66,10 +66,11 @@ test_custom_message() { cat > rule.bzl <<'EOF' def _rule_impl(ctx): out = ctx.actions.declare_file(ctx.label.name + ".txt") - ctx.actions.run_shell( + ctx.actions.run( inputs = ctx.files._data, outputs = [out], - command = ["cp"] + [f.path for f in ctx.files._data] + [out.path], + executable = "cp", + arguments = [f.path for f in ctx.files._data] + [out.path], mnemonic = "copying", progress_message = "Copying implict data dependency for %s" % ctx.label )