Use `ar_archive_writer` for writing COFF import libs on all backends by ChrisDenton · Pull Request #129164 · rust-lang/rust

ChrisDenton · 2024-08-16T19:12:56Z

This is mostly the same as the llvm backend but with the cranelift version copy/pasted in place of the LLVM library.

try-job: x86_64-msvc
try-job: i686-msvc
try-job: i686-mingw
try-job: aarch64-gnu
try-job: aarch64-apple
try-job: test-various
try-job: armhf-gnu

rustbot · 2024-08-16T19:13:04Z

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-08-16T19:13:06Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

compiler/rustc_codegen_llvm/Cargo.toml

compiler/rustc_codegen_cranelift/src/archive.rs

compiler/rustc_codegen_ssa/src/back/archive.rs

compiler/rustc_codegen_llvm/Cargo.toml

jieyouxu

This seems reasonable to me. Do we want any tests (even just a sanity check) for this (in terms of integration w/ ar_archive_writer)? Or does ar_archive_writer have sufficient tests?

compiler/rustc_codegen_ssa/src/back/archive.rs

jieyouxu · 2024-08-16T19:35:16Z

Since this PR would also close the linked issue, is it possible to have a regression test?

ChrisDenton · 2024-08-16T19:41:11Z

Do we want any tests (even just a sanity check) for this (in terms of integration w/ ar_archive_writer)?

I think this should be covered by our existing raw-dylib tests. They should fail if anything goes wrong.

Since this PR would also close the linked issue, is it possible to have a regression test?

I can try.

rustbot · 2024-08-16T20:09:20Z

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

tests/run-make/msvc-wholearchive/rmake.rs

jieyouxu

Thanks!

jieyouxu · 2024-08-17T02:56:53Z

@bors r+

bors · 2024-08-17T02:56:56Z

📌 Commit c9451ac has been approved by jieyouxu

It is now in the queue for this repository.

Use `ar_archive_writer` for writing COFF import libs on all backends This is mostly the same as the llvm backend but with the cranelift version copy/pasted in place of the LLVM library. Also updates ar_archive_writer to 0.4.0 which fixes rust-lang#129020

Rollup of 9 pull requests Successful merges: - rust-lang#128786 (Detect multiple crate versions on method not found) - rust-lang#128982 (Re-enable more debuginfo tests on Windows) - rust-lang#128989 (Emit an error for invalid use of the linkage attribute) - rust-lang#129115 (Re-enable `dump-ice-to-disk` for Windows) - rust-lang#129164 (Use `ar_archive_writer` for writing COFF import libs on all backends) - rust-lang#129167 (mir/pretty: use `Option` instead of `Either<Once, Empty>`) - rust-lang#129168 (Return correct HirId when finding body owner in diagnostics) - rust-lang#129173 (Fix `is_val_statically_known` for floats) - rust-lang#129185 (Port `run-make/libtest-json/validate_json.py` to Rust) r? `@ghost` `@rustbot` modify labels: rollup

ChrisDenton · 2024-08-17T15:43:21Z

Failed in rollup #129198 (comment)

@bors r-

ChrisDenton · 2024-08-17T15:45:29Z

While I figure out what went wrong, let's make sure x86_64 passes

@bors try

bors · 2024-08-17T15:46:40Z

⌛ Trying commit c9451ac with merge eb9ec80...

Use `ar_archive_writer` for writing COFF import libs on all backends This is mostly the same as the llvm backend but with the cranelift version copy/pasted in place of the LLVM library. Also updates ar_archive_writer to 0.4.0 which fixes rust-lang#129020 try-job: x86_64-msvc

bors · 2024-08-17T17:28:38Z

💔 Test failed - checks-actions

jieyouxu · 2024-08-17T17:51:17Z

... weird

ChrisDenton · 2024-08-17T19:08:50Z

Hm, it seems to work on x86_64 with MSVC's link.exe but it seems there's problems with LLVM and i686. It might be that the need the .idata$3 object file to be just so. It might still be possible to workaround that limitation but not easily.

So I think I'll edit this to disable the comdat option and just land the unification of COFF writing for now. It might be a good idea to land them separately anyway.

jieyouxu · 2024-08-18T02:48:25Z

@bors try

bors · 2024-08-18T02:49:37Z

⌛ Trying commit 0156eb5 with merge f1672b9...

Use `ar_archive_writer` for writing COFF import libs on all backends This is mostly the same as the llvm backend but with the cranelift version copy/pasted in place of the LLVM library. try-job: x86_64-msvc try-job: i686-msvc try-job: i686-mingw try-job: aarch64-gnu try-job: aarch64-apple try-job: test-various try-job: armhf-gnu

bors · 2024-08-18T05:11:37Z

☀️ Try build successful - checks-actions
Build commit: f1672b9 (f1672b955d70942cc9a145a28e1a61cbf003b88b)

jieyouxu · 2024-08-18T05:35:34Z

Thanks!

@bors r+

