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.

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

Bazel 7 PATH issues #289505

Open
layus opened this issue Feb 17, 2024 · 3 comments
Open

Bazel 7 PATH issues #289505

layus opened this issue Feb 17, 2024 · 3 comments
Assignees
Labels
0.kind: question Requests for a specific question to be answered 6.topic: best practices

Comments

@layus
Copy link
Member

layus commented Feb 17, 2024

This issue is here to discuss issues with PATH when using bazel 7 as packaged in nix.

When packaging bazel_7, I removed the (action_path patch)[https://github.com/NixOS/nixpkgs/blob/1012b2a368b12c3c5751b06123214de202e28778/pkgs/development/tools/build-managers/bazel/bazel_6/actions_path.patch] from bazel_6, because it did not make sense to have it.

But removing that patch has consequences. Some actions now run with no PATH at all. What I am unable to figure right now is how to fix that.

  1. Does it also fail with vanilla bazel (I think not)
  2. Why does it work with vanilla bazel (probably /usr/bin:/bin/, etc gets used by default by the system or the system shell)
  3. Introduce a fix that is as close as possible to what vanilla bazel users may expect.

It would be nice to setup a MWE to test it on several machines.

AFAIU the issue arises when you run a binary process (that needs no PATH set to run) but that binary ends up running system commands (using system() or calling a bash script) which then fails to find it's dependencies.

As mentioned in some places, we may want to tweak that path without having to recompile bazel.

See
#262152
#285544
https://github.com/sourcegraph/sourcegraph/pull/59359
and possibly some more I have forgotten.

/cc @malt3 @uri-canva @Strum355 @flurie @boltzmannrain @rickvanprim as you might be interested in this discussion.

@layus layus added 0.kind: question Requests for a specific question to be answered 6.topic: best practices labels Feb 17, 2024
@layus layus self-assigned this Feb 17, 2024
@layus layus mentioned this issue Feb 17, 2024
13 tasks
@malt3
Copy link
Contributor

malt3 commented Feb 19, 2024

I added a reproducer: https://github.com/malt3/bazel-path-issues-nix

This is a simplified form of the rules_oci oci_image rule.

@Strum355
Copy link
Contributor

Strum355 commented Feb 19, 2024

the action_path patch is imo a double edged sword. In one sense, it (usually) hid the fact that --action_env=PATH=... isnt always propagated due to use_default_shell_env being generally set to False for various historical (aka no longer relevant) reasons[1], by containing the most likely set of binaries one would need. The flip side being its completely non-obvious why things aren't picking up your specifically set action_env PATH, or how to address it (do I patch a rule? do I patch the bazel derivation? does a user even know where to start looking?).

I would almost be tempted to default use_default_shell_env to True instead of False that it is today, but it may have unexpected knock-on effects to actions that start seeing the "few OS-dependent variables". https://github.com/bazelbuild/bazel/blob/1cbf09dd45a4bb4cc73ace58ae344bb90e709752/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkActionFactoryApi.java#L416

I may have some more thoughts on this, but Id need to formulate them in my mind first 😄

[1]: previously, setting use_default_shell_env on a rule would result in env of the same rule being ignored.

@alexflurieagility
Copy link
Contributor

@Strum355 I wonder if use_default_shell_env should get a trinary value like stamping that defaults to the value of --incompatible_merge_fixed_and_default_shell_env? It would still run the risk of breaking some random workflows, but it would have a fairly simple mitigation.

malt3 added a commit to malt3/rules_ts that referenced this issue Jun 4, 2024
Aspect uses shell wrappers that invoke tools like `dirname` and `uname`.
When the default shell env is not inherited, this prevents tools to be
found.

See also: NixOS/nixpkgs#289505
malt3 added a commit to malt3/rules_ts that referenced this issue Jun 4, 2024
Aspect uses shell wrappers that invoke tools like `dirname` and `uname`.
When the default shell env is not inherited, this prevents tools to be
found.

See also: NixOS/nixpkgs#289505
malt3 added a commit to malt3/rules_ts that referenced this issue Jun 4, 2024
Aspect uses shell wrappers that invoke tools like `dirname` and `uname`.
When the default shell env is not inherited, this prevents tools to be
found.

See also: NixOS/nixpkgs#289505
malt3 added a commit to malt3/rules_ts that referenced this issue Jun 4, 2024
Aspect uses shell wrappers that invoke tools like `dirname` and `uname`.
When the default shell env is not inherited, this prevents tools from
being found on systems without a working fallback $PATH (like NixOS).

See also: NixOS/nixpkgs#289505
malt3 added a commit to malt3/rules_esbuild that referenced this issue Jun 4, 2024
Aspect uses shell wrappers that invoke tools like `dirname` and `uname`.
When the default shell env is not inherited, this prevents tools from
being found on systems without a working fallback $PATH (like NixOS).

See also: NixOS/nixpkgs#289505
malt3 added a commit to malt3/rules_esbuild that referenced this issue Jun 4, 2024
Aspect uses shell wrappers that invoke tools like `dirname` and `uname`.
When the default shell env is not inherited, this prevents tools from
being found on systems without a working fallback $PATH (like NixOS).

See also: NixOS/nixpkgs#289505
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: question Requests for a specific question to be answered 6.topic: best practices
Projects
None yet
Development

No branches or pull requests

4 participants