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;