Fix debuginfo compression in bootstrap#158169
Conversation
|
Thanks! @bors r+ |
This comment has been minimized.
This comment has been minimized.
Fix cc rs flag if supported Found in https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/weird.20code.20in.20fill_target_compiler/with/604588655. When `cc-rs` checks if a compiler flag is supported, it tries to read `out_dir` to generate the source file in. However, when this option is not set, then the check silently fails and the flag is considered to be unsupported. Since we didn't set `out_dir`, all such added flags were simply ignored. Normally, it reads `OUT_DIR`, which is fine if `cc-rs` is used within a build script. But if it is used elsewhere, then this is a pretty gnarly footgun in `cc-rs`, because the failure is [completely silent](https://github.com/rust-lang/cc-rs/blob/main/src/lib.rs#L1483). CC @madsmtm or @NobodyXu in case you have thoughts on that :) r? @bjorn3
This comment has been minimized.
This comment has been minimized.
|
💔 Test for b946733 failed: CI. Failed job:
|
|
@bors try jobs=x86_64-gnu-distcheck |
This comment has been minimized.
This comment has been minimized.
Fix cc rs flag if supported try-job: x86_64-gnu-distcheck
|
Thanks, I've opened an issue #1765, I think this is something we should fix. We could use a tempfile instead for it |
I think you meant rust-lang/cc-rs#1765 😉 |
|
@bors r=bjorn3 Let's try again. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
💔 Test for 4c6d7d3 failed: CI. Failed job:
|
1271947 to
4f747ca
Compare
|
@bors try |
This comment has been minimized.
This comment has been minimized.
Fix debuginfo compression in bootstrap try-job: dist-x86_64-msvc try-job: dist-x86_64-apple try-job: dist-x86_64-linux
4f747ca to
9f7569b
Compare
|
@bors try |
|
⌛ Trying commit 9f7569b with merge f2dc3ea… To cancel the try build, run the command Workflow: https://github.com/rust-lang/rust/actions/runs/28464072089 |
Fix debuginfo compression in bootstrap try-job: dist-x86_64-msvc try-job: dist-x86_64-apple try-job: dist-x86_64-linux
|
💥 Test timed out after |
|
The Apple timeout looks spurious. I enabled the compression only on Linux-like OSes, for now. @rustbot ready |
|
@bors r+ |
…, r=bjorn3 Fix debuginfo compression in bootstrap Found through https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/weird.20code.20in.20fill_target_compiler/with/604588655. This PR solves a few issues with debuginfo compression in bootstrap. The main issue was that debuginfo compression through the `-gz` flag wasn't enabled, by mistake. When `cc-rs` checks if a compiler flag is supported, it tries to read `out_dir` to generate the source file in. However, when this option is not set, then the check silently fails and the flag is considered to be unsupported. Since we didn't set `out_dir`, all such added flags were simply ignored. Normally, it reads `OUT_DIR`, which is fine if `cc-rs` is used within a build script. But if it is used elsewhere, then this is a pretty gnarly footgun in `cc-rs`, because the failure is [completely silent](https://github.com/rust-lang/cc-rs/blob/main/src/lib.rs#L1483). CC @madsmtm or @NobodyXu in case you have thoughts on that :) After that was fixed, I had to change the used compression flag from `-gz` to `--compress-debug-sections`, to support both BFD and LLD linkers. And to solve it properly, and allow `dist` CI jobs to configure debuginfo compression for all targets that they build, I added a new bootstrap option to configure debuginfo compression. I wanted the flag to be configurable both globally and per target. At the same time, the flag applies both to C/C++ and Rust. We currently don't have such config area in `bootstrap.toml`, and `[build]` didn't seem like the right place, so I shoved it into `[rust]` (while documenting that it also applies to C/C++). r? @bjorn3 try-job: dist-x86_64-msvc try-job: dist-x86_64-apple try-job: dist-x86_64-linux
…uwer Rollup of 5 pull requests Successful merges: - #158169 (Fix debuginfo compression in bootstrap) - #158613 (Fix getrandom fallback test on platforms with `panic=abort`) - #158620 (Remove skip_norm_w/i/p().def_id with a helper) - #158633 (Remove unnecessary `Clone` derives on resolver types) - #158634 (Add missing `needs_drop` check to `DroplessArena`.)
…, r=bjorn3 Fix debuginfo compression in bootstrap Found through https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/weird.20code.20in.20fill_target_compiler/with/604588655. This PR solves a few issues with debuginfo compression in bootstrap. The main issue was that debuginfo compression through the `-gz` flag wasn't enabled, by mistake. When `cc-rs` checks if a compiler flag is supported, it tries to read `out_dir` to generate the source file in. However, when this option is not set, then the check silently fails and the flag is considered to be unsupported. Since we didn't set `out_dir`, all such added flags were simply ignored. Normally, it reads `OUT_DIR`, which is fine if `cc-rs` is used within a build script. But if it is used elsewhere, then this is a pretty gnarly footgun in `cc-rs`, because the failure is [completely silent](https://github.com/rust-lang/cc-rs/blob/main/src/lib.rs#L1483). CC @madsmtm or @NobodyXu in case you have thoughts on that :) After that was fixed, I had to change the used compression flag from `-gz` to `--compress-debug-sections`, to support both BFD and LLD linkers. And to solve it properly, and allow `dist` CI jobs to configure debuginfo compression for all targets that they build, I added a new bootstrap option to configure debuginfo compression. I wanted the flag to be configurable both globally and per target. At the same time, the flag applies both to C/C++ and Rust. We currently don't have such config area in `bootstrap.toml`, and `[build]` didn't seem like the right place, so I shoved it into `[rust]` (while documenting that it also applies to C/C++). r? @bjorn3 try-job: dist-x86_64-msvc try-job: dist-x86_64-apple try-job: dist-x86_64-linux
…uwer Rollup of 8 pull requests Successful merges: - #150075 (Implement clamp_to) - #158169 (Fix debuginfo compression in bootstrap) - #158397 (delegation: support simplest output `Self` mapping) - #158613 (Fix getrandom fallback test on platforms with `panic=abort`) - #158620 (Remove skip_norm_w/i/p().def_id with a helper) - #158633 (Remove unnecessary `Clone` derives on resolver types) - #158634 (Add missing `needs_drop` check to `DroplessArena`.) - #158647 (Document `strip_circumfix` behavior on overlapping prefix and suffix.)
|
💔 I suspect this PR failed tests as part of a rollup After fixing the problem, consider running a try job for the failed job before re-approving. Link to failure: #158660 (comment) |
|
This pull request was unapproved. This PR was contained in a rollup (#158660), which was unapproved. |
View all comments
Found through https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/weird.20code.20in.20fill_target_compiler/with/604588655.
This PR solves a few issues with debuginfo compression in bootstrap.
The main issue was that debuginfo compression through the
-gzflag wasn't enabled, by mistake.When
cc-rschecks if a compiler flag is supported, it tries to readout_dirto generate the source file in. However, when this option is not set, then the check silently fails and the flag is considered to be unsupported. Since we didn't setout_dir, all such added flags were simply ignored.Normally, it reads
OUT_DIR, which is fine ifcc-rsis used within a build script. But if it is used elsewhere, then this is a pretty gnarly footgun incc-rs, because the failure is completely silent. CC @madsmtm or @NobodyXu in case you have thoughts on that :)After that was fixed, I had to change the used compression flag from
-gzto--compress-debug-sections, to support both BFD and LLD linkers.And to solve it properly, and allow
distCI jobs to configure debuginfo compression for all targets that they build, I added a new bootstrap option to configure debuginfo compression.I wanted the flag to be configurable both globally and per target. At the same time, the flag applies both to C/C++ and Rust. We currently don't have such config area in
bootstrap.toml, and[build]didn't seem like the right place, so I shoved it into[rust](while documenting that it also applies to C/C++).r? @bjorn3
try-job: dist-x86_64-msvc
try-job: dist-x86_64-apple
try-job: dist-x86_64-linux