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

build(port): CMake improvements #3457

Merged
merged 188 commits into from
Nov 15, 2023
Merged

Conversation

olanti-p
Copy link
Contributor

@olanti-p olanti-p commented Oct 17, 2023

Summary

SUMMARY: Build "[DDA Port] CMake improvements"

Purpose of change

Working towards #3113

Describe the solution

Cherry-pick almost everything from the following PRs:

I had to temporarily revert #3248 because of conflicts; I'll have to figure out how to reapply it in the future.

Additional changes:

  1. MSYS+CMake and MSVC+CMake builds have been added to CI (will run on PR; TODO: make run on upload only)
  2. A few fixes in the source code to make the game compile on newer compilers and MSVC
  3. Changed a few compile flags to closer match what we have right now rather then what DDA has
  4. vcpkg manifest is tied to a fixed version so the dependencies don't suddenly change on us
  5. CMake build will automatically use vcpkg bundled with Visual Studio 2022
  6. Added a patch removing brotli feature from freetype pulled in by vcpkg because I couldn't get the executable to properly link to libbrotli on one of the platforms, and disabling features of dependencies is broken in vcpkg
  7. Expanded CMakePresets.json for use in Visual Studio
  8. For Visual Studio, CMake will automatically configure working directory for debugging
  9. Added docs on using CMake with Visual Studio
  10. Removed from docs mentions on how to use CMake+vcpkg build on Linux. The produced executable segfaults on SDL init, which turned out to be common for both BN and latest DDA. May be related to this: SDL doesn't open a window when compiled with vcpkg microsoft/vcpkg#34457 Not a big deal; the main point of us using vcpkg is having it fetch build dependencies on Windows, where it's a much more painful process than on Linux.
  11. Some changes related to localization files weren't useful for BN since we use a different system, and many of those issues have already been solved on our side.
  12. Added precompiled headers for tests (build speedup)
  13. Removed automatic CMake formatter added in [DDA Port] Various improvements to CMake builds #3208. DDA doesn't seem to be using any automated formatter, but on our side autofix formats the code and introduces merge conflicts. Also, I don't like some of the changes it does, and there's no support for some of the style choices that imo should be the defaults (e.g. ensuring endif() does not repeat the check in if()).
  14. Changed MSYS build in CI to run on pushes to upload rather than on every PR

TODO list

  • Cherry-pick more PRs
  • Fix localization stuff
  • BNify things
  • Add Lua switches where needed
  • Check if we need to keep using specific vcpkg version, and if so, which - updated to version microsoft/vcpkg@c9aba30, which contains SDL 2.26.5 with the fix for Police car's sound is distorted (@'s soundpack) CleverRaven/Cataclysm-DDA#63765
  • Figure out why CMake+MSVC tests are extremely slow - Turned out, build type in CMake must be defined in a different way when it comes to multigenerators like Visual Studio, so it was compiling the fallback Debug instead of Release
  • Figure out why CMake+MSYS2 compilation is slower then Makefile+MSYS2 - There seems to be a fixed 10-20% (eyeballing it here) penalty to build times even after I've aligned most compile flags (-DNDEBUG, -g1, optimization levels). Most likely I'm missing something there, doing test builds in VM gave very inconsistent results (up to being 10% faster then Makefile). Same inconsistent results were present when running jobs on this PR. MSYS2 is low-priority, since our main builds are MXE and MSVC, so I'm willing to overlook this for now.
  • Reapply build(cmake): deduplicate library setup #3248 - This will have to come later, when it's time to reorganize the project. I'd rather not introduce additional changes to the build structure while we're still burdened by the other build systems.
  • Update documentation
  • Move the new doc file into proper folder
  • Get rid of some DDA-specific zlib linker flags
  • Check out ccache crash on MSYS2 Nevermind, it works now. Must've been the wind?
  • Add to CI
  • Test everything

Describe alternatives you've considered

Figuring all of this out by myself.

Testing

Things that were NOT tested because I plan to deal with them in later PRs:

  • CMake install targets
  • Creating releases from CMake builds
  • CMake on macOS

Builds that have been tested manually in the process:

  • MSYS2 + CMake
  • VS2022 + CMake (bundled vcpkg) - both building in IDE and with command line
  • VS2019 + CMake (independent vcpkg) - both building in IDE and with command line
  • VSCode + CMake + Clang on Linux

CI has tested the rest of CMake builds.

Additional context

In VSCode, I had to disable use of presets in the the Microsoft's CMake extension because it forced me to choose a preset and picked the default GCC toolchain, but didn't let me switch over to Clang while still using preset.

@github-actions github-actions bot added src changes related to source code. tests changes related to tests labels Oct 17, 2023
@github-actions github-actions bot added the JSON related to game datas in JSON format. label Oct 17, 2023
@olanti-p olanti-p changed the title [WiP] [DDA Port] Align CMake to Visual Studio projects [WiP] [DDA Port] CMake improvements Oct 17, 2023
@olanti-p olanti-p marked this pull request as ready for review October 25, 2023 00:16
@olanti-p olanti-p force-pushed the cmake-stuffs branch 12 times, most recently from b884c50 to d68920c Compare November 2, 2023 09:28
@scarf005 scarf005 changed the title [WiP] [DDA Port] CMake improvements build(port): [WiP] CMake improvements Nov 13, 2023
@olanti-p olanti-p force-pushed the cmake-stuffs branch 2 times, most recently from 4df8f31 to f6f5ed9 Compare November 14, 2023 15:04
@olanti-p
Copy link
Contributor Author

olanti-p commented Nov 14, 2023

Aaaand it's done. 1 month, 20+ force-pushes and 180+ commits (after squashing).

The new CMake builds pass all checks and are roughly equivalent to existing Makefile / Visual Studio builds.
CI will now validate CMake+MSYS and CMake+MSVC builds, though only on pushes to upload so they don't clog up the waiting queue as much. Plain MSYS CI builds have also been shifted to upload, there's not much reason to run them for every PR.

Clang-tidy failures are unrelated to changes here.
Similarly, Asan timing out is not related (see #3664).
Mac build works, but it might appear not to because it's executed after failing Asan and thus may be skipped.

@olanti-p olanti-p changed the title build(port): [WiP] CMake improvements build(port): CMake improvements Nov 14, 2023
@scarf005 scarf005 added this pull request to the merge queue Nov 15, 2023
Merged via the queue into cataclysmbnteam:upload with commit 91175cb Nov 15, 2023
12 of 16 checks passed
@olanti-p olanti-p deleted the cmake-stuffs branch November 16, 2023 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JSON related to game datas in JSON format. src changes related to source code. tests changes related to tests translation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants