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

Update book: Rust 1.77 changed the default behaviour on debug symbol stripping #13638

Closed
wants to merge 1 commit into from

Conversation

alpitol
Copy link

@alpitol alpitol commented Mar 25, 2024

Rust 1.77 changed the default behaviour on debug symbols stripping. That change seems not to be yet updated to the book, which this PR fixes.

The default value for release builds is not anymore strip = "none", but rather strip = "debuginfo".
@rustbot
Copy link
Collaborator

rustbot commented Mar 25, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (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 (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 25, 2024
Comment on lines 115 to 122
[profile.release]
strip = "debuginfo"
strip = "symbols"
```

Possible string values of `strip` are `"none"`, `"debuginfo"`, and `"symbols"`.
The default is `"none"`.
For debug profiles the default is `"none"`, and for profiles with `debug = false`
the default is `"debuginfo"`.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rust 1.77 changed the default behaviour on debug symbols stripping. That change seems not to be yet updated to the book, which this PR fixes.

I wonder if we need to update the documentation. The idea behind this is to emulate std respecting debug.

@@ -293,7 +294,7 @@ The default settings for the `release` profile are:
opt-level = 3
debug = false
split-debuginfo = '...' # Platform-specific.
strip = "none"
strip = "debuginfo"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems incorrect to me. The default value of strip for release profile is still none.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is strip by default will be equal to none for the release profile if debug is set to false? I read #13677, #13257, #13763, and #13638 but still didn't get it. Thanks in advance!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior of auto-stripping debuginfo is kinda dynamic. The default release profile settings is still static and strip is none when you want to inherit it.

Share these pieces of code for clarity.

fn default_release(trim_paths_enabled: bool) -> Profile {
let trim_paths = trim_paths_enabled.then(|| TomlTrimPathsValue::Object.into());
Profile {
name: InternedString::new("release"),
root: ProfileRoot::Release,
opt_level: InternedString::new("3"),
trim_paths,
..Profile::default()
}
}

impl Default for Profile {
fn default() -> Profile {
Profile {
name: InternedString::new(""),
opt_level: InternedString::new("0"),
root: ProfileRoot::Debug,
lto: Lto::Bool(false),
codegen_backend: None,
codegen_units: None,
debuginfo: DebugInfo::Resolved(TomlDebugInfo::None),
debug_assertions: false,
split_debuginfo: None,
overflow_checks: false,
rpath: false,
incremental: false,
panic: PanicStrategy::Unwind,
strip: Strip::Deferred(StripInner::None),
rustflags: vec![],
trim_paths: None,
}
}
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default release profile settings is still static and strip is none when you want to inherit it.

Hmm.
According to Rust Blog:

Cargo profiles which do not enable debuginfo in outputs (e.g., debug = 0) will enable strip = "debuginfo" by default.

And according to Cargo Book:

[profile.release]
...
debug = false

I made a small test on 1.78.0 and release profile seems indeed have set strip = "debuginfo" by default.
Not sure that behaviour should be, but it seems for me also that documentation is not updated and #13677 make sense.
Or am I missing something here?
Thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The settings are static. The actual flags passed to rustc are deferred later when Cargo see if it can safely strip std or not.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default is still strip=none if you turn on debug for release profile. strip=debuginfo only happens for release profile when debug is off.

But debug is off by default for the release profile, so strip="debuginfo" will take place, no? 🤔
Not sure I get it correctly, but maybe it would be great to add more context to The Cargo Book.
Thanks for the explanation!

@alpitol
Copy link
Author

alpitol commented Mar 25, 2024

Ah, ok. I don't really know how things work in cargo, just thought that this was a small slip of outdated information.
You know it best here, this PR can be closed if this documentation change is not wanted.

@weihanglo
Copy link
Member

That is not anyone's fault. The behavior is a bit hard to explain in one sentence. The default is still strip=none if you turn on debug for release profile. strip=debuginfo only happens for release profile when debug is off. One idea behind the change is let Cargo do what it promises: "0, false, or "none": no debug info at all". Without this change you will still get debuginfo from std even with debug=0. See #4122 (comment).

Thanks for your patch as always. I'll going to close this and hope the explanation help you :)

@ia0
Copy link

ia0 commented May 30, 2024

strip=debuginfo only happens for release profile when debug is off.

The situation is actually more complicated than this, because "debug is off" is from cargo's perspective. When using RUSTFLAGS='-C debuginfo=2' cargo build --release, debuginfo will still be stripped. One has to explicitly override cargo's behavior with RUSTFLAGS='-C debuginfo=2 -C strip=none' cargo build --release.

That's not an issue, but just why the situation is even more complicated and difficult to debug.

@weihanglo
Copy link
Member

RUSTFLAGS is a lower level mechanism. The general idea is if a configuration is provided by Cargo, avoid using RUSTFLAGS to do it. Cargo doesn't parse nor track the content of RUSTFLAGS.

Such situation also applies to RUSTFLAGS=--target xxx

@ia0
Copy link

ia0 commented May 30, 2024

I was going to write the following:

The difference with --target is that it's a cargo flag, not a configuration in the Cargo.toml, so it's easy to pass on the command line. For debug, one would need to generate a new profile on the spot.

But then I realized there's the --config flag. So actually, I can simply --config=profile.release.debug=2 instead of RUSTFLAGS='-C debuginfo=2 -C strip=none'.

Thanks for putting me in the right direction!

@weihanglo
Copy link
Member

Probably a better (?) way to share a set of compiler flags: make it a custom profile in Cargo.toml, and you can specify via --profile myrelease.

@ia0
Copy link

ia0 commented May 30, 2024

Thanks for the suggestion! I indeed considered custom profiles but prefer not to use them because of the combinatoric explosion. I have a tool that invokes cargo. That tool provides options that somehow maps to cargo options (profile options, target, features, etc) and also rust flags that are not cargo options (like link-arg and target-feature, at least as far as I'm aware of they're not cargo options). I don't want to create one profile per possible set of profile options (in particular because I don't own the Cargo.toml file). So being able to create a custom command line for cargo is much more readable and maintenable (in the tool I'm writing).

EDIT: For posterity, here's the PR where I get rid of unnecessary rust flag usage: google/wasefire#501

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants