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

Fix MinGW build by reverting compiler detection #1497

Closed
wants to merge 1 commit into from

Conversation

slipher
Copy link
Member

@slipher slipher commented Jan 12, 2025

Fix the standalone native MinGW toolchain build by reverting #982. The resource compiler chokes on some combination of the quotation marks, spaces, and slashes in defines. Verbose Make logging:

[100%] Building RC object CMakeFiles/dummyapp.dir/src/engine/sys/windows-resource/manifest.rc.obj
C:\lang\mingw-builds\64-14.2-posix-rev0\bin\windres.exe -O coff -DARCH_STRING=amd64 -DDAEMON_ARCH_amd64 -DDAEMON_BUILD_Release -DDAEMON_CXX_COMPILER_MinGW=1 -DDAEMON_CXX_COMPILER_STRING="MinGW 14.2.0/gcc-14.2.0 c++.exe" -DDAEMON_C_COMPILER_MinGW=1 -DDAEMON_C_COMPILER_STRING="MinGW 14.2.0/gcc-14.2.0 gcc.exe" -DDAEMON_USE_ARCH_INTRINSICS=1 -DDAEMON_USE_ARCH_INTRINSICS_amd64=1 -DDAEMON_USE_ARCH_INTRINSICS_i686=1 -DDAEMON_USE_COMPILER_CUSTOMIZATION=1 -DDAEMON_USE_COMPILER_INTRINSICS=1 -DNACL_ANDROID=0 -DNACL_ARCH_STRING=amd64 -DNACL_BUILD_ARCH=x86 -DNACL_BUILD_SUBARCH=64 -DNACL_FREEBSD=0 -DNACL_LINUX=0 -DNACL_OSX=0 -DNACL_WINDOWS=1 -DNOMINMAX -DPDC_FORCE_UTF8 -DPDC_RGB -DPDC_WIDE -DSTRICT -DTHIS_IS_NOT_A_DEBUG_BUILD -DUSELESS_DEFINITION_TO_AVOID_PCH_ISSUE -DUSE_CURSES -DUSE_MUMBLE -DWIN32 -DWINVER=0x501 -I C:\unv\Unvanquished\daemon\src -I C:\unv\Unvanquished\daemon\libs -I C:\unv\Unvanquished\daemon\libs\nacl -I C:\unv\Unvanquished\daemon\external_deps\windows-amd64-mingw_10\include -I C:\unv\Unvanquished\daemon\libs\pdcursesmod -I C:\unv\Unvanquished\daemon\external_deps\windows-amd64-mingw_10\include\opus -I C:\unv\Unvanquished\daemon\external_deps\windows-amd64-mingw_10\include\freetype2 -I C:\unv\Unvanquished\daemon\external_deps\windows-amd64-mingw_10\include\AL -I C:\unv\Unvanquished\daemon\src\engine -I C:\unv\Unvanquished\daemon\external_deps\windows-amd64-mingw_10\include\SDL2  C:\unv\Unvanquished\daemon\src\engine\sys\windows-resource\manifest.rc CMakeFiles\dummyapp.dir\src\engine\sys\windows-resource\manifest.rc.obj
cc1.exe: fatal error: 14.2.0/gcc-14.2.0\: No such file or directory
compilation terminated.
cc1.exe: fatal error: c++.exe: No such file or directory
compilation terminated.
cc1.exe: fatal error: 14.2.0/gcc-14.2.0\: No such file or directory
compilation terminated.
cc1.exe: fatal error: gcc.exe: No such file or directory
compilation terminated.
C:\lang\mingw-builds\64-14.2-posix-rev0\bin\windres.exe: preprocessing failed.
mingw32-make[3]: *** [CMakeFiles\dummyapp.dir\build.make:89: CMakeFiles/dummyapp.dir/src/engine/sys/windows-resource/manifest.rc.obj] Error 1

Revert "framework: report which compiler was used to build the game"

This reverts commit 4313b20.

Revert "cmake: make possible to use PCH with compilers with subcommands"

This reverts commit 098edeb.

Revert "cmake: reverse the PCH PNaCl test"

This reverts commit 1b51cc3.

Revert "cmake: improve the PCH support detection"

This reverts commit e552ff7.

Revert "cmake: add comments for future self to improve the CMake code"

This reverts commit 4cb34f5.

Revert "cmake: improve compiler support"

This reverts commit cbe119e.

Revert "cmake: report wich compiler is used to build the game"

This reverts commit 596020e.
@DolceTriade
Copy link
Contributor

Why revert the compiler changes

@illwieckz
Copy link
Member

illwieckz commented Jan 15, 2025

The compiler stuff is there to know it has been built using the standalone native MinGW toolchain, so it doesn't make sense to revert that.

Here the fault is in the tool driving the compiler (maybewindres here, or the tool calling it, maybe make?), even the antislash \ it chokes on isn't added by us:

windres.exe -O coff […] -DDAEMON_CXX_COMPILER_STRING="MinGW 14.2.0/gcc-14.2.0 c++.exe"
cc1.exe: fatal error: 14.2.0/gcc-14.2.0\: No such file or directory

We may try to replace the space character in the definition by something else, like : or ,.

But this a bug in third-party tools.

@illwieckz
Copy link
Member

I can reproduce the bug in Wine:

20250115-072118-000 wine-mingw64-daemon

@slipher
Copy link
Member Author

slipher commented Jan 15, 2025

Why revert the compiler changes

Because they add defines with special characters that makes the MinGW toolchain unable to build. As a bonus, #982 was merged with only disapproving reviews.

The compiler stuff is there to know it has been built using the standalone native MinGW toolchain

Not really. It generates the same format of string for an MSYS MinGW toolchain.

@illwieckz
Copy link
Member

It is still true fro MinGW in general.

@illwieckz
Copy link
Member

Because they add defines with special characters that makes the MinGW toolchain unable to build.

That's not a reason to revert, that is a reason to fix.

@illwieckz
Copy link
Member

That's a bug in windres.exe.

Using another character like ; instead of space makes it work.

@illwieckz
Copy link
Member

Space is not a special character for a defined string…

@illwieckz
Copy link
Member

Funnily, using | as a separator breaks the command (like a pipe does)… I guess the " character is also taken as part of the string itself…

@illwieckz
Copy link
Member

But using ; makes the string cut when calling c++, so funny.

@illwieckz
Copy link
Member

I have a fix, it works:

20250115-074858-000 wine-mingw64-daemon

@slipher
Copy link
Member Author

slipher commented Jan 19, 2025

Apparently it happens on MSYS MinGW also.

@slipher slipher changed the title Fix standalone MinGW build Fix MinGW build by reverting compiler detection Jan 19, 2025
@illwieckz
Copy link
Member

illwieckz commented Jan 20, 2025

This is clearly a bug in the way the commands are processed on Windows, the string quoting doesn't work properly.

Characters like space, pipe, etc. will break the string. It will break for whatever definition string having a character that is used as some token splitter (or even command splitter) in cmd.exe.

So this is not our fault™.

This PR implement a workaround for that exact use case:

But in the future we may implement something more robust like writing the definitions in a shared header file or something.

Windows is simply broken there, passing a string as a definition should not be considered as being working on Windows.

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.

3 participants