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.

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

Merged
merged 10 commits into from
May 22, 2022
Merged

Fixing build on Mingw #574

merged 10 commits into from
May 22, 2022

Conversation

Honeybunch
Copy link
Contributor

@Honeybunch Honeybunch commented May 13, 2022

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

@CLAassistant
Copy link

CLAassistant commented May 13, 2022

CLA assistant check
All committers have signed the CLA.

@FuXiii
Copy link
Contributor

FuXiii commented May 13, 2022

Great! (๑•̀ㅂ•́)و✧

@MarkCallow
Copy link
Collaborator

MarkCallow commented May 13, 2022

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 (cmake/modules/FindBash.cmake) fixed so it finds the MingW bash. I surprised to hear it is not found because Git for Windows is based on MingW and the search finds that Bash.

@FuXiii
Copy link
Contributor

FuXiii commented May 13, 2022

maybe the "C:\\Program Files\\Git\\bin" in cmake/modules/FindBash.cmake need add some code to find bash.exe from System Environment, my Git path is at F:\\Git\\bin not at "C:\\Program Files\\Git\\bin"

I try to output my git path:
image

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()

@Honeybunch
Copy link
Contributor Author

Not familiar with the bash issue. But I can get something up on Github actions

@Honeybunch
Copy link
Contributor Author

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:

 In file included from D:/a/KTX-Software/KTX-Software/lib/astc-encoder/Source/astcenc_vecmathlib.h:73,
                 from D:/a/KTX-Software/KTX-Software/lib/astc-encoder/Source/astcenc_mathlib.h:433,
                 from D:/a/KTX-Software/KTX-Software/lib/astc-encoder/Source/astcenc_internal.h:37,
                 from D:/a/KTX-Software/KTX-Software/lib/astc-encoder/Source/astcenc_averages_and_directions.cpp:23:
D:/a/KTX-Software/KTX-Software/lib/astc-encoder/Source/astcenc_vecmathlib_avx2_8.h: In function 'void print(vint8)':
D:/a/KTX-Software/KTX-Software/lib/astc-encoder/Source/astcenc_vecmathlib_avx2_8.h:1012:35: error: requested alignment 32 is larger than 16 [-Werror=attributes]
  alignas(ASTCENC_VECALIGN) int v[8];

I'll look into what I can do about this

@Honeybunch
Copy link
Contributor Author

Re: bash. find_package should return a find if bash is anywhere on your path. Both my system and the build machine seem to be happy with where bash is so I'm not sure what's up with that.

@Honeybunch
Copy link
Contributor Author

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

@Honeybunch
Copy link
Contributor Author

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

@Honeybunch
Copy link
Contributor Author

Yeah that worked. I guess I'll upstream that change for others
https://github.com/Honeybunch/KTX-Software/runs/6432087780

@MarkCallow
Copy link
Collaborator

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.

Copy link
Collaborator

@MarkCallow MarkCallow left a 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.

.github/workflows/main.yml Show resolved Hide resolved
.github/workflows/main.yml Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
@MarkCallow
Copy link
Collaborator

@FuXiii is F:\\Git\\bin in your %PATH% environment variable? If so find_program should have found it.

To determine the version number, FindBash has a RE match on the version string output by bash which includes "version". Before executing bash --version it does set(ENV{LANG} "en_US.UTF-8") to ensure the version is printed in English. If F:\\Git\\bin is in your path, please try setting LANG to en_US.UTF-8 and then bash version to see if the output is in English.

@MarkCallow
Copy link
Collaborator

@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.

@Honeybunch
Copy link
Contributor Author

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

@FuXiii
Copy link
Contributor

FuXiii commented May 21, 2022

@MarkCallow Yes you are right.
In my systtem %PATH% environment variable only the F:\Git\cmd had in it, that make some kind weird, then I add the F:\Git\bin in %PATH%. the bash --version output:

image

Previously used git-bash.exe is in git path root, it will automatic load ./bin/bash.exe.

image

@MarkCallow
Copy link
Collaborator

@FuXiii git-bash.exe starts an MSYS2 terminal running bash. It is not itself bash and git-bash.exe --version does not produce any output. There is no way to make FindBash use this to find the actual bash executable needed to run the script in the build. Therefore everything needed to support mingw builds in already in this PR. I will merge shortly.

@FuXiii
Copy link
Contributor

FuXiii commented May 21, 2022

Ok, thanks :)

@MarkCallow MarkCallow merged commit 1f07cb0 into KhronosGroup:master May 22, 2022
@FuXiii
Copy link
Contributor

FuXiii commented May 22, 2022

When I build KTX with mingw32-make:

E:\Turbo\thirdparty\KTX-Software\lib\astc_encode.cpp:437:5: error: unknown type name 'pthread_t'
    pthread_t threadHandle;
    ^
