Skip to content

[CMAKE][GITHUB] Simplify and consolidate CMakePresets for multi-config builds #1235

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

aliendroid1
Copy link

Summary

  • Collapse all per-configuration configurePresets into a single "default" plus generator/toolset-specific presets (ninja, ninja-vc6, vs2022, vs2022-vc6).
    • Note: ninja and ninja-vc6 presets remain separate to avoid requiring binaryDir deletion when switching compilers.
  • Add new presets for Visual Studio 2022 generator (vs2022, vs2022-vc6).
    • Note: vs2022-vc6 (toolset v60) requires Daffodil and VS2010 to be installed.
  • Remove buildPresets: all build configuration logic is now handled in CI or at build time, not by maintaining a large preset matrix.
    • In a follow-up PR, new build presets may be added for selecting sets of targets.
  • CI now loops over Debug, Release, and RelWithDebInfo at build time for each preset.
  • Remove cmakeMinimumRequired (now set in top-level CMakeLists.txt).
  • Downgrade schema version to 5 to enable CMake 3.24 compatibility, which is required for the legacy VS2010 generator (used for certain VC6 debugging workflows).
  • General clean-up: remove redundant workflow/configuration steps.

Benefits

  • Simplifies maintenance: No more combinatorial explosion of presets—much easier to read, update, and review.
  • Restores compatibility: Supports legacy toolchains (VS2010/VC6) required for specific debugging scenarios.
  • Cleaner CI: Shifts all build configuration switching to CI/build time, keeping presets minimal and focused.
  • Increased user flexibility:
    Users can set debug/profile defines at configure time to decouple them from optimization level, or leave them unset and have them switch automatically at build time using generator configuration. This makes mixed setups (like release builds with debug defines) easy and explicit.

@aliendroid1 aliendroid1 force-pushed the ConsolidatePresets branch 4 times, most recently from 3485ab1 to 1883f2c Compare July 6, 2025 11:36
@aliendroid1 aliendroid1 marked this pull request as draft July 6, 2025 11:38
@aliendroid1 aliendroid1 force-pushed the ConsolidatePresets branch 8 times, most recently from 9405621 to e334e47 Compare July 6, 2025 13:33
@aliendroid1 aliendroid1 force-pushed the ConsolidatePresets branch from e334e47 to 9631194 Compare July 6, 2025 13:47
Copy link

@OmniBlade OmniBlade left a comment

Choose a reason for hiding this comment

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

I don't think we are going to see eye to eye on the subject of mixing build configuration with optimisation level.

The vcpkg stuff needs fixing at some point rather than removing too as it will be required for future development and we already have code merged into the code base that depends on it to build.

@@ -49,26 +48,29 @@ if(NOT IS_VS6_BUILD)
target_compile_features(core_config INTERFACE cxx_std_20)
endif()

target_compile_options(core_config INTERFACE ${RTS_FLAGS})
target_compile_options(core_config INTERFACE "/W3")

Choose a reason for hiding this comment

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

Don't hard code warning options, they differ between compilers. Better to pull this from an environment variable and set it either from presets or from the environment directly such as on the CI.

For example when cleaned up we might want the CI to error on warnings to stop them creeping back in, but don't want that hard coded as it might break builds on platforms we haven't tested on or on compilers not run on the CI.

if(RTS_BUILD_OPTION_PROFILE)
target_compile_definitions(core_config INTERFACE RTS_PROFILE)
endif()
# otherwise DEBUG defines are tied to Debug generator config and PROFILE defines to RelWithDebug info generator config

Choose a reason for hiding this comment

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

Disagree with this, it prevents building an end user configuration without optimisation and breaks the current behaviour largely just for the benefit of Visual Studio users who don't build using the command line so they can build two seperate types of build without switching solution. Correct solultion is to have two solutions when developing in my opinion, one with developer instrumentation enabled and one without. This isn't being developed as just and MSVC solution, we aren't tied to the limitations of just that IDE anymore.

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.

2 participants