diff --git a/lib/private/run_binary.bzl b/lib/private/run_binary.bzl index 7ce403824..5dbc141ad 100644 --- a/lib/private/run_binary.bzl +++ b/lib/private/run_binary.bzl @@ -50,13 +50,15 @@ Possible fixes: # Location and Make variable expansion can reference paths that aren't path mapped. can_path_map = True + targets = ctx.attr.tools + ctx.attr.srcs + inputs = ctx.files.tools + ctx.files.srcs for a in ctx.attr.args: - expanded = expand_variables(ctx, ctx.expand_location(a, targets = ctx.attr.srcs), inputs = ctx.files.srcs, outs = outputs) + expanded = expand_variables(ctx, ctx.expand_location(a, targets = targets), inputs = inputs, outs = outputs) can_path_map = can_path_map and expanded == a args.add_all(split_args(expanded)) envs = {} for k, v in ctx.attr.env.items(): - envs[k] = expand_variables(ctx, ctx.expand_location(v, targets = ctx.attr.srcs), inputs = ctx.files.srcs, outs = outputs, attribute_name = "env") + envs[k] = expand_variables(ctx, ctx.expand_location(v, targets = targets), inputs = inputs, outs = outputs, attribute_name = "env") can_path_map = can_path_map and envs[k] == v stamp = maybe_stamp(ctx) @@ -72,6 +74,7 @@ Possible fixes: outputs = outputs, inputs = inputs, executable = ctx.executable.tool, + tools = ctx.files.tools, arguments = [args], resource_set = resource_set(ctx.attr), mnemonic = ctx.attr.mnemonic if ctx.attr.mnemonic else None, @@ -95,6 +98,10 @@ _run_binary = rule( mandatory = True, cfg = "exec", ), + "tools": attr.label_list( + allow_files = True, + cfg = "exec", + ), "env": attr.string_dict(), "srcs": attr.label_list( allow_files = True, @@ -122,6 +129,7 @@ def run_binary( execution_requirements = None, use_default_shell_env = False, stamp = 0, + tools = [], **kwargs): """Runs a binary as a build action. @@ -215,12 +223,19 @@ def run_binary( These files can be read and parsed by the action, for example to pass some values to a linker. + tools: A list of tool dependencies for this action. + + These are treated similarly to srcs, except that they are built in + the exec configuration. Any code that will be executed as part of + the action should generally be listed here instead of in srcs. + **kwargs: Additional arguments """ _run_binary( name = name, tool = tool, srcs = srcs, + tools = tools, args = args, env = env, outs = outs, diff --git a/lib/tests/run_binary/BUILD.bazel b/lib/tests/run_binary/BUILD.bazel index 60fca4c5f..99e25f390 100644 --- a/lib/tests/run_binary/BUILD.bazel +++ b/lib/tests/run_binary/BUILD.bazel @@ -2,10 +2,12 @@ load("@bazel_skylib//rules:write_file.bzl", "write_file") load("@rules_shell//shell:sh_binary.bzl", "sh_binary") +load("@rules_shell//shell:sh_test.bzl", "sh_test") load("//lib:copy_to_directory.bzl", "copy_to_directory") load("//lib:diff_test.bzl", "diff_test") load("//lib:run_binary.bzl", "run_binary") load("//lib:testing.bzl", "assert_contains") +load("//lib:transitions.bzl", "platform_transition_filegroup") write_file( name = "make_dir_a_sh", @@ -80,3 +82,235 @@ assert_contains( actual = ":dir_a_rootpath", expected = "lib/tests/run_binary/dir_a_out", ) + +# Verify that items in `tools` are built in the exec configuration. +# To do this we check that run_binary agrees with genrule on the location of a +# tool dependency. +write_file( + name = "gen_cfg_probe", + out = "cfg_probe.txt", + content = ["probe"], +) + +write_file( + name = "make_echo_arg_sh", + out = "echo_arg.sh", + content = [ + "#!/usr/bin/env bash", + 'echo "$1" > "$2"', + ], +) + +sh_binary( + name = "echo_arg", + srcs = [":echo_arg.sh"], +) + +run_binary( + name = "tools_cfg_location", + outs = ["tools_cfg_location.txt"], + args = [ + "$(execpath :cfg_probe.txt)", + "$@", + ], + tool = ":echo_arg", + tools = [":cfg_probe.txt"], +) + +genrule( + name = "expected_tools_cfg_location", + outs = ["expected_tools_cfg_location.txt"], + cmd = "echo $(execpath :cfg_probe.txt) > $@", + tools = [":cfg_probe.txt"], +) + +diff_test( + name = "tools_exec_cfg_test", + file1 = ":tools_cfg_location.txt", + file2 = ":expected_tools_cfg_location.txt", +) + +# Below we set up a run_binary() target that writes the name of the execution +# platform OS to a file. Since we provide the OS name via the tools parameter, +# it should always be associated with the execution platform rather than the +# target platform. +# +# We do not know what the execution platform is going to be, since that is up +# to whoever is running the test. However, we can assert two things to +# establish some confidence that we are correctly making a distinction between +# the execution and target platforms: +# 1. Regardless of what platform we are targeting, the detected execution +# platform should remain the same. +# 2. The file paths produced for different target platforms should be different. +write_file( + name = "gen_os_name", + out = "os_name.txt", + content = select({ + "@platforms//os:linux": ["linux"], + "@platforms//os:macos": ["macos"], + "@platforms//os:windows": ["windows"], + "//conditions:default": ["other"], + }), + tags = ["manual"], +) + +write_file( + name = "make_cat_file_sh", + out = "cat_file.sh", + content = [ + "#!/usr/bin/env bash", + 'cat "$1" > "$2"', + ], +) + +sh_binary( + name = "cat_file", + srcs = [":cat_file.sh"], +) + +run_binary( + name = "gen_exec_os", + outs = ["exec_os.txt"], + args = [ + "$(execpath :os_name.txt)", + "$@", + ], + tags = ["manual"], + tool = ":cat_file", + tools = [":os_name.txt"], +) + +platform( + name = "linux", + constraint_values = [ + "@platforms//os:linux", + ], +) + +platform( + name = "macos", + constraint_values = [ + "@platforms//os:macos", + ], +) + +platform( + name = "windows", + constraint_values = [ + "@platforms//os:windows", + ], +) + +platform_transition_filegroup( + name = "exec_os_linux", + srcs = [":gen_exec_os"], + target_platform = ":linux", +) + +platform_transition_filegroup( + name = "exec_os_macos", + srcs = [":gen_exec_os"], + target_platform = ":macos", +) + +platform_transition_filegroup( + name = "exec_os_windows", + srcs = [":gen_exec_os"], + target_platform = ":windows", +) + +genrule( + name = "gen_platform_test", + srcs = [ + ":exec_os_linux", + ":exec_os_macos", + ":exec_os_windows", + ], + outs = ["platform_test.sh"], + # The genrule runs in the execroot so execpaths resolve directly. It reads + # the three files and bakes their paths and contents into the test script + # as shell assignments; the assertions then run at test time via sh_test. + cmd = """ + exec_os_linux_path=$(execpath :exec_os_linux) + exec_os_macos_path=$(execpath :exec_os_macos) + exec_os_windows_path=$(execpath :exec_os_windows) + { + echo '#!/usr/bin/env sh' + echo 'set -o errexit -o nounset' + echo "linux_path='$$exec_os_linux_path'" + echo "macos_path='$$exec_os_macos_path'" + echo "windows_path='$$exec_os_windows_path'" + echo "linux_os='$$(cat $$exec_os_linux_path)'" + echo "macos_os='$$(cat $$exec_os_macos_path)'" + echo "windows_os='$$(cat $$exec_os_windows_path)'" + echo 'n=$$(printf "%s\n" "$$linux_path" "$$macos_path" "$$windows_path" | sort -u | wc -l)' + echo 'if [ "$$n" -ne 3 ]; then' + echo ' printf "ERROR: expected different file paths for linux, macos, and windows targets\\n" >&2' + echo ' exit 1' + echo 'fi' + echo 'if [ "$$linux_os" != "$$macos_os" ] || [ "$$linux_os" != "$$windows_os" ]; then' + echo ' printf "ERROR: exec OS should be the same regardless of target platform\\n" >&2' + echo ' printf " linux target produced: $$linux_os\\n" >&2' + echo ' printf " macos target produced: $$macos_os\\n" >&2' + echo ' printf " windows target produced: $$windows_os\\n" >&2' + echo ' exit 1' + echo 'fi' + } > $@ + """, +) + +sh_test( + name = "platform_test", + srcs = [":platform_test.sh"], +) + +# This target copies the OS name to another file, but this time takes in the +# name via the srcs parameter. We test below that the resulting output matches +# the platform we are targeting. +run_binary( + name = "gen_target_os", + srcs = [":os_name.txt"], + outs = ["target_os.txt"], + args = [ + "$(execpath :os_name.txt)", + "$@", + ], + tags = ["manual"], + tool = ":cat_file", +) + +platform_transition_filegroup( + name = "target_os_linux", + srcs = [":gen_target_os"], + target_platform = ":linux", +) + +platform_transition_filegroup( + name = "target_os_macos", + srcs = [":gen_target_os"], + target_platform = ":macos", +) + +platform_transition_filegroup( + name = "target_os_windows", + srcs = [":gen_target_os"], + target_platform = ":windows", +) + +assert_contains( + name = "linux_target_platform_test", + actual = ":target_os_linux", + expected = "linux", +) + +assert_contains( + name = "macos_target_platform_test", + actual = ":target_os_macos", + expected = "macos", +) + +assert_contains( + name = "windows_target_platform_test", + actual = ":target_os_windows", + expected = "windows", +)