-
-
Notifications
You must be signed in to change notification settings - Fork 99
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.
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
feat: chdir_binary is an sh_binary that wraps a tool and does a cd first #251
Closed
Closed
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
25d2a30
feat: chdir_binary is an sh_binary that wraps a tool and does a cd first
alexeagle ef3bb75
refactor: make it a new public api
alexeagle 50ca483
refactor: make it a new public api
alexeagle 67f6b32
feat: add tty_binary
alexeagle add64ec
chore: compile socat from source
alexeagle File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
load("@bazel_skylib//rules:run_binary.bzl", "run_binary") | ||
load("@bazel_skylib//rules:build_test.bzl", "build_test") | ||
load("@aspect_bazel_lib//lib:wrap_binary.bzl", "chdir_binary") | ||
|
||
sh_binary( | ||
name = "fixture", | ||
srcs = ["needs-working-dir.sh"], | ||
) | ||
|
||
chdir_binary( | ||
name = "wrap", | ||
binary = "fixture", | ||
chdir = package_name(), | ||
) | ||
|
||
run_binary( | ||
name = "assert", | ||
outs = ["thing"], | ||
tool = "wrap", | ||
) | ||
|
||
build_test( | ||
name = "test", | ||
targets = ["assert"], | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
#!/usr/bin/env bash | ||
|
||
set -o errexit -o nounset -o pipefail | ||
|
||
if [ "$(dirname $(pwd))" != "wrap_binary" ]; then | ||
echo >&2 "this program must be run with the working directory inside the package, but was $(pwd)" | ||
exit 1 | ||
fi |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
"""Wraps binary rules to make them more compatible with Bazel. | ||
|
||
Currently supports only Bash as the wrapper language, not cmd.exe. | ||
|
||
Future additions might include: | ||
- wrap a binary such that it sees a tty on stdin | ||
- manipulate arguments or environment variables | ||
- redirect stdout/stderr, e.g. to silence buildspam on success | ||
- intercept exit code, e.g. to make an "expect_fail" | ||
- change user, e.g. to deal with containerized build running as root, but tool requires non-root | ||
- intercept signals, e.g. to make a tool behave as a Bazel persistent worker | ||
""" | ||
|
||
load(":paths.bzl", "BASH_RLOCATION_FUNCTION") | ||
load(":utils.bzl", "to_label") | ||
load("@bazel_skylib//rules:write_file.bzl", "write_file") | ||
|
||
def chdir_binary(name, binary, chdir = "$BUILD_WORKSPACE_DIRECTORY", **kwargs): | ||
"""Wrap a *_binary to be executed under a given working directory. | ||
|
||
Note: under `bazel run`, this is similar to the `--run_under "cd $PWD &&"` trick, but is hidden | ||
from the user so they don't need to know about that flag. | ||
|
||
Args: | ||
name: Name of the rule. | ||
binary: Label of an executable target to wrap. | ||
chdir: Argument for the `cd` command. | ||
By default, supports using the binary under `bazel run` by running program in the | ||
root of the Bazel workspace, in the source tree. | ||
**kwargs: Additional named arguments for the resulting sh_binary rule. | ||
""" | ||
|
||
script = "_{}_chdir.sh".format(name) | ||
binary = to_label(binary) | ||
|
||
# It's 2022 and java_binary still cannot be told to cd to the source directory under bazel run. | ||
write_file( | ||
name = "_{}_wrap".format(name), | ||
out = script, | ||
content = [ | ||
"#!/usr/bin/env bash", | ||
BASH_RLOCATION_FUNCTION, | ||
# Remove external/ prefix that is included in $(rootpath) but not supported by $(rlocation) | ||
"bin=$(rlocation ${1#external/})", | ||
# Consume the first argument | ||
"shift", | ||
# Fix the working directory | ||
"cd " + chdir, | ||
# Replace the current process | ||
"exec $bin $@", | ||
], | ||
is_executable = True, | ||
) | ||
|
||
native.sh_binary( | ||
name = name, | ||
srcs = [script], | ||
args = ["$(rootpath {})".format(binary)] + kwargs.pop("args", []), | ||
data = [binary], | ||
deps = ["@bazel_tools//tools/bash/runfiles"], | ||
**kwargs | ||
) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I'm wondering if the
BUILD_WORKSPACE_DIRECTORY
is intuitive. Most Bazel users are accustomed to binaries running either in the execroot or the runfiles root. I feel like defaulting to the source tree could confuse some people.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I agree, also at the use site it's better to make this value explicit.