Skip to content

Use split-debuginfo = "unpacked" for debug builds#10516

Merged
bors merged 1 commit into
rust-lang:masterfrom
Alexendoo:split-debuginfo
Mar 24, 2023
Merged

Use split-debuginfo = "unpacked" for debug builds#10516
bors merged 1 commit into
rust-lang:masterfrom
Alexendoo:split-debuginfo

Conversation

@Alexendoo

Copy link
Copy Markdown
Member

On Windows this has no effect as it's unsupported. On macOS the default set by cargo is already unpacked so no effect there either

For Linux it shaves a bit off the rebuild time, for me in the case of a simple touch + cargo build it goes from 12s to 10s

It saves a good amount of disk space too, on aarch64-unknown-linux-gnu it saves 1.2GB for a plain cargo build, 3GB when also running cargo dev and cargo test --no-run -F internal

r? @flip1995

changelog: none

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 17, 2023
@xFrednet

Copy link
Copy Markdown
Contributor

Out of interest, do you know why unpacked is the default on Mac but not on linux? 🤔

@Alexendoo

Copy link
Copy Markdown
Member Author

I'm not sure, I think it would be up to cargo to make that change but I can't see anything about unpacked on linux in their issue list

@flip1995

Copy link
Copy Markdown
Member

@bors r+

That sounds like a really nice improvement. The Clippy target dir is responsible for 9.5% of the file sizes in my $HOME dir. This will improve things :)

@bors

bors commented Mar 22, 2023

Copy link
Copy Markdown
Contributor

📌 Commit 43f8859 has been approved by flip1995

It is now in the queue for this repository.

@bors

bors commented Mar 22, 2023

Copy link
Copy Markdown
Contributor

⌛ Testing commit 43f8859 with merge 4f1ecfe...

bors added a commit that referenced this pull request Mar 22, 2023
Use `split-debuginfo = "unpacked"` for debug builds

On Windows this has no effect as it's unsupported. On macOS the default set by cargo is already unpacked so no effect there either

For Linux it shaves a bit off the rebuild time, for me in the case of a simple `touch` + `cargo build` it goes from 12s to 10s

It saves a good amount of disk space too, on `aarch64-unknown-linux-gnu` it saves 1.2GB for a plain `cargo build`, 3GB when also running `cargo dev` and `cargo test --no-run -F internal`

r? `@flip1995`

changelog: none
@bors

bors commented Mar 22, 2023

Copy link
Copy Markdown
Contributor

💔 Test failed - checks-action_test

@Alexendoo

Copy link
Copy Markdown
Member Author

The extra files in deps confused the mv command, but I've set the integration build to not use split debuginfo since it's easier to just transfer the binaries

@flip1995

Copy link
Copy Markdown
Member

Thanks!

@bors r+

@bors

bors commented Mar 24, 2023

Copy link
Copy Markdown
Contributor

📌 Commit a90e5cc has been approved by flip1995

It is now in the queue for this repository.

@bors

bors commented Mar 24, 2023

Copy link
Copy Markdown
Contributor

⌛ Testing commit a90e5cc with merge d9e42b0...

bors added a commit that referenced this pull request Mar 24, 2023
Use `split-debuginfo = "unpacked"` for debug builds

On Windows this has no effect as it's unsupported. On macOS the default set by cargo is already unpacked so no effect there either

For Linux it shaves a bit off the rebuild time, for me in the case of a simple `touch` + `cargo build` it goes from 12s to 10s

It saves a good amount of disk space too, on `aarch64-unknown-linux-gnu` it saves 1.2GB for a plain `cargo build`, 3GB when also running `cargo dev` and `cargo test --no-run -F internal`

r? `@flip1995`

changelog: none
@flip1995

Copy link
Copy Markdown
Member

@bors retry (yeeting to rustup PR)

@bors

bors commented Mar 24, 2023

Copy link
Copy Markdown
Contributor

⌛ Testing commit a90e5cc with merge 00e9372...

@bors

bors commented Mar 24, 2023

Copy link
Copy Markdown
Contributor

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 00e9372 to master...

@bors bors merged commit 00e9372 into rust-lang:master Mar 24, 2023
@Alexendoo Alexendoo deleted the split-debuginfo branch March 24, 2023 13:58
@klensy

klensy commented May 25, 2023

Copy link
Copy Markdown
Contributor

Funny thing: on my windows machine with this change rustc(or cargo?) forgets to clean 2.2GB of *.o files in deps folder.

@klensy

klensy commented May 25, 2023

Copy link
Copy Markdown
Contributor

Looks like it preserved here, but should this work for x86_64-pc-windows-msvc? Idk:
https://github.com/rust-lang/rust/blob/0b011b7b7e5d1a1737aa3337f01b79fd5f56cf04/compiler/rustc_codegen_ssa/src/back/link.rs#L1368-L1389

@Alexendoo

Copy link
Copy Markdown
Member Author

hmm I see what's happening I think, we use -Zunstable-options to enable internal rustc lints, but that also enables unsupported split-debuginfo profiles

I'll see if upstream will accept another -Z flag for that instead of using -Zunstable-options

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 14, 2023
…nfo, r=b-naber

Don't print unsupported split-debuginfo modes with `-Zunstable-options`

Currently unsupported `split-debuginfo` options are enabled by `-Zunstable-options`, for projects that have `-Zunstable-options` for other reasons this can be [an unexpected interaction](rust-lang/rust-clippy#10516 (comment))

This PR makes it so that `--print split-debuginfo -Zunstable-options` doesn't print unsupported modes, so that a cargo config of e.g.

```toml
[profile.dev]
split-debuginfo = "unpacked"
```

Would not cause an unsupported mode to be enabled on `x86_64-pc-windows-msvc`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

6 participants