build(presets): Major CMakePresets overhaul, unify vcpkg, modernize VC6 support #1373

aliendroid1 · 2025-07-28T04:25:46Z

Deduplicate configure presets for easier maintenance
Make vcpkg the default and speed up dependency installation for all builds
Enable vcpkg integration for VC6 builds
Switch VC6 builds to Ninja Multi-Config: optimization level can now be switched without reconfiguring
Enable full embedded and pdb debug info for VC6 configurations
Enable both coff and cv debug info for VC6 configurations
VC6 release builds are still multithreaded even with debug info
VC6 environment now requires only the VS6_DIR environment variable to be set
Decouple dev builds from optimization config for more flexible development
Debug builds are excluded from ci because they are not redistributable

If you are using the visual studio 2022 ide then I highly recommend you navigate to tool>options>cmake> and disable unified build and configuration preset menu. This way you won't have to re-configure when switching between build presets with a common configure preset.

For convenient vc6 builds, I recommend setting VS6_DIR to your vc6 directory in your system envvironment variable.
Since it is not a standard environment variable, it should have no affect on other visual studio installations or anything
other than building this project using the vc6 presets.

closes #1235

helmutbuhler · 2025-07-28T05:06:27Z

I wish you had waited with this until #1366 is merged. This is gonna cause endless merge conflicts...

How will you be able to control debuginfo generation with this change?

Regarding the CI: It's outputting this error:

Invalid workflow file: .github/workflows/ci.yml#L99
The workflow is not valid. .github/workflows/ci.yml (Line: 99, Col: 11): Input configurePreset is required, but not provided while calling. .github/workflows/ci.yml (Line: 99, Col: 11): Input buildPreset is required, but not provided while calling.

aliendroid1 · 2025-07-28T06:06:26Z

I wish you had waited with this until #1366 is merged. This is gonna cause endless merge conflicts...

How will you be able to control debuginfo generation with this change?

Regarding the CI: It's outputting this error:
Invalid workflow file: .github/workflows/ci.yml#L99
The workflow is not valid. .github/workflows/ci.yml (Line: 99, Col: 11): Input configurePreset is required, but not provided while calling. .github/workflows/ci.yml (Line: 99, Col: 11): Input buildPreset is required, but not provided while calling.

you control debug info by setting the generator config directly or using the build preset.
thanks for sharing the error message. that helped. its mostly working correctly now. just need to fix the artifact collection part

CMakePresets.json

cmake/compilers.cmake

OmniBlade · 2025-07-29T14:44:59Z

CMakePresets.json

            "cacheVariables": {
                "CMAKE_EXPORT_COMPILE_COMMANDS": "ON",
-                "CMAKE_MSVC_DEBUG_INFORMATION_FORMAT": "$<$<CONFIG:Release,Debug,RelWithDebInfo>:Embedded>",
+                "CMAKE_MSVC_DEBUG_INFORMATION_FORMAT": "$<$<CONFIG:Debug,RelWithDebInfo>:ProgramDatabase>",


Don't really care too much about this, but Embeded is allegedly slightly faster to build even on modern MSVC, which is why I set it to that initially. I doubt it makes a massive difference though and only affects object generation, not the final link pdb.

https://learn.microsoft.com/en-us/cpp/build/reference/z7-zi-zi-debug-information-format?view=msvc-170

That doesn't say anything about which format is most efficient for a multithreaded build and I was already aware of the other differences in the format. As I said it doesn't make much difference for the pdb that is generated, its always a separate pdb (if one is generated) for binaries and shared libraries. It only affects how its stored for objects (and static libraries which are just collections of objects). Also, it should be set for all builds, there is no reason to not be generating debug information for all builds except for VC6 so it can multithread build.

sorry, somehow had misread it. but i think program database should be better for pdb size. It definitely is for vc6 but i'm not sure for modern visual studio.

OmniBlade · 2025-07-29T14:47:23Z

CMakePresets.json

            "cacheVariables": {
-                "RTS_BUILD_OPTION_PROFILE": "ON"
+                "CMAKE_CXX_FLAGS" : "/DWIN32 /D_WINDOWS /EHac /Zm800",


Is it perhaps worth investigating moving all the special VC6 handling to a toolchain file rather than dealing with it all in presets?

The only special vc6 stuff is replacing /Zm1000 with /Zm800 and setting jobs to 1 for some build presets. The latter can't be done in a toolchain file for only select build presets

It should probably be done in compilers.cmake if we are going to say this stuff is required then so it doesn't matter how it was configured, its always set for VC6.

I prefer to have most stuff set through cmakelists. I assumed others preffered it in presets based on other stuff being set in presets

So my take is that "its complicated", flags that are needed for a correct build like these I'd say belong in the CMake so any build gets them. Things like warning overrides and especially converting warnings to errors which I think we should work towards using on the CI need to be passed in from the environment in some way so an end users build doesn't error out because we hard coded it and they tried to build for freebsd or their distro upgraded their compilers.

CMakePresets.json

cmake/compilers.cmake

OmniBlade · 2025-07-31T13:56:13Z

cmake/compilers.cmake

-    # and all link steps go into 'link' (so only one link ever runs since vc6 can't handle multithreaded linking)
-    set(CMAKE_JOB_POOL_LINK link)
+if(NOT IS_VS6_BUILD)
+    set(CMAKE_CXX_STANDARD 20)


Why is this being moved here and set globally rather than on just out targets? Does it not work as advertised when done on targets?

it doesn't work correctly for visual studio generators if done by target and eitherway I think this way is safer because you would want all code to use the same standard for api/abi compatibility. Not much of a benefit to setting it per target.

I don't understand why it wouldn't work for the VS generators given they probably recieve some of the heaviest testing for CMake.

I'm thinking more along the lines of if we FetchContent a library that doesn't set this for itself and doesn't build correctly. Granted we will be using vcpkg for most dependencies but its possible we would want to use something not packaged there.

Guess it doesn't matter for now, we can reevaluate that when it comes to it. Setting things per target rather than globally is just generally considered better CMake practice.

…C6 support - Deduplicate configure presets for easier maintenance - Make vcpkg the default and speed up dependency installation for all builds - Enable vcpkg integration for VC6 builds - Switch VC6 builds to Ninja Multi-Config: optimization level can now be switched without reconfiguring - Enable ful debug info for VC6 configurations - VC6 environment now requires only the VS6_DIR environment variable to be set - Decouple dev builds from optimization config for more flexible development

OmniBlade

I'm pretty happy with this now, just a couple remaining points for attention/clarification on.

OmniBlade · 2025-08-05T12:53:05Z

cmake/compilers.cmake

+    # Generate debug information for all builds
+    add_link_options("/debug")
+
+    set(CMAKE_CONFIGURATION_TYPES "Release;Debug")


Any reason we are limiting this to just Release/Debug?

limit creation of build subdirectories

Do we really care if it creates RelWithDebInfo folders, especially as it is a different optimisation level to Release? Limiting for the CI is fine if we don't build that type, but this limits all end users of this repo unless I'm misunderstanding it.

OmniBlade · 2025-08-05T12:53:42Z

cmake/compilers.cmake

+    set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>DLL")
+
+    set(CMAKE_CXX_FLAGS  "/DWIN32 /D_WINDOWS /EHac")
+    set(CMAKE_C_FLAGS  "/DWIN32 /D_WINDOWS")


This is just reiterating the defaults for C_Flags and can probably be ommitted?

helmutbuhler

Some things I noticed.

helmutbuhler · 2025-08-05T17:06:58Z

.github/workflows/ci.yml

      tools: ${{ matrix.tools }}
      extras: ${{ matrix.extras }}
    secrets: inherit

-  # Note build-generalsmd is split into two jobs for vc6 and win32 because replaycheck-generalsmd


Why delete this comment?

helmutbuhler · 2025-08-05T17:08:40Z

.github/workflows/ci.yml

-      matrix:
-        include:
-          - preset: "vc6+t+e"
-          - preset: "vc6-releaselog+t+e" # optimized build with logging and crashing enabled should be compatible, so we test that here.


Why not check replays for release and releaselog?
The Release version is the one we'll ship, so I think it doesn't hurt to test it as well.
It might also help track down issues when we know that only one version mismatches or doesn't run.

I'm not that good at ci stuff atm. I just fixed stuff on the cmake side and tried my best to adjust the ci accordingly. Perhaps someone else can do the ci update/fix.

helmutbuhler · 2025-08-05T17:10:57Z

.github/workflows/check-replays.yml

+      buildPreset:
+        required: true
+        type: string
+      tools:


Why is tools and extras added here?

helmutbuhler · 2025-08-05T17:16:06Z

.github/workflows/build-toolchain.yml

Debug builds are excluded from ci because they are not redistributable

Do I understand correctly that each Job compiles Debug and Release, but only uploads Release binaries? Why don't we upload the debug builds? We don't need to upload the debug c++ runtime.

helmutbuhler · 2025-08-05T17:18:18Z

.github/workflows/build-toolchain.yml

Here is a comparison of before:

and after:

Can we remove this redundant information and clean it up a bit?

helmutbuhler · 2025-08-05T17:20:15Z

.github/workflows/build-toolchain.yml

It seems like the resulting vc6 binary depends on zlib.dll. Before it didn't require any additional dependencies. Is there a way to keep it that way?

helmutbuhler · 2025-08-05T17:24:28Z

.github/workflows/build-toolchain.yml

Will developers now be required to install vcpkg to compile with VC6? If so, please put a instructions in the first post of the PR. (Ideally in the build instructions, but they are sadly in another repo).

OmniBlade · 2025-08-06T15:00:03Z

cmake/compilers.cmake

+    set(CMAKE_MSVC_DEBUG_INFORMATION_FORMAT "ProgramDatabase")
+    set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>DLL")
+
+    set(CMAKE_CXX_FLAGS  "/DWIN32 /D_WINDOWS /EHac")


As of #1424 this won't be needed for the games at least.

Nice! /EHsc is better

aliendroid1 force-pushed the debugInfo branch 3 times, most recently from 776e30f to 4972671 Compare July 28, 2025 04:48

aliendroid1 marked this pull request as ready for review July 28, 2025 04:50

aliendroid1 closed this Jul 28, 2025

aliendroid1 reopened this Jul 28, 2025

aliendroid1 force-pushed the debugInfo branch from b47365a to 6cb40aa Compare July 28, 2025 05:58

aliendroid1 force-pushed the debugInfo branch from 6cb40aa to a6b85ab Compare July 28, 2025 06:13

OmniBlade requested changes Jul 29, 2025

View reviewed changes

aliendroid1 force-pushed the debugInfo branch 4 times, most recently from 95af92a to 75f053e Compare July 30, 2025 05:51

aliendroid1 requested a review from OmniBlade July 30, 2025 06:02

aliendroid1 force-pushed the debugInfo branch 4 times, most recently from 3dc74e9 to 737bb31 Compare July 31, 2025 03:48

OmniBlade requested changes Jul 31, 2025

View reviewed changes

aliendroid1 added Build Anything related to building, compiling Debug Is mostly debug functionality labels Aug 1, 2025

aliendroid1 force-pushed the debugInfo branch 7 times, most recently from ba32576 to ca87492 Compare August 5, 2025 04:11

aliendroid1 force-pushed the debugInfo branch from ca87492 to 7ad0bd7 Compare August 5, 2025 04:32

aliendroid1 requested a review from OmniBlade August 5, 2025 04:36

aliendroid1 force-pushed the debugInfo branch 5 times, most recently from f187400 to 45b7d9b Compare August 5, 2025 05:51

aliendroid1 mentioned this pull request Aug 5, 2025

Create a page of all newly introduced configuration options TheSuperHackers/GeneralsWiki#68

Open

aliendroid1 force-pushed the debugInfo branch 12 times, most recently from ffe436c to 2a92237 Compare August 5, 2025 08:55

aliendroid1 force-pushed the debugInfo branch from 2a92237 to b7beed8 Compare August 5, 2025 09:18

OmniBlade requested changes Aug 5, 2025

View reviewed changes

helmutbuhler requested changes Aug 5, 2025

View reviewed changes

OmniBlade requested changes Aug 6, 2025

View reviewed changes

build(presets): Major CMakePresets overhaul, unify vcpkg, modernize VC6 support #1373

Are you sure you want to change the base?

build(presets): Major CMakePresets overhaul, unify vcpkg, modernize VC6 support #1373

Uh oh!

Conversation

aliendroid1 commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

helmutbuhler commented Jul 28, 2025

Uh oh!

aliendroid1 commented Jul 28, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OmniBlade left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

helmutbuhler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aliendroid1 commented Jul 28, 2025 •

edited

Loading