Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ship MinGW-w64 runtime DLLs along with rust-lld.exe for -pc-windows-gnu targets #128876

Merged
merged 1 commit into from
Aug 23, 2024
Merged

Ship MinGW-w64 runtime DLLs along with rust-lld.exe for -pc-windows-gnu targets #128876

merged 1 commit into from
Aug 23, 2024

Conversation

ColinFinck
Copy link
Contributor

@ColinFinck ColinFinck commented Aug 9, 2024 •
edited by ChrisDenton
Loading

rust-lld.exe built for x86_64-pc-windows-gnu depends on libgcc_s_seh-1.dll and libwinpthread-1.dll from MinGW-w64. Until now, they were not shipped alongside rust-lld.exe, and you could not run rust-lld.exe on most systems.

This problem didn't surface until now because:

  • Most targets don't use rust-lld by default.
  • Some people had these DLLs in their PATH from some other MinGW binary.
  • rustup used to add bin to the PATH, which contains these DLLs for rustc.exe. But it no longer does that: rust-lang/rustup@ce3c09a

Fixes #125809

try-job: dist-x86_64-mingw

@rustbot
Copy link
Collaborator

rustbot commented Aug 9, 2024

r? @Kobzol

rustbot has assigned @Kobzol.
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 rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Aug 9, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@ColinFinck ColinFinck force-pushed the rust-lld-with-runtime-dlls branch 2 times, most recently from a39d8ab to 7fbb1cb Compare August 9, 2024 13:17
@ColinFinck ColinFinck mentioned this pull request Aug 9, 2024
Closed
Kobzol
Kobzol reviewed Aug 11, 2024
View reviewed changes
Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bootstrap-wise, these changes look OK, but I don't know a lot about windows-gnu and don't have Windows prepared for testing this.

@jieyouxu could you please try to check it on Windows?

@ChrisDenton
Copy link
Member

cc @mati865

@jieyouxu
Copy link
Member

@jieyouxu could you please try to check it on Windows?

I can try a bit later, but no guarantees to reproduce ci environment

@jieyouxu
Copy link
Member

jieyouxu commented Aug 11, 2024 •
edited
Loading

@Kobzol wait what am I supposed to be testing here? Or rather, what's the canonical example that I should try to build with a x86_64-pc-windows-gnu host compiler that used to fail but would pass with this PR?

@Kobzol
Copy link
Contributor

Kobzol commented Aug 11, 2024

I suppose that the goal is to check whether LLD can be used on windows-gnu if you also meet all the requirements described in this PR's description 😅 @ColinFinck Could you describe some reproduction guide to make it easier to test whether this works as intended on Windows?

@jieyouxu

This comment was marked as resolved.

Sign in to view
@jieyouxu

This comment was marked as resolved.

Sign in to view
@jieyouxu

This comment was marked as resolved.

Sign in to view
@ColinFinck
Copy link
Contributor Author

@Kobzol @jieyouxu To reproduce the problem for the current Rust 1.80.0 release, follow these steps on a Windows Command Prompt (cmd):

rustup default 1.80.0-x86_64-pc-windows-gnu
set PATH=%USERPROFILE%\.cargo\bin

cargo new testproject
cd testproject
set RUSTFLAGS=-Clinker=rust-lld
cargo build

This will show a message box about a DLL not being found, followed by a linking with rust-lld failed: exit code: 0xc0000135 error on the command line.
set PATH is important here to ensure that only Rust-provided DLLs are tried.

My PR fixes this by putting the missing DLLs into the right place.

@jieyouxu

This comment was marked as resolved.

Sign in to view
@bors

This comment was marked as resolved.

Sign in to view
@rustbot rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 13, 2024
@rustbot

This comment was marked as resolved.

Sign in to view
@ColinFinck ColinFinck force-pushed the rust-lld-with-runtime-dlls branch from 30aab08 to f94c872 Compare August 13, 2024 15:48
@rustbot rustbot removed has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 13, 2024
@ColinFinck ColinFinck force-pushed the rust-lld-with-runtime-dlls branch from f94c872 to a29b75a Compare August 13, 2024 15:52
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@ColinFinck ColinFinck mentioned this pull request Aug 13, 2024
Merged
@ColinFinck
Ship MinGW-w64 runtime DLLs along with rust-lld.exe for `-pc-window…
71d98a8 
…s-gnu` targets

`rust-lld.exe` built for `x86_64-pc-windows-gnu` depends on `libgcc_s_seh-1.dll` and `libwinpthread-1.dll` from MinGW-w64.
Until now, they were not shipped alongside `rust-lld.exe`, and you could not run `rust-lld.exe` on most systems.

