bazelbuild
Explicitly set `LD_LIBRARY_PATH` in `Rustc` actions.
When investigating ways to reduce the time needed to download toolchains (https://github.com/bazelbuild/rules_rust/pull/1163) I found in a couple of cases where rustc was either not using the correct libraries (occurs when using cc_toolchains downloaded by Bazel) or not able to find the ones it needs (occurs when the rust_toolchain is represented by multiple external repositories). This change explicitly sets LD_LIBRARY_PATH to guide rustc to using paths in registered toolchains for finding the necessary files.
Could you explain the concrete cases where rustc used incorrect libraries (which C++ toolchain, which library was misused, which library should have been used instead) and which libraries were not found when rust_toolchain was represented in multiple repositories?
Using LD_LIBRARY_PATH in Bazel is usually not needed and often leads to nonhermetic builds, so I'd like to understand the scenarios involved. Ideally we should be able to tell the linker where to look for libraries using command line flags.
Could you explain the concrete cases where rustc used incorrect libraries (which C++ toolchain, which library was misused, which library should have been used instead) and which libraries were not found when rust_toolchain was represented in multiple repositories?
I have a collection of custom C++ toolchains I use for one of my projects. These are designed to allow me to do cross compilation to a couple of platforms. Before these changes, I was getting build errors when I tried to build for musl platforms and found this was caused by rustc loading libraries (like libc.so.6) from /lib/x86-64-linux-gnu instead of my toolchain.
The use of LD_LIBRARY_PATH here is intended to only point to paths provided by currently registered toolchains, which I think is not going to be an issue for hermetic builds. My expectation is that the use of LD_LIBRARY_PATH on the host (set to something in your environment before you call bazel) will cause problems.
Does my example give you additional info and is my thinking around LD_LIBRARY_PATH sound?
I have a collection of custom C++ toolchains I use for one of my projects. These are designed to allow me to do cross compilation to a couple of platforms. Before these changes, I was getting build errors when I tried to build for musl platforms and found this was caused by rustc loading libraries (like libc.so.6) from /lib/x86-64-linux-gnu instead of my toolchain.
That makes me think that the fix could be on the C++ toolchain side. If you have a custom toolchain you can tell it to set LD_LIBRARY_PATH. We fetch env variables from the C++ toolchain and set it for Rustc here: https://cs.opensource.google/bazel/rules_rust/+/main:rust/private/rustc.bzl;l=309?q=rustc.bzl.
If I were to fix it, I'd probably not use LD_LIBRARY_PATH, but instead check if all -L/-rpath flags in the C++ toolchain are correct. I suspect there are some flags missing. Which linker are you using?
The use of LD_LIBRARY_PATH here is intended to only point to paths provided by currently registered toolchains, which I think is not going to be an issue for hermetic builds. My expectation is that the use of LD_LIBRARY_PATH on the host (set to something in your environment before you call bazel) will cause problems.
Then IMHO it's the job of the toolchain to prepare the correct environment variables, not rustc.bzl. I'll comment on the code to be more precise in a second.
That makes me think that the fix could be on the C++ toolchain side. If you have a custom toolchain you can tell it to set
LD_LIBRARY_PATH. We fetch env variables from the C++ toolchain and set it for Rustc here: https://cs.opensource.google/bazel/rules_rust/+/main:rust/private/rustc.bzl;l=309?q=rustc.bzl.If I were to fix it, I'd probably not use
LD_LIBRARY_PATH, but instead check if all-L/-rpathflags in the C++ toolchain are correct. I suspect there are some flags missing. Which linker are you using?
Ah cool that sounds like a better approach. I can remove the cc_toolchain stuff from this PR.
Then IMHO it's the job of the toolchain to prepare the correct environment variables, not rustc.bzl. I'll comment on the code to be more precise in a second.
I think I agree. Would it make sense to allow rust_toolchain to set environment variables? Or how could I have the toolchain define this information?
Another ping @hlopko, I think the direction this change is going is rust_toolchain should support setting environment variables?
I'm sorry for the delay, I still failed to find time to try to reproduce and wrap my head around rustc sysroot issue. Maybe @krasimirgg will be faster :)
support setting environment variables?
Another ping @hlopko, I think the direction this change is going is
rust_toolchainshould support setting environment variables?
What exactly do you mean? An attribute with env vars that rustc.bzl then reads from the Rust toolchain info and passes them to each actions? That doesn't sound controversial. Do you also want to "parse" LD_LIBRARY_PATH and add things to it automatically?
What exactly do you mean? An attribute with env vars that rustc.bzl then reads from the Rust toolchain info and passes them to each actions? That doesn't sound controversial. Do you also want to "parse"
LD_LIBRARY_PATHand add things to it automatically?
Yeah, that's exactly what I mean. And yeah, I also want the ability to have the toolchain automatically set the generated sysroot path on a LD_LIBRARY_PATH environment variable it would have in it's env attribute.
edit:
I created https://github.com/bazelbuild/rules_rust/pull/1363 as a separate change to allow rust_toolchain to define environment variables since this seemed like a generally useful thing.
@hlopko @krasimirgg I'm trying to figure out how I can update rust_toolchain to conditionally set LD_LIBRARY_PATH to the rust sysroot based on some flag but I struggle to come up with something that does not require a change to the Rustc action. In order to get link_env (https://github.com/bazelbuild/rules_rust/blob/0.5.0/rust/private/rustc.bzl#L311-L315) don't I need a ton of context from a target that's about to be built? The thought there is that if the cc_toolchain sets these variables too, that rust_toolchain would know to either append or prepend it's own flags. Any thoughts there? Perhaps that's unnecessary simply having a boolean toggle is sufficient for setting it or not?
(I apologize for not replying on this, I was on extended leave not checking my emails much)