Skip to content

Conversation

aaliddell
Copy link

Alternate to #448 that allows still using bash, with the absolute path resolved at templating time, see bazelbuild/rules_rust#3535. Opened as draft for visibility but there may be other scripts that need similar patch and there should also be an error check for repository_ctx.which("bash") returning None if somehow bash is unavailable.

@keith
Copy link
Member

keith commented Aug 15, 2025

probably not great to go this route because folks could easily invalidate the build by this file being different

@fmeum
Copy link
Collaborator

fmeum commented Aug 15, 2025

If we want to embed a path, it should be the one from rules_shell's sh_toolchain. Since this is a repo rule, we would need to add a way to obtain it.

@UebelAndre
Copy link
Contributor

Is there any opposition to the wrappers being converted to pure shell from bash?

If we want to embed a path, it should be the one from rules_shell's sh_toolchain. Since this is a repo rule, we would need to add a way to obtain it.

There's an example of this here https://github.com/bazelbuild/rules_rust/blob/c7f09c03b158a08dcefb22a56da3c41b10acd3c4/util/process_wrapper/private/bootstrap_process_wrapper.bzl#L9-L20

@fmeum
Copy link
Collaborator

fmeum commented Aug 18, 2025

+1 to pure posix sh

@aaliddell
Copy link
Author

Yep suits me fine 👍

@UebelAndre
Copy link
Contributor

#466 it's not quite pure posix shell since there's still uses of dirname, readlink, etc but I think something like this would work and be an improvement.

@aaliddell
Copy link
Author

Superseded by #466

@aaliddell aaliddell closed this Aug 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants