Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support modern location expansions for run_binary #490

Merged
merged 8 commits into from Apr 30, 2024
Merged

Support modern location expansions for run_binary #490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cddde10
Improve user feedback for run_binary location expansions
UebelAndre Mar 2, 2024
7e0c3ee
Merge branch 'main' into location
UebelAndre Apr 6, 2024
ef59d66
PR feedback and tests
UebelAndre Apr 25, 2024
d0aa3eb
always expand
UebelAndre Apr 25, 2024
5a9c0cd
Merge branch 'main' into location
UebelAndre Apr 25, 2024
3a917ca
Merge branch 'main' into location
UebelAndre Apr 30, 2024
891ddd3
Add a test to verify values of form `$(foo ...)` do not get expanded
tetromino Apr 30, 2024
e0e0118
Mention both `$(execpath ...)` and legacy `$(location ...)` in docs a…
tetromino Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions docs/run_binary_doc.md
View file
Open in desktop
Expand Up @@ -22,10 +22,10 @@ This rule does not require Bash (unlike `native.genrule`).
| 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-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-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-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-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-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-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 `$(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 [`$(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 [`$(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 `$(execpath)` and `$(location)` expansion in `args` and `env`. | <a href="https://bazel.build/concepts/labels">Label</a> | required | |


22 changes: 11 additions & 11 deletions rules/run_binary.bzl
View file
Open in desktop
Expand Up @@ -20,10 +20,10 @@ Runs a binary as a build action. This rule does not require Bash (unlike native.

load("//lib:dicts.bzl", "dicts")

def _impl(ctx):
def _run_binary_impl(ctx):
tool_as_list = [ctx.attr.tool]
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).
# (We can add this feature later if users ask for it.)
Expand All @@ -33,12 +33,12 @@ def _impl(ctx):
# 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
# 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
]
envs = {
# Expand $(location) / $(locations) in the values.
k: ctx.expand_location(v, tool_as_list) if "$(location" in v else v
# Expand $(execpath ...) / $(execpaths ...) / $(location ...) / $(locations ...) in the values.
k: ctx.expand_location(v, tool_as_list)
for k, v in ctx.attr.env.items()
}
ctx.actions.run(
Expand All @@ -57,7 +57,7 @@ def _impl(ctx):
)

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" +
" `native.genrule`).",
attrs = {
Expand All @@ -66,30 +66,30 @@ run_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`.",
" `$(execpath)` and `$(location)` expansion in `args` and `env`.",
executable = True,
allow_files = True,
mandatory = True,
cfg = "exec",
),
"env": attr.string_dict(
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.",
),
"srcs": attr.label_list(
allow_files = True,
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(
mandatory = True,
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(
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.",
),
},
Expand Down
27 changes: 22 additions & 5 deletions tests/run_binary/BUILD
View file
Open in desktop
Expand Up @@ -24,6 +24,8 @@ write_file(
"arg2=(bar)",
"ENV_LOCATION=(a tests/run_binary/BUILD)",
"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_PATH_BASH=($PATH)",
"ENV_PATH_CMD=(%PATH%)",
Expand Down Expand Up @@ -51,12 +53,14 @@ run_binary(
# Bash-tokenized, and should appear the same for Windows .bat files and Bash
# .sh scripts.
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
# predictable. The path of generated files depends on the target
# platform.
"ENV_LOCATION": "a $(location BUILD)",
"ENV_LOCATIONS": "b\\ $(locations :dummy_srcs)",
"ENV_COMPLEX": "xx/yy \\\"zz",
"ENV_PATH_BASH": "$PATH",
"ENV_PATH_CMD": "%PATH%",
"OUT": "$(location run_script.out)",
Expand All @@ -76,6 +80,8 @@ write_file(
"@echo>>%OUT% arg2=(%2)",
"@echo>>%OUT% ENV_LOCATION=(%ENV_LOCATION%)",
"@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_PATH_BASH=(%ENV_PATH_BASH%)",
"@echo>>%OUT% ENV_PATH_CMD=(%ENV_PATH_CMD%)",
Expand All @@ -86,6 +92,8 @@ write_file(
"echo >> \"$OUT\" \"arg2=($2)\"",
"echo >> \"$OUT\" \"ENV_LOCATION=($ENV_LOCATION)\"",
"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_PATH_BASH=($ENV_PATH_BASH)\"",
"echo >> \"$OUT\" \"ENV_PATH_CMD=($ENV_PATH_CMD)\"",
Expand Down Expand Up @@ -114,9 +122,13 @@ write_file(
"arg5=(tests/run_binary/BUILD)",
"arg6=(tests/run_binary/BUILD tests/run_binary/printargs.cc)",
"arg7=('tests/run_binary/BUILD $tests/run_binary/BUILD')",
"arg8=($PATH)",
"arg9=($$PATH)",
"arg10=(${PATH})",
"arg8=(tests/run_binary/BUILD)",
"arg9=(tests/run_binary/BUILD tests/run_binary/printargs.cc)",
"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.
"",
],
Expand All @@ -136,15 +148,20 @@ run_binary(
"\"c d\"",
"e\\ f",
"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
# platform.
"$(execpath BUILD)",
"$(execpaths :dummy_srcs)",
"'$(execpath BUILD) $$(execpath BUILD)'",
# Test the legacy 'location' expansions
"$(location BUILD)",
"$(locations :dummy_srcs)",
"'$(location BUILD) $$(location BUILD)'",
"$PATH",
"$$PATH",
"${PATH}",
"$(echo hello)",
],
# Not testing any complex envvars here, because we already did in
# "run_script".
Expand Down