This problem didn't surface until now because:
* Most targets don't use `rust-lld` by default.
* Some people had these DLLs in their `PATH` from some other MinGW binary.
* `rustup` used to add `bin` to the `PATH`, which contains these DLLs for `rustc.exe`. But it no longer does that: rust-lang/rustup@ce3c09a

Fixes rust-lang#125809
@ColinFinck ColinFinck force-pushed the rust-lld-with-runtime-dlls branch from a29b75a to 71d98a8 Compare August 13, 2024 16:15
@bors
Copy link
Contributor

bors commented Aug 20, 2024

⌛ Trying commit 71d98a8 with merge 7f116c7...

@bors
Copy link
Contributor

bors commented Aug 20, 2024

☀️ Try build successful - checks-actions
Build commit: 7f116c7 (7f116c7f3b8cb1aebb226f1a978e9ebb2fda04a5)

@ChrisDenton
Copy link
Member

ChrisDenton commented Aug 20, 2024 •
edited
Loading

Ok, so if anyone wants to try this then they can install it locally using:

cargo install rustup-toolchain-install-master
rustup-toolchain-install-master 7f116c7f3b8cb1aebb226f1a978e9ebb2fda04a5 -i x86_64-pc-windows-gnu -c cargo rust-std rust-mingw

EDIT: updated the toolchain install command to add missing components.

@mati865
Copy link
Contributor

mati865 commented Aug 20, 2024

Apparently ld.exe doesn't need libgcc_s_*-1.dll for some reason.

It's written in C, so it doesn't pull in libstdc++. LLVM (and therefore LLD) built by Rust has statically linked libstdc++.

@jieyouxu

This comment was marked as resolved.

Sign in to view
@ChrisDenton
Copy link
Member

ChrisDenton commented Aug 21, 2024 •
edited
Loading

I think I missed some needed components from my install instructions. I've updated them. You may need to also use --force to reinstall.

@jieyouxu
Copy link
Member

jieyouxu commented Aug 21, 2024 •
edited
Loading

I think I missed some needed components from my install instructions. I've updated them. You may need to also use --force to reinstall.

I can confirm whatever the CI built Works On My Machine™. So this LGTM (insofar as whatever the CI built is what the end users get)!

Repro instructions:

cargo install rustup-toolchain-install-master
rustup-toolchain-install-master 7f116c7f3b8cb1aebb226f1a978e9ebb2fda04a5 -i x86_64-pc-windows-gnu -c cargo rust-std rust-mingw

then in PowerShell

> $env:PATH="$TOOLCHAIN\lib\rustlib\x86_64-pc-windows-gnu\bin;$TOOLCHAIN\bin"
> $env:RUSTFLAGS="-Clinker=rust-lld"
> cargo -vV // <- checked is from built toolchain
> cargo build // a new hello_world project from `cargo new --bin`

seems to work without any missing DLL errors 👍

@jieyouxu
Copy link
Member

jieyouxu commented Aug 21, 2024 •
edited
Loading

Feel free to also r=me.

@ColinFinck
Copy link
Contributor Author

Can also confirm that the CI-built toolchain of this PR does exactly what it's supposed to do and fixes #125809
Would love to see this being merged :)

@ColinFinck
Copy link
Contributor Author

@rustbot ready

@rustbot 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 Aug 22, 2024
@jieyouxu
Copy link
Member

AFAICT this is good to go, therefore:

@bors r=Kobzol,petrochenkov,jieyouxu

@bors
Copy link
Contributor

bors commented Aug 22, 2024

📌 Commit 71d98a8 has been approved by Kobzol,petrochenkov,jieyouxu

It is now in the queue for this repository.

@bors 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 22, 2024
@jieyouxu
Copy link
Member

May the machine gods bless windows-gnu and may this target be free from bugs 🙏

tgross35 added a commit to tgross35/rust that referenced this pull request Aug 22, 2024
@tgross35
Rollup merge of rust-lang#128876 - ColinFinck:rust-lld-with-runtime-d…
75fbcac 
…lls, r=Kobzol,petrochenkov,jieyouxu

Ship MinGW-w64 runtime DLLs along with `rust-lld.exe` for `-pc-windows-gnu` targets

`rust-lld.exe` built for `x86_64-pc-windows-gnu` depends on `libgcc_s_seh-1.dll` and `libwinpthread-1.dll` from MinGW-w64. Until now, they were not shipped alongside `rust-lld.exe`, and you could not run `rust-lld.exe` on most systems.

This problem didn't surface until now because:
* Most targets don't use `rust-lld` by default.
* Some people had these DLLs in their `PATH` from some other MinGW binary.
* `rustup` used to add `bin` to the `PATH`, which contains these DLLs for `rustc.exe`. But it no longer does that: rust-lang/rustup@ce3c09a

Fixes rust-lang#125809

