rules_rust icon indicating copy to clipboard operation
rules_rust copied to clipboard

Build script env is overridden by use_default_shell_env in Bazel 6

Open thesayyn opened this issue 1 year ago • 9 comments

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 https://github.com/bazelbuild/bazel/issues/19317

https://github.com/bazelbuild/rules_rust/blob/df80ce61e418ea1c45c5bd51f88a440a7fb9ebc9/cargo/private/cargo_build_script.bzl#L316-L319

thesayyn avatar May 24 '24 18:05 thesayyn

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 avatar May 24 '24 18:05 thesayyn

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

thesayyn avatar May 24 '24 18:05 thesayyn

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

archshift avatar Jun 04 '24 22:06 archshift

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?

illicitonion avatar Jun 06 '24 10:06 illicitonion

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.

Silcet avatar Jul 15 '24 12:07 Silcet

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 https://github.com/bazelbuild/rules_rust/issues/2859 but I ran into this just trying to get rules_rust upgraded and working as a local repo.

sthornington avatar Sep 12 '24 15:09 sthornington

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" ?

sthornington avatar Sep 12 '24 15:09 sthornington

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.

illicitonion avatar Sep 12 '24 16:09 illicitonion

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

sthornington avatar Sep 12 '24 16:09 sthornington