Skip to content

Replace SYSROOT env var with --sysroot flag #2039

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

Closed
daivinhtran opened this issue Jun 29, 2023 · 3 comments
Closed

Replace SYSROOT env var with --sysroot flag #2039

daivinhtran opened this issue Jun 29, 2023 · 3 comments

Comments

@daivinhtran
Copy link
Contributor

daivinhtran commented Jun 29, 2023

rust_toolchain generates a rust sysroot from collection of toolchain components. The sysroot path is then assigned to RUST_SYSROOT make variable and ToolchainInfo.sysroot. When constructing rustc compile action, we set ToolchainInfo.sysroot to SYSROOT env var.

Apparently, SYSROOT env var isn't read by rustc and I suspect it is currently a no-op. To reproduce the no-op of SYSROOT, simply change it to any random string and expect the builds to still succeed.

Per rustc doc, we can set --sysroot to override the system sysroot (i.e. `rustc --print sysroot). I believe this is needed to prevent rustc from managing to escape the Bazel sandbox and seeing unspecified inputs.

As a concrete example, say we have a toolchain defined as

rust_toolchain(
  name = "foo",
  rustc = "prebuilt/bin/rustc",
  rustc_lib = "sysroot_with_stdlibs", # outside of $WORKSPACE/prebuilt
  rust_std = "stdlibs_built_from_source", # outside of $WORKSPACE/prebuilt
)

The system sysroot is

$ ~/prebuilt/bin/rustc --print sysroot
~/prebuilt

Under ~/prebuilt, there are prebuilt stdlibs

<target-tripple>/
  lib/
    libstd-<hash>.rlib|so

So in addition to the system sysroot which always points to prebuilt stdlibs, rustc compile action sets -L to the path of stdlibs built from source. Hence, it fails with

error[E0464]: multiple candidates for `rlib` dependency `std` found
 = note: candidate #1 bazel-built libstd-<hash>.rlib
 = note: candidate #1 prebuilt libstd-<hash>.so.  <------- Unspecified in rust_toolchain foo

Fortunately, we can fix this by setting rustc_flags = ["--sysroot=$(RUST_SYSROOT)"] to the rust_toolchain. However, the error with multiple std candidates can be confusing because we don't specify the prebuilt libstd in rust_toolchain at all. rustc should not be able to escape the Bazel sandbox.

To sort-of fix this issue, one solution is to add --sysroot rustc flag to point to the generated sysroot to avoid this issue.

@UebelAndre
Copy link
Collaborator

I think this would be a great change! Would you be willing to make a pull-request for it where a new build flag is added (experimental_rustc_sysroot_flag or something) that can set this flag instead of setting the environment variable?

@daivinhtran
Copy link
Contributor Author

@UebelAndre I'll be more than happy to contribute a PR. I'm not 100% sure but the fix might also remove the need for the custom handling in #1500. I'll verify that.

@daivinhtran
Copy link
Contributor Author

daivinhtran commented Oct 27, 2023

Hi @UebelAndre . I finally got some time for this issue. Please take a look #2223.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants