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

Pass --strip-debug to GccLinker when building without debuginfo #49212

Merged
merged 1 commit into from
Mar 26, 2018

Conversation

kyrias
Copy link
Contributor

@kyrias kyrias commented Mar 20, 2018

C.f. #46034


This brings a hello-world built by passing rustc no command line options from 2.9M to 592K on Linux.

(This might need to special case MacOS or Windows, not sure if the linkers there support --strip-debug, and there is an annoying lack of dependable docs for the linkers there.)

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 20, 2018
@retep998
Copy link
Member

retep998 commented Mar 20, 2018

This might need to special case MacOS or Windows

For pc-windows-gnu it depends on whether the MinGW linker needs to shell out to another binary to handle stripping, or whether it does it itself, and also whether it is even capable of doing so.

pc-windows-msvc already uses split debuginfo so this is a non-issue for that target.

@kyrias
Copy link
Contributor Author

kyrias commented Mar 20, 2018

It's a linker argument, so it's passed to the linker. Specifically I am not sure whether ld.exe and the MacOS ld supports it, and there's no good docs for either of them that I can find at all. So the only way to find out that I know of would be a compile test.

@petrochenkov
Copy link
Contributor

r? @michaelwoerister probably

@michaelwoerister
Copy link
Member

Looks good, @kyrias! On OSX the flag is called -S. Can't hurt to add it there too.

@kyrias
Copy link
Contributor Author

kyrias commented Mar 21, 2018

Ah, you sure that they don't have --strip-debug as well? I prefer the long and readable names, but the Linux ld has -S as well so we don't need to match on the different platforms either way.

@alexcrichton
Copy link
Member

Are we sure we want to do this? Just because the final compilation doesn't have -g doesn't mean that the intermediate debuginfo is not desired, right?

This indeed does fix a "first impression" of Rust on linux sort of but I think it could hurt legitimate use cases where debuginfo is only applied to intermediate crates, right?

@michaelwoerister
Copy link
Member

Good point, @alexcrichton!

I see two options:

  • We leave things as they are and tell people to run strip if binary size is a concern for them; and point them to @alexcrichton's reasoning.
  • We strip by default (since this is probably the more common case) and add a flag that allows to control this behavior.

@michaelwoerister
Copy link
Member

@kyrias The man page at least does not list the long version.

@kyrias
Copy link
Contributor Author

kyrias commented Mar 23, 2018

I would prefer the latter option, especially since anyone who would want to do it already needs to do some funky custom build system calling rustc directly.

@kyrias kyrias force-pushed the strip-debug-no-debuginfo branch 2 times, most recently from dbd54d4 to 368c66a Compare March 24, 2018 19:17
@kyrias
Copy link
Contributor Author

kyrias commented Mar 24, 2018

I added the boolean -Znever-strip-debug-info now. So now we by default tell the linker to strip the debug info whenever we're building with debuginfo=0, unless -Znever-strip-debug-info is passed to rustc.

@alexcrichton
Copy link
Member

Can the defaults remain as they were? Until we stabilize the option I don't think we should change the default behavior

…out it

Signed-off-by: Johannes Löthberg <johannes@kyriasis.com>
@kyrias
Copy link
Contributor Author

kyrias commented Mar 24, 2018

Done, now you need to pass -Zstrip-debuginfo-if-disabled=yes.

@michaelwoerister
Copy link
Member

Thanks, @kyrias!

@bors r+

@bors
Copy link
Contributor

bors commented Mar 25, 2018

📌 Commit 2f0dd4a has been approved by michaelwoerister

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 25, 2018
@bors
Copy link
Contributor

bors commented Mar 25, 2018

⌛ Testing commit 2f0dd4a with merge d351805...

bors added a commit that referenced this pull request Mar 25, 2018
…ister

Pass --strip-debug to GccLinker when building without debuginfo

C.f. #46034

---

This brings a hello-world built by passing rustc no command line options from 2.9M to 592K on Linux.

(This might need to special case MacOS or Windows, not sure if the linkers there support `--strip-debug`, and there is an annoying lack of dependable docs for the linkers there.)
@bors
Copy link
Contributor

bors commented Mar 26, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing d351805 to master...

@bors bors merged commit 2f0dd4a into rust-lang:master Mar 26, 2018
@kyrias kyrias deleted the strip-debug-no-debuginfo branch March 26, 2018 06:40
@ghost
Copy link

ghost commented Oct 19, 2018

shouldnt this be --strip-all (-s) instead of --strip-debug (-S)?

rust-lang/cargo#4122 (comment)

@mati865
Copy link
Contributor

mati865 commented Oct 19, 2018

@cup for most users -strip-debug is a safer bet, --strip-all isn't good choice for everything as removes things like relocation table.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants