Add 'windows-mingw-release' CMake preset

[Kuxe]

Adds a 'windows-mingw-release'-preset (configure, build and test).

Two commits 8846fba and 100664e fixes compilation errors using said preset.

Full disclosure: 100664e is not tested locally. I haven't gotten around to actually running locally built binaries yet. Let me know if you want me to test this locally, or if you were able to test it yourself.

[kambala-decapitator]

iirc there's a GNU check

[Kuxe]

I think that'd include installations of GCC on osx, which is not something I've tested

[kambala-decapitator]

which is not something anyone has ever tested :)

[Kuxe]

yep, so let's keep MinGW

[kambala-decapitator]

well, what about Makefile or Ninja generator on macOS regardless of the compiler? They should also work by simply specifying the "compiler launcher".

I'd rather just add a check for those generators here (maybe excluding Windows OS, not sure how ccache works there in command-line builds).

[Kuxe]

Not sure what you mean by

maybe excluding Windows OS

this PR is for Windows, using the gcc port mingw-w64 that is used on host-os Windows targeting Windows (not cross-compilation). I know mingw can be used to cross-compile on Linux targeting Windows, but that's unrelated to this PR. I could rewrite it into

if(ENABLE_CCACHE AND (LINUX OR (CMAKE_HOST_WIN32 AND MINGW)))

so I don't wind up unintentionally supporting ccache for platforms using MinGW other than windows (only such case would be cross-compilation from linux to windows afaik).. but I thought that was a nice side-effect so I just skipped checking for CMAKE_HOST_WIN32.

well, what about Makefile or Ninja generator on macOS regardless of the compiler?
It doesn't matter if you use Makefile or Ninja generator. This PR is enabling ccache for windows-mingw-release, both Makefile and Ninja generator.

I just now found out there's no GNU-variable in CMake (I also thought it did exist), at least not listed here: https://cmake.org/cmake/help/latest/manual/cmake-variables.7.html

I'm all for supporting other platforms with ccache :), but it's out of scope of this PR

[kambala-decapitator]

my main remark is that instead of adding every single platform or compiler here we should instead check the CMake generator that's used. I definitely know that ccache works through the standard compiler launcher variables for Makefile and Ninja generators, and definitely neither for Xcode nor for Visual Studio (that's already handled separately).
 

this PR is for Windows

right, was thinking about msvc :)

[Kuxe]

my main remark is that instead of adding every single platform or compiler here we should instead check the CMake generator that's used.

Ah, now I see what you mean. I'll make a new commit. EDIT: See #3421 (comment)

[Kuxe]

FYI I also have a windows-mingw-debug-preset locally in my CMakeUserPresets.json. Let me know if I should carry it over to CMakePresets.json

[Kuxe]

Running into a crash when starting vcmiserver:

CObjectClassesHandler.cpp:181:

obj->objects[index] = object;

index is 20 but size of obj->objects is 6. The crash is preceded by this warning.

Object wog:creatureBank.grotto - attempt to load object with preset index! This option is reserved for built-in mod

Added to CObjectClassesHandler::loadSubObject

	if(index > obj->objects.size())
	{
		logMod->error("Object %s:%s.%s - index out of bounds (%i > %i)!", obj->modScope, obj->handlerName, identifier, index, obj->objects.size());
		return;
	}

and I seem to be hitting this for several objects, see attached log.
log.txt

Could this be local setup issues? Or is this a known issue? Didn't find any GitHub issue that resembles this. EDIT: I don't encounter it when building on linux. Could be MinGW related. I have to dig deeper. EDIT: Right, I don't have wog installed on my linux machine. Apparently I had a wog installation in C:\Users\kuxe\Documents\My Games\vcmi\Mods from 2019. Removed it. Crash is gone. Not MinGW related.

I can also start the game now, although I am greeted by a "TLS Initialziation Error" on starting VCMI_launcher.exe. I don't get this error when running 1.4.2 release. Investigating...

[kambala-decapitator]

FYI I also have a windows-mingw-debug-preset locally in my CMakeUserPresets.json. Let me know if I should carry it over to CMakePresets.json

and how does it differ from release build?

I can also start the game now, although I am greeted by a "TLS Initialziation Error" on starting VCMI_launcher.exe. I don't get this error when running 1.4.2 release. Investigating...

I believe your Qt version was built without openssl support

[Kuxe]

FYI I also have a windows-mingw-debug-preset locally in my CMakeUserPresets.json. Let me know if I should carry it over to CMakePresets.json

and how does it differ from release build?

It passes CMAKE_BUILD_TYPE="debug" instead of CMAKE_BUILD_TYPE="release"

I can also start the game now, although I am greeted by a "TLS Initialziation Error" on starting VCMI_launcher.exe. I don't get this error when running 1.4.2 release. Investigating...

I believe your Qt version was built without openssl support

Thanks! I'll rebuild with openssl and do some more manual smoke testing to see if there's anything obviously broken with MinGW build.