try-job: dist-x86_64-mingw
@tgross35 tgross35 mentioned this pull request Aug 22, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 22, 2024
@bors
Auto merge of rust-lang#129407 - tgross35:rollup-q5vnsge, r=tgross35
be29189 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#126985 (Implement `-Z embed-source` (DWARFv5 source code embedding extension))
 - rust-lang#128349 (Enable `f16` on x86 and x86-64)
 - rust-lang#128876 (Ship MinGW-w64 runtime DLLs along with `rust-lld.exe` for `-pc-windows-gnu` targets)
 - rust-lang#129190 (Add f16 and f128 to tests/ui/consts/const-float-bits-conv.rs)
 - rust-lang#129257 (Allow rust staticlib to work with MSVC's /WHOLEARCHIVE)
 - rust-lang#129386 (Use a LocalDefId in ResolvedArg.)
 - rust-lang#129400 (Update `compiler_builtins` to `0.1.120`)

r? `@ghost`
`@rustbot` modify labels: rollup
@mati865
Copy link
Contributor

mati865 commented Aug 22, 2024

May the machine gods bless windows-gnu and may this target be free from bugs 🙏

Sorry, could not resist: https://youtu.be/EIWHvPP0U64?si=ZpitnEb2lNjQSSJ9

@matthiaskrgr matthiaskrgr mentioned this pull request Aug 23, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 23, 2024
@bors
Auto merge of rust-lang#129443 - matthiaskrgr:rollup-tbgdj0p, r=matth…
087fc49 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#127623 (fix: fs::remove_dir_all: treat internal ENOENT as success)
 - rust-lang#128876 (Ship MinGW-w64 runtime DLLs along with `rust-lld.exe` for `-pc-windows-gnu` targets)
 - rust-lang#129055 (Migrate `x86_64-fortanix-unknown-sgx-lvi` `run-make` test to rmake)
 - rust-lang#129386 (Use a LocalDefId in ResolvedArg.)
 - rust-lang#129400 (Update `compiler_builtins` to `0.1.120`)
 - rust-lang#129414 (Fix extern crates not being hidden with `doc(hidden)`)
 - rust-lang#129417 (Don't trigger refinement lint if predicates reference errors)
 - rust-lang#129433 (Fix a missing import in a doc in run-make-support)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 23, 2024
@bors
Auto merge of rust-lang#129443 - matthiaskrgr:rollup-tbgdj0p, r=matth…
c8b14ba 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#127623 (fix: fs::remove_dir_all: treat internal ENOENT as success)
 - rust-lang#128876 (Ship MinGW-w64 runtime DLLs along with `rust-lld.exe` for `-pc-windows-gnu` targets)
 - rust-lang#129055 (Migrate `x86_64-fortanix-unknown-sgx-lvi` `run-make` test to rmake)
 - rust-lang#129386 (Use a LocalDefId in ResolvedArg.)
 - rust-lang#129400 (Update `compiler_builtins` to `0.1.120`)
 - rust-lang#129414 (Fix extern crates not being hidden with `doc(hidden)`)
 - rust-lang#129417 (Don't trigger refinement lint if predicates reference errors)
 - rust-lang#129433 (Fix a missing import in a doc in run-make-support)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6a1418a into rust-lang:master Aug 23, 2024
7 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 23, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 23, 2024
@rust-timer
Unrolled build for rust-lang#128876
d0b41d8 
Rollup merge of rust-lang#128876 - ColinFinck:rust-lld-with-runtime-dlls, r=Kobzol,petrochenkov,jieyouxu

Ship MinGW-w64 runtime DLLs along with `rust-lld.exe` for `-pc-windows-gnu` targets

`rust-lld.exe` built for `x86_64-pc-windows-gnu` depends on `libgcc_s_seh-1.dll` and `libwinpthread-1.dll` from MinGW-w64. Until now, they were not shipped alongside `rust-lld.exe`, and you could not run `rust-lld.exe` on most systems.

This problem didn't surface until now because:
* Most targets don't use `rust-lld` by default.
* Some people had these DLLs in their `PATH` from some other MinGW binary.
* `rustup` used to add `bin` to the `PATH`, which contains these DLLs for `rustc.exe`. But it no longer does that: rust-lang/rustup@ce3c09a

Fixes rust-lang#125809

try-job: dist-x86_64-mingw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kobzol Kobzol Kobzol left review comments

@jieyouxu jieyouxu jieyouxu approved these changes

@petrochenkov petrochenkov petrochenkov approved these changes

Assignees

@Kobzol Kobzol

@petrochenkov petrochenkov

Labels
O-windows-gnu Toolchain: GNU, Operating system: Windows S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
1.82.0
9 participants
@ColinFinck @rustbot @rust-log-analyzer @ChrisDenton @jieyouxu @Kobzol @bors @mati865 @petrochenkov