From 598c356c6e698a6a79dc87249981ca7af8d43c75 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 9 Jun 2026 08:56:34 +0200 Subject: [PATCH 1/3] Add new tests --- shell/BUILD | 36 ++++++- shell/private/BUILD | 1 + shell/private/repositories/sh_config.bzl | 42 ++++++-- shell/private/run_shell.bzl | 129 +++++++++++++++++++++++ shell/private/sh_executable.bzl | 13 ++- shell/run_shell.bzl | 21 ++++ shell/toolchains/BUILD | 8 ++ shell/toolchains/sh_exec_toolchain.bzl | 52 +++++++++ shell/toolchains/sh_toolchain.bzl | 2 + tests/run_shell/exec_fixtures.bzl | 12 ++- tests/run_shell/fixtures.bzl | 44 ++++++-- 11 files changed, 325 insertions(+), 35 deletions(-) create mode 100644 shell/private/run_shell.bzl create mode 100644 shell/run_shell.bzl create mode 100644 shell/toolchains/sh_exec_toolchain.bzl diff --git a/shell/BUILD b/shell/BUILD index 4148901..4097d9e 100644 --- a/shell/BUILD +++ b/shell/BUILD @@ -1,6 +1,10 @@ +load("@bazel_skylib//:bzl_library.bzl", "bzl_library") + # A runtime toolchain for shell scripts. # -# Use `sh_toolchain` to register a toolchain for this type. +# Used at runtime by sh_binary and sh_test targets. +# +# Register `sh_toolchain` targets for this type. # # Every toolchain registered for this type has the following attributes: # - `path`: The path to the shell interpreter for the target platform. @@ -9,21 +13,43 @@ # Bazel's sh_* rules. # # Toolchains registered for this type should have target constraints. - -load("@bazel_skylib//:bzl_library.bzl", "bzl_library") - toolchain_type( name = "toolchain_type", visibility = ["//visibility:public"], ) +# An exec toolchain for shell commands. +# +# Used in rule implementations that use `run_shell` to run shell commands at build time. +# +# Register `sh_exec_toolchain` targets for this type. +# +# Every toolchain registered for this type has the following attributes: +# - `path`: The path to the shell interpreter for the execution platform. +# - `max_command_length`: The maximum command length before spilling to a helper script. +# +# Other attribute may be present but are considered implementation details of +# the `run_shell` implementation. +# +# Toolchains registered for this type should have exec constraints. +toolchain_type( + name = "exec_toolchain_type", + visibility = ["//visibility:public"], +) + bzl_library( name = "rules_bzl", srcs = [ + "run_shell.bzl", "sh_binary.bzl", + "sh_binary_info.bzl", + "sh_info.bzl", "sh_library.bzl", "sh_test.bzl", ], visibility = ["//visibility:public"], - deps = ["//shell/private:private_bzl"], + deps = [ + "//shell/private:private_bzl", + "//shell/toolchains:toolchains_bzl", + ], ) diff --git a/shell/private/BUILD b/shell/private/BUILD index 23eb387..7926fd4 100644 --- a/shell/private/BUILD +++ b/shell/private/BUILD @@ -4,6 +4,7 @@ bzl_library( name = "private_bzl", srcs = [ "providers.bzl", + "run_shell.bzl", "sh_binary.bzl", "sh_executable.bzl", "sh_library.bzl", diff --git a/shell/private/repositories/sh_config.bzl b/shell/private/repositories/sh_config.bzl index 0928599..4279615 100644 --- a/shell/private/repositories/sh_config.bzl +++ b/shell/private/repositories/sh_config.bzl @@ -23,31 +23,52 @@ _DEFAULT_SHELL_PATHS = { "openbsd": "/usr/local/bin/bash", } -_UNIX_SH_TOOLCHAIN_TEMPLATE = """ +_UNIX_SH_TOOLCHAINS_TEMPLATE = """ sh_toolchain( name = "{os}_sh", path = {sh_path}, ) + +sh_exec_toolchain( + name = "{os}_sh_exec", + path = {sh_path}, + max_command_length = 64000, +) """ -_WINDOWS_SH_TOOLCHAIN_TEMPLATE = """ +_WINDOWS_SH_TOOLCHAINS_TEMPLATE = """ sh_toolchain( name = "{os}_sh", path = {sh_path}, launcher = "@bazel_tools//tools/launcher", launcher_maker = "@bazel_tools//tools/launcher:launcher_maker", ) + +sh_exec_toolchain( + name = "{os}_sh_exec", + path = {sh_path}, + max_command_length = 8000, +) if {sh_path} else None """ -_TOOLCHAIN_TEMPLATE = """ +_TOOLCHAINS_TEMPLATE = """ toolchain( name = "{os}_sh_toolchain", toolchain = ":{os}_sh", - toolchain_type = "@rules_shell//shell:toolchain_type", + toolchain_type = SH_TOOLCHAIN_TYPE, target_compatible_with = [ "@platforms//os:{os}", ], ) + +toolchain( + name = "{os}_sh_exec_toolchain", + toolchain = ":{os}_sh_exec", + toolchain_type = SH_EXEC_TOOLCHAIN_TYPE, + exec_compatible_with = [ + "@platforms//os:{os}", + ], +) if {sh_path} else None """ def _sh_config_impl(repository_ctx): @@ -66,23 +87,26 @@ def _sh_config_impl(repository_ctx): if is_host: # This toolchain was first added before optional toolchains were # available, so instead of not registering a toolchain if we - # couldn't find the shell, we register a toolchain with an empty - # path. + # couldn't find the shell, we register a runtim toolchain with an + # empty path. The exec toolchain is new and not registered if no + # shell is found. sh_path = _detect_local_shell_path(repository_ctx) or "" else: sh_path = default_shell_path - sh_toolchain_template = _WINDOWS_SH_TOOLCHAIN_TEMPLATE if os == "windows" else _UNIX_SH_TOOLCHAIN_TEMPLATE + sh_toolchain_template = _WINDOWS_SH_TOOLCHAINS_TEMPLATE if os == "windows" else _UNIX_SH_TOOLCHAINS_TEMPLATE toolchains.append(sh_toolchain_template.format( os = os, sh_path = repr(sh_path), )) - toolchains.append(_TOOLCHAIN_TEMPLATE.format( + toolchains.append(_TOOLCHAINS_TEMPLATE.format( os = os, + sh_path = repr(sh_path), )) repository_ctx.file("BUILD", """ -load("@rules_shell//shell/toolchains:sh_toolchain.bzl", "sh_toolchain") +load("@rules_shell//shell/toolchains:sh_toolchain.bzl", "sh_toolchain", "SH_TOOLCHAIN_TYPE") +load("@rules_shell//shell/toolchains:sh_exec_toolchain.bzl", "sh_exec_toolchain", "SH_EXEC_TOOLCHAIN_TYPE") """ + "\n".join(toolchains)) sh_config = repository_rule( diff --git a/shell/private/run_shell.bzl b/shell/private/run_shell.bzl new file mode 100644 index 0000000..bcbe388 --- /dev/null +++ b/shell/private/run_shell.bzl @@ -0,0 +1,129 @@ +# Copyright 2024 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""A wrapper around `ctx.actions.run` that runs a shell command. + +This mirrors the behavior of the native `ctx.actions.run_shell` function. + +Rules calling `run_shell` must depend on the shell exec toolchain via +`toolchains = [SH_EXEC_TOOLCHAIN_TYPE]` (loaded from +`@rules_shell//shell/toolchains:sh_exec_toolchain.bzl`). +""" + +load("@rules_shell//shell/toolchains:sh_exec_toolchain.bzl", "SH_EXEC_TOOLCHAIN_TYPE") + +visibility("public") + +def run_shell( + ctx, + *, + command, + outputs, + inputs = [], + tools = [], + arguments = [], + mnemonic = None, + progress_message = None, + use_default_shell_env = False, + env = None, + execution_requirements = None, + exec_group = None, + shadowed_action = None, + resource_set = None): + """Creates an action that runs a shell command. + + Args: + ctx: The rule context. The calling rule must depend on the shell + toolchain type, e.g. via `toolchains = ["//shell:toolchain_type"]`. + command: Shell command to execute. Unlike the native + `ctx.actions.run_shell`, only a string is accepted; passing a + sequence of strings is deprecated and rejected. The command is + executed as `sh -c "" `, which makes the + `arguments` available as `$1`, `$2`, etc. If an `Args` object of + unknown size is passed as part of `arguments`, then the strings + will be at unknown indices; in this case the `$@` shell + substitution (retrieve all arguments) may be useful. + outputs: List of the output files of the action. + inputs: List or depset of the input files of the action. + tools: List or depset of any tools needed by the action. Tools are + executable inputs that may have their own runfiles which are + automatically made available to the action. + arguments: Command line arguments of the action. Must be a list of + strings or `actions.args()` objects. Bazel passes the elements in + this attribute as arguments to the command. The command can access + these arguments using shell variable substitutions such as `$1`, + `$2`, etc. Note that since `Args` objects are flattened before + indexing, if there is an `Args` object of unknown size then all + subsequent strings will be at unpredictable indices. + mnemonic: A one-word description of the action, for example, + CppCompile or GoLink. + progress_message: Progress message to show to the user during the + build, for example, "Compiling foo.cc to create foo.o". The message + may contain `%{label}`, `%{input}`, or `%{output}` patterns, which + are substituted with label string, first input, or output's path, + respectively. Prefer to use patterns instead of static strings, + because the former are more efficient. + use_default_shell_env: Whether the action should use the default shell + environment, which consists of a few OS-dependent variables as well + as variables set via `--action_env`. If both `use_default_shell_env` + and `env` are set, values set in `env` will overwrite the default + shell environment. + env: Sets the dictionary of environment variables. + execution_requirements: Information for scheduling the action. + exec_group: The execution group for the action. The shell exec toolchain + will be obtained from this group. + """ + if type(command) != type(""): + fail("'command' must be of type string, got %s" % type(command)) + + toolchains = ctx.exec_groups[exec_group] if exec_group else ctx.toolchains + sh_exec_toolchain = toolchains[SH_EXEC_TOOLCHAIN_TYPE].sh_exec_toolchain + + interpreter_args = ctx.actions.args() + if len(command) <= sh_exec_toolchain.max_command_length: + interpreter_args.add("-c") + interpreter_args.add(command) + + # Preserve the long-standing behavior of `ctx.actions.run_shell` passing + # an empty string as $0 (if any args are passed). + if arguments: + interpreter_args.add("") + else: + # Spill the command into a helper script and reference it instead. The + # script is executed directly rather than via ` -c` so that + # the given interpreter is used without the need to synthesize a + # shebang. + interpreter_args.use_param_file("%s", use_always = True) + interpreter_args.set_param_file_format("multiline") + interpreter_args.add(command) + + # shadowed_action doesn't allow an explicit None value. + run_kwargs = {"shadowed_action": shadowed_action} if shadowed_action else {} + + ctx.actions.run( + executable = sh_exec_toolchain.interpreter, + arguments = [interpreter_args] + arguments, + outputs = outputs, + inputs = inputs, + tools = tools, + mnemonic = mnemonic, + progress_message = progress_message, + use_default_shell_env = use_default_shell_env, + env = env, + execution_requirements = execution_requirements, + toolchain = SH_EXEC_TOOLCHAIN_TYPE, + exec_group = exec_group, + resource_set = resource_set, + **run_kwargs + ) diff --git a/shell/private/sh_executable.bzl b/shell/private/sh_executable.bzl index 9561f3d..32300d3 100644 --- a/shell/private/sh_executable.bzl +++ b/shell/private/sh_executable.bzl @@ -14,12 +14,11 @@ """Common code for sh_binary and sh_test rules.""" +load("//shell/toolchains:sh_toolchain.bzl", "SH_TOOLCHAIN_TYPE") load(":providers.bzl", "ShBinaryInfo", "ShInfo") visibility(["//shell"]) -_SH_TOOLCHAIN_TYPE = Label("//shell:toolchain_type") - def _to_rlocation_path(ctx, file): if file.short_path.startswith("../"): return file.short_path[3:] @@ -47,7 +46,7 @@ def _sh_executable_impl(ctx): # error. shebang = "" else: - shell = ctx.toolchains[_SH_TOOLCHAIN_TYPE].path + shell = ctx.toolchains[SH_TOOLCHAIN_TYPE].path shebang = "#!{}".format(shell) ctx.actions.write( entrypoint, @@ -159,7 +158,7 @@ def _create_windows_exe_launcher(ctx, sh_toolchain, primary_output): outputs = [bash_launcher], arguments = [launcher_artifact.path, launch_info, bash_launcher.path], use_default_shell_env = True, - toolchain = _SH_TOOLCHAIN_TYPE, + toolchain = SH_TOOLCHAIN_TYPE, ) return bash_launcher @@ -171,14 +170,14 @@ def _launcher_for_windows(ctx, primary_output, main_file): fail("Source file is a Windows executable file, target name extension should match source file extension") # bazel_tools should always registers a toolchain for Windows, but it may have an empty path. - sh_toolchain = ctx.toolchains[_SH_TOOLCHAIN_TYPE] + sh_toolchain = ctx.toolchains[SH_TOOLCHAIN_TYPE] if not sh_toolchain or not sh_toolchain.path: # Let fail print the toolchain type with an apparent repo name. fail( """No suitable shell toolchain found: * if you are running Bazel on Windows, set the BAZEL_SH environment variable to the path of bash.exe * if you are running Bazel on a non-Windows platform but are targeting Windows, register an sh_toolchain for the""", - _SH_TOOLCHAIN_TYPE, + SH_TOOLCHAIN_TYPE, "toolchain type", ) @@ -232,7 +231,7 @@ most build rules. ), } | extra_attrs, toolchains = [ - config_common.toolchain_type(_SH_TOOLCHAIN_TYPE, mandatory = False), + config_common.toolchain_type(SH_TOOLCHAIN_TYPE, mandatory = False), ], provides = [ShBinaryInfo], **kwargs diff --git a/shell/run_shell.bzl b/shell/run_shell.bzl new file mode 100644 index 0000000..55215f1 --- /dev/null +++ b/shell/run_shell.bzl @@ -0,0 +1,21 @@ +# Copyright 2024 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""A wrapper around `ctx.actions.run` that runs a shell command.""" + +load("//shell/private:run_shell.bzl", _run_shell = "run_shell") + +visibility("public") + +run_shell = _run_shell diff --git a/shell/toolchains/BUILD b/shell/toolchains/BUILD index e69de29..bb177bc 100644 --- a/shell/toolchains/BUILD +++ b/shell/toolchains/BUILD @@ -0,0 +1,8 @@ +bzl_library( + name = "toolchains_bzl", + srcs = [ + "sh_exec_toolchain.bzl", + "sh_toolchain.bzl", + ], + visibility = ["//visibility:public"], +) diff --git a/shell/toolchains/sh_exec_toolchain.bzl b/shell/toolchains/sh_exec_toolchain.bzl new file mode 100644 index 0000000..ca6e850 --- /dev/null +++ b/shell/toolchains/sh_exec_toolchain.bzl @@ -0,0 +1,52 @@ +# Copyright 2018 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Defines a toolchain rule for shell build actions.""" + +visibility("public") + +SH_EXEC_TOOLCHAIN_TYPE = Label("//shell:exec_toolchain_type") + +ShExecToolchainInfo = provider( + doc = "A toolchain used to execute shell commands in a build action.", + fields = { + "interpreter": "(str|FilesToRunProvider) The shell interpreter to use, either non-hermetic (absolute path) or hermetic (FilesToRunProvider).", + "max_command_length": "(int) The maximum command length before spilling to a helper script.", + }, +) + +def _sh_exec_toolchain_impl(ctx): + """sh_exec_toolchain rule implementation.""" + return [ + platform_common.ToolchainInfo( + sh_exec_toolchain = ShExecToolchainInfo( + interpreter = ctx.attr.path, + max_command_length = ctx.attr.max_command_length, + ), + ), + ] + +sh_exec_toolchain = rule( + doc = "A toolchain used to execute shell commands in a build action.", + attrs = { + "path": attr.string( + doc = "Absolute path to the non-hermetic shell interpreter.", + mandatory = True, + ), + "max_command_length": attr.int( + doc = "The maximum command length before spilling to a helper script.", + mandatory = True, + ), + }, + implementation = _sh_exec_toolchain_impl, +) diff --git a/shell/toolchains/sh_toolchain.bzl b/shell/toolchains/sh_toolchain.bzl index 64d2d3c..0250128 100644 --- a/shell/toolchains/sh_toolchain.bzl +++ b/shell/toolchains/sh_toolchain.bzl @@ -15,6 +15,8 @@ visibility("public") +SH_TOOLCHAIN_TYPE = Label("//shell:toolchain_type") + def _sh_toolchain_impl(ctx): """sh_toolchain rule implementation.""" return [ diff --git a/tests/run_shell/exec_fixtures.bzl b/tests/run_shell/exec_fixtures.bzl index 64a4b18..e342565 100644 --- a/tests/run_shell/exec_fixtures.bzl +++ b/tests/run_shell/exec_fixtures.bzl @@ -1,4 +1,4 @@ -"""Rules that execute `ctx.actions.run_shell` and expose the produced output. +"""Rules that execute the `run_shell` function and expose the produced output. The single output file of each target is compared against a golden file with `diff_test`, which exercises the runtime behavior of `run_shell` (command @@ -7,10 +7,13 @@ long commands) without a Bazel-in-Bazel integration test. """ load("@with_cfg.bzl", "with_cfg") +load("//shell:run_shell.bzl", "run_shell") +load("//shell/toolchains:sh_exec_toolchain.bzl", "SH_EXEC_TOOLCHAIN_TYPE") def _run_shell_output_impl(ctx): out = ctx.actions.declare_file(ctx.label.name + ".out") - ctx.actions.run_shell( + run_shell( + ctx, outputs = [out], command = ctx.attr.command, arguments = [out.path] + ctx.attr.extra_arguments, @@ -30,6 +33,7 @@ run_shell_output = rule( "env": attr.string_dict(), "use_default_shell_env": attr.bool(), }, + toolchains = [SH_EXEC_TOOLCHAIN_TYPE], ) # Same as run_shell_output, but transitions --action_env so that targets can @@ -112,7 +116,8 @@ def _run_shell_long_output_impl(ctx): # the smaller-stacked bash on Windows. The output path is embedded directly # because positional arguments are not forwarded into the helper script. command = "echo done > %s #%s" % (out.path, "x" * 70000) - ctx.actions.run_shell( + run_shell( + ctx, outputs = [out], command = command, mnemonic = "RunShellLongOutput", @@ -121,4 +126,5 @@ def _run_shell_long_output_impl(ctx): run_shell_long_output = rule( implementation = _run_shell_long_output_impl, + toolchains = [SH_EXEC_TOOLCHAIN_TYPE], ) diff --git a/tests/run_shell/fixtures.bzl b/tests/run_shell/fixtures.bzl index 231851e..137ad6f 100644 --- a/tests/run_shell/fixtures.bzl +++ b/tests/run_shell/fixtures.bzl @@ -1,9 +1,13 @@ -"""Rules under test that exercise `ctx.actions.run_shell`.""" +"""Rules under test that exercise the `run_shell` function.""" + +load("//shell:run_shell.bzl", "run_shell") +load("//shell/toolchains:sh_exec_toolchain.bzl", "SH_EXEC_TOOLCHAIN_TYPE") def _command_string_impl(ctx): out_a = ctx.actions.declare_file(ctx.label.name + "_a.txt") out_b = ctx.actions.declare_file(ctx.label.name + "_b.img") - ctx.actions.run_shell( + run_shell( + ctx, inputs = ctx.files.srcs, outputs = [out_a, out_b], arguments = ["--a", "--b"], @@ -17,11 +21,13 @@ def _command_string_impl(ctx): command_string = rule( implementation = _command_string_impl, attrs = {"srcs": attr.label_list(allow_files = True)}, + toolchains = [SH_EXEC_TOOLCHAIN_TYPE], ) def _command_no_arguments_impl(ctx): out = ctx.actions.declare_file(ctx.label.name + ".out") - ctx.actions.run_shell( + run_shell( + ctx, outputs = [out], command = "echo foo123 > " + out.path, ) @@ -30,11 +36,13 @@ def _command_no_arguments_impl(ctx): command_no_arguments = rule( implementation = _command_no_arguments_impl, attrs = {}, + toolchains = [SH_EXEC_TOOLCHAIN_TYPE], ) def _command_with_tools_impl(ctx): out = ctx.actions.declare_file(ctx.label.name + ".out") - ctx.actions.run_shell( + run_shell( + ctx, inputs = ctx.files.tool if ctx.attr.tool_in_inputs else [], tools = ctx.files.tool, outputs = [out], @@ -48,11 +56,13 @@ command_with_tools = rule( "tool": attr.label(allow_files = True, cfg = "exec"), "tool_in_inputs": attr.bool(default = False), }, + toolchains = [SH_EXEC_TOOLCHAIN_TYPE], ) def _command_list_impl(ctx): out = ctx.actions.declare_file(ctx.label.name + ".out") - ctx.actions.run_shell( + run_shell( + ctx, outputs = [out], mnemonic = "DummyMnemonic", command = ["dummy_command", "--arg1", "--arg2"], @@ -62,11 +72,13 @@ def _command_list_impl(ctx): command_list = rule( implementation = _command_list_impl, attrs = {}, + toolchains = [SH_EXEC_TOOLCHAIN_TYPE], ) def _invalid_mnemonic_impl(ctx): out = ctx.actions.declare_file(ctx.label.name + ".out") - ctx.actions.run_shell( + run_shell( + ctx, outputs = [out], command = "false", mnemonic = "@@@", @@ -76,13 +88,15 @@ def _invalid_mnemonic_impl(ctx): invalid_mnemonic = rule( implementation = _invalid_mnemonic_impl, attrs = {}, + toolchains = [SH_EXEC_TOOLCHAIN_TYPE], ) def _lazy_args_impl(ctx): out = ctx.actions.declare_file(ctx.label.name + ".out") args = ctx.actions.args() args.add("--foo") - ctx.actions.run_shell( + run_shell( + ctx, outputs = [out], arguments = [args], mnemonic = "DummyMnemonic", @@ -93,6 +107,7 @@ def _lazy_args_impl(ctx): lazy_args = rule( implementation = _lazy_args_impl, attrs = {}, + toolchains = [SH_EXEC_TOOLCHAIN_TYPE], ) def _long_command_impl(ctx): @@ -100,7 +115,8 @@ def _long_command_impl(ctx): long_command = "( %s ; ) > $1" % " ; ".join( ["echo xxx%d" % i for i in range(0, 7000)], ) - ctx.actions.run_shell( + run_shell( + ctx, outputs = [out], command = long_command, mnemonic = "LongMnemonic", @@ -111,6 +127,7 @@ def _long_command_impl(ctx): long_command = rule( implementation = _long_command_impl, attrs = {}, + toolchains = [SH_EXEC_TOOLCHAIN_TYPE], ) def _medium_command_impl(ctx): @@ -121,7 +138,8 @@ def _medium_command_impl(ctx): medium_command = "( %s ; ) > $1" % " ; ".join( ["echo zzz%d" % i for i in range(0, 1000)], ) - ctx.actions.run_shell( + run_shell( + ctx, outputs = [out], command = medium_command, mnemonic = "MediumMnemonic", @@ -132,6 +150,7 @@ def _medium_command_impl(ctx): medium_command = rule( implementation = _medium_command_impl, attrs = {}, + toolchains = [SH_EXEC_TOOLCHAIN_TYPE], ) def _two_long_commands_impl(ctx): @@ -143,13 +162,15 @@ def _two_long_commands_impl(ctx): command2 = "( %s ; ) > $1" % " ; ".join( ["echo yyy%d" % i for i in range(0, 7000)], ) - ctx.actions.run_shell( + run_shell( + ctx, outputs = [out1], command = command1, mnemonic = "Mnemonic1", arguments = [out1.path], ) - ctx.actions.run_shell( + run_shell( + ctx, outputs = [out2], command = command2, mnemonic = "Mnemonic2", @@ -160,4 +181,5 @@ def _two_long_commands_impl(ctx): two_long_commands = rule( implementation = _two_long_commands_impl, attrs = {}, + toolchains = [SH_EXEC_TOOLCHAIN_TYPE], ) From 40620820f07a50d7a0f3fbb748ce62525a26921e Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 9 Jun 2026 15:30:29 +0200 Subject: [PATCH 2/3] Fix run_shell bugs/inconsistencies and forward args to spilled scripts - Load bzl_library in shell/toolchains/BUILD (was a build failure) - Correct run_shell ctx docstring to reference the exec toolchain type and document shadowed_action/resource_set - Guard the Unix sh_exec_toolchain target on a non-empty shell path, matching the Windows template; fix "runtim" typo - Route run_shell_script_args and run_shell_helper_script_args through the new run_shell wrapper, and add the missing exec toolchain to those rules - run_shell forwards `arguments` into the spilled helper script, so update the helper_script_args_spilled golden and comments accordingly, fixing https://github.com/bazelbuild/bazel/issues/29365 - Misc comment/grammar fixes Co-Authored-By: Claude Opus 4.8 (1M context) --- shell/BUILD | 4 ++-- shell/private/repositories/sh_config.bzl | 4 ++-- shell/private/run_shell.bzl | 9 +++++++-- shell/toolchains/BUILD | 2 ++ tests/run_shell/BUILD | 5 +++-- tests/run_shell/exec_fixtures.bzl | 10 +++++++--- tests/run_shell/helper_script_args_spilled.golden | 2 +- tests/run_shell/run_shell_tests.bzl | 2 +- 8 files changed, 25 insertions(+), 13 deletions(-) diff --git a/shell/BUILD b/shell/BUILD index 4097d9e..c5f09b0 100644 --- a/shell/BUILD +++ b/shell/BUILD @@ -9,7 +9,7 @@ load("@bazel_skylib//:bzl_library.bzl", "bzl_library") # Every toolchain registered for this type has the following attributes: # - `path`: The path to the shell interpreter for the target platform. # -# Other attribute may be present but are considered implementation details of +# Other attributes may be present but are considered implementation details of # Bazel's sh_* rules. # # Toolchains registered for this type should have target constraints. @@ -28,7 +28,7 @@ toolchain_type( # - `path`: The path to the shell interpreter for the execution platform. # - `max_command_length`: The maximum command length before spilling to a helper script. # -# Other attribute may be present but are considered implementation details of +# Other attributes may be present but are considered implementation details of # the `run_shell` implementation. # # Toolchains registered for this type should have exec constraints. diff --git a/shell/private/repositories/sh_config.bzl b/shell/private/repositories/sh_config.bzl index 4279615..cbca69a 100644 --- a/shell/private/repositories/sh_config.bzl +++ b/shell/private/repositories/sh_config.bzl @@ -33,7 +33,7 @@ sh_exec_toolchain( name = "{os}_sh_exec", path = {sh_path}, max_command_length = 64000, -) +) if {sh_path} else None """ _WINDOWS_SH_TOOLCHAINS_TEMPLATE = """ @@ -87,7 +87,7 @@ def _sh_config_impl(repository_ctx): if is_host: # This toolchain was first added before optional toolchains were # available, so instead of not registering a toolchain if we - # couldn't find the shell, we register a runtim toolchain with an + # couldn't find the shell, we register a runtime toolchain with an # empty path. The exec toolchain is new and not registered if no # shell is found. sh_path = _detect_local_shell_path(repository_ctx) or "" diff --git a/shell/private/run_shell.bzl b/shell/private/run_shell.bzl index bcbe388..7530290 100644 --- a/shell/private/run_shell.bzl +++ b/shell/private/run_shell.bzl @@ -44,8 +44,9 @@ def run_shell( """Creates an action that runs a shell command. Args: - ctx: The rule context. The calling rule must depend on the shell - toolchain type, e.g. via `toolchains = ["//shell:toolchain_type"]`. + ctx: The rule context. The calling rule must depend on the shell exec + toolchain type via `toolchains = [SH_EXEC_TOOLCHAIN_TYPE]` (loaded + from `@rules_shell//shell/toolchains:sh_exec_toolchain.bzl`). command: Shell command to execute. Unlike the native `ctx.actions.run_shell`, only a string is accepted; passing a sequence of strings is deprecated and rejected. The command is @@ -83,6 +84,10 @@ def run_shell( execution_requirements: Information for scheduling the action. exec_group: The execution group for the action. The shell exec toolchain will be obtained from this group. + shadowed_action: An action whose inputs and environment are made + available to this action in addition to its own. + resource_set: A callback returning a dictionary of resource estimates + (e.g. memory, CPU) for scheduling the action locally. """ if type(command) != type(""): fail("'command' must be of type string, got %s" % type(command)) diff --git a/shell/toolchains/BUILD b/shell/toolchains/BUILD index bb177bc..bcf6b45 100644 --- a/shell/toolchains/BUILD +++ b/shell/toolchains/BUILD @@ -1,3 +1,5 @@ +load("@bazel_skylib//:bzl_library.bzl", "bzl_library") + bzl_library( name = "toolchains_bzl", srcs = [ diff --git a/tests/run_shell/BUILD b/tests/run_shell/BUILD index 71b8d92..e03edc0 100644 --- a/tests/run_shell/BUILD +++ b/tests/run_shell/BUILD @@ -102,8 +102,9 @@ diff_test( file2 = "helper_script_args_inline.golden", ) -# ...but the same command spilled into a helper script does not: $1 is empty. -# Codifies https://github.com/bazelbuild/bazel/issues/29365 (currently open bug). +# ...and the same command spilled into a helper script behaves identically: +# run_shell forwards `arguments` into the helper script, fixing +# https://github.com/bazelbuild/bazel/issues/29365. run_shell_helper_script_args( name = "helper_script_args_spilled", long = True, diff --git a/tests/run_shell/exec_fixtures.bzl b/tests/run_shell/exec_fixtures.bzl index e342565..fab253f 100644 --- a/tests/run_shell/exec_fixtures.bzl +++ b/tests/run_shell/exec_fixtures.bzl @@ -64,7 +64,8 @@ echo "args=($*)" > "$OUT" if ctx.attr.forward_arguments: command += " \"$@\"" - ctx.actions.run_shell( + run_shell( + ctx, outputs = [out], tools = [script], command = command, @@ -79,6 +80,7 @@ run_shell_script_args = rule( attrs = { "forward_arguments": attr.bool(), }, + toolchains = [SH_EXEC_TOOLCHAIN_TYPE], ) def _run_shell_helper_script_args_impl(ctx): @@ -90,7 +92,8 @@ def _run_shell_helper_script_args_impl(ctx): # written to a helper script without otherwise changing its behavior. command += " #" + ("x" * 70000) - ctx.actions.run_shell( + run_shell( + ctx, outputs = [out], command = command, arguments = ["the_argument"], @@ -104,6 +107,7 @@ run_shell_helper_script_args = rule( attrs = { "long": attr.bool(), }, + toolchains = [SH_EXEC_TOOLCHAIN_TYPE], ) def _run_shell_long_output_impl(ctx): @@ -114,7 +118,7 @@ def _run_shell_long_output_impl(ctx): # trailing comment rather than many statements: a deeply nested command list # (e.g. tens of thousands of `;`-separated commands) overflows the stack of # the smaller-stacked bash on Windows. The output path is embedded directly - # because positional arguments are not forwarded into the helper script. + # to keep the command self-contained. command = "echo done > %s #%s" % (out.path, "x" * 70000) run_shell( ctx, diff --git a/tests/run_shell/helper_script_args_spilled.golden b/tests/run_shell/helper_script_args_spilled.golden index 474b415..43a1696 100644 --- a/tests/run_shell/helper_script_args_spilled.golden +++ b/tests/run_shell/helper_script_args_spilled.golden @@ -1 +1 @@ -arg= +arg=the_argument diff --git a/tests/run_shell/run_shell_tests.bzl b/tests/run_shell/run_shell_tests.bzl index dcfd637..6a6d7ef 100644 --- a/tests/run_shell/run_shell_tests.bzl +++ b/tests/run_shell/run_shell_tests.bzl @@ -1,4 +1,4 @@ -"""Analysis tests for `ctx.actions.run_shell`.""" +"""Analysis tests for the `run_shell` function.""" load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite") load("@rules_testing//lib:truth.bzl", "matching") From 3146d6f356a6081f1abd8c741efbfbcc0b9874c0 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 9 Jun 2026 15:33:10 +0200 Subject: [PATCH 3/3] Pass output path as an argument in run_shell_long_output Now that run_shell forwards arguments into the spilled helper script, the output path no longer needs to be embedded directly in the command. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/run_shell/exec_fixtures.bzl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/run_shell/exec_fixtures.bzl b/tests/run_shell/exec_fixtures.bzl index fab253f..51a20d6 100644 --- a/tests/run_shell/exec_fixtures.bzl +++ b/tests/run_shell/exec_fixtures.bzl @@ -117,14 +117,14 @@ def _run_shell_long_output_impl(ctx): # run_shell spills the command into a helper script. The length comes from a # trailing comment rather than many statements: a deeply nested command list # (e.g. tens of thousands of `;`-separated commands) overflows the stack of - # the smaller-stacked bash on Windows. The output path is embedded directly - # to keep the command self-contained. - command = "echo done > %s #%s" % (out.path, "x" * 70000) + # the smaller-stacked bash on Windows. + command = "echo done > $1 #%s" % ("x" * 70000) run_shell( ctx, outputs = [out], command = command, mnemonic = "RunShellLongOutput", + arguments = [out.path], ) return [DefaultInfo(files = depset([out]))]