Conversation
| next_command(creator, Ok(MockChild::new(exit_status(0), sysroot, ""))); | ||
| next_command(creator, Ok(MockChild::new(exit_status(0), sysroot, ""))); | ||
| let sysroot_and_target_libdir = | ||
| format!("{sysroot}\n{sysroot}/lib/rustlib/x86_64-unknown-linux-gnu/lib"); |
There was a problem hiding this comment.
hardcording x86_64 ?!
sorry
There was a problem hiding this comment.
The function is only used in test code, afaics, and other tests used the same target. That's why I left it. But it's fair, I can fix it.
On second thought, it matches the mocked command output above, so it should probably stay like this.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2852 +/- ##
==========================================
+ Coverage 76.32% 76.44% +0.11%
==========================================
Files 72 72
Lines 40180 40317 +137
==========================================
+ Hits 30669 30821 +152
+ Misses 9511 9496 -15 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
By querying rustc for its actual target-libdir, linux distro provided rust toolchains installed in, for example, lib64 rather than lib are now packaged correctly as well. Additionally filtering down to actually needed files prevents the toolchain from including all of /usr/bin in the toolchain package. As a side effect this should also make the toolchain packages for standalone rust toolchains a bit more compact.
| /// src -> the standard library sources | ||
| /// etc -> the gdb/lldb pretty-printers. | ||
| #[cfg(feature = "dist-client")] | ||
| #[allow(unused)] |
There was a problem hiding this comment.
could we use the same linux cfg as the ToolchainPackager impl instead of #[allow(unused)] on these 3 items?
By querying rustc for its actual target-libdir, linux distro provided rust toolchains installed in, for example, lib64 rather than lib are now packaged correctly as well.
Additionally filtering down to actually needed files prevents the toolchain from including all of /usr/{bin,lib} in the toolchain package.
As a side effect this should also make the toolchain packages for standalone rust toolchains a bit more compact.
I kept a couple longer comments in that explain the choices/reasoning. If you'd rather I remove them before merging that's fine with me, but I figured they would be handy for the review at least.
Related
Potentially fixes #1870 (hard to tell from the error messages, but I also started with gcc working and rust failing).
Partially conflicts/overlaps with #2850 (Fix 4). Similar conclusions, but the approach here also considers lib64. #2850 in turn retains the old behavior as fallback functionality, while the approach here errors out.