bors · 2024-08-18T05:35:36Z

📌 Commit 0156eb5 has been approved by jieyouxu

It is now in the queue for this repository.

Rollup of 5 pull requests Successful merges: - rust-lang#129164 (Use `ar_archive_writer` for writing COFF import libs on all backends) - rust-lang#129173 (Fix `is_val_statically_known` for floats) - rust-lang#129185 (Port `run-make/libtest-json/validate_json.py` to Rust) - rust-lang#129203 (Use cnum for extern crate data key) - rust-lang#129221 (Remove JohnTitor from review rotation) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#129164 - ChrisDenton:comdat, r=jieyouxu Use `ar_archive_writer` for writing COFF import libs on all backends This is mostly the same as the llvm backend but with the cranelift version copy/pasted in place of the LLVM library. try-job: x86_64-msvc try-job: i686-msvc try-job: i686-mingw try-job: aarch64-gnu try-job: aarch64-apple try-job: test-various try-job: armhf-gnu

rustbot assigned jieyouxu Aug 16, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 16, 2024

ChrisDenton force-pushed the comdat branch from 6047d9c to 086c26d Compare August 16, 2024 19:15

bjorn3 reviewed Aug 16, 2024

View reviewed changes

compiler/rustc_codegen_llvm/Cargo.toml Outdated Show resolved Hide resolved

bjorn3 reviewed Aug 16, 2024

View reviewed changes

compiler/rustc_codegen_cranelift/src/archive.rs Show resolved Hide resolved

bjorn3 reviewed Aug 16, 2024

View reviewed changes

compiler/rustc_codegen_ssa/src/back/archive.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

bjorn3 reviewed Aug 16, 2024

View reviewed changes

compiler/rustc_codegen_llvm/Cargo.toml Outdated Show resolved Hide resolved

ChrisDenton force-pushed the comdat branch from 9ad3bb1 to f21d635 Compare August 16, 2024 19:32

jieyouxu approved these changes Aug 16, 2024

View reviewed changes

compiler/rustc_codegen_ssa/src/back/archive.rs Outdated Show resolved Hide resolved

rustbot added the A-run-make Area: port run-make Makefiles to rmake.rs label Aug 16, 2024

This comment has been minimized.

Sign in to view

ChrisDenton force-pushed the comdat branch from 6bc4465 to 3b4de45 Compare August 16, 2024 20:15

bjorn3 reviewed Aug 16, 2024

View reviewed changes

tests/run-make/msvc-wholearchive/rmake.rs Outdated Show resolved Hide resolved

ChrisDenton force-pushed the comdat branch from 3b4de45 to c9451ac Compare August 16, 2024 20:31

jieyouxu approved these changes Aug 17, 2024

View reviewed changes

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 Aug 17, 2024

tgross35 mentioned this pull request Aug 17, 2024

Rollup of 9 pull requests #129198

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 17, 2024

This comment has been minimized.

Sign in to view

ChrisDenton force-pushed the comdat branch from c9451ac to f21d635 Compare August 17, 2024 19:09

Always use ar_archive_writer for import libs

0156eb5

ChrisDenton force-pushed the comdat branch from f21d635 to 0156eb5 Compare August 17, 2024 19:10

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 18, 2024

jieyouxu mentioned this pull request Aug 18, 2024

Rollup of 5 pull requests #129225

Merged

bors merged commit c4cf437 into rust-lang:master Aug 18, 2024

rustbot added this to the 1.82.0 milestone Aug 18, 2024

Zalathar mentioned this pull request Feb 7, 2025

Remove dead code from rustc_codegen_llvm and the LLVM wrapper #136653

Merged

Uh oh!

Conversation

ChrisDenton commented Aug 16, 2024 • edited by jieyouxu Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Aug 16, 2024

Uh oh!

rustbot commented Aug 16, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

jieyouxu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jieyouxu commented Aug 16, 2024

Uh oh!

ChrisDenton commented Aug 16, 2024

Uh oh!

rustbot commented Aug 16, 2024

Uh oh!

This comment has been minimized.

Uh oh!

jieyouxu left a comment

Choose a reason for hiding this comment

Uh oh!

jieyouxu commented Aug 17, 2024

Uh oh!

bors commented Aug 17, 2024

Uh oh!

ChrisDenton commented Aug 17, 2024

Uh oh!

ChrisDenton commented Aug 17, 2024

Uh oh!

bors commented Aug 17, 2024

Uh oh!

This comment has been minimized.

bors commented Aug 17, 2024

Uh oh!

jieyouxu commented Aug 17, 2024

Uh oh!

ChrisDenton commented Aug 17, 2024

Uh oh!

jieyouxu commented Aug 18, 2024

Uh oh!

bors commented Aug 18, 2024

Uh oh!

bors commented Aug 18, 2024

Uh oh!

jieyouxu commented Aug 18, 2024

Uh oh!

bors commented Aug 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ChrisDenton commented Aug 16, 2024 •

edited by jieyouxu

Loading