-
Notifications
You must be signed in to change notification settings - Fork 231
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
Fixing build on Mingw #574
Conversation
Great! (๑•̀ㅂ•́)و✧ |
This is great. Thank you. I need MingW to be added to the CI builds to prevent bit rot and breakage from future changes before I merge this. Please add a build either in GitHub actions (preferred) or as an addition to the Appveyor build. I also want the search for Bash ( |
maybe the Is this path point to that cmake code: if(NOT BASH_EXECUTABLE)
# Fallback search in default locations
# WSL bash did not work in my case :(
find_program (
BASH_EXECUTABLE
bash
)
endif() |
Not familiar with the bash issue. But I can get something up on Github actions |
Mingw action works and does seem to want to build but the version of gcc being used (8.1.0) is far older than what I tested on my system (11.2.0) And thus I get the wonderful error:
I'll look into what I can do about this |
Re: bash. |
Okay the older GCC version has more problems than just the alignment bug. It also doesn't seem to have snprintf properly declared? I'm going to spin up a clean VM and try this with the same GCC version as the build machine |
Oh I see, the script that the build machine uses to install mingw from chocolatey is out of date. It thinks that the 10.2 build is latest when really the latest is 11.2... I may have to go patch that |
Yeah that worked. I guess I'll upstream that change for others |
Awesome. Thanks for all this. I don't have time to review right now but I've approved running the github action in the PR. Hopefully tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I have a very minor request re a comment plus I am concerned about using non-canonical sources for ninja and mingw. See my review comments.
@FuXiii is To determine the version number, FindBash has a RE match on the version string output by bash which includes "version". Before executing |
@Honeybunch how is progress on upstreaming your change to the script for installing mingw? If it is close I'd like to wait for it and update this PR accordingly. As for the ninja install, since it is coming from a location on GitHub I'm okay with using that fork if that is the best alternative. I'm looking into the build problems on Travis. Of the 2 jobs that filed, one succeeded on retrying. I'm retrying the other as I write this. After that I will try the ones that errored. I can see no reason for the errors. |
That change is just kind of sitting there. Worried that the owner may not be active. I'll try to chase it down after my vacation this week. Otherwise the options are to just rely on my fork or for KhronosGroup to fork my patched copy and rely on that. That way you don't have to rely on mine; but I want this change alive for my own uses so it should be alive for as long as github will host the fork |
@MarkCallow Yes you are right. Previously used |
@FuXiii |
Ok, thanks :) |
When you create a new cmake configuration, i.e. when you select the generator and the compiler and they differ from any existing config in the output directory, cmake runs a test to see if pthread is available. This is the output I see when configuring for Xcode & clang:
I see the same in our CI build under Mingw which specifies Ninja and gcc. What compiler are you specifying in the configure step and what do you see in the configure output with regards to pthread? |
I use clang/clang++
|
@FuXiii, it is not finding the declaration in the screenshot you posted from astc_encode.cpp because there is a On the other hand since CMake is finding thread support without these declarations, there must be some other way to get the thread support in Mingw & Clang. @Honeybunch do you know? |
I realized after much prodding that I can't repro this because of how my copy of LLVM was compiled with the default triple & sysroot being MSVC rather than MinGW:
I just configured with and build with I tried getting my copy of LLVM to build with my mingw sysroot but I think scoop's packaging of mingw is not suitable for this as this test: Fails with: I'll have to spin up a VM real quick to get a clean environment where I can repro this |
I have a mingw environment set up in msys2 now and I'm actually still having a hard time reproducing this. There are still some things I can fix here; it doesn't build. But I'm not getting hung up on the same issue. My version of clang and gcc both compile with
The packages here I installed from msys2 are |
I got something building but still didn't run into a problem with threads. I configured with: I had to turn off the FEATURE_LOADTEST_APPS because some binary support libs (like SDL2) are built against the MSVC ABI and will not link with the mingw version. I didn't run into this because that define is off by default. Maybe a task for the future. Note that in my msys2 environment that the built-in cmake from pacman didn't support the "Mingw Makefiles" generator so I had to install cmake from the web and add it to my msys2 bash path. I also had to use I'm going to put up a PR to use the built in |
I did manage to come up with something that resembles FuXiii's issue. Building from a standard windows powershell prompt with:
Here I'm using the msvc based LLVM from chocolately with the sysroot provided by chocolately. Unlike the version of gcc provided by scoop, this one from chocolatey is the same version but seems to include some of the libraries scoop was missing (gcc_s).
This isn't the same issue that was seen before. I get linker errors like If I'm reading FuXiii's post correctly I see @FuXiii could you do me a favor: If I force my MSVC built clang to use the gnu triple with This is a strange config but means that I'll have to figure out a good way to get CMake to report pthreads support |
Put up an update to #579 which should result in much better detection of pthreads support on Windows I just built for windows with: All of them worked |
and add Mingw build to Github Actions.
and add Mingw build to Github Actions.
and add Mingw build to Github Actions.
and add Mingw build to Github Actions.
and add Mingw build to Github Actions.
I was interested in getting this working with mingw for a side project of mine. Dug around and made a small change to make sure that everything works. Trickiest thing was digging up the mangled names for the .def files for gcc.
Tested this by building the root cmake project with gcc & ninja.
Fixes #328, #572