From 1e2ce703151a852b723f613d926956bd7523cb10 Mon Sep 17 00:00:00 2001 From: "Marco Antonio J. Costa" Date: Sun, 26 Jul 2026 12:04:48 -0300 Subject: [PATCH] Stamp the build's identity into GameVersion.cpp from CMake GameVersion.cpp becomes a configure_file template, so the version strings are a build input rather than something a CI step seds into the working tree mid-run. That is what lets a crash report be matched to its exact build's PDB for offline symbolization. The two strings get distinct jobs. czVersionString is the machine-readable build identity: the bare short commit SHA and nothing else. A `git describe` string cannot serve here -- czVersionString is 16 bytes because it is stored in the savegame header as SAVED_GAME_HEADER::zGameVersionNumber and strcmp'd on load, and describe puts the SHA last, so truncation ate exactly the discriminating part and could make two different commits compare equal. CI passes 9 hex; anything that would not fit is now a configure error rather than a silent clip. zBuildInformation is the display string, untruncated, and it is no longer printed beside czVersionString: that drops the duplication in the version line ("JA2 1.13 V4-555-G6A941C0 2026-07-25 V4-555-G6A941C06") and stops showing a token that is now machine-only. CI's copy leads with GAME_VERSION, because the version line is now the only place a player reads which release they are running. Finish what the sed step's TODO asked for: the workflow pins GAME_BUILD_INFORMATION and GIT_SHA once, next to the gamedir SHAs it already pins, and hands them to CMake with -D. Every parallel build job therefore stamps one agreed identity, and the compile checkout needs no git history at all. A build nobody identifies is a local build and says so -- czVersionString reads "local" -- rather than deriving a SHA from HEAD. A derived SHA could not stay true: nothing re-runs configure once HEAD moves, so an incremental build would keep whatever it was configured with, and keeping it honest would mean reconfiguring on every commit. It would also buy nothing, because only packaged builds are archived with their PDBs; a local build's crash reports are symbolized against the PDB sitting next to the exe. Co-Authored-By: Claude Opus 5 --- .github/workflows/build.yml | 38 ++++++++++------- Ja2/CMakeLists.txt | 46 ++++++++++++++++++++- Ja2/GameSettings.cpp | 2 +- Ja2/{GameVersion.cpp => GameVersion.cpp.in} | 4 +- Ja2/GameVersion.h | 10 ++++- Ja2/jascreens.cpp | 4 +- sgp/DEBUG.cpp | 2 +- 7 files changed, 81 insertions(+), 25 deletions(-) rename Ja2/{GameVersion.cpp => GameVersion.cpp.in} (84%) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 19d51e761..b2a948cc9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -48,7 +48,9 @@ jobs: SOURCE_COMMIT_DATETIME=$(TZ=UTC0 git log -1 --date=iso-strict-local --format=%cd $GITHUB_SHA) SOURCE_COMMIT_DATE=$(TZ=UTC0 git log -1 --date=short-local --format=%cd $GITHUB_SHA) - # GAME_VERSION is used to detect outdated save games and is the main version used for tracking + # GAME_VERSION is the release's name: it labels the dist archives and leads + # the in-game version line. Outdated save games are detected with GIT_SHA + # below, which is what czVersionString holds. if [[ "$GITHUB_REF_TYPE" == 'tag' ]] then # if we build for a specific tag, use that as the game version @@ -62,10 +64,13 @@ jobs: # - v1.13-5-7g7ffa GAME_VERSION="$(git describe --tags --match='v[0-9]*' $GITHUB_SHA || echo v0-$(git rev-list --skip 1 --count $GITHUB_SHA)-g${GITHUB_SHA:0:8})" fi - # max 15 CHAR8 + # keeps archive names bounded; nothing binary depends on this any more GAME_VERSION="${GAME_VERSION:0:15}" - GAME_BUILD_INFORMATION="$SOURCE_COMMIT_DATE GitHub $GITHUB_REPOSITORY" + # the version line shown in game. It leads with GAME_VERSION because + # czVersionString is no longer displayed beside it: this is the only + # place a player can read which release they are running. + GAME_BUILD_INFORMATION="$GAME_VERSION $SOURCE_COMMIT_DATE GitHub $GITHUB_REPOSITORY" # in case of a branch or if the tag is truncated if [[ "$GAME_VERSION" != *"$GITHUB_REF_NAME"* ]] then @@ -73,11 +78,19 @@ jobs: fi # max 255 CHAR16 GAME_BUILD_INFORMATION="${GAME_BUILD_INFORMATION:0:255}" - + + # the machine-readable build identity stamped into czVersionString, which + # is both the savegame compatibility key and the crash-report key. Pinned + # here rather than derived per job, so the parallel builds cannot disagree, + # and left unsuffixed: this names a commit whose tree we really built. + # See Ja2/CMakeLists.txt for why it is 9 hex and nothing else. + GIT_SHA="${GITHUB_SHA:0:9}" + echo -n " SOURCE_COMMIT_DATETIME=$SOURCE_COMMIT_DATETIME GAME_VERSION=$GAME_VERSION GAME_BUILD_INFORMATION=$GAME_BUILD_INFORMATION + GIT_SHA=$GIT_SHA " >> ../dist/versions.env # due to building everything in parallel, versions should be pinned at the start so everything builds based on the same versions @@ -144,17 +157,6 @@ jobs: shell: bash run: cat artifacts/versions.env >> $GITHUB_ENV - # TODO: Move this into CMake, just taking the GitHub Actions parameters as -D variables - - name: Update GameVersion.cpp - shell: bash - run: | - set -eux - GAME_VERSION=$(echo "$GAME_VERSION" | tr -cd '[:print:]' | tr -d '"\\') - GAME_BUILD=$(echo "$GAME_BUILD_INFORMATION" | tr -cd '[:print:]' | tr -d '"\\') - sed -i "s|@Version@|${GAME_VERSION:0:15}|" Ja2/GameVersion.cpp - sed -i "s|@Build@|${GAME_BUILD:0:255}|" Ja2/GameVersion.cpp - cat Ja2/GameVersion.cpp - - name: Prepare build properties shell: bash run: | @@ -176,7 +178,11 @@ jobs: run: | # sccache can only cache MSVC objects when the debug information is embedded # in them (/Z7) instead of collected in a shared PDB (/Zi, the CMake default) - cmake -S . -B build -GNinja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DApplications="$Env:JA2Application" + # + # The build identity comes from versions.env, pinned once for the whole + # workflow. Handing it over is what makes this a packaged build rather + # than a local one; CMake never asks git who we are. + cmake -S . -B build -GNinja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DApplications="$Env:JA2Application" -DGAME_BUILD_INFORMATION="$Env:GAME_BUILD_INFORMATION" -DGIT_SHA="$Env:GIT_SHA" - name: Build run: | cmake --build build/ -- -v diff --git a/Ja2/CMakeLists.txt b/Ja2/CMakeLists.txt index db8929539..741d2e3e1 100644 --- a/Ja2/CMakeLists.txt +++ b/Ja2/CMakeLists.txt @@ -1,3 +1,47 @@ +# Stamp the build's identity into GameVersion.cpp. Both strings come from the +# caller with -D, and CI passes them: it pins them once, where it decides what the +# release is, so the parallel build jobs cannot stamp identities that disagree. +# +# A build nobody identified is a local build, and says so rather than guessing +# from HEAD. Keeping such a guess honest would mean re-running configure on every +# commit, and it would buy nothing: only what we package is archived with its PDB, +# and a local crash report is symbolized against the PDB next to the exe. +# +# GIT_SHA lands in czVersionString, which is both the savegame compatibility key +# and the crash-report key. It is the bare short SHA and nothing else — a `git +# describe` string cannot serve here, because czVersionString is only 16 bytes and +# describe puts the SHA last, so it is exactly the discriminating part that would +# get clipped off. The budget is 15 chars + NUL: the string is stored in the +# savegame header as SAVED_GAME_HEADER::zGameVersionNumber (GAME_VERSION_LENGTH), +# so its size is a binary format constant and cannot grow. CI passes 9 hex, well +# past what uniqueness needs (git's own auto-abbrev picks 8 for this repo). +# Overshooting it is an error rather than a truncation, because a clipped SHA is +# how two different builds end up claiming to be the same one. +if(NOT GIT_SHA) + set(GIT_SHA "local") +endif() +string(LENGTH "${GIT_SHA}" _length) +if(_length GREATER 15) + message(FATAL_ERROR "GIT_SHA is ${_length} characters; czVersionString holds 15") +endif() + +# zBuildInformation is the version line shown on screen, CHAR16[256]. Nothing +# parses it, so it carries the version in full and truncation costs only +# readability. +if(NOT GAME_BUILD_INFORMATION) + string(TIMESTAMP _buildDate "%Y-%m-%d" UTC) + set(GAME_BUILD_INFORMATION "local build ${_buildDate}") +endif() +string(LENGTH "${GAME_BUILD_INFORMATION}" _length) +if(_length GREATER 255) + string(SUBSTRING "${GAME_BUILD_INFORMATION}" 0 255 GAME_BUILD_INFORMATION) +endif() + +configure_file( + "${CMAKE_CURRENT_SOURCE_DIR}/GameVersion.cpp.in" + "${CMAKE_CURRENT_BINARY_DIR}/GameVersion.cpp" + @ONLY) + set(Ja2Src "${CMAKE_CURRENT_SOURCE_DIR}/aniviewscreen.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/Credits.cpp" @@ -7,7 +51,7 @@ set(Ja2Src "${CMAKE_CURRENT_SOURCE_DIR}/gameloop.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/gamescreen.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/GameSettings.cpp" -"${CMAKE_CURRENT_SOURCE_DIR}/GameVersion.cpp" +"${CMAKE_CURRENT_BINARY_DIR}/GameVersion.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/HelpScreen.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/Init.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/Intro.cpp" diff --git a/Ja2/GameSettings.cpp b/Ja2/GameSettings.cpp index 6b9ab20af..6046435e3 100644 --- a/Ja2/GameSettings.cpp +++ b/Ja2/GameSettings.cpp @@ -4299,7 +4299,7 @@ void FreeGameExternalOptions() void DisplayGameSettings( ) { //Display the version number - ScreenMsg( FONT_MCOLOR_LTYELLOW, MSG_INTERFACE, L"%s: %s %S %s", pMessageStrings[ MSG_VERSION ], zProductLabel, czVersionString, zBuildInformation ); + ScreenMsg( FONT_MCOLOR_LTYELLOW, MSG_INTERFACE, L"%s: %s %s", pMessageStrings[ MSG_VERSION ], zProductLabel, zBuildInformation ); //Display the difficulty level ScreenMsg( FONT_MCOLOR_LTYELLOW, MSG_INTERFACE, L"%s: %s", gzGIOScreenText[ GIO_DIF_LEVEL_TEXT ], zDiffSetting[gGameOptions.ubDifficultyLevel].szDiffName ); diff --git a/Ja2/GameVersion.cpp b/Ja2/GameVersion.cpp.in similarity index 84% rename from Ja2/GameVersion.cpp rename to Ja2/GameVersion.cpp.in index 6a38c8a5c..2c7fea5b4 100644 --- a/Ja2/GameVersion.cpp +++ b/Ja2/GameVersion.cpp.in @@ -39,7 +39,7 @@ #endif -CHAR8 czVersionString[16] = { "@Version@" }; -CHAR16 zBuildInformation[256] = { L"@Build@" }; +CHAR8 czVersionString[16] = { "@GIT_SHA@" }; +CHAR16 zBuildInformation[256] = { L"@GAME_BUILD_INFORMATION@" }; // SAVE_GAME_VERSION is defined in header, change it there diff --git a/Ja2/GameVersion.h b/Ja2/GameVersion.h index 8cc25371d..454fec370 100644 --- a/Ja2/GameVersion.h +++ b/Ja2/GameVersion.h @@ -13,9 +13,15 @@ extern "C" { // name of the product, Unfinished Business, Map Editor etc.. extern CHAR16 zProductLabel[64]; -// used for save game comparison +// this build's identity, stamped in by CMake from -DGIT_SHA. A packaged build sets +// it to the bare short commit SHA and nothing else, so it names that build exactly; +// a build nobody identified reads "local". Used for save game comparison, and +// stamped into crash reports so a packaged build's report can be matched to the PDB +// archived beside it for offline symbolization. extern CHAR8 czVersionString[16]; -// can contain information regarding the build: what git ref was the base (tag, branch), by whom, commit date, build date, etc.. +// human-readable build description, stamped in from -DGAME_BUILD_INFORMATION: the +// release name, its date and where it was built, or "local build ". For +// display only — never compared against, so it is free to change shape. extern CHAR16 zBuildInformation[256]; //ADB: I needed these here so I moved them, and why put them in *.cpp anyways? diff --git a/Ja2/jascreens.cpp b/Ja2/jascreens.cpp index 1b41bfd43..dc30fd437 100644 --- a/Ja2/jascreens.cpp +++ b/Ja2/jascreens.cpp @@ -354,9 +354,9 @@ UINT32 InitScreenHandle(void) SetFontForeground( FONT_MCOLOR_WHITE ); #ifdef _DEBUG - mprintf( 10, 10, L"%s: %s Debug %S %s", pMessageStrings[ MSG_VERSION ], zProductLabel, czVersionString, zBuildInformation ); + mprintf( 10, 10, L"%s: %s Debug %s", pMessageStrings[ MSG_VERSION ], zProductLabel, zBuildInformation ); #else - mprintf( 10, 10, L"%s: %s %S %s", pMessageStrings[ MSG_VERSION ], zProductLabel, czVersionString, zBuildInformation ); + mprintf( 10, 10, L"%s: %s %s", pMessageStrings[ MSG_VERSION ], zProductLabel, zBuildInformation ); #endif #if defined JA2BETAVERSION diff --git a/sgp/DEBUG.cpp b/sgp/DEBUG.cpp index ca26d3a94..b08300ae4 100644 --- a/sgp/DEBUG.cpp +++ b/sgp/DEBUG.cpp @@ -395,7 +395,7 @@ void _FailMessage(const char* message, unsigned lineNum, const char * functionNa sgp::dumpStackTrace(message); - mprintf( 10, 10, L"%s: %s %S %s", pMessageStrings[ MSG_VERSION ], zProductLabel, czVersionString, zBuildInformation ); + mprintf( 10, 10, L"%s: %s %s", pMessageStrings[ MSG_VERSION ], zProductLabel, zBuildInformation ); std::stringstream basicInformation; basicInformation << "Assertion Failure [Line " << lineNum;