1 error generated.
mingw32-make[2]: *** [thirdparty\KTX-Software\CMakeFiles\ktx.dir\build.make:500: thirdparty/KTX-Software/CMakeFiles/ktx.dir/lib/astc_encode.cpp.obj] Error 1
mingw32-make[1]: *** [CMakeFiles\Makefile2:3057: thirdparty/KTX-Software/CMakeFiles/ktx.dir/all] Error 2
mingw32-make: *** [Makefile:155: all] Error 2
The terminal process "C:\WINDOWS\System32\WindowsPowerShell\v1.0\powershell.exe -Command mingw32-make" terminated with exit code: 1.

the vscode clangd can find the define of pthread_t
image
but the compiler can't find it...
What should I do?
ε(┬┬﹏┬┬)3

@MarkCallow
Copy link
Collaborator

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:

Looking for pthread.h
Looking for pthread.h - found
Performing Test CMAKE_HAVE_LIBC_PTHREAD
Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success

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?

@FuXiii
Copy link
Contributor

FuXiii commented May 22, 2022

I use clang/clang++

C:\Users\g1018>clang++ --version
clang version 14.0.0 (https://github.com/llvm/llvm-project.git 329fda39c507e8740978d10458451dcdb21563be)
Target: x86_64-w64-windows-gnu
Thread model: posix
InstalledDir: F:/llvm-mingw-20220323-msvcrt-x86_64/bin

CMake output:

$ cmake . -B build -G "MinGW Makefiles" -D KTX_FEATURE_LOADTEST_APPS=ON -D KTX_FEATURE_DOC=OFF -D KTX_FEATURE_STATIC_LIBRARY=ON -D CMAKE_EXPORT_COMPILE_COMMANDS=1 -D CMAKE_CXX_COMPILER=clang++ -D CMAKE_C_COMPILER=clang
-- Found Bash: F:/Git/usr/bin/bash.exe
-- The C compiler identification is Clang 14.0.0
-- The CXX compiler identification is Clang 14.0.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: F:/llvm-mingw-20220323-msvcrt-x86_64/bin/clang.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: F:/llvm-mingw-20220323-msvcrt-x86_64/bin/clang++.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Looking for CL_VERSION_2_2
-- Looking for CL_VERSION_2_2 - not found
-- Looking for CL_VERSION_2_1
-- Looking for CL_VERSION_2_1 - not found
-- Looking for CL_VERSION_2_0
-- Looking for CL_VERSION_2_0 - not found
-- Looking for CL_VERSION_1_2
-- Looking for CL_VERSION_1_2 - not found
-- Looking for CL_VERSION_1_1
-- Looking for CL_VERSION_1_1 - not found
-- Looking for CL_VERSION_1_0
-- Looking for CL_VERSION_1_0 - not found
-- Could NOT find OpenCL (missing: OpenCL_LIBRARY OpenCL_INCLUDE_DIR)
-- Found Vulkan: F:/VulkanSDK/1.3.204.1/Lib/vulkan-1.lib
-- Found Perl: F:/Git/usr/bin/perl.exe (found version "5.26.2")
--   AVX2 backend     - ON
--   SSE4.1 backend   - OFF
--   SSE2 backend     - OFF
--   NEON backend     - OFF
--   NONE backend     - OFF
--   NATIVE backend   - OFF
--   Decompressor     - OFF
--   No invariance    - OFF
--   Diagnostics      - OFF
--   ASAN             - OFF
--   Unit tests       - OFF
-- Found OpenGL: opengl32
OPENGL_ES_EMULATOR not set. Will not build OpenGL ES load tests applications.
-- Found Vulkan at F:/VulkanSDK/1.3.204.1/Include & F:/VulkanSDK/1.3.204.1/Lib/vulkan-1.lib
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Failed
-- Looking for pthread_create in pthreads
-- Looking for pthread_create in pthreads - not found
-- Looking for pthread_create in pthread
-- Looking for pthread_create in pthread - found
-- Found Threads: TRUE
-- Configuring done
-- Generating done
-- Build files have been written to: E:/Lib/KTX-Software/build

@MarkCallow
Copy link
Collaborator

@FuXiii, it is not finding the declaration in the screenshot you posted from astc_encode.cpp because there is a !defined(__MINGW32__) just before it. I am not familiar with MingW but it looks like that code should only be excluded when gcc is being used with Mingw.

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?

@Honeybunch
Copy link
Contributor Author

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:

λ clang --version
clang version 14.0.3
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Users\Arsine\scoop\apps\llvm\current\bin

I just configured with cmake -B build-clang-mingw -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -G "MinGW Makefiles" -DCMAKE_MAKE_PROGRAM=make -DKTX_FEATURE_LOADTEST_APPS=ON -DKTX_FEATURE_DOC=OFF -DKTX_FEATURE_STATIC_LIBRARY=ON

and build with cmake --build build-clang-mingw and it just worked fine (after tweaking an .rc file to be UTF8 rather than UTF16)

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: clang++ --target=x86_64-w64-windows-gnu --sysroot=%USERPROFILE%/scoop/apps/gcc/11.2.0 -o test test.cpp -v

Fails with: ld.exe: cannot find -lgcc_s

I'll have to spin up a VM real quick to get a clean environment where I can repro this

@Honeybunch
Copy link
Contributor Author

Honeybunch commented May 22, 2022

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 __MINGW32__ defined.

$ clang --version
clang version 14.0.3
Target: x86_64-w64-windows-gnu
Thread model: posix
InstalledDir: C:/msys64/mingw64/bin
$ gcc --version
gcc.exe (Rev1, Built by MSYS2 project) 12.1.0
Copyright (C) 2022 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

The packages here I installed from msys2 are mingw-w64-x86_64-gcc and mingw-w64-x86_64-clang

@Honeybunch
Copy link
Contributor Author

I got something building but still didn't run into a problem with threads. I configured with: cmake . -B build -G "MinGW Makefiles" -DCMAKE_BUILD_TYPE=Debug -D KTX_FEATURE_LOADTEST_APPS=OFF -D KTX_FEATURE_DOC=OFF -D KTX_FEATURE_STATIC_LIBRARY=ON -D CMAKE_EXPORT_COMPILE_COMMANDS=1 -D CMAKE_CXX_COMPILER=clang++ -D CMAKE_C_COMPILER=clang

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 mingw-w64-x86_64-make. Kind of a weird setup.

I'm going to put up a PR to use the built in MINGW var that CMake provides which, in my experience, will detect MinGW usage properly in and outside of MSYS2 with either GCC or Clang but will properly return false when using clang with MSVC.

@Honeybunch
Copy link
Contributor Author

Honeybunch commented May 22, 2022

I did manage to come up with something that resembles FuXiii's issue.

Building from a standard windows powershell prompt with:

cmake -B build-clang-mingw -G "Ninja Multi-Config" -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_FLAGS="--target=x86_64-w64-windows-gnu --sysroot=C:/ProgramData/chocolatey/lib/mingw/tools/install/mingw64" -DCMAKE_CXX_FLAGS="--target=x86_64-w64-windows-gnu --sysroot=C:/ProgramData/chocolatey/lib/mingw/tools/install/mingw64"

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).

