-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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.
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
Windows build fails with runner image 20221120.1 #6627
Comments
Further information: as far as I found out here error code
This doesn't seem to be related to our build. See also above that this is very likely a regression and that it appeared only with the latest runner image. |
Hi @Albrecht-S , thank you, we will investigate. |
Hi @Albrecht-S can you provide minimal repro steps? |
It's hard to say if I can do this because I don't know what is happening and I can't reproduce it on my local system. If my interpretation of the error code is correct (can you confirm this?) then the error message doesn't tell me which dll is the culprit. :-( Anyway, I investigated on my local system (Windows 10 Home, Version 22H2, Build 19045.2311), if this is of any help. The tool we build (
As you can see all DLL's are Windows system DLL's. I could try to build a minimal reproducer but I have some questions:
If you could answer my questions this could help me to get a minimal repro working. The problem I see is that |
FWIW: the program
|
This was necessary because the latest GitHub "runner" image (20221120.1) includes incompatible image libs (libjpeg, libpng, and zlib). This caused `fluid-cmd.exe` to fail with error code -1073741511. For further information please refer to this GitHub Issue: actions/runner-images#6627 The previous runner version (20221027.1) did not include these libs and our build used the internal libs and worked. The fix is to disable the search for system libs in CMake. Note: this does not explain *why* these libs are incompatible.
Short info: I found the culprit and I could fix it in our GitHub workflow file. But ... Long info: It's a change in the GitHub runner that now includes image libraries (libjpeg, libpng) and zlib. These libs were found by our CMake configure process and obviously linked in The fix (workaround?) in our build instructions was to disable the system library search so it uses the internal, bundled libraries libjpeg, libpng, and zlib. In the previous runner image this was used as a fallback because the libs didn't exist. Summary:
For me this issue is resolved but it's still something you might want to investigate further. However, please feel free to close it. FTR: the relevant part of the failed jobs is:
What is this "Strawberry" thing? Looks like something related to perl ... Anyway, just in case it's still of any value: I found this by reducing our build to a minimal process and eventually in the logs. The minimal build process can be found in my FLTK fork in branch github-minimal. Note that I will delete this branch as soon as this issue is closed. Thanks for your investigation. |
We're also start hitting this issue where CMake starts picking libraries from For more reference, love2d/love@b738668 compiles successfully, which is approximately 4 days ago. At the Windows configure step HarfBuzz and other libraries are NOT found when it configures our bundled FreeType, which is intended. However less than hour ago (as of writing), we're hitting compile failure at love2d/love@6599697. At the configure step, CMake starts picking libraries from This could be a regression. |
Yes it seems to be that recently the windows images (I noticed it on windows-2019) include However it contains binaries (executables and libraries) compiled with MSYS2 MinGW GCC which are incompatible with MSVC compiled object files/binaries. Or at least "sometimes". E.g. There may be other similar subtle differences where things "mostly work" |
In my case, I have to delete the whole Failed build commit to test the |
Together with my Boost colleague Peter Dimov we figured out what is going on: CMake was updated to 3.25 which includes a change to find Summary:
The CMake maintainers are aware of the potential trouble that change might cause and included it in the release only "on watch", i.e. it is likely to be removed in the next CMake release as we now have evidence that this is actually an issue with Meson creating wrongly named files not with CMake not finding them and the change causes more trouble than it solves. See the linked issue for details. A workaround is to pass As @MikuAuahDark noticed this will still pick up So |
@Flamefire Thank you very much for your analysis and for posting your results here. I appreciate this very much. |
Proper solution would be: #5459 strawberry perl should just be shunned for its deployment practices, or - at the very least - patched to not gratuitously inject a broken toolchain into PATH. Upstream strawberry is stubbornly refusing to do this: StrawberryPerl/Perl-Dist-Strawberry#11 |
To be fair: Perl needs a working compile environment to fully function, hence StrawberryPerl has a good reason to bundle a toolchain which is accessible when Perl is accessible (which is the case for the default installation, but only on request for the "portable" installation as they mention), so no, they do not "inject a broken toolchain into PATH" The toolchain they "inject" works very well. It is only that you must not combine MSVC ABI binaries and GNU ABI binaries. So someone using the GNU ABI binaries from StrawberryPerl with MSVC makes a mistake. To avoid this the practice was that GNU ABI import libraries had the extension So if you want to blame someone then the people responsible for naming both static and import libraries the same on Windows. Everything else (i.e. all the tools mentioned) just tried to provide the best user experience given that situation. So for a "proper solution" I'd suggest to use the portable StrawberryPerl or any other means such that Perl is available on request only on GHA images or simply use I hope that helps to understand why this issue is difficult and how seemingly good decisions led us to this situation and it is hard to tell which decision is now to blame as each one is "right" given the technical reasons. |
This is fallaciously wrong (although the propaganda comes from the Strawberry devs -- do not believe everything you read). Perl does not need a working compile environment to fully function -- perl.exe is needed to fully function -- but it may be needed internally by module building routines launched via perl.exe in the event that people both need a functioning perl and a functioning CPAN.
A functioning CPAN is not a worthwhile tradeoff for Github Actions to make, when this doesn't just break recent versions of cmake -- it breaks any build system that notices GCC and assumes it is supposed to build with GCC instead of MSVC. GCC is in PATH while MSVC needs to be configured with something like https://github.com/ilammy/msvc-dev-cmd, and some build systems like Meson will also try to set up an MSVC dev environment for you -- but not if GCC is detected first. Strawberry Perl has a solution for that, you can select at installation time to simply not add its mingw userland to the PATH. Actual Perl users possess the domain specific knowledge of Perl to add the extra path in if they actually need to go building PerlXS modules. People who have never heard of Perl in their lives have to play "hunt the weird bug", then learn what Perl is, and why they don't care about it, and then add Perl-specific workarounds like deleting Strawberry Perl does NOT have a solution for pkg-config.bat shipping alongside perl.exe -- the only solution is to get rid of perl altogether (or find a different Perl distributor, maybe ActivePerl?) but that's not such a big problem as all that.
Except for Strawberry Perl, which is uninterested in the best user experience for anything other than Strawberry Perl. Which isn't totally strange, because this mostly breaks for people who didn't want to install perl at all, but had it baked into the Github Actions VM image -- and sometimes, had it recommended in the OpenSSL build instructions as a good place to get a perl.exe interpreter. The broken experiences don't happen to the Strawberry Perl target userbase -- just the Github target userbase. 🤣 |
Hello! Just few observations from me:
|
That's what I meant by "fully function".
I wouldn't say it "breaks" because as far as I understood using the StrawberryPerl GCC with the StrawberryPerl libraries works as they are ABI compatible (obviously).
If you wanted to use the StrawberryPerl GCC with CMake then it does the right thing. If you don't want to use what is in the environment and hence auto-detected then you can tell CMake to ignore certain paths (see above how to do that) or not add the PATHs in the first place (see my recommendation about the "portable installation" of StrawberryPerl)
CMake has other means of finding stuff (CMake configs, Find-Modules) so it does not need pkg-config for most things. @eli-schwartz Don't get me wrong: I was also bitten by this change in CMake and had lost hours trying to understand what is happening. I'm just trying to understand why things are as they are, what has changed to lead to the situation and what the reasons for decisions (which seem to be right in isolation) were.
You could say the same about Meson creating MSVC ABI Again: I do understand the reasons of Meson for that change and they added a workaround especially for StrawberryPerl as they seemingly noticed the issue. It is only now the situation they you cannot have Meson, CMake and StrawberryPerl all in use at the same time with all features. (And maybe other distributions of MinGW toolchains not using the shell-environment approach)
This is about the MinGW inside StrawberryPerl: It comes with a full MSYS2 toolchain to be able to build Perl modules if a user needs to.
Correct: CMake 3.25 now finds |
To be precise: @mikhailkoliada I see two (three) options to proceed for the current Windows runner images:
I could see that the relevant CMake milestone is due on Nov 30 which is very soon now but unfortunately I don't know of a release schedule, i.e. how long it will take until 3.25.1 will be released. |
But normally a build system is supposed to be able to guess well (that is why people make build systems, to do things for them) which is why build systems allow guessing in the first place. There's a reasonable assumption that your environment should only contain sensible things which you intended to install. Strawberry Perl breaks this -- and breaks the build by by successfully building invalid products which cannot be used for what you tried running a build system in order to do -- by injecting Strawberry GCC into your PATH when you're trying to develop with MSVC. Arguing that it isn't broken because the compiler you didn't want to compile with is compatible with the library you didn't want to compile against, seems to be missing the point.
I think we probably agree that it's a bad idea for Github to add the PATHs in the first place, yeah.
I don't think this is the best comparison. :) I say that Strawberry Perl is "uninterested in the best user experience for anything other than Strawberry Perl" because:
However, Meson's decision was made based on the understanding that it is useful for autotools, cmake, and Visual Studio users, not just Meson users. In fact, per @nirbheek's elaboration above, it is probably still the correct decision, because autotools, meson, and even cmake will produce MSVC-ABI libraries using the .a and .dll.a convention, and people should be able to link with them -- which is why Meson looks for them, will continue to look for them, and hasn't had issues like the recent cmake fallout. Whether one agrees or disagrees with that assessment, it's certainly true that Meson cares about the experience of cmake and cmake users. And I agree that as cmake stands today, it's too much of a regression to simply flip a switch and check for these libraries. Reverting the cmake commit was the right move. (But I think that cmake should be able to handle the situation correctly for MSVC-ABI libraries built with mingw GCC and the UCRT, and in the long run, cmake should still be looking for them.) And we aren't going to avoid discussing the topic, or lock mesonbuild tickets when people try to question our decision. :p ... Point is, that Strawberry Perl leaves the collective software community with our hands tied, there's no choice other than to not use it (and the runner image currently makes us use it by default). There's nothing else to discuss and nowhere to discuss it.
Again: this is technically true by the dictionary definition of "functional" -- it does something when you run it -- but it does not function the way even a single user anywhere wants it to. This toolchain exists solely for building PerlXS modules, it exists internally to Strawberry Perl, and it unpredictably contains whatever Strawberry Perl currently believes is necessary for PerlXS and PerlXS alone. |
I absolutely agree 100% -- this would be a beautiful solution. People who want perl.exe would have to explicitly ask for it, but Github would distribute it in the VM images and make it easy to load it. It's nice to have software that is so preinstalled that you don't even need to ask for it before running it, but sometimes that conflicts with other software and you just have to ask for it before using it. |
Cmake version updated |
CMake is accidentally picking up zlib from the CI environment, see actions/runner-images#6627 (comment). Disable zlib on Windows since we don't want the dependency there, this is also what Rust did a while back: rust-lang/rust#85762
CMake is accidentally picking up zlib from the CI environment, see actions/runner-images#6627 (comment), the build log now has this line: ``` -- Found ZLIB: C:/Strawberry/c/lib/libz.a (found version "1.2.11") ``` Disable zlib on Windows since we don't want the dependency there, this is also what Rust did a while back: rust-lang/rust#85762
CMake is accidentally picking up zlib from the CI environment, see actions/runner-images#6627 (comment), the build log now has this line: ``` -- Found ZLIB: C:/Strawberry/c/lib/libz.a (found version "1.2.11") ``` Disable zlib on Windows since we don't want the dependency there, this is also what Rust did a while back: rust-lang/rust#85762 (cherry picked from commit ea445e4)
CMake is accidentally picking up zlib from the CI environment, see actions/runner-images#6627 (comment), the build log now has this line: ``` -- Found ZLIB: C:/Strawberry/c/lib/libz.a (found version "1.2.11") ``` Disable zlib on Windows since we don't want the dependency there, this is also what Rust did a while back: rust-lang/rust#85762 (cherry picked from commit ea445e4)
- Observed in windows-2022 and windows 2019 version 20221120.1 - It is a known issue on how CMake 3.25 discovers static libraries. - It consists in CMake wrongly picking static libraries from the Perl Strawberry distribution. Upstream issue: - actions/runner-images#6627
this is still an issue when building python wheels, as they will pull cmake from https://pypi.org/project/cmake/, which is still at 3.25.0. |
You need to contact the package maintainer of such package. GitHub Actions is already correct. https://github.com/scikit-build/cmake-python-distributions |
🤔 What if... ... maybe... ... Github Actions stopped configuring their Perl installation to leak dangerous As stated above, these all cause numerous issues, and it's never actually right to use them. CMake is just one more recent example of the breakage caused by installing Perl like this. Please, can we fix the root of the problem by fixing Github Actions? |
Fixes 32-bit builds, which were picking up libz.a and libssh2.a found in this directory. More information: actions/runner-images#6627
Fixes 32-bit builds, which were picking up libz.a and libssh2.a found in this directory. More information: actions/runner-images#6627
Fixes 32-bit builds, which were picking up libz.a and libssh2.a found in this directory. More information: actions/runner-images#6627
Fixes 32-bit builds, which were picking up libz.a and libssh2.a found in this directory. More information: actions/runner-images#6627
Fixes 32-bit builds, which were picking up libz.a and libssh2.a found in this directory. More information: actions/runner-images#6627
Fixes 32-bit builds, which were picking up libz.a and libssh2.a found in this directory. More information: actions/runner-images#6627
Description
We are using a tool (
fluid-cmd.exe
) which is created during the build and used later several times to convertfluid
(.fl
) files to source code (.h
and.cxx
). This tool fails with error code-1073741511
(hexC0000139
).I consider this a bug (regression of the runner image) because it worked flawlessly until Nov. 22 with the previous runner image (
20221027.1
).Since Nov. 22 building with the old runner worked and some "random" builds with the new runner failed. Since today it seems that all builds fail with the new runner. See more info below.
Current build fail/success can be viewed here: https://github.com/fltk/fltk/commits/master
Platforms affected
Runner images affected
Image version and build link
Runner Image
Failed build:
Latest successful build:
Successful build with runner image
Is it regression?
Yes. Works fine with image
20221027.1
(see above)Expected behavior
We are using a tool (
fluid-cmd.exe
) which is created during the build and used later several times to convertfluid
(.fl
) files to source code (.h
and.cxx
). This tool should create the files and return no error as it did with runner version20221027.1
(see above).Actual behavior
This tool (
fluid-cmd.exe
) fails with the current runner image20221120.1
with error code-1073741511
.Excerpt from the build log:
Similar errors occur later for several other identical custom build steps.
It is noteworthy that we first got some intermittent build failures since Nov. 22, 2022 (first failed build) without changing anything related to the error. It appears that all failed builds were with the new runner version whereas the working builds were with the old runner version.
This makes it very likely that this is a regression in the runner image.
Repro steps
There's nothing I can provide here. A local build with Visual Studio 2019 works for me, others have reported that VS 2022 works as well.
The text was updated successfully, but these errors were encountered: