From c03cc75173042fcbdb13902b94b49da73754ca76 Mon Sep 17 00:00:00 2001 From: Nick Newson Date: Sun, 13 Sep 2026 09:28:11 +0100 Subject: [PATCH] Move the vcpkg baseline, and stop the tool drifting behind it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Baseline ea1a7396 (Aug 2026) -> a1cae005 (Sep 2026). A small move as these go: imgui 1.92.8#1 -> 1.92.9 and catch2 3.15.3 -> 3.16.0, with everything else unchanged. vulkan-headers and vulkan-loader stay at 1.4.357.0, still matching the SDK installed on the development machine, which is what keeps the mixed-vulkan-hpp trap out of reach rather than merely survivable. Taken now because the rule says between items and before a perf item, never after one: the next arc includes measurement work, and a number taken across a toolchain move cannot be attributed to the change that was supposed to cause it. THE CHECKOUT IS PINNED TO THE SAME COMMIT, which is the substantive half of this change. Every job checked vcpkg out at master, so the baseline fixed port VERSIONS while the tool, the triplets and the toolchain scripts floated free. Two consequences, and we have now met both: CI could break with no change in this repository, and a green run said nothing about the next one. A recent macOS failure looked for all the world like our problem and turned out to be a mid-stream HTTP/2 reset fetching a tarball — the diagnosis cost more than it should have precisely because "nothing changed here" was not evidence of anything. The SHA now appears in three places that must agree, and they move together or not at all: the baseline in vcpkg-configuration.json, the ref: on all four Checkout vcpkg steps, and VCPKG_COMMIT in container-run.sh. The Docker replica fetches BY SHA rather than by branch, so a persistent volume already holding the pin does no network work, and one holding an older pin moves to exactly this commit instead of to whatever master is today — which is what makes it a replica of CI rather than a second, differently-drifting build. The native macOS replica is the exception and is documented as such: it uses whatever VCPKG_ROOT the machine has, because pinning it would mean a script taking charge of someone's personal checkout. It is therefore the one runner that can drift, and a macOS-only failure CI does not reproduce should be treated as a suspect local checkout first. Verified the way a bump has to be. Full --clean-first rebuild on both platforms, because vcpkg preserves upstream file timestamps and an upgraded header can land older than the objects including it — the imgui bump that produced a runtime IMGUI_CHECKVERSION abort from a green incremental build is the precedent. tests-full on macOS (1707 cases) and on Linux in the Docker replica (1707 cases, all six guards), the Release contract on both, clang-tidy clean on Linux, and the render smoke with validation enabled: alive, SIGTERM 143, zero VUIDs, and no version-mismatch line. The determinism golden did not move on either platform, which is the result a dependency bump should produce. A moved hash here would have been a regression to investigate, not a golden to rewrite. --- .github/workflows/ci.yml | 20 ++++++++++++++++++++ CLAUDE.md | 17 ++++++++++++++--- README.md | 7 ++++--- docs/review-order.md | 2 +- tools/ci/container-run.sh | 17 ++++++++++++++--- vcpkg-configuration.json | 2 +- 6 files changed, 54 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 173674b..1050da5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -51,6 +51,11 @@ jobs: with: repository: microsoft/vcpkg path: vcpkg + # PINNED to the same commit as the manifest baseline. The baseline fixes port VERSIONS; + # this fixes the TOOL, the triplets and the toolchain scripts, which are what an unpinned + # checkout would let drift. Without it a green run proves nothing about the next one, and + # a build can break with no change in this repository. Move both together, never one. + ref: a1cae005c39be7b18ba319fced856b68d7276271 - name: Bootstrap vcpkg run: ./vcpkg/bootstrap-vcpkg.sh @@ -109,6 +114,11 @@ jobs: with: repository: microsoft/vcpkg path: vcpkg + # PINNED to the same commit as the manifest baseline. The baseline fixes port VERSIONS; + # this fixes the TOOL, the triplets and the toolchain scripts, which are what an unpinned + # checkout would let drift. Without it a green run proves nothing about the next one, and + # a build can break with no change in this repository. Move both together, never one. + ref: a1cae005c39be7b18ba319fced856b68d7276271 - name: Bootstrap vcpkg run: ./vcpkg/bootstrap-vcpkg.sh @@ -157,6 +167,11 @@ jobs: with: repository: microsoft/vcpkg path: vcpkg + # PINNED to the same commit as the manifest baseline. The baseline fixes port VERSIONS; + # this fixes the TOOL, the triplets and the toolchain scripts, which are what an unpinned + # checkout would let drift. Without it a green run proves nothing about the next one, and + # a build can break with no change in this repository. Move both together, never one. + ref: a1cae005c39be7b18ba319fced856b68d7276271 - name: Bootstrap vcpkg run: ./vcpkg/bootstrap-vcpkg.sh @@ -201,6 +216,11 @@ jobs: with: repository: microsoft/vcpkg path: vcpkg + # PINNED to the same commit as the manifest baseline. The baseline fixes port VERSIONS; + # this fixes the TOOL, the triplets and the toolchain scripts, which are what an unpinned + # checkout would let drift. Without it a green run proves nothing about the next one, and + # a build can break with no change in this repository. Move both together, never one. + ref: a1cae005c39be7b18ba319fced856b68d7276271 - name: Bootstrap vcpkg run: ./vcpkg/bootstrap-vcpkg.sh diff --git a/CLAUDE.md b/CLAUDE.md index 073571f..2205918 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -97,10 +97,21 @@ since a loader/ICD change can alter device capabilities. **Do it in the gap BETW before a perf item rather than after one** — measurements taken across a toolchain move cannot attribute a change to the work. -Current pin: `ea1a7396` (Aug 2026) — `vulkan-headers`/`vulkan-loader` **1.4.357.0**, matching the +Current pin: `a1cae005` (Sep 2026) — `vulkan-headers`/`vulkan-loader` **1.4.357.0**, matching the SDK installed here, which is what keeps the mixed-vulkan-hpp trap below out of reach; plus `glfw3 -3.5.1`, `glslang 16.4.0`, `spirv-tools 1.4.357.0`, `imgui 1.92.8#1`, `shaderc 2026.2`, `ktx 4.4.2`, -`fastgltf 0.9.0`, `catch2 3.15.3`, `vulkan-memory-allocator 3.4.0`. +3.5.1`, `glslang 16.4.0`, `spirv-tools 1.4.357.0`, `imgui 1.92.9`, `shaderc 2026.2`, `ktx 4.4.2`, +`fastgltf 0.9.0`, `catch2 3.16.0`, `vulkan-memory-allocator 3.4.0`. + +**The vcpkg CHECKOUT is pinned to that same commit, and the two move together.** The baseline fixes +port *versions*; the checkout fixes everything else vcpkg supplies — the tool, the triplets, the +toolchain scripts — and an unpinned checkout means CI can break with no change in this repository +and a green run says nothing about the next one. The SHA therefore appears in three places that must +agree: `vcpkg-configuration.json`'s baseline, the `ref:` on all four `Checkout vcpkg` steps in +`.github/workflows/ci.yml`, and `VCPKG_COMMIT` in `tools/ci/container-run.sh` (which fetches by SHA, +so a persistent Docker volume already holding the pin does no network work). The native macOS +replica is the exception: it uses whatever `VCPKG_ROOT` the machine has, so it is the one runner +that can drift — treat a macOS-only failure that CI does not reproduce as a suspect local checkout +first. **"Full rebuild" there means `--clean-first`, and that is not pedantry.** vcpkg preserves each port's *upstream* file timestamps, so an upgraded header can land with an mtime OLDER than the object diff --git a/README.md b/README.md index ca4beb9..b03f387 100644 --- a/README.md +++ b/README.md @@ -426,9 +426,10 @@ baseline is not comparable from frame one. ## Dependencies Managed via the vcpkg manifest (`vcpkg.json`); every version comes from the baseline pinned in -`vcpkg-configuration.json`, currently `ea1a7396` (Aug 2026) — Vulkan headers + loader **1.4.357.0**, -`glfw3 3.5.1`, `glslang 16.4.0`, `spirv-tools 1.4.357.0`, `imgui 1.92.8`, `shaderc 2026.2`, -`ktx 4.4.2`, `fastgltf 0.9.0`, `catch2 3.15.3`, `vulkan-memory-allocator 3.4.0`: +`vcpkg-configuration.json`, currently `a1cae005` (Sep 2026) — Vulkan headers + loader **1.4.357.0**, +`glfw3 3.5.1`, `glslang 16.4.0`, `spirv-tools 1.4.357.0`, `imgui 1.92.9`, `shaderc 2026.2`, +`ktx 4.4.2`, `fastgltf 0.9.0`, `catch2 3.16.0`, `vulkan-memory-allocator 3.4.0`. CI checks out vcpkg +at that same commit, so the tool and the ports move together: - `vulkan-headers` — Vulkan API headers (the Vulkan **loader** + `glfw3` arrive transitively, so both come from vcpkg — no system Vulkan SDK / GLFW needed to build) diff --git a/docs/review-order.md b/docs/review-order.md index 7a10b07..b9fcb4c 100644 --- a/docs/review-order.md +++ b/docs/review-order.md @@ -19,7 +19,7 @@ Read these first when a change touches build configuration, CI, or local tooling | `CMakePresets.json` | The `vcpkg` preset pins Apple Clang on macOS, selects `Dev`, and exports `compile_commands.json` for `clangd`. | | `CMakeLists.txt` | `Dev` is the default build type (`-O2 -g`, no `NDEBUG`); `FIRE_ENGINE_WARNINGS_AS_ERRORS` is CI-only by default; CTest registers `test_fire_engine` (`~[slow]`) so plain CTest stays fast; `tests-full` runs the all-tags Catch2 binary plus the layering, shared-shader-block, shadow-bias, GPU-limits, shadow-matrix and release-contract guards. Both of those are DEFAULT Catch2 runs, so neither sees a hidden (`[.]`) case — which is what keeps the `[release-contract]` sentinel (`tests/release_contract.cpp`, registered first in the test target) out of the Dev suites while an explicit tag selection picks it up; `shaders/` is on the C++ include path (`BUILD_INTERFACE`, PUBLIC) because the public header `graphics/gpu_limits.hpp` includes `shaders/gpu_limits.glsl` — the dual-language limits file; `run-clang-tidy` appears only when `clang-tidy` is installed. **Read the include-order block near the top before touching dependency includes**: `CMAKE_NO_SYSTEM_FROM_IMPORTED` + `include_directories(BEFORE …)` force vcpkg's headers to arrive as `-I`, because clang searches `/usr/local/include` ahead of every `-isystem` path — with a Vulkan SDK installed there, our TUs compiled against ITS vulkan-hpp while includes resolved relative to a vcpkg header got vcpkg's, and the first RAII call aborted on a header-version assert with no bad C++ behind it (real: SDK 1.4.357 beside the pinned 1.4.335). The guard checks both `VCPKG_INSTALLED_DIR` spellings and hard-errors if the directory is missing, since a silent miss restores the mixed-header build. `SHADER_INCLUDES` lists the shared GLSL includes (`gpu_limits.glsl`, `light_ubo.glsl`, `material.glsl`, `shadow_push.glsl`, `shadow_depth.glsl`, `self_shadow_second.glsl`) so editing one rebuilds every shader — a stale `.spv` against a changed block is the layout bug the include exists to prevent. Consequence: third-party headers are no longer warning-suppressed, so a finding gets a narrow `#pragma` at the include site (`src/graphics/frame_capture.cpp`), never a weakened flag. | | `.clang-format` / `.clang-tidy` | Formatting is CI-gated. Tidy is the first-pass static-analysis config for engine `src/` plus `include/fire_engine/`; disabled checks are documented inline. | -| `.github/workflows/ci.yml` + `tools/ci/ci-stages.sh` | Five parallel jobs: a clang-format dry-run, `run-clang-tidy`, `tests-full` on Ubuntu and on macOS/arm64 (each validating its own determinism golden), and `release-contract` — the only job that builds `Release`, which is the only configuration where the suite's `#ifdef NDEBUG` bodies are real code. The stage BODIES live in `ci-stages.sh` and are shared with both local replicas, so a stage edited in one place cannot drift from the others; the `release-contract` job sources that file rather than restating its commands, and `all` includes the stage on Linux only (`CI_RELEASE_CONTRACT`). Read `ci_release_contract` together with the `vcpkg-release` preset: it builds `test_fire_engine` alone into `build-release/` while pointing `VCPKG_INSTALLED_DIR` back at `build/vcpkg_installed`, which is what lets the CI cache serve both trees. | +| `.github/workflows/ci.yml` + `tools/ci/ci-stages.sh` | Five parallel jobs: a clang-format dry-run, `run-clang-tidy`, `tests-full` on Ubuntu and on macOS/arm64 (each validating its own determinism golden), and `release-contract` — the only job that builds `Release`, which is the only configuration where the suite's `#ifdef NDEBUG` bodies are real code. Every job checks vcpkg out **at the commit the manifest baseline names** (`ref:` on each `Checkout vcpkg`), and `tools/ci/container-run.sh` pins the same SHA in `VCPKG_COMMIT` — the baseline fixes port versions, the checkout fixes the tool, the triplets and the toolchain scripts, and the two are moved together or not at all. The stage BODIES live in `ci-stages.sh` and are shared with both local replicas, so a stage edited in one place cannot drift from the others; the `release-contract` job sources that file rather than restating its commands, and `all` includes the stage on Linux only (`CI_RELEASE_CONTRACT`). Read `ci_release_contract` together with the `vcpkg-release` preset: it builds `test_fire_engine` alone into `build-release/` while pointing `VCPKG_INSTALLED_DIR` back at `build/vcpkg_installed`, which is what lets the CI cache serve both trees. | | `tools/assetgen/geometry.py` | Primitive builders, all returning the same `(positions, normals, indices)` triple: box (24 verts, per-face normals), tetrahedron, UV sphere, flat-shaded mesh-from-triangles, and `combine_geometry` for compounds. Plus the shared vector helpers. Everything is authored at true size with node scale left at 1. | | `tools/assetgen/quaternions.py` | glTF `[x, y, z, w]` order throughout — the easiest thing to get wrong when hand-authoring. `look_at_quat` builds a camera orientation (glTF cameras look down −Z); `quat_from_to` is the "aim this at that" helper. | | `tools/assetgen/scene.py` | The `Scene` assembler: one self-contained `.gltf` with the binary embedded as a base64 data URI (no sidecar `.bin`, no textures). Note the two layers — `node`/`box`/`sphere` are generic and pass `extras` through verbatim, while `box_body`/`sphere_body`/`compound_body`/`static_mesh_body`/`static_floor` are physics wrappers that inject `extras.Physics` and default the collider to match the mesh. `generator` is a constructor argument, not a hard-coded string. `write_gltf`'s 2-space-plus-newline formatting is part of the contract: the build regenerates the committed assets, so a formatting change is churn in every one. | diff --git a/tools/ci/container-run.sh b/tools/ci/container-run.sh index 4d3606d..ca266d4 100755 --- a/tools/ci/container-run.sh +++ b/tools/ci/container-run.sh @@ -24,15 +24,26 @@ sync_source() /repo/ /work/fireEngine/ } +# The vcpkg TOOL commit, pinned to the same SHA as the manifest baseline in +# vcpkg-configuration.json and as the GitHub jobs' checkout. The baseline pins port versions; this +# pins everything else vcpkg brings (the tool itself, the triplets, the toolchain scripts). A +# replica that tracked master would drift away from the CI it exists to reproduce. Move it with the +# baseline, never on its own. +VCPKG_COMMIT=a1cae005c39be7b18ba319fced856b68d7276271 + ensure_vcpkg() { if [ ! -d "${VCPKG_ROOT}/.git" ]; then rm -rf "${VCPKG_ROOT:?}"/* git clone https://github.com/microsoft/vcpkg "${VCPKG_ROOT}" - else - git -C "${VCPKG_ROOT}" fetch --depth=1 origin master - git -C "${VCPKG_ROOT}" checkout --detach FETCH_HEAD fi + # Fetched BY SHA rather than by branch: the volume persists between runs, so a checkout already + # holding the pinned commit does no network work at all, and one holding an older pin moves to + # exactly this commit rather than to whatever master is today. + if ! git -C "${VCPKG_ROOT}" cat-file -e "${VCPKG_COMMIT}^{commit}" 2>/dev/null; then + git -C "${VCPKG_ROOT}" fetch --depth=1 origin "${VCPKG_COMMIT}" + fi + git -C "${VCPKG_ROOT}" checkout --detach "${VCPKG_COMMIT}" "${VCPKG_ROOT}/bootstrap-vcpkg.sh" -disableMetrics } diff --git a/vcpkg-configuration.json b/vcpkg-configuration.json index d37d8b4..8333403 100644 --- a/vcpkg-configuration.json +++ b/vcpkg-configuration.json @@ -1,7 +1,7 @@ { "default-registry": { "kind": "git", - "baseline": "ea1a7396b05637a53bf23c078647ecc0edee4b80", + "baseline": "a1cae005c39be7b18ba319fced856b68d7276271", "repository": "https://github.com/microsoft/vcpkg" }, "registries": [