[CI] Touch build.ninja after untar to prevent cascade rebuild#197
Merged
Conversation
Diagnostic in #195 showed test-side ninja invocations cascade-rebuild because tar -m sets per-file mtimes from sequential extraction order, leaving build.ninja older than CMakeCache.txt within the same build dir: ninja explain: output build.ninja older than most recent input CMakeCache.txt (1778450813199019645 vs 1778450820955083819) That triggers ninja's auto-regen rule, cmake regenerates build.ninja with subtly different command lines, and ninja's command-hash check then rebuilds every .o it has on file. Fix: explicitly touch build.ninja after untar so it's the newest file in the build tree. Trivial 3-file touch, applies to all 3 test jobs via the shared "Untar build trees" step pattern.
lamb-j
added a commit
that referenced
this pull request
May 11, 2026
Same fix as #197 for the Linux variant. tar -m sets per-file mtimes from sequential extraction order; build.ninja ends up older than CMakeCache.txt, triggering ninja's regen rule and cascade rebuild. Touch build.ninja explicitly to make it the newest file in the tree.
lamb-j
added a commit
to ROCm/llvm-project
that referenced
this pull request
May 11, 2026
Same fix as ROCm/SPIRV-LLVM-Translator#197. tar -m sets per-file mtimes from sequential extraction order; build.ninja ends up older than CMakeCache.txt, triggering ninja's regen rule and cascade rebuild. Touch build.ninja explicitly to make it the newest file in the tree.
Contributor
🔴 New failures (0) — likely caused by this PR(none) 🟢 Fixed by this PR (0) — failing on baseline, passing here(none)
|
lamb-j
added a commit
that referenced
this pull request
May 13, 2026
* [CI] Add Windows variant (Build + 3 test jobs) Adds Windows::release dispatcher job + spirv-ci-windows.yml — full parity with the Linux variant: Build + Test SPIRV translator lit (with baseline-diff + gate) + Test LLVM SPIRV codegen + Test Comgr. Mirrors TheRock's Windows CI conventions: - Runner: azure-windows-scale-rocm - No container (Windows runs natively) - MSVC env via ilammy/msvc-dev-cmd action - Pinned ninja 1.12.1 via Chocolatey (1.13.0 has a known bug) - bash shell defaults - git long paths enabled - No strip step (PE/COFF debug info lives in separate .pdb) Translator-lit uses a separate sticky-comment marker (spirv-ci:translator-lit-windows) so it doesn't fight with the Linux variant's comment on the same PR. After this lands the PR rollup gets 4 more checks: SPIRV Compiler CI / Windows::release / Build SPIRV Compiler CI / Windows::release / Test SPIRV translator lit SPIRV Compiler CI / Windows::release / Test LLVM SPIRV codegen SPIRV Compiler CI / Windows::release / Test Comgr Wall-time risk: Windows build is typically 25-45 min vs ~14 min on Linux. Total wall-clock for a PR roughly 2x. Test-side risk: Windows lit + Comgr fixture generation are unproven on this runner. If individual test jobs flake, can break them out into follow-up PRs. Companion: ROCm/llvm-project copy of this workflow doesn't get the Windows variant in this PR — defer until Windows is proven on the translator side. * [CI] Touch build.ninja after untar (Windows variant) Same fix as #197 for the Linux variant. tar -m sets per-file mtimes from sequential extraction order; build.ninja ends up older than CMakeCache.txt, triggering ninja's regen rule and cascade rebuild. Touch build.ninja explicitly to make it the newest file in the tree. * [CI debug] Locate AMDDeviceLibsConfig.cmake on Windows Comgr's find_package(AMDDeviceLibs) fails on Windows. Add a one-shot debug step before the Comgr configure to print: - $PWD (validates path expansion in MSVC bash) - file path of AMDDeviceLibs*.cmake under build-device-libs - any cmake/ subdirs under build-device-libs Once we see where the file actually lands we can pin AMDDeviceLibs_DIR explicitly. Revert this debug after the targeted fix. * [CI] Use pwd -W on Windows for cmake -D paths Comgr's find_package(AMDDeviceLibs) failed on Windows even though the config file was at the expected build-device-libs/lib/cmake/AMDDeviceLibs/ location. Cause: $PWD in MSYS bash returns a Unix-style path (/c/home/runner/...) that cmake's find_package can't traverse on Windows. Switch device-libs and Comgr configures to compute PWD_WIN=$(pwd -W) which returns Windows-style with forward slashes (C:/home/runner/...) that cmake handles natively. Also pin AMDDeviceLibs_DIR explicitly as belt-and-suspenders. Drop the debug step. * [CI] Skip Comgr gtests on Windows (lit + ctest only) Comgr's HotswapMCTests gtest binary fails to link on Windows because the hotswap apply* path is fully MSVC-guarded out (see ROCm/llvm-project#2479). Replace `ninja check-comgr` with `ninja test test-lit` on Windows so we run the lit suite and ctest without dragging in the gtest aggregator (test-unit). Linux variant unchanged — still runs full check-comgr including the gtest binaries. * [CI] Skip Comgr test-unit subdirectory on Windows Drop test-unit (gtest binaries) at configure time so check-comgr's lit and ctest layers still work on Windows. HotswapMCTests fails to link because of the MSVC-guarded hotswap apply* path (ROCm/llvm-project#2479); rather than special-casing the test step, sed out add_subdirectory(test-unit) from Comgr's CMakeLists.txt before configuring. This is a workflow-side hack — when the upstream Comgr issue is resolved (or Comgr exposes a CMake option to gate test-unit), drop the sed and revert to plain check-comgr. * [CI] Enable zstd via vcpkg for clang-offload-bundler (Windows) Two of the Comgr lit failures on Windows (unbundle, cache) were hitting "Compression not supported" from clang-offload-bundler — LLVM was built without zstd or zlib. The manylinux container ships zstd dev libs on Linux; on Windows we install via vcpkg (already present at $VCPKG_ROOT) and pass CMAKE_TOOLCHAIN_FILE + LLVM_ENABLE_ZSTD=FORCE_ON so the configure fails loudly if zstd isn't located. * [CI] Use $VCPKG_ROOT vcpkg.exe to install zstd (Windows) The runner has two vcpkg installs: C:\vcpkg (first in PATH) and the MSVC-bundled one at $VCPKG_ROOT. Plain `vcpkg install` resolved to C:\vcpkg, but Configure LLVM's CMAKE_TOOLCHAIN_FILE points at the MSVC-bundled vcpkg.cmake, so find_package(zstd) couldn't see the package. Invoke $VCPKG_ROOT\vcpkg.exe directly so install and lookup share the same root. * [CI] Switch to C:\vcpkg for classic-mode install (Windows) The MSVC-bundled vcpkg at $VCPKG_ROOT is manifest-mode only — `vcpkg install <pkg>` errors out with "this distribution does not have a classic mode instance". The other vcpkg on the runner (C:\vcpkg, what plain `vcpkg` resolves to via PATH) supports classic mode. Point both the install command and CMAKE_TOOLCHAIN_FILE at C:\vcpkg so the package zstd installs and the LLVM cmake find_package locate it. * [CI] Pass vcpkg toolchain to device-libs and Comgr (Windows) LLVM built with zstd exports LLVMSupport linking against zstd::libzstd_shared. Downstream find_package(LLVM) callers in device-libs and Comgr need CMAKE_TOOLCHAIN_FILE pointing at the same vcpkg.cmake so the imported zstd target is defined when the export file is processed; otherwise cmake errors with "target was not found". * [CI] Install zstd on Windows test runners (tactical) build.ninja embeds C:/vcpkg/installed/.../zstd.lib as a build-edge input that ninja validates at planning time. Test jobs running ninja against the downloaded artifact need the same vcpkg state as the build runner. Install zstd in each Windows test job to match. This pattern doesn't scale — every new build-time system dep requires updates to N test jobs. Followup planned to drop ninja from test jobs in favor of llvm-lit-direct invocation, which removes the test-time dependency on build-time system libs. * [CI] Drop SED workaround for Comgr test-unit on Windows ROCm/llvm-project#2491 (merged to amd-staging) fixes the HotswapMCTests link failure on Windows that #2479 tracked. The SED that stripped add_subdirectory(test-unit) is no longer needed — test-unit builds and runs cleanly. check-comgr on Windows now exercises lit + ctest + gtest, matching the Linux variant. * [CI] Pin test jobs to Build job's resolved llvm-project SHA Build and test jobs both used \`ref: amd-staging\` for llvm-project checkout. If amd-staging advances mid-run (e.g., upstream merge lands between Build and Test), test jobs land on a newer commit than what Build produced. ninja then sees source > object, decides to rebuild, and uses the OLD compile command from build.ninja — which can lack include paths added by the newer commit (recently hit when libc/shared became a Support dependency for APFloat). Build job now exports the resolved SHA as a job output. Test jobs check out llvm-project at that exact SHA via needs.build.outputs. Both Linux and Windows variants.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test-side ninja invocations (codegen, comgr, translator-lit) cascade-rebuild even with `tar -xmf`. Diagnosed in #195:
```
ninja explain: output build.ninja older than most recent input CMakeCache.txt
(1778450813199019645 vs 1778450820955083819)
```
`tar -m` sets each extracted file's mtime to its individual extraction time. Tar processes files sequentially, so within the build dir some files end up newer than others — `build.ninja` (large, extracted earlier) ends up older than `CMakeCache.txt` (extracted later). Ninja's regen rule sees an "input newer than build.ninja" → cmake regen → command-hash drift → rebuild everything.
Fix
After untar, explicitly `touch build/build.ninja build-comgr/build.ninja build-device-libs/build.ninja` so build.ninja is the newest file in the tree. Ninja's regen check passes, no rebuild.
Why this is the right scope of fix
TheRock avoids this entirely by never shipping build trees — they ship installed artifacts and run `ctest` directly against pre-built binaries (no ninja involved). That's the right long-term direction and probably how this workflow looks once it moves into TheRock. For now this 1-line fix unblocks the in-flight PRs (#196 Windows, #2451 llvm-project both hit the same bug) without a full restructure.
After this lands I'll close #195 (the debug PR) and apply the same fix to #196 + #2451.