PS C:\Users\User\KTX-Software> clang --version
clang version 14.0.3
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Program Files\LLVM\bin

This isn't the same issue that was seen before. I get linker errors like C:/Users/User/KTX-Software/lib/astc_encode.cpp:486: undefined reference to 'pthread_join'

If I'm reading FuXiii's post correctly I see InstalledDir: F:/llvm-mingw-20220323-msvcrt-x86_64/bin and that msvcrt gives me pause.

@FuXiii could you do me a favor:
Create a test file with touch test.c (the file can be blank)
Run clang -dM -E test.c > defines.h
This will report all of the defines that the compiler provides to the preprocessor by default. Do you see __MINGW32__ in there?

If I force my MSVC built clang to use the gnu triple with clang --target=x86_64-w64-windows-gnu --sysroot=C:/ProgramData/chocolatey/lib/mingw/tools/install/mingw64 -dM -E test.c > defines.h the defines still don't report __MINGW32__

This is a strange config but means that __MINGW32__ is a pretty unreliable define when caring about Clang on MinGW.

I'll have to figure out a good way to get CMake to report pthreads support

@Honeybunch
Copy link
Contributor Author

Put up an update to #579 which should result in much better detection of pthreads support on Windows

I just built for windows with:
LLVM + Mingw in MSYS2
LLVM + Mingw with that fucked-up sysroot nonsense in powershell
GCC + Mingw in powershell
CL + MSVC in VS2022 dev env

All of them worked

KaperD pushed a commit to KaperD/KTX-Software that referenced this pull request Feb 21, 2024
and add Mingw build to Github Actions.
KaperD pushed a commit to KaperD/KTX-Software that referenced this pull request Feb 22, 2024
and add Mingw build to Github Actions.
KaperD pushed a commit to KaperD/KTX-Software that referenced this pull request Feb 22, 2024
and add Mingw build to Github Actions.
KaperD pushed a commit to KaperD/KTX-Software that referenced this pull request Feb 22, 2024
and add Mingw build to Github Actions.
KaperD pushed a commit to KaperD/KTX-Software that referenced this pull request Feb 22, 2024
and add Mingw build to Github Actions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please add support for MinGW / gcc under Windows
4 participants