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

Manually moving shared libraries to a multiarch libdir, eg, /usr/lib/x86_64-linux-gnu, breaks after 1.67 #109994

Closed
cheese opened this issue Apr 6, 2023 · 11 comments · Fixed by #110281
Assignees
Labels
C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression.

Comments

@cheese
Copy link

cheese commented Apr 6, 2023

Description

Debian deploys a multiarch schema. For Rust, librustc_driver-*.so and other shared libraries needed for rustc to run are installed to /usr/lib/x86_64-linux-gnu on x86_64. And rustlib directory is installed as /usr/lib/rustlib.

During Rust build, Debian just lets x.py install the shared libraries to /usr/lib, and then manually moves them to /usr/lib/x86_64-linux-gnu.

This works before 1.67.

But since 1.67, rustc will fail to find std crate, since rustc tries to find rustlib directory through a relative path based on librustc_driver-*.so. rustc guesses rustlib directory is /usr/lib/x86_64-linux-gnu/../lib/rustlib aka /usr/lib/lib/rustlib, which does not exist.

To work around this, Ubuntu deploys a downstream patch. But as discussed with @bjorn3, that patch actually breaks what Rust 1.67 trying to get.

I hope Rust upstream can provide a better solution for this installation schema to work without downstream patch.

Version it worked on

It most recently worked on: Rust 1.66.1

Version with regression

1.67

@cheese cheese added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Apr 6, 2023
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Apr 6, 2023
@apiraino
Copy link
Contributor

apiraino commented Apr 6, 2023

Additional context from the linked u.r.l.o forum:

I think improve filesearch::get_or_default_sysroot r=ozkanonur · rust-lang/rust@71a3a48 · GitHub assumes librustc_driver.so will be in $sysroot/lib, while in your case it would be in $sysroot/lib/x86_64-linux-gnu. It should probably use the CFG_LIBDIR_RELATIVE env var set during compilation of rustc. The relevant code existed before this commit but was previously only used for finding the location of codegen backends other than LLVM. As LLVM is the only backend available on stable this didn't cause any issues until this commit which reuses the same logic for finding the sysroot (which in your case is /usr but ends up getting calculated as /usr/lib) Please open an issue to track a fix.

and

The correct fix would be to take cfg!("CFG_LIBDIR_RELATIVE") into account to find the sysroot from the librustc_driver.so location.

cc: @ozkanonur

@onur-ozkan
Copy link
Member

onur-ozkan commented Apr 6, 2023

Thank you for the report. I can take care of it over the weekend.

--

If anyone has time, feel free taking/fixing this before the weekend.

@onur-ozkan
Copy link
Member

Just remember we have already patched sys detection for supporting multiarch systems.

Related commit 2186358

It's added to 1.69.0 milestone.

@cheese
Copy link
Author

cheese commented Apr 8, 2023

Just remember we have already patched sys detection for supporting multiarch systems.

Related commit 2186358

It's added to 1.69.0 milestone.

Oh! The same author of the Ubuntu patch.

@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Apr 8, 2023
@nc7s
Copy link

nc7s commented Apr 12, 2023

Hi, I'm experiencing the same issue when packaging 1.67 for Debian. 2186358 as mentioned by @ozkanonur didn't work. The fix is behind a check dir.ends_with(crate::config::host_triple()), which evaluates to false because host_triple() is x86_64-unknown-linux-gnu on x86_64 and dir is /usr/lib/x86_64-linux-gnu, where Debian installs multi-arch libs for x86_64. I'm testing with a || clause that simply checks against host_triple().replace("-unknown", ""), but open to better solutions.

@onur-ozkan
Copy link
Member

Hi, I'm experiencing the same issue when packaging 1.67 for Debian. 2186358 as mentioned by @ozkanonur didn't work. The fix is behind a check dir.ends_with(crate::config::host_triple()), which evaluates to false because host_triple() is x86_64-unknown-linux-gnu on x86_64 and dir is /usr/lib/x86_64-linux-gnu, where Debian installs multi-arch libs for x86_64. I'm testing with a || clause that simply checks against host_triple().replace("-unknown", ""), but open to better solutions.

How did you test that patch? It's not shipped with 1.67

@nc7s
Copy link

nc7s commented Apr 12, 2023

How did you test that patch? It's not shipped with 1.67

The fix isn't shipped with 1.67, but the problematic code it's for is. I was building 1.68 with this 1.67, met this issue, cherry picked this fix, still met this issue, and figured the check evaluates to false, causing the fix to not take effect.

@onur-ozkan onur-ozkan reopened this Apr 12, 2023
@onur-ozkan onur-ozkan self-assigned this Apr 12, 2023
@onur-ozkan
Copy link
Member

The fix isn't shipped with 1.67, but the problematic code it's for is. I was building 1.68 with this 1.67, met this issue, cherry picked this fix, still met this issue, and figured the check evaluates to false, causing the fix to not take effect.

Thank you for your feedback, I will check it this week.

@nc7s
Copy link

nc7s commented Apr 13, 2023

A deeper look concluded my previous finding partially wrong.

In default_from_rustc_driver_dll(), before the aforementioned check, two levels are unconditionally chopped off from the path of librustc_driver-*.so. On Debian it's currently installed to /usr/lib/x86_64-linux-gnu/librustc_driver-*.so, so after that dir becomes /usr/lib. The check will, of course, evaluate to false, because there's no more arch specific directory on the path.

The comment line "dll will be in one of the following two" may need to be changed to three.

Update: A test that special-case's Debian's situation worked, see https://salsa.debian.org/ncts/rust/-/commit/92d0b8f4aeb38f26803cf11ab8af0cfdb653796b.

@onur-ozkan
Copy link
Member

I have tested this on Debian 11, worked just fine. It's under review progress now.

@nc7s
Copy link

nc7s commented Apr 13, 2023

I see, thanks for the help 🥂 I'll cherry-pick that then.

@bors bors closed this as completed in 7f94b31 Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants