Fix debuginfo compression in bootstrap

[Kobzol]

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 -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. 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
try-job: dist-various-1
try-job: dist-various-2

[bjorn3]

Thanks!

@bors r+

[rust-bors]

📌 Commit 1bc29dc has been approved by bjorn3

It is now in the queue for this repository.

[rust-bors]

💔 Test for b946733 failed: CI. Failed job:

• auto - x86_64-gnu-distcheck (web logs, enhanced plaintext logs)

[Kobzol]

@bors try jobs=x86_64-gnu-distcheck

[NobodyXu]

Thanks, I've opened an issue #1765, I think this is something we should fix.

We could use a tempfile instead for it

[ChrisDenton]

Thanks, I've opened an issue #1765

I think you meant rust-lang/cc-rs#1765 😉

[rust-bors]

☀️ Try build successful (CI)
Build commit: d269546 (d269546b8320f5f3255b5c7e6268def1a4808374)
Base parent: 4008bbd (4008bbdf34b583ce44a467043aae3703afc0a76f)

[Kobzol]

@bors r=bjorn3

Let's try again.

[rust-bors]

📌 Commit d59253e has been approved by bjorn3

It is now in the queue for this repository.

[Kobzol]

@bors try

[rust-bors]

☀️ Try build successful (CI)
Build commit: 217ff87 (217ff875d701cb36dbe8f2cc525b968e4d2bda9c)
Base parent: c397dae (c397dae808f70caebab1fc4e11b3edf7e59f58c7)

[Kobzol]

Ok, makes sense.

@rustbot ready

[rustbot]

Some changes occurred in src/tools/cargo

cc @ehuss

[Kobzol]

@bors r=bjorn3 rollup=never

Another attempt!

[rust-bors]

📌 Commit 7c1d8b8 has been approved by bjorn3

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 10. This pull request will be tested once the tree is reopened.

[jhpratt]

Threading between rollups

@bors p=4

[bjorn3]

dist-x86_64-illumos has been running for 5 hours now.

[rust-bors]

☀️ Test successful - CI
Approved by: bjorn3
Duration: 5h 4m 42s
Pushing 7148b31 to main...

[Kobzol]

It took way longer to compile LLVM and LLD in the Illumos job, hmm.

[github-actions]

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 4eb6253 (parent) -> 7148b31 (this PR)

Test differences

Show 3 test diffs

Stage 2

• [run-make] tests/run-make/compressed-debuginfo-zstd: ignore (ignored if LLVM wasn't build with zstd for ELF section compression or LLVM is not the default codegen backend) -> pass (J0)

Additionally, 2 doctest diffs were found. These are ignored, as they are noisy.

Job group index

• J0: x86_64-gnu-nopt

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 7148b31956911d5d5496900bf564ca5212bf199b --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

1. dist-x86_64-solaris: 1h 1m -> 3h 42m (+259.7%)
2. dist-x86_64-illumos: 1h 46m -> 4h 59m (+179.8%)
3. dist-sparcv9-solaris: 1h 33m -> 3h 58m (+154.8%)
4. dist-apple-various: 1h 39m -> 3h 25m (+107.1%)
5. dist-aarch64-apple: 1h 25m -> 2h 53m (+102.1%)
6. dist-x86_64-musl: 2h 19m -> 4h 36m (+97.6%)
7. dist-x86_64-freebsd: 1h 22m -> 2h 41m (+95.4%)
8. dist-arm-linux-musl: 1h 43m -> 2h 55m (+70.7%)
9. dist-loongarch64-musl: 1h 30m -> 2h 34m (+69.8%)
10. dist-ohos-armv7: 1h 16m -> 2h 7m (+65.7%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

[rust-timer]

Finished benchmarking commit (7148b31): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary -0.1%, secondary 1.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.1%	[2.0%, 2.4%]	4
Regressions ❌ (secondary)	3.0%	[1.9%, 4.2%]	6
Improvements ✅ (primary)	-1.9%	[-5.6%, -0.5%]	5
Improvements ✅ (secondary)	-2.2%	[-2.3%, -2.1%]	2
All ❌✅ (primary)	-0.1%	[-5.6%, 2.4%]	9

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 489.956s -> 489.438s (-0.11%)
Artifact size: 393.31 MiB -> 388.50 MiB (-1.22%)

[Kobzol]

Let's see if the dist job duration increase stays in the next merged PR. If not, then probably we started passing different flags to the LLVM build, and sccache should catch up. If yes, then something is wrong.

[Kobzol]

It went back (#158740 (comment)), so it was just sccache. Still might be interesting to take a look at exactly what flags have changed.. probably because we now pass out_dir to cc.

[bjorn3]

We now also pass the compression flag when building LLVM, don't we?