| Message ID | 20250121081220.35589-1-jst@mbs-solutions.de |
|---|---|
| State | New |
| Headers | show |
| Series | cargo: remove LD_LIBRARY_PATH wrapper | expand |
On Tue, 2025-01-21 at 09:12 +0100, Jan Strater-Büddefeld via lists.openembedded.org wrote: > Fixes [YOCTO #15579] > > This commit removes the LD_LIBRARY_PATH wrapper around `cargo`. > Setting the LD_LIBRARY_PATH causes many problems. > Some build scripts will not run because the build scripts execute > binaries from the host system that are not compatible with the target > libraries. > Even a simple `cargo help build` can fail because it uses `less` > internally, which should not be linked against the target libraries. > > There might be cases where the LD_LIBRARY_PATH is needed for some hosts, > like the case described in 388e7cac9f90e79ce8c3c1683d8ee0f4df1bc907 > but one can always set LD_LIBRARY_PATH manually if needed. > > A current working workaround without this commit would be always using > `cargo.real` instead of `cargo`, which can lead to confusion. > > Signed-off-by: Jan Strater-Büddefeld <jst@mbs-solutions.de> > --- > meta/recipes-devtools/rust/cargo_1.81.0.bb | 8 -------- > 1 file changed, 8 deletions(-) That wrapper was added for a reason. Is that reason still valid and how do you plan to mitigate against the issues it was originally added for? The commit message needs to mention this. Cheers, Richard
Am 21.01.25 um 09:55 schrieb Richard Purdie:
On Tue, 2025-01-21 at 09:12 +0100, Jan Strater-Büddefeld via lists.openembedded.org wrote:
Fixes [YOCTO #15579]
This commit removes the LD_LIBRARY_PATH wrapper around `cargo`.
Setting the LD_LIBRARY_PATH causes many problems.
Some build scripts will not run because the build scripts execute
binaries from the host system that are not compatible with the target
libraries.
Even a simple `cargo help build` can fail because it uses `less`
internally, which should not be linked against the target libraries.
There might be cases where the LD_LIBRARY_PATH is needed for some hosts,
like the case described in 388e7cac9f90e79ce8c3c1683d8ee0f4df1bc907
but one can always set LD_LIBRARY_PATH manually if needed.
A current working workaround without this commit would be always using
`cargo.real` instead of `cargo`, which can lead to confusion.
Signed-off-by: Jan Strater-Büddefeld <jst@mbs-solutions.de><mailto:jst@mbs-solutions.de>
---
meta/recipes-devtools/rust/cargo_1.81.0.bb | 8 --------
1 file changed, 8 deletions(-)
That wrapper was added for a reason. Is that reason still valid and how
do you plan to mitigate against the issues it was originally added for?
The commit message needs to mention this.
Cheers,
Richard
Sorry for not being clear enough.
Cargo still doesn't set LD_LIBRARY_PATH to /lib/, so the issue remains. On the other hand, Cargo itself only sets LD_LIBRARY_PATH for some commands if needed, and not always like the wrapper does.
As mentioned in the commit message, I would argue that setting the environment variable always is wrong, and one should instead set LD_LIBRARY_PATH manually if needed.
Another, maybe better, approach would be to remove the wrapper and patch Cargo itself, but I was not able to do so and also don't know what side effects that will have.
Kindly regards,
Jan
diff --git a/meta/recipes-devtools/rust/cargo_1.81.0.bb b/meta/recipes-devtools/rust/cargo_1.81.0.bb index 123032cdf7..091ac3ae97 100644 --- a/meta/recipes-devtools/rust/cargo_1.81.0.bb +++ b/meta/recipes-devtools/rust/cargo_1.81.0.bb @@ -44,14 +44,6 @@ do_install () { install -m 755 "${B}/target/${CARGO_TARGET_SUBDIR}/cargo" "${D}${bindir}" } -do_install:append:class-nativesdk() { - # To quote the cargo docs, "Cargo also sets the dynamic library path when compiling - # and running binaries with commands like `cargo run` and `cargo test`". Sadly it - # sets to libdir but not base_libdir leading to symbol mismatches depending on the - # host OS. Fully set LD_LIBRARY_PATH to contain both to avoid this. - create_wrapper ${D}/${bindir}/cargo LD_LIBRARY_PATH=${libdir}:${base_libdir} -} - # Disabled due to incompatibility with libgit2 0.28.x (https://github.com/rust-lang/git2-rs/issues/458, https://bugs.gentoo.org/707746#c1) # as shipped by Yocto Dunfell. # According to https://github.com/rust-lang/git2-rs/issues/458#issuecomment-522567539, there are no compatibility guarantees between
Fixes [YOCTO #15579] This commit removes the LD_LIBRARY_PATH wrapper around `cargo`. Setting the LD_LIBRARY_PATH causes many problems. Some build scripts will not run because the build scripts execute binaries from the host system that are not compatible with the target libraries. Even a simple `cargo help build` can fail because it uses `less` internally, which should not be linked against the target libraries. There might be cases where the LD_LIBRARY_PATH is needed for some hosts, like the case described in 388e7cac9f90e79ce8c3c1683d8ee0f4df1bc907 but one can always set LD_LIBRARY_PATH manually if needed. A current working workaround without this commit would be always using `cargo.real` instead of `cargo`, which can lead to confusion. Signed-off-by: Jan Strater-Büddefeld <jst@mbs-solutions.de> --- meta/recipes-devtools/rust/cargo_1.81.0.bb | 8 -------- 1 file changed, 8 deletions(-)