[kambala-decapitator]

It passes CMAKE_BUILD_TYPE="debug" instead of CMAKE_BUILD_TYPE="release"

such stuff can be passed on the command line, no need to create another preset for it

[IvanSavenko]

Running into a crash when starting vcmiserver:

CObjectClassesHandler.cpp:181:

obj->objects[index] = object;

index is 20 but size of obj->objects is 6. The crash is preceded by this warning.

Huh, interesting find. I did have a suspicion that we have memory corruption somewhere in CObjectClassesHandler on some systems - due to reported crashes. Might be caused by such outdated wog mod.

Noted, will fix. Should not be just a warning in log.

[Kuxe]

It passes CMAKE_BUILD_TYPE="debug" instead of CMAKE_BUILD_TYPE="release"

such stuff can be passed on the command line, no need to create another preset for it

Sure, most things can be done command-line. But presets allow for IDE integrations, which I think is handy :). In gif below I select one configure preset out of all available presets and then start configuring it. Fuzzy-search, visually listing available presets etc. Things you usually don't get in your terminal.

For me personally I can have the debug preset in my CMakeUserPresets.json. Let's keep it there until the unlikely event comes that someone requests a debug preset for mingw.

[Kuxe]

Here are the solutions to the aforementioned OpenSSL problem:

a) Use pacman -S mingw-w64-x86_64-qt5-base that loads openssl at runtime (not loadtime nor compile time) and copy libssl-3-x64.dll, libcrypto-3-x64.dll (from firadaemons OpenSSL binaries 3.1.4 or elsewhere) into .\out\build\windows-mingw-debug-kuxe\bin. Would probably require some special handling with cpack.

b) Use pacman -S mingw-w64-x86_64-qt5-base-static. This will statically link qt5 into VCMI_launcher.exe and VCMI_mapeditor.exe. I wanted to check how much this affected binary size:

Release build

file	static	static (stripped)	non-static	non-static (stripped)
VCMI_launcher.exe	38669kb	21880kb	1798kb	1222kb
VCMI_mapeditor.exe	37164kb	21253kb	2698kb	1722kb

Debug build

file	static	static (stripped)	non-static	non-static (stripped)
VCMI_launcher.exe	74258kb	21620kb	31114kb	1015kb
VCMI_mapeditor.exe	125748kb	20891kb	84883kb	1345kb

c) Build Qt5 from source

d) Use conan on Windows to get Qt5 with statically linked OpenSSL

I'll go with option b) for now. It's the path of least resistance and there are no plans to use Windows MinGW-build for shipping so larger binary sizes is OK and it's easier to maintain than a). c) and d) will take much more effort to get it right.

EDIT: I did run a cpack for windows-mingw-release in .\out\build\windows-mingw-release and installed it alongside VCMI 1.4.2. Seems to work as intended. I can start launcher and run a singleplayer game without any obvious errors. The total install size is 128MB via windows-mingw-release in comparison with released VCMI 1.4.2 at 127MB. So all-in-all statically linking to Qt5 is not a biggie.

I think this is ready to be merged now.

[kambala-decapitator]

But presets allow for IDE integrations, which I think is handy

right, I always forget about those IDEs that work with CMake directly :)

But isn't it possible to define custom variables right inside the IDE? For example, Qt Creator has such feature iirc

For me personally I can have the debug preset in my CMakeUserPresets.json. Let's keep it there until the unlikely event comes that someone requests a debug preset for mingw.

I agree

Here are the solutions to the aforementioned OpenSSL problem:

thanks a lot for the investigation! Static libs are fine.

P.S. OpenSSL is needed only for the launcher to make https network calls

[Kuxe]

But isn't it possible to define custom variables right inside the IDE? For example, Qt Creator has such feature iirc

In VSCode I think that's possible to do via "cmake.configureArgs": [<args goes here>] but it's not possible to have separate args for special "types" of configurations / builds, e.g only passing -g in some situations. It's possible to just edit the workspace-file over and over again but I think it's more ergonomic to just stuff whatever arguments to cmake you want in a preset and select the right preset

[kambala-decapitator]

I think it's more ergonomic to just stuff whatever arguments to cmake you want in a preset and select the right preset

sure, makes complete sense

[Kuxe]

491dfd8 removes the platform checks. I don't even check if generator is ninja or make, since CMake won't do anything if other generators are used (see https://cmake.org/cmake/help/latest/prop_tgt/LANG_COMPILER_LAUNCHER.html#prop_tgt:%3CLANG%3E_COMPILER_LAUNCHER) - so checking if generator is ninja or make is redundant.

Meaning that if user sets the ENABLE_CCACHE-option, regardless of compiler or platform, CMake will set CMAKE_C(XX)_COMPILER_LAUNCHER, but it's only actually used if generator is a makefile generator or ninja. Special handling for compilers can be handled separately, as is currently done for e.g MSVC.