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.

Sign up for GitHub

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

Jump to bottom

Add 'windows-mingw-release' CMake preset #3421

Merged
merged 8 commits into from
Jan 4, 2024
Merged

Add 'windows-mingw-release' CMake preset #3421

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8846fba
Fixes compilation error "ResourceSet.h:33:21: error: function 'Resour…
Kuxe Dec 29, 2023
3536e21
Add 'windows-mingw-release' to CMakePresets.json
Kuxe Dec 29, 2023
30b851d
Merge branch 'develop' of https://github.com/vcmi/vcmi into windows_m…
Kuxe Jan 1, 2024
100664e
Fix compilation error where gcc 12.1.0 (MinGW) errors with "writing 1…
Kuxe Jan 2, 2024
eeab16a
Enable ccache support for MinGW
Kuxe Jan 2, 2024
a78a3b3
Added documentation on how to build using windows-mingw-release preset
Kuxe Jan 2, 2024
c8ed138
Use statically linked Qt5 for Windows MinGW builds. This solves an is…
Kuxe Jan 3, 2024
491dfd8
Remove platform related checks for setting 'CMAKE_C(XX)_COMPILER_LAUN…
Kuxe Jan 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ if(ENABLE_CCACHE)
find_program(CCACHE ccache REQUIRED)
endif()

# On Linux, use ccache via CMAKE_CXX_COMPILER_LAUNCHER.
# On Linux and MinGW, use ccache via CMAKE_CXX_COMPILER_LAUNCHER.
# The XCode and MSVC builds each require some more configuration further down.
if(ENABLE_CCACHE AND LINUX)
if(ENABLE_CCACHE AND (LINUX OR MINGW))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc there's a GNU check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which is not something anyone has ever tested :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, so let's keep MinGW

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@Kuxe Kuxe Jan 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@Kuxe Kuxe Jan 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

set(CMAKE_C_COMPILER_LAUNCHER "ccache")
set(CMAKE_CXX_COMPILER_LAUNCHER "ccache")
endif()
Expand Down
19 changes: 19 additions & 0 deletions CMakePresets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,15 @@
"CMAKE_CXX_COMPILER": "/usr/bin/g++"
}
},
{
"name": "windows-mingw-release",
kambala-decapitator marked this conversation as resolved.
Show resolved Hide resolved
"displayName": "Windows x64 MinGW Release",
"description": "VCMI Windows Ninja using MinGW",
"inherits": "default-release",
"cacheVariables": {
"CMAKE_BUILD_TYPE": "Release"
}
},
{
"name": "windows-msvc-release",
"displayName": "Windows x64 RelWithDebInfo",
Expand Down Expand Up @@ -352,6 +361,11 @@
"configurePreset": "macos-arm-conan-ninja-release",
"inherits": "default-release"
},
{
"name": "windows-mingw-release",
"configurePreset": "windows-mingw-release",
"inherits": "default-release"
},
{
"name": "windows-msvc-release",
"configurePreset": "windows-msvc-release",
Expand Down Expand Up @@ -438,6 +452,11 @@
"configurePreset": "macos-ninja-release",
"inherits": "default-release"
},
{
"name": "windows-mingw-release",
"configurePreset": "windows-mingw-release",
"inherits": "default-release"
},
{
"name": "windows-msvc-release",
"configurePreset": "windows-msvc-release",
Expand Down
9 changes: 4 additions & 5 deletions client/render/Colors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ std::optional<ColorRGBA> Colors::parseColor(std::string text)
{
std::smatch match;
std::regex expr("^#([0-9a-fA-F]{6})$");
ui8 rgb[3] = {0, 0, 0};
std::vector<ui8> rgb;
rgb.reserve(3);
if(std::regex_search(text, match, expr))
{
std::string tmp = boost::algorithm::unhex(match[1].str());
std::copy(tmp.begin(), tmp.end(), rgb);
boost::algorithm::unhex(match[1].str(), std::back_inserter(rgb));
return ColorRGBA(rgb[0], rgb[1], rgb[2]);
}

Expand All @@ -42,8 +42,7 @@ std::optional<ColorRGBA> Colors::parseColor(std::string text)
for(auto & color : colors) {
if(boost::algorithm::to_lower_copy(color.first) == boost::algorithm::to_lower_copy(text))
{
std::string tmp = boost::algorithm::unhex(color.second.String());
std::copy(tmp.begin(), tmp.end(), rgb);
boost::algorithm::unhex(color.second.String(), std::back_inserter(rgb));
return ColorRGBA(rgb[0], rgb[1], rgb[2]);
}
}
Expand Down
8 changes: 8 additions & 0 deletions docs/developers/Building_Windows.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,14 @@ Extract `ccache` to a folder of your choosing, add the folder to the `PATH` envi
- Right click on `BUILD_ALL` project. This `BUILD_ALL` project should be in `CMakePredefinedTargets` tree in Solution Explorer.
- VCMI will be built in `%VCMI_DIR%/build/bin` folder!

## Compile VCMI with MinGW via MSYS2
- Install MSYS2 from https://www.msys2.org/
- Start the `MSYS MinGW x64`-shell
- Install dependencies: `pacman -S mingw-w64-x86_64-SDL2 mingw-w64-x86_64-SDL2_image mingw-w64-x86_64-SDL2_mixer mingw-w64-x86_64-SDL2_ttf mingw-w64-x86_64-boost mingw-w64-x86_64-gcc mingw-w64-x86_64-ninja mingw-w64-x86_64-qt5-static`
- Generate and build solution from VCMI-root dir: `cmake --preset windows-mingw-release && cmake --build --preset windows-mingw-release`

**NOTE:** This will link Qt5 statically to `VCMI_launcher.exe` and `VCMI_Mapeditor.exe`. See [PR #3421](https://github.com/vcmi/vcmi/pull/3421) for some background.

# Create VCMI installer (This step is not required for just building & development)

Make sure NSIS is installed to default directory or have registry entry so CMake can find it.
Expand Down
2 changes: 2 additions & 0 deletions lib/ResourceSet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

VCMI_LIB_NAMESPACE_BEGIN

ResourceSet::ResourceSet() = default;

ResourceSet::ResourceSet(const JsonNode & node)
{
for(auto i = 0; i < GameConstants::RESOURCE_QUANTITY; i++)
Expand Down
2 changes: 1 addition & 1 deletion lib/ResourceSet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ResourceSet
public:
// read resources set from json. Format example: { "gold": 500, "wood":5 }
DLL_LINKAGE ResourceSet(const JsonNode & node);
DLL_LINKAGE ResourceSet() = default;
DLL_LINKAGE ResourceSet();


#define scalarOperator(OPSIGN) \
Expand Down
Loading