Implement raw-dylib support for windows-gnu #90782

ricobbe · 2021-11-10T20:39:12Z

Add support for #[link(kind = "raw-dylib")] on windows-gnu targets. Work around binutils's linker's inability to read import libraries produced by LLVM by calling out to the binutils dlltool utility to create an import library from a temporary .DEF file; this approach is effectively a slightly refined version of @mati865's earlier attempt at this strategy in PR #88801. (In particular, this attempt at this strategy adds support for #[link_ordinal(...)] as well.)

In support of #58713.

rust-highfive · 2021-11-10T20:39:15Z

r? @michaelwoerister

(rust-highfive has picked a reviewer for you, use r? to override)

ricobbe · 2021-11-10T20:39:35Z

@petrochenkov this may interest you

compiler/rustc_codegen_llvm/src/back/archive.rs

joshtriplett · 2021-11-16T15:00:52Z

One nit in a comment, and also I do think we need better handling for errors writing files. But otherwise this looks good to me.

michaelwoerister · 2021-11-17T12:24:18Z

@joshtriplett, did you review this closely enough to approve it? If not, I'll take a closer look tomorrow.

ricobbe · 2021-11-17T18:32:53Z

General note: I have seen @joshtriplett 's requests for modifications, and I'll look into those. It may be a while before I can get to that, unfortunately, as an unrelated high-priority project has come up at work that is consuming all my bandwidth at the moment. I have no intention of abandoning this PR, but I'm not sure how quickly I'll be able to respond.

ricobbe · 2021-11-23T19:08:22Z

@rustbot label -S-waiting-on-author +S-waiting-on-review

mati865 · 2021-11-24T00:04:03Z

cc @nagisa who left some unresolved comments in the original PR.

compiler/rustc_codegen_llvm/src/back/archive.rs

joshtriplett · 2021-11-24T09:39:22Z

@michaelwoerister Closely enough to post a comment and feedback, but not closely enough to r+ it myself.

michaelwoerister

Thanks for the PR, @ricobbe! This looks great. I appreciate the solid tests.

I left a few comments below.

compiler/rustc_codegen_llvm/src/back/archive.rs

src/test/run-make/raw-dylib-stdcall-ordinal/output.txt

src/test/run-make/raw-dylib-stdcall-ordinal/lib.rs

src/test/ui/feature-gates/feature-gate-raw-dylib.rs

compiler/rustc_codegen_llvm/src/back/archive.rs

ricobbe · 2021-12-02T01:30:51Z

@rustbot label -S-waiting-on-author +S-waiting-on-review

ricobbe · 2021-12-02T01:31:53Z

I went ahead and marked this "waiting on review" even though there's an unresolved comment; I'd like to hear feedback on my response before starting to work on that particular point.

mati865 · 2021-12-02T21:33:58Z

Once if works with dlltool.exe shipped in rust-mingw component (in other words there is no external mingw-w64 toolchain in the PATH) and @nagisa's comments from #88801 are addressed I think this would be good to go.

ricobbe · 2022-01-06T00:59:40Z

@michaelwoerister , @mati865 I'd particularly like to hear your feedback on the question I raised in a comment in rustc_codegen_llvm/src/back/archive.rs, line 453

compiler/rustc_codegen_llvm/src/back/archive.rs

ricobbe · 2022-01-11T16:51:22Z

Are there any other outstanding concerns with this PR? We'd really like to get it merged soon, if that's at all possible.

michaelwoerister · 2022-01-12T09:48:32Z

Looks good to me now. I think it's better to merge it and then do the rustup testing. Trying to coax CI to produce the right binaries before merging seems like an unnecessary complication since this is all still behind a feature flag anyway.

@ricobbe, would you mind squashing the commits? The history has become a bit too messy to merge as is.

mati865 · 2022-01-12T17:08:57Z

I still doubt this will work without external mingw-w64 based toolchain in PATH but given the fact this is nightly feature I agree there is no need to block this PR.
Verifying (and likely fixing) that should block stabilisation though.

michaelwoerister · 2022-01-13T15:22:17Z

@bors r+

bors · 2022-01-13T15:22:18Z

📌 Commit 0cf7fd1 has been approved by michaelwoerister

…woerister Implement raw-dylib support for windows-gnu Add support for `#[link(kind = "raw-dylib")]` on windows-gnu targets. Work around binutils's linker's inability to read import libraries produced by LLVM by calling out to the binutils `dlltool` utility to create an import library from a temporary .DEF file; this approach is effectively a slightly refined version of `@mati865's` earlier attempt at this strategy in PR rust-lang#88801. (In particular, this attempt at this strategy adds support for `#[link_ordinal(...)]` as well.) In support of rust-lang#58713.

…woerister Implement raw-dylib support for windows-gnu Add support for `#[link(kind = "raw-dylib")]` on windows-gnu targets. Work around binutils's linker's inability to read import libraries produced by LLVM by calling out to the binutils `dlltool` utility to create an import library from a temporary .DEF file; this approach is effectively a slightly refined version of ``@mati865's`` earlier attempt at this strategy in PR rust-lang#88801. (In particular, this attempt at this strategy adds support for `#[link_ordinal(...)]` as well.) In support of rust-lang#58713.

…woerister Implement raw-dylib support for windows-gnu Add support for `#[link(kind = "raw-dylib")]` on windows-gnu targets. Work around binutils's linker's inability to read import libraries produced by LLVM by calling out to the binutils `dlltool` utility to create an import library from a temporary .DEF file; this approach is effectively a slightly refined version of ```@mati865's``` earlier attempt at this strategy in PR rust-lang#88801. (In particular, this attempt at this strategy adds support for `#[link_ordinal(...)]` as well.) In support of rust-lang#58713.

ricobbe · 2022-01-18T17:12:31Z

Just checking to make sure I understand the state of this PR: it's been approved, but there have been troubles getting a clean roll-up PR. Is that correct?

Also, just to confirm: no action is needed on my part at the moment, right?

mati865 · 2022-01-18T19:04:43Z

Correct, you can see it's position in the queue over https://bors.rust-lang.org/queue/rust
I think it should land by tomorrow if nothing goes bad.

ricobbe · 2022-01-18T20:14:01Z

Sounds good. Thanks for the confirmation!

…askrgr Rollup of 9 pull requests Successful merges: - rust-lang#90782 (Implement raw-dylib support for windows-gnu) - rust-lang#91150 (Let qpath contain NtTy: `<$:ty as $:ty>::…`) - rust-lang#92425 (Improve SIMD casts) - rust-lang#92692 (Simplify and unify rustdoc sidebar styles) - rust-lang#92780 (Directly use ConstValue for single literals in blocks) - rust-lang#92924 (Delete pretty printer tracing) - rust-lang#93018 (Remove some unused `Ord` derives based on `Span`) - rust-lang#93026 (fix typo in `max` description for f32/f64) - rust-lang#93035 (Fix stdarch submodule pointing to commit outside tree) Failed merges: - rust-lang#92861 (Rustdoc mobile: put out-of-band info on its own line) r? `@ghost` `@rustbot` modify labels: rollup

…dylib, r=michaelwoerister Fix MinGW target detection in raw-dylib LLVM target doesn't have to be the same as Rust target so relying on it is wrong. It was one of concerns in rust-lang#88801 that was not fixed in rust-lang#90782.

rust-highfive assigned michaelwoerister Nov 10, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 10, 2021

This comment has been minimized.

Sign in to view

petrochenkov self-assigned this Nov 11, 2021

apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 11, 2021

joshtriplett reviewed Nov 16, 2021

View reviewed changes

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

wesleywiser mentioned this pull request Nov 17, 2021

Tracking issue for RFC 2627: #[link(kind="raw-dylib")] #58713

Open

15 tasks

michaelwoerister 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 18, 2021

petrochenkov removed their assignment Nov 19, 2021

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 23, 2021

mati865 reviewed Nov 24, 2021

View reviewed changes

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

michaelwoerister reviewed Nov 29, 2021

View reviewed changes

michaelwoerister 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 29, 2021

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 2, 2021

michaelwoerister reviewed Jan 10, 2022

View reviewed changes

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

Call out to binutils' dlltool for raw-dylib on windows-gnu platforms.

0cf7fd1

ricobbe force-pushed the binutils-dlltool branch from 4bb9086 to 0cf7fd1 Compare January 12, 2022 18:27

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 Jan 13, 2022

matthiaskrgr mentioned this pull request Jan 14, 2022

Rollup of 13 pull requests #92878

Closed

matthiaskrgr mentioned this pull request Jan 14, 2022

Rollup of 10 pull requests #92880

Closed

matthiaskrgr mentioned this pull request Jan 14, 2022

Rollup of 8 pull requests #92905

Closed

matthiaskrgr mentioned this pull request Jan 18, 2022

Rollup of 9 pull requests #93048

Merged

bors merged commit dd621a4 into rust-lang:master Jan 19, 2022

rustbot added this to the 1.60.0 milestone Jan 19, 2022

ricobbe deleted the binutils-dlltool branch January 19, 2022 16:30

mati865 mentioned this pull request Feb 25, 2022

Fix MinGW target detection in raw-dylib #94365

Merged

mati865 mentioned this pull request Aug 23, 2022

Stabilize raw-dylib for non-x86 #99916

Merged

michaelwoerister mentioned this pull request Nov 17, 2022

Stabilize raw_dylib (#[link(kind = "raw-dylib")]) #104218

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement raw-dylib support for windows-gnu #90782

Implement raw-dylib support for windows-gnu #90782

ricobbe commented Nov 10, 2021

rust-highfive commented Nov 10, 2021

ricobbe commented Nov 10, 2021

This comment has been minimized.

joshtriplett commented Nov 16, 2021

michaelwoerister commented Nov 17, 2021

ricobbe commented Nov 17, 2021

ricobbe commented Nov 23, 2021

mati865 commented Nov 24, 2021

joshtriplett commented Nov 24, 2021

michaelwoerister left a comment

This comment has been minimized.

ricobbe commented Dec 2, 2021

ricobbe commented Dec 2, 2021

mati865 commented Dec 2, 2021

ricobbe commented Jan 6, 2022

ricobbe commented Jan 11, 2022

michaelwoerister commented Jan 12, 2022

mati865 commented Jan 12, 2022

michaelwoerister commented Jan 13, 2022

bors commented Jan 13, 2022

ricobbe commented Jan 18, 2022

mati865 commented Jan 18, 2022

ricobbe commented Jan 18, 2022

Implement raw-dylib support for windows-gnu #90782

Implement raw-dylib support for windows-gnu #90782

Conversation

ricobbe commented Nov 10, 2021

rust-highfive commented Nov 10, 2021

ricobbe commented Nov 10, 2021

This comment has been minimized.

joshtriplett commented Nov 16, 2021

michaelwoerister commented Nov 17, 2021

ricobbe commented Nov 17, 2021

ricobbe commented Nov 23, 2021

mati865 commented Nov 24, 2021

joshtriplett commented Nov 24, 2021

michaelwoerister left a comment

Choose a reason for hiding this comment

This comment has been minimized.

ricobbe commented Dec 2, 2021

ricobbe commented Dec 2, 2021

mati865 commented Dec 2, 2021

ricobbe commented Jan 6, 2022

ricobbe commented Jan 11, 2022

michaelwoerister commented Jan 12, 2022

mati865 commented Jan 12, 2022

michaelwoerister commented Jan 13, 2022

bors commented Jan 13, 2022

ricobbe commented Jan 18, 2022

mati865 commented Jan 18, 2022

ricobbe commented Jan 18, 2022