-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Strip debuginfo when debuginfo is not requested #13257
Conversation
r? @ehuss (rustbot has picked a reviewer for you, use r? to override) |
c50fa19
to
ef67dfb
Compare
ef67dfb
to
e954099
Compare
#[serde(rename_all = "lowercase")] | ||
pub enum Strip { | ||
/// A strip option that is fixed and will not change. | ||
Resolved(StripInner), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would make sense to introduce some kind of generic Resolved/Deferred
wrapper and use it both here and in DebugInfo
?
The FCP has been accepted. |
Thanks! @bors r |
☀️ Test successful - checks-actions |
Update cargo 10 commits in 84976cd699f4aea56cb3a90ce3eedeed9e20d5a5..1cff2ee6b92e0ad3f87c44b70b28f788b2528b3c 2024-01-12 15:55:43 0000 to 2024-01-16 16:56:57 0000 - doc: add a heading to highlight "How to find features enabled on dependencies" (rust-lang/cargo#13305) - fix(cargo-update): `--precise` accept arbitrary git revisions (rust-lang/cargo#13250) - Strip debuginfo when debuginfo is not requested (rust-lang/cargo#13257) - Update ahash dependency to 0.8.7 (rust-lang/cargo#13301) - docs: add more links to pkgid spec chapter (rust-lang/cargo#13298) - fix(metadata): Stabilize id format as PackageIDSpec (rust-lang/cargo#12914) - Introduce `-Zprecise-pre-release` unstable flag (rust-lang/cargo#13296) - Delete sentence about parentheses being unsupported in license (rust-lang/cargo#13292) - Add guidance on setting homepage in manifest (rust-lang/cargo#13293) - Clarify the function of the test options (rust-lang/cargo#13236) r? ghost
I think this is worth mentioning in the docs too? |
I would say that the docs already mentioned this ( |
Fix debuginfo strip when using `--target` This fixes an issue where automatic `strip` of debuginfo added in #13257 wasn't working when the `--target` flag was used. The problem is that the adjustment code was only running in the optimization pass that is done when `--target` is *not* specified. The solution is to just always run the unit graph rebuild. I believe it should be safe to do so, since the adjustments it makes should be conditional on just the scenarios that matter when `--target` is not specified. The downside is that this might be a small performance hit when `--target` is used. Trying to avoid that I think would be quite challenging. Fixes #13617
Do not strip debuginfo by default for MSVC This PR disables the logic which enables debuginfo stripping by default in release mode (#13257) for MSVC, since it causes problems there (rust-lang/rust#122857). I'm not sure if this is the correct way to gate the logic on a specific target triple. The root of the issue is that `-Cstrip=debuginfo` currently behaves like `-Cstrip=symbols` on MSVC, which causes the original optimization to break backtraces on Windows. Ultimately, this should probably be fixed in `rustc`, but that might take some further design work, therefore a faster solution would be to just special case this in Cargo for now, and remove the special case later, once it gets fixed on the `rustc` side. Related issue: rust-lang/rust#122857
Pkgsrc changes: * Adapt checksums and patches. Upstream chnages: Version 1.77.0 (2024-03-21) ========================== - [Reveal opaque types within the defining body for exhaustiveness checking.] (rust-lang/rust#116821) - [Stabilize C-string literals.] (rust-lang/rust#117472) - [Stabilize THIR unsafeck.] (rust-lang/rust#117673) - [Add lint `static_mut_refs` to warn on references to mutable statics.] (rust-lang/rust#117556) - [Support async recursive calls (as long as they have indirection).] (rust-lang/rust#117703) - [Undeprecate lint `unstable_features` and make use of it in the compiler.] (rust-lang/rust#118639) - [Make inductive cycles in coherence ambiguous always.] (rust-lang/rust#118649) - [Get rid of type-driven traversal in const-eval interning] (rust-lang/rust#119044), only as a [future compatiblity lint] (rust-lang/rust#122204) for now. - [Deny braced macro invocations in let-else.] (rust-lang/rust#119062) Compiler -------- - [Include lint `soft_unstable` in future breakage reports.] (rust-lang/rust#116274) - [Make `i128` and `u128` 16-byte aligned on x86-based targets.] (rust-lang/rust#116672) - [Use `--verbose` in diagnostic output.] (rust-lang/rust#119129) - [Improve spacing between printed tokens.] (rust-lang/rust#120227) - [Merge the `unused_tuple_struct_fields` lint into `dead_code`.] (rust-lang/rust#118297) - [Error on incorrect implied bounds in well-formedness check] (rust-lang/rust#118553), with a temporary exception for Bevy. - [Fix coverage instrumentation/reports for non-ASCII source code.] (rust-lang/rust#119033) - [Fix `fn`/`const` items implied bounds and well-formedness check.] (rust-lang/rust#120019) - [Promote `riscv32{im|imafc}-unknown-none-elf` targets to tier 2.] (rust-lang/rust#118704) - Add several new tier 3 targets: - [`aarch64-unknown-illumos`] (rust-lang/rust#112936) - [`hexagon-unknown-none-elf`] (rust-lang/rust#117601) - [`riscv32imafc-esp-espidf`] (rust-lang/rust#119738) - [`riscv32im-risc0-zkvm-elf`] (rust-lang/rust#117958) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Implement `From<&[T; N]>` for `Cow<[T]>`.] (rust-lang/rust#113489) - [Remove special-case handling of `vec.split_off (0)`.](rust-lang/rust#119917) Stabilized APIs --------------- - [`array::each_ref`] (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_ref) - [`array::each_mut`] (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_mut) - [`core::net`] (https://doc.rust-lang.org/stable/core/net/index.html) - [`f32::round_ties_even`] (https://doc.rust-lang.org/stable/std/primitive.f32.html#method.round_ties_even) - [`f64::round_ties_even`] (https://doc.rust-lang.org/stable/std/primitive.f64.html#method.round_ties_even) - [`mem::offset_of!`] (https://doc.rust-lang.org/stable/std/mem/macro.offset_of.html) - [`slice::first_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk) - [`slice::first_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk_mut) - [`slice::split_first_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk) - [`slice::split_first_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk_mut) - [`slice::last_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk) - [`slice::last_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk_mut) - [`slice::split_last_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk) - [`slice::split_last_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk_mut) - [`slice::chunk_by`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by) - [`slice::chunk_by_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by_mut) - [`Bound::map`] (https://doc.rust-lang.org/stable/std/ops/enum.Bound.html#method.map) - [`File::create_new`] (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.create_new) - [`Mutex::clear_poison`] (https://doc.rust-lang.org/stable/std/sync/struct.Mutex.html#method.clear_poison) - [`RwLock::clear_poison`] (https://doc.rust-lang.org/stable/std/sync/struct.RwLock.html#method.clear_poison) Cargo ----- - [Extend the build directive syntax with `cargo::`.] (rust-lang/cargo#12201) - [Stabilize metadata `id` format as `PackageIDSpec`.] (rust-lang/cargo#12914) - [Pull out as `cargo-util-schemas` as a crate.] (rust-lang/cargo#13178) - [Strip all debuginfo when debuginfo is not requested.] (rust-lang/cargo#13257) - [Inherit jobserver from env for all kinds of runners.] (rust-lang/cargo#12776) - [Deprecate rustc plugin support in cargo.] (rust-lang/cargo#13248) Rustdoc ----- - [Allows links in markdown headings.] (rust-lang/rust#117662) - [Search for tuples and unit by type with `()`.] (rust-lang/rust#118194) - [Clean up the source sidebar's hide button.] (rust-lang/rust#119066) - [Prevent JS injection from `localStorage`.] (rust-lang/rust#120250) Misc ---- - [Recommend version-sorting for all sorting in style guide.] (rust-lang/rust#115046) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Add more weirdness to `weird-exprs.rs`.] (rust-lang/rust#119028)
The debuginfo is used by the "bcachefs debug" and "bcachefs list_bkeys" commands. Rust 1.77 [1] changed Cargo's release profile to strip debuginfo by default, but we always want it included. [1] rust-lang/cargo#13257 Signed-off-by: Thomas Bertschinger <[email protected]>
The debuginfo is used by the "bcachefs debug" and "bcachefs list_bkeys" commands. Rust 1.77 [1] changed Cargo's release profile to strip debuginfo by default, but we always want it included. [1] rust-lang/cargo#13257 Signed-off-by: Thomas Bertschinger <[email protected]>
The debuginfo is used by the "bcachefs debug" and "bcachefs list_bkeys" commands. Rust 1.77 [1] changed Cargo's release profile to strip debuginfo by default, but we always want it included. [1] rust-lang/cargo#13257 Signed-off-by: Thomas Bertschinger <[email protected]>
The debuginfo is used by the "bcachefs debug" and "bcachefs list_bkeys" commands. Rust 1.77 [1] changed Cargo's release profile to strip debuginfo by default, but we always want it included. [1] rust-lang/cargo#13257 Signed-off-by: Thomas Bertschinger <[email protected]>
The debuginfo is used by the "bcachefs debug" and "bcachefs list_bkeys" commands. Rust 1.77 [1] changed Cargo's release profile to strip debuginfo by default, but we always want it included. [1] rust-lang/cargo#13257 Signed-off-by: Thomas Bertschinger <[email protected]>
The debuginfo is used by the "bcachefs debug" and "bcachefs list_bkeys" commands. Rust 1.77 [1] changed Cargo's release profile to strip debuginfo by default, but we always want it included. [1] rust-lang/cargo#13257 Signed-off-by: Thomas Bertschinger <[email protected]>
The debuginfo is used by the "bcachefs debug" and "bcachefs list_bkeys" commands. Rust 1.77 [1] changed Cargo's release profile to strip debuginfo by default, but we always want it included. [1] rust-lang/cargo#13257 Signed-off-by: Thomas Bertschinger <[email protected]>
This seems to have changed in rust-lang/cargo#13257. Also do not install libusb-1.0 during setup. This doesn't seem necessary anymore.
This seems to have changed in rust-lang/cargo#13257. Also do not install libusb-1.0 during setup. This doesn't seem necessary anymore.
The debuginfo is used by the "bcachefs debug" and "bcachefs list_bkeys" commands. Rust 1.77 [1] changed Cargo's release profile to strip debuginfo by default, but we always want it included. [1] rust-lang/cargo#13257 Signed-off-by: Thomas Bertschinger <[email protected]>
The debuginfo is used by the "bcachefs debug" and "bcachefs list_bkeys" commands. Rust 1.77 [1] changed Cargo's release profile to strip debuginfo by default, but we always want it included. [1] rust-lang/cargo#13257 Signed-off-by: Thomas Bertschinger <[email protected]>
What does this PR try to resolve?
This PR implements this proposal. It contains a detailed description of the change.
As a summary, this PR modifies Cargo so that if the user doesn't set
strip
explicitly, and debuginfo is not enabled for any package being compiled, Cargo will implicitly setstrip = "debuginfo"
, to strip pre-existing debuginfo coming from the standard library. This reduces the default size of release binaries considerably (~4.5 MiB => ~450 KiB for helloworld on Linux x64).Perhaps we could only add the
-Cstrip
option for the leaf/root target (i.e., a binary), but cargo already passes-Cstrip
to libraries if it's set by[profile.<...>.package.<lib>]
, so it seems harmless.Fixes: #4122
How should we test and review this PR?
Best reviewed commit by commit. There is one commit that fixes an existing related test that was using wrong assertion.
Additional information
The implementation of the deferred option was inspired by
DebugInfo
, which already uses a similar concept.Unresolved questions
Should we also do this on macOS by default? It seems that there can be some issues with
strip
there. If it doesn't work, it basically inhibits compilation in release mode, with no easy way to opt out (unless the user explicitly requests debuginfo, but that's not the same as the previous state).