Respect config.lld_enabled when building windows-gnu.

[jfgoog]

We (Android toolchain) set config.lld_enabled to false for our Linux and Darwin builds of rustc because we maintain our linker separately and don't want an extra copy of it. We'd like to build a Windows version of rustc, and preserve the same behavior.

This change is essentially regrouping/moving code so we can wrap most of it in an if block.

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ozkanonur (or someone else) soon.

Please see the contribution instructions for more information.

[onur-ozkan]

r? @albertlarsan68

[albertlarsan68]

LGTM, but I would like to have @Mark-Simulacrum take a look.

[mati865]

I'm in favor of change like that (so MSYS2 could drop https://github.com/msys2/MINGW-packages/blob/eea9ecca1839b4a5550bb12109898500e56f792e/mingw-w64-rust/PKGBUILD#L193-L196) but linking it to LLD enablement doesn't sound right.

[jfgoog]

I'm in favor of change like that (so MSYS2 could drop https://github.com/msys2/MINGW-packages/blob/eea9ecca1839b4a5550bb12109898500e56f792e/mingw-w64-rust/PKGBUILD#L193-L196) but linking it to LLD enablement doesn't sound right.

The intent of this particular change is only to make config.lld_enabled behave consistently for windows-gnu as it does for other host triples. It doesn't change anything with respect to copying libgcc_blah.dll and libwinpthread.dll, since those are required to run the rust executables.

(Not sure if that's what you were referring to, lmk)

[mati865]

The intent of this particular change is only to make config.lld_enabled behave consistently for windows-gnu as it does for other host triples.

I don't understand what you mean. Looking at this search this option controls only whether LLD is built and installed for all targets.

It doesn't change anything with respect to copying libgcc_blah.dll and libwinpthread.dll, since those are required to run the rust executables.

It disables copying of mingw-w64 which is welcome.

[jfgoog]

Thanks for looking closely at this.

My understanding (based on this) is that config.lld_enabled also causes the linker from the original toolchain used to bootstrap to be copied into the toolchain.

For example, in my .rustup directory, I have a directory toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin/gcc-ld which contains ld and ld64.

We disable this with the rust.lld flag on our custom Linux builds, and we think the analogous behavior for pc-windows-gnu should be the same.

As I understand it, make_win_dist copies the linker from MinGW, a license file for the linker, and some .a files required to use the linker, so it seems correct to omit those based on config.lld_enabled, just like we omit gcc-ld for Linux targets.

The DLLs, though, seem to be required to be required to run the compiler at all. I think omitting them would require more extensive refactoring to the bootstrap process -- perhaps copying them temporarily, or perhaps modifying the path in the environment. (Not sure, I'm not a Windows expert)

I think you are absolutely correct that the flag also controls whether to include the rust-lld linker built during the build. We don't want that either, but I think that part of the logic works the same for linux and windows targets.

[mati865]

My understanding (based on this) is that config.lld_enabled also causes the linker from the original toolchain used to bootstrap to be copied into the toolchain.

IMO this is different thing. gcc-ld is just a wrapper for allowing using LLD bundled with Rust, with windows-gnu targets the actual linker is copied.

As I understand it, make_win_dist copies the linker from MinGW, a license file for the linker, and some .a files required to use the linker, so it seems correct to omit those based on config.lld_enabled, just like we omit gcc-ld for Linux targets.

gcc-ld is a general thing that also applies (or should apply but is not implemented yet) to windows-gnu target that is directly connected with use of LLD.
Mingw-w64 tools that are copied on windows-target can be considered as "self-contained" feature. It provides not only the linker (that is not related to gcc-ld in any way) but also other binaries and import libraries.

[jfgoog]

My understanding (based on this) is that config.lld_enabled also causes the linker from the original toolchain used to bootstrap to be copied into the toolchain.

IMO this is different thing. gcc-ld is just a wrapper for allowing using LLD bundled with Rust, with windows-gnu targets the actual linker is copied.

I took another look and, yes, you are correct. I think I was misled by the name, because it doesn't really have anything to do with gcc...or does it?

I will take a closer look next week, and see if I can address your suggestions, now that I have a better understanding of what's going on.

[mati865]

I took another look and, yes, you are correct. I think I was misled by the name, because it doesn't really have anything to do with gcc...or does it?

Yes, it has nothing to do with GCC. You can read more about it in #89288 and items linked to that PR.

I will take a closer look next week, and see if I can address your suggestions, now that I have a better understanding of what's going on.

I'm not a member or reviewer so my comments are nothing more than a suggestions that are not blocking 😉

[Mark-Simulacrum]

Generally the goal this is asking for seems relatively reasonable, but I agree with @mati865 that this doesn't seem like the right flag for you.

@rustbot author

[jfgoog]

Generally the goal this is asking for seems relatively reasonable, but I agree with @mati865 that this doesn't seem like the right flag for you.

@rustbot author

Yup, I agree. Good idea, wrong flag.

It sounds like I should add a new flag to config.toml, and I welcome suggestions. In the meantime, I will work on updating this PR to use some new flag, possibly badly named, since changing the name should be relatively easy.

Also, I did some further investigation and found I was mistaken about something else. The DLLs from MinGW do not seem to be necessary for bootstrapping the rustc toolchain, so I think they can be excluded as well.

[jfgoog]

I created a new PR: #108581