Skip to content

Commit

Permalink
Enable incompatible_run_shell_command_string by default
Browse files Browse the repository at this point in the history
Fixes #5903

RELNOTES: --incompatible_run_shell_command_string is enabled by default (#5903)
PiperOrigin-RevId: 339641875
  • Loading branch information
laurentlb authored and copybara-github committed Oct 29, 2020
1 parent 655c4f8 commit 213043a
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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"));

Expand Down
15 changes: 9 additions & 6 deletions src/test/shell/bazel/external_path_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions src/test/shell/bazel/starlark_rule_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/test/shell/bazel/tags_propagation_starlark_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down

0 comments on commit 213043a

Please sign in to comment.