rust icon indicating copy to clipboard operation
rust copied to clipboard

Upgrade mingw-w64 on CI

Open mati865 opened this issue 1 year ago • 9 comments

Continuation of https://github.com/rust-lang/rust/pull/99162

This will require copying related archives from https://github.com/niXman/mingw-builds-binaries/releases/tag/12.2.0-rt_v10-rev0 to Rust's mirror. Those binaries are provided by the same person who previously maintained them at sourceforge. Binaries from both sources were built using these scripts https://github.com/niXman/mingw-builds so it's possible to build them manually if needed.

This should fix multiple issues with currently used ancient releases but I'll ask folks to retest them using nightly once it goes live.

mati865 avatar Aug 05 '22 19:08 mati865

r? @pietroalbini

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

rust-highfive avatar Aug 05 '22 19:08 rust-highfive

:umbrella: The latest upstream changes (presumably #99967) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Aug 21 '22 02:08 bors

So I wonder maybe we should use a slightly older version of mingw on CI, if possible (as long as it supports removing llvm.allow-old-toolchain). The newest version is already tested by anyone using MSYS2 and updating packages regularly (e.g. me), but some older version support may accidentally break without a CI check.

petrochenkov avatar Aug 26 '22 22:08 petrochenkov

It's not easy to obtain them: we would have to build them ourselves. Also there are issues like https://github.com/rust-lang/rust/issues/88704 that have been fixed in recent GCC versions (it shouldn't appear with GCC 11/12) but I have no clue how old compiler we could built and still have that fix.

mati865 avatar Aug 26 '22 22:08 mati865

niXman's github has this oldest release - https://github.com/niXman/mingw-builds-binaries/releases/tag/11.2.0-rt_v9-rev1, that may be good enough.

petrochenkov avatar Aug 26 '22 23:08 petrochenkov

