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

Build script env is overridden by use_default_shell_env in Bazel 6 #2665

Open
thesayyn opened this issue May 24, 2024 · 9 comments
Open

Build script env is overridden by use_default_shell_env in Bazel 6 #2665

thesayyn opened this issue May 24, 2024 · 9 comments

Comments

@thesayyn
Copy link

We have bumped into an issue where rust_build_script rule sets some custom environment variables alongside the use_default_shell_env.

Attribute in ctx.actions.run use_default_shell_env overrides the env attribute due to bazelbuild/bazel#19317

env = env,
toolchain = None,
# Set use_default_shell_env so that $PATH is set, as tools like cmake may want to probe $PATH for helper tools.
use_default_shell_env = True,

@thesayyn
Copy link
Author

This leads to errors that looks like;

 Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
thread 'main' panicked at external/rules_rust/cargo/cargo_build_script_runner/bin.rs:33:59:
CARGO_MANIFEST_DIR was not set: NotPresent
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@thesayyn
Copy link
Author

Current workaround is to either patch rules_rust to disable use_default_shell_env or use --incompatible_merge_fixed_and_default_shell_env on > Bazel 7

@archshift
Copy link

This issue seems to have started manifesting with version 0.43. Any news on how to mitigate for Bazel 6?

@illicitonion
Copy link
Collaborator

I'd be happy to review a change which added a config option in https://github.com/bazelbuild/rules_rust/blob/main/rust/settings/BUILD.bazel for making this behaviour optional, if someone wanted to prepare one?

@Silcet
Copy link

Silcet commented Jul 15, 2024

Hey @illicitonion I'm currently looking into this issue now and I'd like to make a PR.

I've been doing some debugging and I can see that use_default_shell_env is set to True to pass the PATH to the tool according to the comment on cargo_build_script.bzl:325. However, in cargo_build_script.bzl:164 the default environment is already captured and put into the env variable which is later passed as the running env to the run action.

My suggestion would be to then set the use_default_shell_env = False and let the PATH be passed in the env. I believe this should fix it for all cases.

Also, I've been trying to reproduce the issue using Bazel 6.5.0 and 6.4.0 and trying to build different examples of the repo from crates_universe but I can't reproduce it. However, I can reproduce it easily in my own Bazel environment. Any suggestion of a test or example to use? Otherwise, I'll try to include one on the PR to prevent regression.

@sthornington
Copy link
Contributor

I am running into this too. Honestly I'm so far down the yaks here I don't know if it's worth it, I was originally trying to work on #2859 but I ran into this just trying to get rules_rust upgraded and working as a local repo.

@sthornington
Copy link
Contributor

If people are downgrading to 0.42 to avoid this issue, doesn't that make it a little more serious than "I would welcome a PR" ?

@illicitonion
Copy link
Collaborator

New versions of rules_rust no longer support Bazel 6, and are no longer tested against it, so our official recommendation for Bazel 6 support is: Use a version of rules_rust which works in Bazel 6 (which in this case sounds like it's 0.42).

I'm happy to review something that makes Bazel 6 easier to use, if it doesn't have a large maintenance cost, but as (volunteer!) maintainers, we're not going to invest a lot of effort into Bazel 6 support.

@sthornington
Copy link
Contributor

Aha thanks for the background! I'll see what it would take for us to upgrade Bazel.

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

No branches or pull requests

5 participants