bazel
/
bazel-skylib
mirror of https://github.com/bazelbuild/bazel-skylib
2
0
Fork 0
Code Issues Releases Activity

Support modern location expansions for run_binary (#490) 

A common point of confusion I see around `run_binary` is that it's hard coded to only expand `$(location` values which in codebases I work in are otherwise completely eliminated due to it being described as "legacy"

> location: A synonym for either execpath or rootpath, depending on the attribute being expanded. This is legacy pre-Starlark behavior and not recommended unless you really know what it does for a particular rule. See [#2475](https://github.com/bazelbuild/bazel/issues/2475#issuecomment-339318016) for details.

If `execpath` is used instead as the appropriate alternative, the rule does no do any expansion and fails the action. This change adds support for expanding all available patterns whenever they're provided.
This commit is contained in:
UebelAndre 2024-04-30 14:19:34 -07:00 committed by GitHub
parent 2b546aff9e
commit 16bf90d4ae
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 38 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
docs/run_binary_doc.md
View File

@ -22,10 +22,10 @@ This rule does not require Bash (unlike `native.genrule`).
| Name | Description | Type | Mandatory | Default | | Name | Description | Type | Mandatory | Default |
| :------------- | :------------- | :------------- | :------------- | :------------- | | :------------- | :------------- | :------------- | :------------- | :------------- |
| <a id="run_binary-name"></a>name | A unique name for this target. | <a href="https://bazel.build/concepts/labels#target-names">Name</a> | required | | | <a id="run_binary-name"></a>name | A unique name for this target. | <a href="https://bazel.build/concepts/labels#target-names">Name</a> | required | |
| <a id="run_binary-srcs"></a>srcs | Additional inputs of the action.<br><br>These labels are available for `$(location)` expansion in `args` and `env`. | <a href="https://bazel.build/concepts/labels">List of labels</a> | optional | `[]` | | <a id="run_binary-srcs"></a>srcs | Additional inputs of the action.<br><br>These labels are available for `$(execpath)` and `$(location)` expansion in `args` and `env`. | <a href="https://bazel.build/concepts/labels">List of labels</a> | optional | `[]` |
| <a id="run_binary-outs"></a>outs | Output files generated by the action.<br><br>These labels are available for `$(location)` expansion in `args` and `env`. | List of labels | required | | | <a id="run_binary-outs"></a>outs | Output files generated by the action.<br><br>These labels are available for `$(execpath)` and `$(location)` expansion in `args` and `env`. | List of labels | required | |
| <a id="run_binary-args"></a>args | Command line arguments of the binary.<br><br>Subject to [`$(location)`](https://bazel.build/reference/be/make-variables#predefined_label_variables) expansion. | List of strings | optional | `[]` | | <a id="run_binary-args"></a>args | Command line arguments of the binary.<br><br>Subject to [`$(execpath)` and `$(location)`](https://bazel.build/reference/be/make-variables#predefined_label_variables) expansion. | List of strings | optional | `[]` |
| <a id="run_binary-env"></a>env | Environment variables of the action.<br><br>Subject to [`$(location)`](https://bazel.build/reference/be/make-variables#predefined_label_variables) expansion. | <a href="https://bazel.build/rules/lib/dict">Dictionary: String -> String</a> | optional | `{}` | | <a id="run_binary-env"></a>env | Environment variables of the action.<br><br>Subject to [`$(execpath)` and `$(location)`](https://bazel.build/reference/be/make-variables#predefined_label_variables) expansion. | <a href="https://bazel.build/rules/lib/dict">Dictionary: String -> String</a> | optional | `{}` |
| <a id="run_binary-tool"></a>tool | The tool to run in the action.<br><br>Must be the label of a *_binary rule, of a rule that generates an executable file, or of a file that can be executed as a subprocess (e.g. an .exe or .bat file on Windows or a binary with executable permission on Linux). This label is available for `$(location)` expansion in `args` and `env`. | <a href="https://bazel.build/concepts/labels">Label</a> | required | | | <a id="run_binary-tool"></a>tool | The tool to run in the action.<br><br>Must be the label of a *_binary rule, of a rule that generates an executable file, or of a file that can be executed as a subprocess (e.g. an .exe or .bat file on Windows or a binary with executable permission on Linux). This label is available for `$(execpath)` and `$(location)` expansion in `args` and `env`. | <a href="https://bazel.build/concepts/labels">Label</a> | required | |

22
rules/run_binary.bzl
View File

@ -20,10 +20,10 @@ Runs a binary as a build action. This rule does not require Bash (unlike native.
load("//lib:dicts.bzl", "dicts") load("//lib:dicts.bzl", "dicts")
def _impl(ctx): def _run_binary_impl(ctx):
tool_as_list = [ctx.attr.tool] tool_as_list = [ctx.attr.tool]
args = [ args = [
# Expand $(location) / $(locations) in args. # Expand $(execpath ...) / $(execpaths ...) / $(location ...) / $(locations ...) in args.
# #
# To keep the rule simple, do not expand Make Variables (like *_binary.args usually would). # To keep the rule simple, do not expand Make Variables (like *_binary.args usually would).
# (We can add this feature later if users ask for it.) # (We can add this feature later if users ask for it.)
@ -33,12 +33,12 @@ def _impl(ctx):
# tokenization they would have to write args=["'a b'"] or args=["a\\ b"]. There's no # tokenization they would have to write args=["'a b'"] or args=["a\\ b"]. There's no
# documented tokenization function anyway (as of 2019-05-21 ctx.tokenize exists but is # documented tokenization function anyway (as of 2019-05-21 ctx.tokenize exists but is
# undocumented, see https://github.com/bazelbuild/bazel/issues/8389). # undocumented, see https://github.com/bazelbuild/bazel/issues/8389).
ctx.expand_location(a, tool_as_list) if "$(location" in a else a ctx.expand_location(a, tool_as_list)
for a in ctx.attr.args for a in ctx.attr.args
] ]
envs = { envs = {
# Expand $(location) / $(locations) in the values. # Expand $(execpath ...) / $(execpaths ...) / $(location ...) / $(locations ...) in the values.
k: ctx.expand_location(v, tool_as_list) if "$(location" in v else v k: ctx.expand_location(v, tool_as_list)
for k, v in ctx.attr.env.items() for k, v in ctx.attr.env.items()
} }
ctx.actions.run( ctx.actions.run(
@ -57,7 +57,7 @@ def _impl(ctx):
) )
run_binary = rule( run_binary = rule(
implementation = _impl, implementation = _run_binary_impl,
doc = "Runs a binary as a build action.\n\nThis rule does not require Bash (unlike" + doc = "Runs a binary as a build action.\n\nThis rule does not require Bash (unlike" +
" `native.genrule`).", " `native.genrule`).",
attrs = { attrs = {
@ -66,7 +66,7 @@ run_binary = rule(
" of a rule that generates an executable file, or of a file that can be" + " of a rule that generates an executable file, or of a file that can be" +
" executed as a subprocess (e.g. an .exe or .bat file on Windows or a binary" + " executed as a subprocess (e.g. an .exe or .bat file on Windows or a binary" +
" with executable permission on Linux). This label is available for" + " with executable permission on Linux). This label is available for" +
" `$(location)` expansion in `args` and `env`.", " `$(execpath)` and `$(location)` expansion in `args` and `env`.",
executable = True, executable = True,
allow_files = True, allow_files = True,
mandatory = True, mandatory = True,
@ -74,22 +74,22 @@ run_binary = rule(
), ),
"env": attr.string_dict( "env": attr.string_dict(
doc = "Environment variables of the action.\n\nSubject to " + doc = "Environment variables of the action.\n\nSubject to " +
" [`$(location)`](https://bazel.build/reference/be/make-variables#predefined_label_variables)" + " [`$(execpath)` and `$(location)`](https://bazel.build/reference/be/make-variables#predefined_label_variables)" +
" expansion.", " expansion.",
), ),
"srcs": attr.label_list( "srcs": attr.label_list(
allow_files = True, allow_files = True,
doc = "Additional inputs of the action.\n\nThese labels are available for" + doc = "Additional inputs of the action.\n\nThese labels are available for" +
" `$(location)` expansion in `args` and `env`.", " `$(execpath)` and `$(location)` expansion in `args` and `env`.",
), ),
"outs": attr.output_list( "outs": attr.output_list(
mandatory = True, mandatory = True,
doc = "Output files generated by the action.\n\nThese labels are available for" + doc = "Output files generated by the action.\n\nThese labels are available for" +
" `$(location)` expansion in `args` and `env`.", " `$(execpath)` and `$(location)` expansion in `args` and `env`.",
), ),
"args": attr.string_list( "args": attr.string_list(
doc = "Command line arguments of the binary.\n\nSubject to" + doc = "Command line arguments of the binary.\n\nSubject to" +
" [`$(location)`](https://bazel.build/reference/be/make-variables#predefined_label_variables)" + " [`$(execpath)` and `$(location)`](https://bazel.build/reference/be/make-variables#predefined_label_variables)" +
" expansion.", " expansion.",
), ),
}, },

27
tests/run_binary/BUILD
View File

@ -24,6 +24,8 @@ write_file(
"arg2=(bar)", "arg2=(bar)",
"ENV_LOCATION=(a tests/run_binary/BUILD)", "ENV_LOCATION=(a tests/run_binary/BUILD)",
"ENV_LOCATIONS=(b\\ tests/run_binary/BUILD tests/run_binary/printargs.cc)", "ENV_LOCATIONS=(b\\ tests/run_binary/BUILD tests/run_binary/printargs.cc)",
"ENV_EXECPATH=(a tests/run_binary/BUILD)",
"ENV_EXECPATHS=(b\\ tests/run_binary/BUILD tests/run_binary/printargs.cc)",
"ENV_COMPLEX=(xx/yy \\\"zz)", "ENV_COMPLEX=(xx/yy \\\"zz)",
"ENV_PATH_BASH=($PATH)", "ENV_PATH_BASH=($PATH)",
"ENV_PATH_CMD=(%PATH%)", "ENV_PATH_CMD=(%PATH%)",
@ -51,12 +53,14 @@ run_binary(
# Bash-tokenized, and should appear the same for Windows .bat files and Bash # Bash-tokenized, and should appear the same for Windows .bat files and Bash
# .sh scripts. # .sh scripts.
env = { env = {
"ENV_COMPLEX": "xx/yy \\\"zz",
"ENV_EXECPATH": "a $(execpath BUILD)",
"ENV_EXECPATHS": "b\\ $(execpaths :dummy_srcs)",
# Testing $(location) expansion only on source files so the result is # Testing $(location) expansion only on source files so the result is
# predictable. The path of generated files depends on the target # predictable. The path of generated files depends on the target
# platform. # platform.
"ENV_LOCATION": "a $(location BUILD)", "ENV_LOCATION": "a $(location BUILD)",
"ENV_LOCATIONS": "b\\ $(locations :dummy_srcs)", "ENV_LOCATIONS": "b\\ $(locations :dummy_srcs)",
"ENV_COMPLEX": "xx/yy \\\"zz",
"ENV_PATH_BASH": "$PATH", "ENV_PATH_BASH": "$PATH",
"ENV_PATH_CMD": "%PATH%", "ENV_PATH_CMD": "%PATH%",
"OUT": "$(location run_script.out)", "OUT": "$(location run_script.out)",
@ -76,6 +80,8 @@ write_file(
"@echo>>%OUT% arg2=(%2)", "@echo>>%OUT% arg2=(%2)",
"@echo>>%OUT% ENV_LOCATION=(%ENV_LOCATION%)", "@echo>>%OUT% ENV_LOCATION=(%ENV_LOCATION%)",
"@echo>>%OUT% ENV_LOCATIONS=(%ENV_LOCATIONS%)", "@echo>>%OUT% ENV_LOCATIONS=(%ENV_LOCATIONS%)",
"@echo>>%OUT% ENV_EXECPATH=(%ENV_EXECPATH%)",
"@echo>>%OUT% ENV_EXECPATHS=(%ENV_EXECPATHS%)",
"@echo>>%OUT% ENV_COMPLEX=(%ENV_COMPLEX%)", "@echo>>%OUT% ENV_COMPLEX=(%ENV_COMPLEX%)",
"@echo>>%OUT% ENV_PATH_BASH=(%ENV_PATH_BASH%)", "@echo>>%OUT% ENV_PATH_BASH=(%ENV_PATH_BASH%)",
"@echo>>%OUT% ENV_PATH_CMD=(%ENV_PATH_CMD%)", "@echo>>%OUT% ENV_PATH_CMD=(%ENV_PATH_CMD%)",
@ -86,6 +92,8 @@ write_file(
"echo >> \"$OUT\" \"arg2=($2)\"", "echo >> \"$OUT\" \"arg2=($2)\"",
"echo >> \"$OUT\" \"ENV_LOCATION=($ENV_LOCATION)\"", "echo >> \"$OUT\" \"ENV_LOCATION=($ENV_LOCATION)\"",
"echo >> \"$OUT\" \"ENV_LOCATIONS=($ENV_LOCATIONS)\"", "echo >> \"$OUT\" \"ENV_LOCATIONS=($ENV_LOCATIONS)\"",
"echo >> \"$OUT\" \"ENV_EXECPATH=($ENV_EXECPATH)\"",
"echo >> \"$OUT\" \"ENV_EXECPATHS=($ENV_EXECPATHS)\"",
"echo >> \"$OUT\" \"ENV_COMPLEX=($ENV_COMPLEX)\"", "echo >> \"$OUT\" \"ENV_COMPLEX=($ENV_COMPLEX)\"",
"echo >> \"$OUT\" \"ENV_PATH_BASH=($ENV_PATH_BASH)\"", "echo >> \"$OUT\" \"ENV_PATH_BASH=($ENV_PATH_BASH)\"",
"echo >> \"$OUT\" \"ENV_PATH_CMD=($ENV_PATH_CMD)\"", "echo >> \"$OUT\" \"ENV_PATH_CMD=($ENV_PATH_CMD)\"",
@ -114,9 +122,13 @@ write_file(
"arg5=(tests/run_binary/BUILD)", "arg5=(tests/run_binary/BUILD)",
"arg6=(tests/run_binary/BUILD tests/run_binary/printargs.cc)", "arg6=(tests/run_binary/BUILD tests/run_binary/printargs.cc)",
"arg7=('tests/run_binary/BUILD $tests/run_binary/BUILD')", "arg7=('tests/run_binary/BUILD $tests/run_binary/BUILD')",
"arg8=($PATH)", "arg8=(tests/run_binary/BUILD)",
"arg9=($$PATH)", "arg9=(tests/run_binary/BUILD tests/run_binary/printargs.cc)",
"arg10=(${PATH})", "arg10=('tests/run_binary/BUILD $tests/run_binary/BUILD')",
"arg11=($PATH)",
"arg12=($$PATH)",
"arg13=(${PATH})",
"arg14=($(echo hello))",
# Add trailing newline, as printed by printargs. # Add trailing newline, as printed by printargs.
"", "",
], ],
@ -136,15 +148,20 @@ run_binary(
"\"c d\"", "\"c d\"",
"e\\ f", "e\\ f",
"xx/yy\\ \\\"zz", "xx/yy\\ \\\"zz",
# Testing $(location) expansion only on source files so the result is # Testing $(execpath) expansion only on source files so the result is
# predictable. The path of generated files depends on the target # predictable. The path of generated files depends on the target
# platform. # platform.
"$(execpath BUILD)",
"$(execpaths :dummy_srcs)",
"'$(execpath BUILD) $$(execpath BUILD)'",
# Test the legacy 'location' expansions
"$(location BUILD)", "$(location BUILD)",
"$(locations :dummy_srcs)", "$(locations :dummy_srcs)",
"'$(location BUILD) $$(location BUILD)'", "'$(location BUILD) $$(location BUILD)'",
"$PATH", "$PATH",
"$$PATH", "$$PATH",
"${PATH}", "${PATH}",
"$(echo hello)",
], ],
# Not testing any complex envvars here, because we already did in # Not testing any complex envvars here, because we already did in
# "run_script". # "run_script".