That didn't work out at all:

  = note: D:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: D:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/lib/../lib/crt2.o:crtexe.c:(.rdata$.refptr.mingw_app_type[.refptr.mingw_app_type]+0x0): undefined reference to `mingw_app_type'
          D:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: D:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/lib/../lib/crt2.o:crtexe.c:(.rdata$.refptr.mingw_initcharmax[.refptr.mingw_initcharmax]+0x0): undefined reference to `mingw_initcharmax'
          D:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: D:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/lib/../lib/crt2.o:crtexe.c:(.rdata$.refptr.mingw_initltssuo_force[.refptr.mingw_initltssuo_force]+0x0): undefined reference to `mingw_initltssuo_force'
          D:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: D:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/lib/../lib/crt2.o:crtexe.c:(.rdata$.refptr.mingw_initltsdyn_force[.refptr.mingw_initltsdyn_force]+0x0): undefined reference to `mingw_initltsdyn_force'
          D:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: D:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/lib/../lib/crt2.o:crtexe.c:(.rdata$.refptr.mingw_initltsdrot_force[.refptr.mingw_initltsdrot_force]+0x0): undefined reference to `mingw_initltsdrot_force'
          collect2.exe: error: ld returned 1 exit status

So upgraded to newer GCC 12.2 release instead and tested again both 32-bit and 64-bit builds locally.

mati865 avatar Aug 27 '22 12:08 mati865

Likely won't have time in the near future to look into this. r? @Mark-Simulacrum

pietroalbini avatar Sep 18 '22 12:09 pietroalbini

I don't have any real opinion on whether to go for older or more recent release binaries, but am happy to mirror binaries as needed when we're ready to do so. Will leave this as waiting on author for now though to figure out a path toward a clear answer for which binaries to copy up.

Mark-Simulacrum avatar Sep 18 '22 17:09 Mark-Simulacrum

Binaries with GCC 11 and older mingw-w64 didn't work and upon inspection tarballs called GCC 12.1 and GCC 12.2 differ only in GCC version (mingw-w64 and Binutils are the same) so I'd go with GCC 12,2 since it carries mostly bugfixes over GCC 12.1 (so there are hopefully less chances for issues with things like building LLVM).

@rustbot ready

mati865 avatar Sep 18 '22 19:09 mati865

:umbrella: The latest upstream changes (presumably #98483) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Sep 24 '22 23:09 bors

Uploaded the binaries in question to the mirror -- so I think from an infra perspective this should be good to go.

I'm marking this as relnotes since it sounds (https://github.com/rust-lang/rust/pull/100178#issuecomment-1229021255) like this could break some workflows; along those lines I wonder if it makes sense to have this version be listed as part of the platform support page? Currently that just says MinGW -- should it have a version?

r? @wesleywiser perhaps to confirm the policy angle here vis-a-vis platform support (mostly since you might have more Windows context than me).

Mark-Simulacrum avatar Oct 01 '22 19:10 Mark-Simulacrum

I worry there is only one way to find out if this breaks some workflows.

I'm marking this as relnotes since it sounds (https://github.com/rust-lang/rust/pull/100178#issuecomment-1229021255) like this could break some workflows;

Maybe it would be worth to call for testing or something like that?

along those lines I wonder if it makes sense to have this version be listed as part of the platform support page? Currently that just says MinGW -- should it have a version?

It should at very least say to use mingw-w64 and Dwarf-2/SEH exception models for i686/x86_64 respectively.

Regarding the version, declaring GCC 12 and mingw-w64 10 as lowest supported versions would be very restrictive but those are the versions that would be tested on CI. Staying on GCC 6 is not an option because LLVM 16 will remove option that allows it to be built with GCC older than 7.1.

mati865 avatar Oct 01 '22 21:10 mati865

I have to admit, I'm not super familiar with the details of the windows-gnu targets.

What sort of workflows do we think could theoretically break?

Does this have any effect on the minimum supported version of Windows for these targets?

wesleywiser avatar Oct 04 '22 20:10 wesleywiser

What sort of workflows do we think could theoretically break?

Hopefully the only visible change (apart from numerous fixes) will be for people who rely on rust-mingw component instead of using external toolchain: I imagine static libs built with that setup would no longer work with old toolchains but in turn they would start working with new toolchains. I expect this situation to be very rare as most of the users have external mingw-w64 toolchain installed in their system.

Does this have any effect on the minimum supported version of Windows for these targets?

Apparently mingw-w64 has increased default Windows version to Windows 10 last year. So it might not work with Windows 7 which I cannot easily test.

mati865 avatar Oct 05 '22 00:10 mati865

people who rely on rust-mingw component instead of using external toolchain

Ah so people that use mingw components we distribute with the toolchain instead of using the same components from mingw itself? That feels analogous to the LLVM tools we ship on various platforms. Users could rely on those and we might break their workflow during an LLVM upgrade but that isn't really a supported scenario. I think from that perspective, my concern is resolved.

Apparently mingw-w64 has increased default Windows version to Windows 10 last year. So it might not work with Windows 7 which I cannot easily test.

This is more concerning to me since {i686,x86_64}-pc-windows-gnu are Tier 1 platforms which claim to support Windows 7+. I think we will probably need to go through a similar process as we did in #95026 which raised the platform requirements for the Linux targets.

wesleywiser avatar Oct 07 '22 14:10 wesleywiser

This is going to be more urgent when we start the LLVM 16 upgrade, which will require at least GCC 7.1 (i.e. no more option to allow-old-toolchain here). If we can avoid the Windows 7 question for now by choosing an older build, or building our own, I think that should be strongly considered.

cuviper avatar Oct 17 '22 18:10 cuviper

This is going to be more urgent when we start the LLVM 16 upgrade, which will require at least GCC 7.1 (i.e. no more option to allow-old-toolchain here).

Yep, that was the main motivation for me to look at it again.

If we can avoid the Windows 7 question for now by choosing an older build

For some various reasons older builds do not work for Rust.

building our own

Scripts used to build toolchain used in this PR are available at https://github.com/niXman/mingw-builds, building from scratch is also possible albeit annoying with some outdated guides like https://sourceforge.net/p/mingw-w64/wiki2/Cross%20Win32%20and%20Win64%20compiler/.

I think there is no defined process on how to handle this though.

mati865 avatar Oct 19 '22 22:10 mati865

Could we use the toolchain from MSYS2? They're in the process of phasing out Windows 7 too, but maybe we can take builds from before they started those changes?

cuviper avatar Oct 21 '22 17:10 cuviper

IIRC Rust has used them previously but switched to standalone builds because MSYS2 is rolling release and it was deemed more complicated to "freeze" it than using standalone builds. It tries to follow Arch Linux principals in this regard. I guess one could install minimal set of toolchain packages (Binutils + GCC + GDB + mingw-w64) and simply tar that directory. Then use it the same way as we install other toolchains.

There are some downsides to that approach like mingw-w64 version. MSYS2 uses its latest git revision (for various good reasons that do not matter for Rust) which would mean CI would test compatibility only with very very CRT version that should be released around spring next year.

mati865 avatar Oct 21 '22 19:10 mati865

I guess one could install minimal set of toolchain packages (Binutils + GCC + GDB + mingw-w64) and simply tar that directory. Then use it the same way as we install other toolchains.

Yeah, I was thinking it would be something like that.

There are some downsides to that approach like mingw-w64 version. MSYS2 uses its latest git revision (for various good reasons that do not matter for Rust) which would mean CI would test compatibility only with very very CRT version that should be released around spring next year.

We do recommend MSYS2 in the README though, so it seems nicer to build and test with that rather than a 3rd-party build. Are there specific version-compatibility changes you're concerned about?

cuviper avatar Oct 21 '22 20:10 cuviper

Hm, to be clear, we don't claim to support compiling Rust itself on Windows 7? Like it may or may not work but we don't make any promises.

ChrisDenton avatar Oct 21 '22 21:10 ChrisDenton

We do recommend MSYS2 in the README though, so it seems nicer to build and test with that rather than a 3rd-party build. Are there specific version-compatibility changes you're concerned about?

Rust provides rust-mingw component which bundles mingw-w64 static and import libraries and binaries like GCC and ld.bfd from build machine. I don't know if it's case that Rust wants to support (I really hope it's not the case) but if user took Rust 1.64 and used bundled rust-mingw component to create static libraries (assuming it's possible?) and then used that static library while linking some binary they could notice it fails with nightly after this merge. Since MSYS2 uses unreleased mingw-w64 version there might be impossible for them to use matching toolchain but again this is already super fragile to rely on.

Another thing that came to my mind is debuginfo, old toolchains like one shipped right now default to dwarf-2 but recent versions use Dwarf 4 (IIRC it was bumped to version 5 and then downgraded to version 4 but I could be wrong). It's unclear to me how it affects compatibility with old toolchains.

Hm, to be clear, we don't claim to support compiling Rust itself on Windows 7? Like it may or may not work but we don't make any promises.

Dunno but this change may break running Rust on Windows 7 if LLVM or compatibility layers like winpthreads start using some new API because _WIN32_WINNT tells them it's fine.

mati865 avatar Oct 21 '22 22:10 mati865

How should we move forward? The longer it takes the less time is left to iron out eventual issues before LLVM 16 testing starts.

mati865 avatar Nov 01 '22 00:11 mati865

I'm not a stakeholder for this target, but it seems to me the options are:

  1. Use the niXman build and drop support for Windows 7. That's a Tier-1 support change, which we've been leaning toward already, but could take a while for approval.
  2. Use a snapshot of the MSYS2 toolchain. We might have to grab a build before this hardware change, if that's even available, or else risk that our builds become slightly less compatible.
  3. Build our own toolchain with whatever compat settings we want. This may be a lot of effort, but at least it's infrequent...

cuviper avatar Nov 01 '22 23:11 cuviper

I don't feel like the person to take decision here either and I have no idea on how the decision should be taken.

mati865 avatar Nov 02 '22 23:11 mati865

Let's ask the compiler team to consider the compatibility aspect, at least.

@rustbot label +I-compiler-nominated

cuviper avatar Nov 02 '22 23:11 cuviper

Visited during the weekly T-compiler meeting, follow-up discussion will happen on this Zulip stream

@rustbot label -I-compiler-nominated

apiraino avatar Nov 10 '22 16:11 apiraino

Visited during the weekly T-compiler meeting, follow-up discussion will happen on this Zulip stream

Is this the right link? The last activity there was in 2020.

nikic avatar Dec 07 '22 08:12 nikic

Is this the right link? The last activity there was in 2020.

Yes. T-compiler visiting this PR happened here on Zulip and was suggested to keep the conversation on that older Zulip thread. I may set another reminder for the next meeting :)

apiraino avatar Dec 07 '22 08:12 apiraino

Build our own toolchain with whatever compat settings we want. This may be a lot of effort, but at least it's infrequent...

So, I looked into how hard it would be to redo the niXman/mingw-builds builds with a lower WINNT requirement, and it turns out that this is not hard at all. The build script supports a --with-default-win32-winnt option, so adjusting the GA workflow to pass --with-default-win32-winnt=0x0601 would produce a Windows 7 compatible toolchain.

And while looking into this, I found that we're not the first people who want this... https://github.com/starg2/mingw-builds/commit/316ee4370be86d9936dac95416d247e608616c22 is the necessary change to the GA workflow, and https://github.com/starg2/mingw-builds/actions/runs/3350177834 are the build artifacts for that commit.

I think we can just go ahead and grab those builds?

nikic avatar Dec 14 '22 19:12 nikic