-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Document variance in strip #13677
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
base: master
Are you sure you want to change the base?
Document variance in strip #13677
Conversation
Two variances in strip will remain, even after MSVC shenanigans settle: - Its default based on profile. - That it can sometimes make backtraces nonsensical. In my experience, people will complain to me if backtraces are fucked up, but sometimes it was their own decision to make the backtrace really bad. Inform them of this. Hopefully this also encourages people to report if their backtraces regress in some platform-dependent way without enabling `strip = "symbols"`, which we would want to know about.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
@@ -117,7 +117,9 @@ strip = "debuginfo" | |||
``` | |||
|
|||
Possible string values of `strip` are `"none"`, `"debuginfo"`, and `"symbols"`. | |||
The default is `"none"`. | |||
The default is `"none"` by default, and `"debuginfo"` in release profile. |
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.
Similar to #13638, I don't think this is entirely correct. The default strip behavior depends on if debug is set (including overrides).
In #13257, we somewhat intentionally did not document this, as it should be a completely internal matter that should be transparent to the user. It is only running strip
to deal with the particular way the standard library is distributed.
I'm not yet entirely convinced this should be documented, since it is a relatively complicated thing to explain to the user (something they shouldn't even need to know about) which could just contribute to confusion.
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.
...Frankly, I don't know why there are even two options if one affects how the other is interpreted.
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.
In any case, I do not believe these behaviors are actually completely internal matters that are transparent to the user, as the effective behavior seems to change wildly based on the target, host, or available binaries, e.g.
strip
binary isPATH
dependent which breaks builds in MacOS rust#123114- 1.77 produced release binaries die w/ SIGKILL on macos 14.4 rust#122902
- Figure out pstack and Rust strip = "symbols" interaction oxidecomputer/helios#147
Perhaps we can make it one with rust-lang/rust#123151 but it isn't now.
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.
@ehuss The current default lying about being "none"
made it much harder today to debug a problem where the compiler is stripping all symbols on MacOS if debug = false
is specified: rust-lang/rust#135028.
I agree with you however that the wording by @workingjubilee is not entirely correct, the default does not depend on the profile, it depends on whether debug
is specified.
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.
@ehuss What should the behavior be when we replace a strip binary with Rust library code?
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.
There isn't actually anything "transparent" here when they can set these options, examine a backtrace, and then set these options again, and examine a different backtrace.
Be aware that stripping `"symbols"` may make backtraces incomprehensible, | ||
depending on the platform. |
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.
For the most part, the cargo documentation doesn't dive into the particulars of explaining the different options and their consequences. That documentation should generally live in the rustc book (so we don't have to duplicate it in two remote places). Adding warnings about the consequences of stripping symbols would probably be appropriate over there (and seems good to include).
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.
It is my experience that people often do not actually click through, simply put.
Would it may be better to just mention that by default cargo will choose the value based on the other options without getting into the details? That makes it clear that when a specific value is needed it should be set explicitly and otherwise using the default is fine. |
I am not sure how to accurately document that. There are two things: The default value for profile inheritance. This is static. And the final value passing to rustc at runtime. |
Two variances in strip will remain, even after MSVC shenanigans settle:
In my experience, people will complain to me if backtraces are fucked up, but sometimes it was their own decision to make the backtrace really bad. Inform them of this. Hopefully this also encourages people to report if their backtraces regress in some platform-dependent way without enabling
strip = "symbols"
, which we would want to know about.