Migrate run-make/compressed-debuginfo to rmake.rs
#126629
Conversation
rustbot
added
A-run-make
|
This PR modifies cc @jieyouxu |
|
The makefile is quite subtle to handle cases where the optional compression type is not compiled in, as it requires enabling them in llvm before hand and readelf must be recent enough to support some as well (ie, it seems possible the translated test can fail because of missing zstd 🤔 ) |
|
I found the Makefile really strange and I tried to translate what it was doing as best as possible. Do you see other cases I didn't handle correctly? |
|
Yes I also find this test too unclear, but maybe we can wait to improve it until enabling zstd unconditionally. Here, it normalized between unknown and missing cases that I believe was to handle the cases where zstd was missing at build time / present at build time but missing at run time (which is another touchy subtlety of the current implementation), and this may be what’s missing. We may need to test all the combinations to make sure (but I’m on my phone rn :3). Maybe check that just in case? It’s hard to predict the contributors and CI systems that will execute it, and we wouldn’t want the test to fail for some people. The lack of directives to help, and reliance on system readelf with unknown compression support also make the test hard to write in general. Maybe we can make some improvements to the test in #125642 eg use the llvm-readelf so that compression support is known in advance, or maybe add compiletest directives to better check fallback cases with revisions for example. |
| let out = rustc() | ||
| .crate_name("foo") | ||
| .crate_type("lib") | ||
| .emit("obj") | ||
| .arg("-Cdebuginfo=full") | ||
| .arg(&format!("-Zdebuginfo-compression={compression}")) | ||
| .input("foo.rs") | ||
| .run(); | ||
| let stderr = out.stderr_utf8(); | ||
| if stderr.is_empty() { | ||
| cmd("readelf").arg("-t").arg(path("foo.o")).run().assert_stdout_contains(to_find); | ||
| } else { | ||
| assert_contains(&stderr, &format!("unknown debuginfo compression algorithm {compression}")); | ||
| } |
There was a problem hiding this comment.
Discussion: I want to say that you may want to run each check inside run_in_tmpdir to prevent the output artifacts from potentially causing the checks to interfere with each other (at present, I think both check invocations work on the same foo.o). Don't think this is actually happening, but yeah.
|
@rustbot author |
rustbot
added
S-waiting-on-author
GuillaumeGomez
force-pushed
the
migrate-run-make-compressed-debuginfo
branch
from
cca9b2e
to
62431b7
Compare
|
Added FIXMEs and made test run in |
|
@rustbot ready |
rustbot
added
S-waiting-on-review
|
@bors r+ |
bors
added
S-waiting-on-bors
…mpressed-debuginfo, r=jieyouxu Migrate `run-make/compressed-debuginfo` to `rmake.rs` Part of rust-lang#121876. r? `@jieyouxu`
Rollup of 7 pull requests Successful merges: - rust-lang#124807 (Migrate `run-make/rustdoc-io-error` to `rmake.rs`) - rust-lang#126095 (Migrate `link-args-order`, `ls-metadata` and `lto-readonly-lib` `run-make` tests to `rmake`) - rust-lang#126308 (Ban `ArrayToPointer` and `MutToConstPointer` from runtime MIR) - rust-lang#126620 (Actually taint InferCtxt when a fulfillment error is emitted) - rust-lang#126629 (Migrate `run-make/compressed-debuginfo` to `rmake.rs`) - rust-lang#126644 (Rewrite `extern-flag-rename-transitive`. `debugger-visualizer-dep-info`, `metadata-flag-frobs-symbols`, `extern-overrides-distribution` and `forced-unwind-terminate-pof` `run-make` tests to rmake) - rust-lang#126650 (Rename a bunch of things in the new solver and `rustc_type_ir`) r? `@ghost` `@rustbot` modify labels: rollup
…mpressed-debuginfo, r=jieyouxu Migrate `run-make/compressed-debuginfo` to `rmake.rs` Part of rust-lang#121876. r? ``@jieyouxu``
…mpressed-debuginfo, r=jieyouxu Migrate `run-make/compressed-debuginfo` to `rmake.rs` Part of rust-lang#121876. r? ```@jieyouxu```
Rollup of 9 pull requests Successful merges: - rust-lang#126095 (Migrate `link-args-order`, `ls-metadata` and `lto-readonly-lib` `run-make` tests to `rmake`) - rust-lang#126534 (Migrate `run-make/comment-section` to `rmake.rs`) - rust-lang#126620 (Actually taint InferCtxt when a fulfillment error is emitted) - rust-lang#126629 (Migrate `run-make/compressed-debuginfo` to `rmake.rs`) - rust-lang#126644 (Rewrite `extern-flag-rename-transitive`. `debugger-visualizer-dep-info`, `metadata-flag-frobs-symbols`, `extern-overrides-distribution` and `forced-unwind-terminate-pof` `run-make` tests to rmake) - rust-lang#126650 (Rename a bunch of things in the new solver and `rustc_type_ir`) - rust-lang#126698 (Migrate `unknown-mod-stdin`, `issue-68794-textrel-on-minimal-lib`, `raw-dylib-cross-compilation` and `used-cdylib-macos` `run-make` tests to rmake) - rust-lang#126703 (reword the hint::blackbox non-guarantees) - rust-lang#126708 (Minimize `can_begin_literal_maybe_minus` usage) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#126095 (Migrate `link-args-order`, `ls-metadata` and `lto-readonly-lib` `run-make` tests to `rmake`) - rust-lang#126629 (Migrate `run-make/compressed-debuginfo` to `rmake.rs`) - rust-lang#126644 (Rewrite `extern-flag-rename-transitive`. `debugger-visualizer-dep-info`, `metadata-flag-frobs-symbols`, `extern-overrides-distribution` and `forced-unwind-terminate-pof` `run-make` tests to rmake) - rust-lang#126735 (collect attrs in const block expr) - rust-lang#126737 (Remove `feature(const_closures)` from libcore) - rust-lang#126740 (add `needs-unwind` to UI test) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#126629 - GuillaumeGomez:migrate-run-make-compressed-debuginfo, r=jieyouxu Migrate `run-make/compressed-debuginfo` to `rmake.rs` Part of rust-lang#121876. r? ````@jieyouxu````
Part of #121876.
r? @jieyouxu