From bde6cfc0be0affb720b23a9783f6f3766c13b7b1 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Tue, 22 Sep 2026 03:01:12 +0200 Subject: [PATCH 1/2] build/ci: one Catch2 for every configure, fetched and never found (fixes #674, refs #666) `find_package(Catch2 CONFIG QUIET)` handed the build whatever Catch2 the machine had, and seventeen Linux legs installed `catch2` from apt without pinning it. clang-tidy therefore analysed `REQUIRE`/`TEST_CASE` expansions against a different release on a runner than on a workstation, and disagreed about what counted as a finding -- silently, because nothing recorded which Catch2 a given build used. The mechanism was one level lower than #674 states, and the ticket's own closing condition reads as satisfied by that. It is not: the compile database records no Catch2 include path at all, because `find_package` resolved to an imported target whose INTERFACE_INCLUDE_DIRECTORIES is `/usr/include`, which CMake drops as an implicit compiler directory. The header that got included was simply the one the compiler's default system search path found. Same divergence, reached by a different route, so the fix is the one the ticket names. The root CMakeLists.txt now fetches v3.8.1 unconditionally, with `SYSTEM`. `SYSTEM` is the load-bearing word. Without it a fetched dependency's include directory arrives as `-I`, clang-tidy classifies Catch2's macro expansions as user code, and `tests/test_executor.cpp` goes from 1 diagnostic to 45 (21 cppcoreguidelines-avoid-do-while and 11 misc-use-anonymous-namespace out of REQUIRE/TEST_CASE, plus notes). Measured both ways on this commit, clang-tidy 22.1.8, two configures differing only in that keyword. The fetch moved *above* the examples rather than staying in the Tests section, and that is a bug fix rather than tidying. examples/{concepts,bank, vetted_hmac} are add_subdirectory()'d before that section, and each ran its own `find_package(Catch2 3 CONFIG QUIET)` with a `message(WARNING)` fallback -- so on any machine without the distro package those three suites were silently not built while the configure still succeeded. With apt's catch2 gone from CI, leaving the fetch where it was would have dropped bank's and concepts' suites from every leg that builds them, and each leg would still have reported success. All five `find_package(Catch2 ...)` sites are now `if(NOT TARGET Catch2::Catch2WithMain)` with FATAL_ERROR: the target either exists because the root fetched it, or the build ordering is broken and says so. The nine `*/tests/.clang-tidy` files are rewritten, not renumbered. Their prose asserted "CI pins catch2 3.4.0 -- ubuntu-24.04's package, which predates that comment"; v3.8.1 carries the `NOLINT(bugprone-chained-comparison)` those files attribute to 3.15.3, so the sentence is now false and the suppression inert. (It was already inert on master for any contributor without the distro package, who got 3.8.1 through the existing fallback.) The rewritten prose names no version at all -- the pin lives in `MORPH_CATCH2_TAG` and nowhere else -- says the entry is measured to subtract nothing today, and says why it stays: the argument is about the macro, not about a release. `scripts/check_catch2_pin.sh` and its self-test are retired rather than re-pointed, and that is the decision the ticket asks for. Its behavioural half read `/usr/include/catch2`, which nothing installs any more and the build never consults -- a gate asserting a pin nothing installs is precisely the failure this cluster is made of. Its textual half compared prose against `CATCH2_VERSION` in ci.yml, which is gone because ci.yml no longer decides the Catch2. Re-pointing it at the CMake pin would have required the nine files to name the version again, manufacturing nine duplicate strings so that a gate had something to compare -- a gate whose subject it created. The one real duplication the new design would have introduced, `morph_cache_dep`'s tag against `GIT_TAG` three lines below, is removed by a variable instead of gated. `catch2` is dropped from `vcpkg.json` and from CONTRIBUTING.md as well. vcpkg is the Windows legs' version of "whatever the machine has", and with the find_package gone its Catch2 would be installed and never consumed. Removing the manifest entry adds no cost -- the four Windows presets stop using vcpkg's Catch2 the moment the find_package goes, whatever the manifest says -- it only stops them installing a package nothing links. The real and unmeasured cost is that those four presets now build Catch2 from source with no compiler cache at all (the Windows job has neither sccache nor MORPH_DEP_CACHE), which #674's cost analysis counted only for the seventeen Linux legs. If a Windows leg becomes the critical path, that is the line to revisit. Verified on this commit, Arch Linux, clang 22.1.8, cmake 4.4.3: - configure from empty, MORPH_BUILD_TESTS=ON: the compile database carries `-isystem .../_deps/catch2-src/src/catch2/..` and `-isystem .../_deps/catch2-build/generated-includes`, and no `-I` for Catch2; - clang-tidy -p tests/test_executor.cpp -> 1 diagnostic; same TU against a build differing only by the missing `SYSTEM` -> 45; - full build and `ctest -j8`: `100% tests passed out of 1588`, `Total Test time (real) = 66.85 sec`, including morph_concepts_tests, which is one of the three suites the old ordering would have skipped; - configure from empty with MORPH_BUILD_LADDER=ON MORPH_BUILD_QT=ON MORPH_BUILD_BANK_EXAMPLE=ON MORPH_BUILD_OFFLINE_SQLITE=ON: all 9 rungs register and generate succeeds, so examples/common's TARGET guard and `include(Catch)` in cmake/morph_add_rung.cmake both resolve against the fetched Catch2; - check_rung_filters.sh, check_tidy_suppression_scope.sh, check_workflow_job_banners.py, check_workflow_option_coverage.py, check_ci_clang_pin.sh, check_bidi_controls.py and test_check_sanitizer_instrumentation.sh all pass. Not verified: the ladder and bank binaries were configured but not built or run here; no measurement was taken on a GitHub runner, so the build-time cost (~1.7 min added to the critical path, ~28 runner-minutes, near zero warm via sccache) is carried over from #674's measurements and the warm figure remains inferred. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW --- .github/workflows/ci.yml | 75 +++------ .github/workflows/mutation.yml | 4 +- CMakeLists.txt | 103 +++++++++--- CONTRIBUTING.md | 10 +- examples/bank/CMakeLists.txt | 14 +- examples/bank/tests/.clang-tidy | 33 +++- examples/bookmarks/tests/.clang-tidy | 33 +++- examples/common/CMakeLists.txt | 7 +- examples/common/testkit/.clang-tidy | 33 +++- examples/concepts/CMakeLists.txt | 22 ++- examples/crm/tests/.clang-tidy | 33 +++- examples/kanban/tests/.clang-tidy | 33 +++- examples/ledger/tests/.clang-tidy | 33 +++- examples/lims/tests/.clang-tidy | 33 +++- examples/pastebin/tests/.clang-tidy | 33 +++- examples/polls/tests/.clang-tidy | 33 +++- examples/vetted_hmac/CMakeLists.txt | 35 ++--- scripts/check_catch2_pin.sh | 218 -------------------------- scripts/test_check_catch2_pin.sh | 226 --------------------------- vcpkg.json | 1 - 20 files changed, 384 insertions(+), 628 deletions(-) delete mode 100755 scripts/check_catch2_pin.sh delete mode 100755 scripts/test_check_catch2_pin.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 53bfac2b1..66f26b4f2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,27 +85,14 @@ env: # the cache itself -- jobs still read each other's entries. FASTCACHE_PREFETCH_GROUP: "${{ github.run_id }}-${{ github.job }}" CLANG_VERSION: "22" - # The Catch2 the clang-tidy-diff job analyses against. Not a version this - # workflow installs -- `apt-get install -y catch2` takes whatever - # ubuntu-24.04 ships -- but a record of what that has been measured to be, - # which the "Assert the Catch2 this job analyses against" step below reads - # back off the runner and fails on if it has moved. - # - # It is worth recording because it is load-bearing and invisible. Which - # Catch2 is on the include path decides whether - # readability-function-cognitive-complexity findings on a TEST_CASE body - # reach this job at all, so a workstation with a different one can run the - # same clang-tidy over the same diff with the same flags and exit 0 where - # this job exits 1 -- silently, reporting nothing rather than reporting less - # (morph#666). An unpinned package that decides a gate's outcome and moves - # without notice is the shape this repository keeps getting caught by. - # - # ubuntu-24.04's package is 3.4.0-1build1 (Launchpad, noble Release pocket), - # and the clang-tidy-report artifact of run 35581623269 prints - # `/usr/include/catch2/internal/catch_test_registry.hpp:121` with - # `INTERNAL_CATCH_TESTCASE2( INTERNAL_CATCH_UNIQUE_NAME( dummyFunction ) )` - # and no `, __VA_ARGS__` -- v3.4.0's text exactly, and not v3.5.3's. - CATCH2_VERSION: "3.4.0" + # No CATCH2_VERSION here any more, deliberately (morph#674). This workflow + # used to record which Catch2 the clang-tidy-diff job would find on the + # runner, because the distro package it installed pinned nothing and the + # answer decided which findings inside REQUIRE/TEST_CASE expansions + # reached the job. The build no longer asks the runner: the root + # CMakeLists.txt fetches one pinned Catch2 for every configure, with + # SYSTEM, and no job installs the distro package. The version is `MORPH_CATCH2_TAG` in the root + # CMakeLists.txt, which is also the only place it appears. # MORPH_BUILD_FORMS_QML needs Qt 6.5+; ubuntu-24.04 apt still ships 6.4.2. QT_VERSION: "6.8.1" @@ -269,7 +256,7 @@ jobs: sudo apt-get install -y software-properties-common sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test sudo apt-get update -q - sudo apt-get install -y gcc-15 g++-15 ninja-build catch2 + sudo apt-get install -y gcc-15 g++-15 ninja-build sudo update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-15 15 sudo update-alternatives --install /usr/bin/g++ g++ /usr/bin/g++-15 15 @@ -277,7 +264,7 @@ jobs: if: startsWith(matrix.preset, 'clang-') run: | sudo apt-get update -q - sudo apt-get install -y ninja-build catch2 + sudo apt-get install -y ninja-build # Download, check, then execute -- not # `wget -qO- https://apt.llvm.org/llvm.sh | sudo bash` (morph#681). # `wget -q` writes no error document, so on an HTTP 4xx/5xx it exits @@ -490,7 +477,7 @@ jobs: - name: Install Clang ${{ env.CLANG_VERSION }} from apt.llvm.org run: | sudo apt-get update -q - sudo apt-get install -y ninja-build catch2 libsqlite3-dev + sudo apt-get install -y ninja-build libsqlite3-dev # --fail, a file, and a non-empty check -- never `wget -qO- | sudo # bash`: see linux-compilers' identical install step for why the piped # form reported success having installed nothing (morph#681). @@ -658,7 +645,7 @@ jobs: - name: Install Clang ${{ env.CLANG_VERSION }} from apt.llvm.org run: | sudo apt-get update -q - sudo apt-get install -y ninja-build catch2 libsqlite3-dev + sudo apt-get install -y ninja-build libsqlite3-dev # --fail, a file, and a non-empty check -- never `wget -qO- | sudo # bash`: see linux-compilers' identical install step for why the piped # form reported success having installed nothing (morph#681). @@ -881,7 +868,7 @@ jobs: # coverage leg, ladder-tests, linux-all-features) for Qt's GL platform # integration; carried here for the same reason even though this leg's # test run itself stays off-GUI. - sudo apt-get install -y ninja-build catch2 libsqlite3-dev \ + sudo apt-get install -y ninja-build libsqlite3-dev \ unixodbc-dev libsqliteodbc libyaml-cpp-dev libzip-dev libgl1-mesa-dev # --fail, a file, and a non-empty check -- never `wget -qO- | sudo # bash`: see linux-compilers' identical install step for why the piped @@ -1081,7 +1068,7 @@ jobs: # through CPM; libgl1-mesa-dev for Qt's GL platform integration, # which every job configuring MORPH_BUILD_QT=ON alongside a Qt GUI # target installs. - sudo apt-get install -y ninja-build catch2 libsqlite3-dev \ + sudo apt-get install -y ninja-build libsqlite3-dev \ unixodbc-dev libsqliteodbc libyaml-cpp-dev libzip-dev libgl1-mesa-dev # --fail, a file, and a non-empty check -- never `wget -qO- | sudo # bash`: see linux-compilers' identical install step for why the piped @@ -1261,13 +1248,13 @@ jobs: key: apt-qt-${{ hashFiles('.github/workflows/ci.yml') }} restore-keys: apt-qt- - - name: Install GCC 15, ninja, catch2, Qt6 WebSockets + - name: Install GCC 15, ninja, Qt6 WebSockets run: | sudo apt-get update -q sudo apt-get install -y software-properties-common sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test sudo apt-get update -q - sudo apt-get install -y gcc-15 g++-15 ninja-build catch2 \ + sudo apt-get install -y gcc-15 g++-15 ninja-build \ qt6-base-dev qt6-websockets-dev qt6-tools-dev libgl1-mesa-dev sudo update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-15 15 sudo update-alternatives --install /usr/bin/g++ g++ /usr/bin/g++-15 15 @@ -1506,7 +1493,7 @@ jobs: key: apt-qt-${{ hashFiles('.github/workflows/ci.yml') }} restore-keys: apt-qt- - - name: Install GCC 15, ninja, catch2 + - name: Install GCC 15, ninja if: steps.filter.outputs.run == 'true' run: | sudo apt-get update -q @@ -1530,7 +1517,7 @@ jobs: # the moment MORPH_BUILD_LADDER=ON pulls Lightweight in. # Qt itself is installed by the aqtinstall step below, not apt: see # that step's comment for why the distro package is unusable here. - sudo apt-get install -y gcc-15 g++-15 ninja-build catch2 \ + sudo apt-get install -y gcc-15 g++-15 ninja-build \ libsqlite3-dev libyaml-cpp-dev libzip-dev libgl1-mesa-dev \ unixodbc-dev libsqliteodbc sudo update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-15 15 @@ -1860,7 +1847,7 @@ jobs: key: apt-ladder-asan-${{ hashFiles('.github/workflows/ci.yml') }} restore-keys: apt-ladder-asan- - - name: Install Clang ${{ env.CLANG_VERSION }}, ninja, catch2, ODBC + - name: Install Clang ${{ env.CLANG_VERSION }}, ninja, ODBC if: steps.filter.outputs.run == 'true' run: | sudo apt-get update -q @@ -1869,7 +1856,7 @@ jobs: # whose fixtures open a real `DRIVER=SQLite3` connection at test # time. libyaml-cpp-dev/libzip-dev/libgl1-mesa-dev round out the # same set every other ladder-building Linux job installs. - sudo apt-get install -y ninja-build catch2 libsqlite3-dev \ + sudo apt-get install -y ninja-build libsqlite3-dev \ unixodbc-dev libsqliteodbc libyaml-cpp-dev libzip-dev libgl1-mesa-dev # --fail, a file, and a non-empty check -- never `wget -qO- | sudo # bash`: see linux-compilers' identical install step for why the piped @@ -2131,7 +2118,7 @@ jobs: # packages, not through CPM — without these, Lightweight's configure # fails with "could not find a package configuration file" the # moment MORPH_BUILD_LADDER=ON pulls it in here. - sudo apt-get install -y ninja-build catch2 \ + sudo apt-get install -y ninja-build \ libsqlite3-dev libsodium-dev libssl-dev \ unixodbc-dev libsqliteodbc \ libyaml-cpp-dev libzip-dev \ @@ -2375,13 +2362,13 @@ jobs: key: apt-valgrind-${{ hashFiles('.github/workflows/ci.yml') }} restore-keys: apt-valgrind- - - name: Install GCC 15, ninja, catch2, valgrind + - name: Install GCC 15, ninja, valgrind run: | sudo apt-get update -q sudo apt-get install -y software-properties-common sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test sudo apt-get update -q - sudo apt-get install -y gcc-15 g++-15 ninja-build catch2 valgrind + sudo apt-get install -y gcc-15 g++-15 ninja-build valgrind sudo update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-15 15 sudo update-alternatives --install /usr/bin/g++ g++ /usr/bin/g++-15 15 @@ -2569,7 +2556,7 @@ jobs: # SQLite ODBC *driver* is not strictly exercised here; it is installed # alongside unixodbc-dev to keep the four-package set identical to the # jobs that do, rather than subtly divergent. - sudo apt-get install -y ninja-build catch2 \ + sudo apt-get install -y ninja-build \ libsqlite3-dev libsodium-dev libssl-dev \ unixodbc-dev libsqliteodbc libyaml-cpp-dev libzip-dev \ libgl1-mesa-dev libxkbcommon-x11-0 libxcb-cursor0 libxcb-icccm4 \ @@ -2595,20 +2582,6 @@ jobs: - name: Self-test the clang-tidy suppression-scope checker run: bash scripts/test_check_tidy_suppression_scope.sh clang-tidy-${{ env.CLANG_VERSION }} - # The Catch2 on this runner's include path decides whether a - # readability-function-cognitive-complexity finding on a TEST_CASE body - # reaches this job, and `apt-get install -y catch2` above pins nothing. - # This step reads the version out of the headers the step above just - # installed and fails if it is not the one CATCH2_VERSION records, so a - # move in the runner image is a red job rather than a quiet change of - # what this gate measures (morph#666). Its self-test runs first, for the - # same reason the suppression-scope checker's does. - - name: Self-test the catch2-pin checker - run: bash scripts/test_check_catch2_pin.sh - - - name: Assert the Catch2 this job analyses against - run: bash scripts/check_catch2_pin.sh . --strict - # Catches morph#632's bug class: tests/.clang-tidy's thirteen # suppressions are argued as Catch2 and raw-syscall idiom, which is true # of test sources and says nothing about include/morph/** -- yet diff --git a/.github/workflows/mutation.yml b/.github/workflows/mutation.yml index 77e319d4c..b77dee307 100644 --- a/.github/workflows/mutation.yml +++ b/.github/workflows/mutation.yml @@ -72,10 +72,10 @@ jobs: steps: - uses: actions/checkout@v4 - - name: Install Clang ${{ env.CLANG_VERSION }}, ninja, catch2 + - name: Install Clang ${{ env.CLANG_VERSION }}, ninja run: | sudo apt-get update -q - sudo apt-get install -y ninja-build catch2 + sudo apt-get install -y ninja-build # --fail, a file, and a non-empty check -- never `wget -qO- | sudo # bash`: see ci.yml's linux-compilers install step for why the piped # form reported success having installed nothing (morph#681). diff --git a/CMakeLists.txt b/CMakeLists.txt index 7ebc6a13c..2db92fbc1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -370,8 +370,8 @@ unset(_morph_optional_components) # morph_forms_moduleplugin just below). The tests/offline_sqlite subdirectory # that also depends on MORPH_BUILD_OFFLINE_SQLITE stays deferred to its # original spot, after the "Tests" section, since it links -# Catch2::Catch2WithMain and that target only exists once Catch2 has been -# found/fetched there. +# Catch2::Catch2WithMain and that target only exists once the "Catch2: +# fetched, never found" section below has run. if(MORPH_BUILD_OFFLINE_SQLITE) find_package(SQLite3 REQUIRED) @@ -488,7 +488,7 @@ endif() # point it is named). add_subdirectory(src/qt/forms) itself (which builds the # actual MorphForms module/plugin, plus a Catch2-linked test executable) is # deferred to just after the "Tests" section further below, since Catch2 is -# only found/fetched there and its test executable names Catch2::Catch2 +# only fetched below this point and its test executable names Catch2::Catch2 # directly. # Emscripten builds this too. MorphForms is a plain Qt Quick QML module over # header-only morph code — nothing in it is host-only — and a WASM ladder @@ -526,6 +526,73 @@ if(MORPH_BUILD_FORMS_QML) ) endif() +# ── Catch2: fetched, never found (morph#674) ──────────────────────────────── +# +# One Catch2, chosen here, for every configure of this repository -- CI, +# workstation, and whatever a contributor happens to have installed. There is +# deliberately no find_package() in front of this: a `find_package(Catch2)` +# that succeeds hands the build the *machine's* Catch2, which is how +# clang-tidy came to analyse `REQUIRE`/`TEST_CASE` expansions against a +# different release on a runner than on a workstation, and to disagree about +# what is a finding (morph#666, morph#674). Nothing in the tree records which +# Catch2 a given build used, so that divergence is silent by construction. +# Fetching unconditionally is what removes it; a gate over an unpinned apt +# package could only report it. +# +# The mechanism was subtler than "the compile database records the runner's +# include path" -- it records no Catch2 include path at all. An imported +# target whose INTERFACE_INCLUDE_DIRECTORIES is /usr/include contributes +# nothing to the command line, because CMake drops implicit compiler +# directories. The header that got included was simply the one the compiler's +# default system search path led to. Same divergence, one level lower. +# +# SYSTEM is load-bearing, not tidiness. A fetched dependency's include +# directory arrives as `-I`, so clang-tidy classifies every Catch2 macro +# expansion as user code and reports findings inside it: measured on +# tests/test_executor.cpp, one TU went from 1 finding to 45 (21 +# cppcoreguidelines-avoid-do-while and 11 misc-use-anonymous-namespace out of +# REQUIRE/TEST_CASE, plus the pre-existing one). With SYSTEM the same TU is +# back at 1 -- the same finding. Removing this keyword turns the +# clang-tidy-diff job permanently red across 134 tests/ TUs and every example +# suite. CMake 3.25+; this project already requires 3.25. +# +# Placed *above* the examples below rather than in the Tests section, which is +# where it used to live. examples/{concepts,bank,vetted_hmac} are +# add_subdirectory()'d before that section and each one used to run its own +# `find_package(Catch2 3 CONFIG QUIET)` with a message(WARNING) fallback -- so +# on any machine without a system Catch2 those three suites were silently not +# built, while the configure still succeeded. Nothing installs a system Catch2 +# any more, so leaving the fetch where it was would have skipped bank's and +# concepts' suites on every leg of CI. They now test a target that already +# exists. +if(MORPH_BUILD_TESTS) + # One tag, used twice below. The dep cache keys its directory on the tag + # (cmake/DepCache.cmake), so the two must agree or a bumped pin reuses the + # old checkout -- the failure that still builds, which is the hardest kind + # to notice. A variable removes the disagreement instead of gating it. + set(MORPH_CATCH2_TAG v3.8.1) + + include(FetchContent) + include(cmake/DepCache.cmake) + morph_cache_dep(Catch2 https://github.com/catchorg/Catch2.git ${MORPH_CATCH2_TAG}) + FetchContent_Declare( + Catch2 + GIT_REPOSITORY https://github.com/catchorg/Catch2.git + GIT_TAG ${MORPH_CATCH2_TAG} + GIT_SHALLOW TRUE + SYSTEM + ) + FetchContent_MakeAvailable(Catch2) + + # catch_discover_tests() lives in Catch2's extras/, which a fetched Catch2 + # does not put on the module path for its consumer (an installed one does, + # via its package config directory). Appended once here, at the top level, + # rather than by each of the eleven directories that call include(Catch): + # CMAKE_MODULE_PATH is inherited by every subdirectory added after this + # point, and every one of those eleven is. + list(APPEND CMAKE_MODULE_PATH "${catch2_SOURCE_DIR}/extras") +endif() + # ── Demo executable ────────────────────────────────────────────────────────── if(MORPH_BUILD_EXAMPLES) # The console demo uses the thread-pool executor; skip it for the @@ -557,20 +624,6 @@ endif() # ── Tests ──────────────────────────────────────────────────────────────────── if(MORPH_BUILD_TESTS) - find_package(Catch2 CONFIG QUIET) - if(NOT Catch2_FOUND) - include(FetchContent) - include(cmake/DepCache.cmake) - morph_cache_dep(Catch2 https://github.com/catchorg/Catch2.git v3.8.1) - FetchContent_Declare( - Catch2 - GIT_REPOSITORY https://github.com/catchorg/Catch2.git - GIT_TAG v3.8.1 - GIT_SHALLOW TRUE - ) - FetchContent_MakeAvailable(Catch2) - endif() - # ── The `--log-level` gate, shared by every Catch2 suite ──────────────── # # morph::log used to default to LogLevel::debug, so a plain `morph_tests` @@ -618,19 +671,19 @@ endif() # ── Application ladder (optional) ─────────────────────────────────────────── # Deferred to here (after the Tests section above), the same way # MORPH_BUILD_FORMS_QML's src/qt/forms subdirectory is deferred further below: -# examples/common/CMakeLists.txt calls find_package(Catch2 3 CONFIG QUIET) and -# treats "not found" as a hard FATAL_ERROR (its own Catch2 does not get -# fetched -- it relies on MORPH_BUILD_TESTS=ON having already resolved one). -# Adding examples/ before this point would let that find_package() run before -# the Tests section's FetchContent fallback ever executes, breaking the -# no-system-Catch2 case even though MORPH_BUILD_TESTS=ON. +# the rungs' test binaries link morph_test_main, which the Tests section above +# defines, and a plain (non-namespaced) target name has to exist before it is +# named. Catch2 itself is no longer a reason -- it is fetched above the +# examples, not here (see "Catch2: fetched, never found"), so +# examples/common/CMakeLists.txt's guard is now a TARGET check that cannot +# depend on what the machine has installed. if(MORPH_BUILD_LADDER) add_subdirectory(examples) endif() # ── Qt/QML forms renderer (optional) ───────────────────────────────────────── # The actual MorphForms module/plugin (src/qt/forms), deferred to here (after -# Catch2 is found/fetched above) since its own CMakeLists.txt links a Catch2 +# Catch2 is fetched above) since its own CMakeLists.txt links a Catch2 # test executable directly against Catch2::Catch2 when MORPH_BUILD_TESTS is # ON -- the same condition that guards both. Qt6 Quick/Qml was already found # and morph::qt_forms already created earlier (before "Demo executable"), so @@ -756,7 +809,7 @@ endif() # "SQLite-backed durable offline queue: library target" above, before # "Application ladder"'s add_subdirectory(examples)) -- only this suite's own # subdirectory stays deferred to here, since it links Catch2::Catch2WithMain -# and Catch2 is only found/fetched in the "Tests" section just above. +# and morph_test_main, the latter defined in the "Tests" section just above. if(MORPH_BUILD_OFFLINE_SQLITE AND MORPH_BUILD_TESTS) add_subdirectory(tests/offline_sqlite) endif() diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 46f5637eb..00fdfbaa0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,9 +3,13 @@ ## Toolchain morph is a header-only C++23 library. You need a C++23 compiler, CMake with -Ninja, and the dependencies declared in `vcpkg.json` (Glaze, Catch2; Qt 6 only -when building the optional Qt integration, `-DMORPH_BUILD_QT=ON`). CMake -presets are provided — `cmake --list-presets` shows the configured matrix; the +Ninja, and the dependencies declared in `vcpkg.json` (Glaze; Qt 6 only when +building the optional Qt integration, `-DMORPH_BUILD_QT=ON`). Catch2 is not +among them and does not need installing: the build fetches one pinned version +itself, on every platform, so that a local clang-tidy run and CI analyse the +same `REQUIRE` expansion (morph#674). + +CMake presets are provided — `cmake --list-presets` shows the configured matrix; the README documents the full set of build options (`MORPH_BUILD_TESTS`, `MORPH_BUILD_EXAMPLES`, `MORPH_BUILD_QT`, `MORPH_BUILD_FORMS_QML`, …). diff --git a/examples/bank/CMakeLists.txt b/examples/bank/CMakeLists.txt index cb15341df..a0a5c77f6 100644 --- a/examples/bank/CMakeLists.txt +++ b/examples/bank/CMakeLists.txt @@ -190,9 +190,16 @@ endif() # ── Tests ──────────────────────────────────────────────────────────────────── if(MORPH_BUILD_TESTS) - find_package(Catch2 3 CONFIG QUIET) - if(NOT Catch2_FOUND) - message(WARNING "Catch2 not found; bank example tests will not be built.") + # Catch2 is fetched by the root CMakeLists.txt above every + # add_subdirectory() in the tree (morph#674), so this is a build-system + # invariant rather than an environment question. It used to be a + # `find_package(Catch2 3 CONFIG QUIET)` with a message(WARNING) fallback, + # which meant a machine without the distro package configured fine and + # silently built no tests here at all. + if(NOT TARGET Catch2::Catch2WithMain) + message(FATAL_ERROR + "Catch2::Catch2WithMain does not exist; the root CMakeLists.txt's " + "Catch2 section must run before add_subdirectory(examples/bank).") else() add_executable(bank_tests tests/test_account.cpp @@ -223,7 +230,6 @@ if(MORPH_BUILD_TESTS) apply_sanitizers(bank_tests ${AF_SANITIZER}) endif() - list(APPEND CMAKE_MODULE_PATH ${Catch2_DIR}) include(Catch) # LABELS "bank" on all three bank suites (morph#679). Bank is not a # rung, so morph_add_rung()'s "ladder"/"ladder-" labels never diff --git a/examples/bank/tests/.clang-tidy b/examples/bank/tests/.clang-tidy index d7ad0d3f1..9a4529ba8 100644 --- a/examples/bank/tests/.clang-tidy +++ b/examples/bank/tests/.clang-tidy @@ -9,14 +9,33 @@ # test source compares three things: the `<=` it objects to is Catch2's own # capture hook, and both fixes it suggests -- parenthesise, or split with a # logical operator -- would defeat the expression decomposition that makes a -# failing assertion print its two operands. Catch2 says so itself; the macro +# failing assertion print its two operands. +# +# Whether the finding reaches a build at all is a property of the Catch2 +# release rather than of this code. Catch2 says as much itself: the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which -# predates that comment -- which is why the finding reaches CI here and not on -# a workstation with a current Catch2. That number is checked rather than -# asserted: scripts/check_catch2_pin.sh reads the runner's installed version -# back off the include path and fails this line and the clang-tidy-diff job -# together if the package moves (morph#666). +# line in the release this repository pins, and older releases do not. Which +# release that was used to be the machine's business -- the build took +# whichever Catch2 happened to be installed, so one diff produced this finding +# on a runner and not on a workstation, and scripts/check_catch2_pin.sh +# existed to make that divergence loud (morph#666). The root CMakeLists.txt +# now fetches one pinned Catch2 for every configure, found nowhere and +# installed by nothing (morph#674), so the question the gate asked no longer +# has per-machine answers and the gate went with it. The version lives in +# exactly one place, `MORPH_CATCH2_TAG` in the root CMakeLists.txt, and is +# deliberately not repeated here: nine copies of a number are nine chances to +# be wrong about it. +# +# On that pin this entry subtracts nothing, and that is measured, not assumed: +# with clang-tidy 22.1.8 against the fetched Catch2, bugprone-chained- +# comparison reports zero findings in these suites whether this line is +# present or absent, because the NOLINT above suppresses it first. It stays +# anyway -- the reasoning above is about the macro, not about a release, and a +# pin bump to a Catch2 without that NOLINT, or a clang-tidy that stops +# honouring a NOLINT reached through a system include, puts the finding back +# on every REQUIRE in the tree. An inert suppression whose inertness is +# written down costs a paragraph; rediscovering why the finding is not a +# defect costs a session. # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/bookmarks/tests/.clang-tidy b/examples/bookmarks/tests/.clang-tidy index d7ad0d3f1..9a4529ba8 100644 --- a/examples/bookmarks/tests/.clang-tidy +++ b/examples/bookmarks/tests/.clang-tidy @@ -9,14 +9,33 @@ # test source compares three things: the `<=` it objects to is Catch2's own # capture hook, and both fixes it suggests -- parenthesise, or split with a # logical operator -- would defeat the expression decomposition that makes a -# failing assertion print its two operands. Catch2 says so itself; the macro +# failing assertion print its two operands. +# +# Whether the finding reaches a build at all is a property of the Catch2 +# release rather than of this code. Catch2 says as much itself: the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which -# predates that comment -- which is why the finding reaches CI here and not on -# a workstation with a current Catch2. That number is checked rather than -# asserted: scripts/check_catch2_pin.sh reads the runner's installed version -# back off the include path and fails this line and the clang-tidy-diff job -# together if the package moves (morph#666). +# line in the release this repository pins, and older releases do not. Which +# release that was used to be the machine's business -- the build took +# whichever Catch2 happened to be installed, so one diff produced this finding +# on a runner and not on a workstation, and scripts/check_catch2_pin.sh +# existed to make that divergence loud (morph#666). The root CMakeLists.txt +# now fetches one pinned Catch2 for every configure, found nowhere and +# installed by nothing (morph#674), so the question the gate asked no longer +# has per-machine answers and the gate went with it. The version lives in +# exactly one place, `MORPH_CATCH2_TAG` in the root CMakeLists.txt, and is +# deliberately not repeated here: nine copies of a number are nine chances to +# be wrong about it. +# +# On that pin this entry subtracts nothing, and that is measured, not assumed: +# with clang-tidy 22.1.8 against the fetched Catch2, bugprone-chained- +# comparison reports zero findings in these suites whether this line is +# present or absent, because the NOLINT above suppresses it first. It stays +# anyway -- the reasoning above is about the macro, not about a release, and a +# pin bump to a Catch2 without that NOLINT, or a clang-tidy that stops +# honouring a NOLINT reached through a system include, puts the finding back +# on every REQUIRE in the tree. An inert suppression whose inertness is +# written down costs a paragraph; rediscovering why the finding is not a +# defect costs a session. # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/common/CMakeLists.txt b/examples/common/CMakeLists.txt index d996eabeb..37b1c6c9e 100644 --- a/examples/common/CMakeLists.txt +++ b/examples/common/CMakeLists.txt @@ -166,9 +166,10 @@ unset(_morph_saved_skip_install_rules) include(${PROJECT_SOURCE_DIR}/cmake/morph_demote_interface_includes.cmake) morph_demote_lightweight_odbc_includes() -find_package(Catch2 3 CONFIG QUIET) -if(NOT Catch2_FOUND) - message(FATAL_ERROR "Catch2 not found; MORPH_BUILD_TESTS=ON should have fetched it already (see root CMakeLists.txt).") +# Fetched by the root CMakeLists.txt, unconditionally, above every +# add_subdirectory() in the tree (morph#674) -- never found on the machine. +if(NOT TARGET Catch2::Catch2WithMain) + message(FATAL_ERROR "Catch2::Catch2WithMain does not exist; MORPH_BUILD_TESTS=ON should have fetched it already (see root CMakeLists.txt).") endif() # ── morph_ladder_testkit: pump/fixtures/rig/fault-proxy/interleaver ───────── diff --git a/examples/common/testkit/.clang-tidy b/examples/common/testkit/.clang-tidy index 21f86b275..ad25f11f9 100644 --- a/examples/common/testkit/.clang-tidy +++ b/examples/common/testkit/.clang-tidy @@ -9,14 +9,33 @@ # test source compares three things: the `<=` it objects to is Catch2's own # capture hook, and both fixes it suggests -- parenthesise, or split with a # logical operator -- would defeat the expression decomposition that makes a -# failing assertion print its two operands. Catch2 says so itself; the macro +# failing assertion print its two operands. +# +# Whether the finding reaches a build at all is a property of the Catch2 +# release rather than of this code. Catch2 says as much itself: the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which -# predates that comment -- which is why the finding reaches CI here and not on -# a workstation with a current Catch2. That number is checked rather than -# asserted: scripts/check_catch2_pin.sh reads the runner's installed version -# back off the include path and fails this line and the clang-tidy-diff job -# together if the package moves (morph#666). +# line in the release this repository pins, and older releases do not. Which +# release that was used to be the machine's business -- the build took +# whichever Catch2 happened to be installed, so one diff produced this finding +# on a runner and not on a workstation, and scripts/check_catch2_pin.sh +# existed to make that divergence loud (morph#666). The root CMakeLists.txt +# now fetches one pinned Catch2 for every configure, found nowhere and +# installed by nothing (morph#674), so the question the gate asked no longer +# has per-machine answers and the gate went with it. The version lives in +# exactly one place, `MORPH_CATCH2_TAG` in the root CMakeLists.txt, and is +# deliberately not repeated here: nine copies of a number are nine chances to +# be wrong about it. +# +# On that pin this entry subtracts nothing, and that is measured, not assumed: +# with clang-tidy 22.1.8 against the fetched Catch2, bugprone-chained- +# comparison reports zero findings in these suites whether this line is +# present or absent, because the NOLINT above suppresses it first. It stays +# anyway -- the reasoning above is about the macro, not about a release, and a +# pin bump to a Catch2 without that NOLINT, or a clang-tidy that stops +# honouring a NOLINT reached through a system include, puts the finding back +# on every REQUIRE in the tree. An inert suppression whose inertness is +# written down costs a paragraph; rediscovering why the finding is not a +# defect costs a session. # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/concepts/CMakeLists.txt b/examples/concepts/CMakeLists.txt index a05b1cc0e..f7db7493e 100644 --- a/examples/concepts/CMakeLists.txt +++ b/examples/concepts/CMakeLists.txt @@ -18,17 +18,16 @@ if(NOT TARGET morph::morph) endif() if(MORPH_BUILD_TESTS) - # This directory is add_subdirectory()'d from the MORPH_BUILD_EXAMPLES - # block, which runs *before* the top-level MORPH_BUILD_TESTS block that - # find_package()s/FetchContent's Catch2 (see root CMakeLists.txt), so - # Catch2's `Catch` CMake module (needed by include(Catch) below) is not - # yet on CMAKE_MODULE_PATH here even though the Catch2::Catch2WithMain - # *target* itself resolves fine (CMake defers target_link_libraries() - # resolution). Same fix as examples/vetted_hmac/CMakeLists.txt's tests - # block: look up Catch2 independently and extend CMAKE_MODULE_PATH from here. - find_package(Catch2 3 CONFIG QUIET) - if(NOT Catch2_FOUND) - message(WARNING "Catch2 not found; examples/concepts tests will not be built.") + # Catch2 is fetched by the root CMakeLists.txt above every + # add_subdirectory() in the tree (morph#674), so this is a build-system + # invariant rather than an environment question. It used to be a + # `find_package(Catch2 3 CONFIG QUIET)` with a message(WARNING) fallback, + # which meant a machine without the distro package configured fine and + # silently built no tests here at all. + if(NOT TARGET Catch2::Catch2WithMain) + message(FATAL_ERROR + "Catch2::Catch2WithMain does not exist; the root CMakeLists.txt's " + "Catch2 section must run before add_subdirectory(examples/concepts).") else() add_executable(morph_concepts_tests getting_started.cpp @@ -51,7 +50,6 @@ if(MORPH_BUILD_TESTS) apply_sanitizers(morph_concepts_tests ${AF_SANITIZER}) endif() - list(APPEND CMAKE_MODULE_PATH ${Catch2_DIR}) include(Catch) catch_discover_tests(morph_concepts_tests DISCOVERY_MODE PRE_TEST) endif() diff --git a/examples/crm/tests/.clang-tidy b/examples/crm/tests/.clang-tidy index d7ad0d3f1..9a4529ba8 100644 --- a/examples/crm/tests/.clang-tidy +++ b/examples/crm/tests/.clang-tidy @@ -9,14 +9,33 @@ # test source compares three things: the `<=` it objects to is Catch2's own # capture hook, and both fixes it suggests -- parenthesise, or split with a # logical operator -- would defeat the expression decomposition that makes a -# failing assertion print its two operands. Catch2 says so itself; the macro +# failing assertion print its two operands. +# +# Whether the finding reaches a build at all is a property of the Catch2 +# release rather than of this code. Catch2 says as much itself: the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which -# predates that comment -- which is why the finding reaches CI here and not on -# a workstation with a current Catch2. That number is checked rather than -# asserted: scripts/check_catch2_pin.sh reads the runner's installed version -# back off the include path and fails this line and the clang-tidy-diff job -# together if the package moves (morph#666). +# line in the release this repository pins, and older releases do not. Which +# release that was used to be the machine's business -- the build took +# whichever Catch2 happened to be installed, so one diff produced this finding +# on a runner and not on a workstation, and scripts/check_catch2_pin.sh +# existed to make that divergence loud (morph#666). The root CMakeLists.txt +# now fetches one pinned Catch2 for every configure, found nowhere and +# installed by nothing (morph#674), so the question the gate asked no longer +# has per-machine answers and the gate went with it. The version lives in +# exactly one place, `MORPH_CATCH2_TAG` in the root CMakeLists.txt, and is +# deliberately not repeated here: nine copies of a number are nine chances to +# be wrong about it. +# +# On that pin this entry subtracts nothing, and that is measured, not assumed: +# with clang-tidy 22.1.8 against the fetched Catch2, bugprone-chained- +# comparison reports zero findings in these suites whether this line is +# present or absent, because the NOLINT above suppresses it first. It stays +# anyway -- the reasoning above is about the macro, not about a release, and a +# pin bump to a Catch2 without that NOLINT, or a clang-tidy that stops +# honouring a NOLINT reached through a system include, puts the finding back +# on every REQUIRE in the tree. An inert suppression whose inertness is +# written down costs a paragraph; rediscovering why the finding is not a +# defect costs a session. # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/kanban/tests/.clang-tidy b/examples/kanban/tests/.clang-tidy index d7ad0d3f1..9a4529ba8 100644 --- a/examples/kanban/tests/.clang-tidy +++ b/examples/kanban/tests/.clang-tidy @@ -9,14 +9,33 @@ # test source compares three things: the `<=` it objects to is Catch2's own # capture hook, and both fixes it suggests -- parenthesise, or split with a # logical operator -- would defeat the expression decomposition that makes a -# failing assertion print its two operands. Catch2 says so itself; the macro +# failing assertion print its two operands. +# +# Whether the finding reaches a build at all is a property of the Catch2 +# release rather than of this code. Catch2 says as much itself: the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which -# predates that comment -- which is why the finding reaches CI here and not on -# a workstation with a current Catch2. That number is checked rather than -# asserted: scripts/check_catch2_pin.sh reads the runner's installed version -# back off the include path and fails this line and the clang-tidy-diff job -# together if the package moves (morph#666). +# line in the release this repository pins, and older releases do not. Which +# release that was used to be the machine's business -- the build took +# whichever Catch2 happened to be installed, so one diff produced this finding +# on a runner and not on a workstation, and scripts/check_catch2_pin.sh +# existed to make that divergence loud (morph#666). The root CMakeLists.txt +# now fetches one pinned Catch2 for every configure, found nowhere and +# installed by nothing (morph#674), so the question the gate asked no longer +# has per-machine answers and the gate went with it. The version lives in +# exactly one place, `MORPH_CATCH2_TAG` in the root CMakeLists.txt, and is +# deliberately not repeated here: nine copies of a number are nine chances to +# be wrong about it. +# +# On that pin this entry subtracts nothing, and that is measured, not assumed: +# with clang-tidy 22.1.8 against the fetched Catch2, bugprone-chained- +# comparison reports zero findings in these suites whether this line is +# present or absent, because the NOLINT above suppresses it first. It stays +# anyway -- the reasoning above is about the macro, not about a release, and a +# pin bump to a Catch2 without that NOLINT, or a clang-tidy that stops +# honouring a NOLINT reached through a system include, puts the finding back +# on every REQUIRE in the tree. An inert suppression whose inertness is +# written down costs a paragraph; rediscovering why the finding is not a +# defect costs a session. # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/ledger/tests/.clang-tidy b/examples/ledger/tests/.clang-tidy index d7ad0d3f1..9a4529ba8 100644 --- a/examples/ledger/tests/.clang-tidy +++ b/examples/ledger/tests/.clang-tidy @@ -9,14 +9,33 @@ # test source compares three things: the `<=` it objects to is Catch2's own # capture hook, and both fixes it suggests -- parenthesise, or split with a # logical operator -- would defeat the expression decomposition that makes a -# failing assertion print its two operands. Catch2 says so itself; the macro +# failing assertion print its two operands. +# +# Whether the finding reaches a build at all is a property of the Catch2 +# release rather than of this code. Catch2 says as much itself: the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which -# predates that comment -- which is why the finding reaches CI here and not on -# a workstation with a current Catch2. That number is checked rather than -# asserted: scripts/check_catch2_pin.sh reads the runner's installed version -# back off the include path and fails this line and the clang-tidy-diff job -# together if the package moves (morph#666). +# line in the release this repository pins, and older releases do not. Which +# release that was used to be the machine's business -- the build took +# whichever Catch2 happened to be installed, so one diff produced this finding +# on a runner and not on a workstation, and scripts/check_catch2_pin.sh +# existed to make that divergence loud (morph#666). The root CMakeLists.txt +# now fetches one pinned Catch2 for every configure, found nowhere and +# installed by nothing (morph#674), so the question the gate asked no longer +# has per-machine answers and the gate went with it. The version lives in +# exactly one place, `MORPH_CATCH2_TAG` in the root CMakeLists.txt, and is +# deliberately not repeated here: nine copies of a number are nine chances to +# be wrong about it. +# +# On that pin this entry subtracts nothing, and that is measured, not assumed: +# with clang-tidy 22.1.8 against the fetched Catch2, bugprone-chained- +# comparison reports zero findings in these suites whether this line is +# present or absent, because the NOLINT above suppresses it first. It stays +# anyway -- the reasoning above is about the macro, not about a release, and a +# pin bump to a Catch2 without that NOLINT, or a clang-tidy that stops +# honouring a NOLINT reached through a system include, puts the finding back +# on every REQUIRE in the tree. An inert suppression whose inertness is +# written down costs a paragraph; rediscovering why the finding is not a +# defect costs a session. # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/lims/tests/.clang-tidy b/examples/lims/tests/.clang-tidy index d7ad0d3f1..9a4529ba8 100644 --- a/examples/lims/tests/.clang-tidy +++ b/examples/lims/tests/.clang-tidy @@ -9,14 +9,33 @@ # test source compares three things: the `<=` it objects to is Catch2's own # capture hook, and both fixes it suggests -- parenthesise, or split with a # logical operator -- would defeat the expression decomposition that makes a -# failing assertion print its two operands. Catch2 says so itself; the macro +# failing assertion print its two operands. +# +# Whether the finding reaches a build at all is a property of the Catch2 +# release rather than of this code. Catch2 says as much itself: the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which -# predates that comment -- which is why the finding reaches CI here and not on -# a workstation with a current Catch2. That number is checked rather than -# asserted: scripts/check_catch2_pin.sh reads the runner's installed version -# back off the include path and fails this line and the clang-tidy-diff job -# together if the package moves (morph#666). +# line in the release this repository pins, and older releases do not. Which +# release that was used to be the machine's business -- the build took +# whichever Catch2 happened to be installed, so one diff produced this finding +# on a runner and not on a workstation, and scripts/check_catch2_pin.sh +# existed to make that divergence loud (morph#666). The root CMakeLists.txt +# now fetches one pinned Catch2 for every configure, found nowhere and +# installed by nothing (morph#674), so the question the gate asked no longer +# has per-machine answers and the gate went with it. The version lives in +# exactly one place, `MORPH_CATCH2_TAG` in the root CMakeLists.txt, and is +# deliberately not repeated here: nine copies of a number are nine chances to +# be wrong about it. +# +# On that pin this entry subtracts nothing, and that is measured, not assumed: +# with clang-tidy 22.1.8 against the fetched Catch2, bugprone-chained- +# comparison reports zero findings in these suites whether this line is +# present or absent, because the NOLINT above suppresses it first. It stays +# anyway -- the reasoning above is about the macro, not about a release, and a +# pin bump to a Catch2 without that NOLINT, or a clang-tidy that stops +# honouring a NOLINT reached through a system include, puts the finding back +# on every REQUIRE in the tree. An inert suppression whose inertness is +# written down costs a paragraph; rediscovering why the finding is not a +# defect costs a session. # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/pastebin/tests/.clang-tidy b/examples/pastebin/tests/.clang-tidy index d7ad0d3f1..9a4529ba8 100644 --- a/examples/pastebin/tests/.clang-tidy +++ b/examples/pastebin/tests/.clang-tidy @@ -9,14 +9,33 @@ # test source compares three things: the `<=` it objects to is Catch2's own # capture hook, and both fixes it suggests -- parenthesise, or split with a # logical operator -- would defeat the expression decomposition that makes a -# failing assertion print its two operands. Catch2 says so itself; the macro +# failing assertion print its two operands. +# +# Whether the finding reaches a build at all is a property of the Catch2 +# release rather than of this code. Catch2 says as much itself: the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which -# predates that comment -- which is why the finding reaches CI here and not on -# a workstation with a current Catch2. That number is checked rather than -# asserted: scripts/check_catch2_pin.sh reads the runner's installed version -# back off the include path and fails this line and the clang-tidy-diff job -# together if the package moves (morph#666). +# line in the release this repository pins, and older releases do not. Which +# release that was used to be the machine's business -- the build took +# whichever Catch2 happened to be installed, so one diff produced this finding +# on a runner and not on a workstation, and scripts/check_catch2_pin.sh +# existed to make that divergence loud (morph#666). The root CMakeLists.txt +# now fetches one pinned Catch2 for every configure, found nowhere and +# installed by nothing (morph#674), so the question the gate asked no longer +# has per-machine answers and the gate went with it. The version lives in +# exactly one place, `MORPH_CATCH2_TAG` in the root CMakeLists.txt, and is +# deliberately not repeated here: nine copies of a number are nine chances to +# be wrong about it. +# +# On that pin this entry subtracts nothing, and that is measured, not assumed: +# with clang-tidy 22.1.8 against the fetched Catch2, bugprone-chained- +# comparison reports zero findings in these suites whether this line is +# present or absent, because the NOLINT above suppresses it first. It stays +# anyway -- the reasoning above is about the macro, not about a release, and a +# pin bump to a Catch2 without that NOLINT, or a clang-tidy that stops +# honouring a NOLINT reached through a system include, puts the finding back +# on every REQUIRE in the tree. An inert suppression whose inertness is +# written down costs a paragraph; rediscovering why the finding is not a +# defect costs a session. # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/polls/tests/.clang-tidy b/examples/polls/tests/.clang-tidy index d7ad0d3f1..9a4529ba8 100644 --- a/examples/polls/tests/.clang-tidy +++ b/examples/polls/tests/.clang-tidy @@ -9,14 +9,33 @@ # test source compares three things: the `<=` it objects to is Catch2's own # capture hook, and both fixes it suggests -- parenthesise, or split with a # logical operator -- would defeat the expression decomposition that makes a -# failing assertion print its two operands. Catch2 says so itself; the macro +# failing assertion print its two operands. +# +# Whether the finding reaches a build at all is a property of the Catch2 +# release rather than of this code. Catch2 says as much itself: the macro # definition carries `/* NOLINT(bugprone-chained-comparison) */` on that very -# line in 3.15.3. CI pins catch2 3.4.0 -- ubuntu-24.04's package, which -# predates that comment -- which is why the finding reaches CI here and not on -# a workstation with a current Catch2. That number is checked rather than -# asserted: scripts/check_catch2_pin.sh reads the runner's installed version -# back off the include path and fails this line and the clang-tidy-diff job -# together if the package moves (morph#666). +# line in the release this repository pins, and older releases do not. Which +# release that was used to be the machine's business -- the build took +# whichever Catch2 happened to be installed, so one diff produced this finding +# on a runner and not on a workstation, and scripts/check_catch2_pin.sh +# existed to make that divergence loud (morph#666). The root CMakeLists.txt +# now fetches one pinned Catch2 for every configure, found nowhere and +# installed by nothing (morph#674), so the question the gate asked no longer +# has per-machine answers and the gate went with it. The version lives in +# exactly one place, `MORPH_CATCH2_TAG` in the root CMakeLists.txt, and is +# deliberately not repeated here: nine copies of a number are nine chances to +# be wrong about it. +# +# On that pin this entry subtracts nothing, and that is measured, not assumed: +# with clang-tidy 22.1.8 against the fetched Catch2, bugprone-chained- +# comparison reports zero findings in these suites whether this line is +# present or absent, because the NOLINT above suppresses it first. It stays +# anyway -- the reasoning above is about the macro, not about a release, and a +# pin bump to a Catch2 without that NOLINT, or a clang-tidy that stops +# honouring a NOLINT reached through a system include, puts the finding back +# on every REQUIRE in the tree. An inert suppression whose inertness is +# written down costs a paragraph; rediscovering why the finding is not a +# defect costs a session. # # Directory-scoped, and no wider. clang-tidy resolves configuration by walking # up from the file it is analysing and offers no finer granularity than a diff --git a/examples/vetted_hmac/CMakeLists.txt b/examples/vetted_hmac/CMakeLists.txt index f5a9fe348..aa8e6848c 100644 --- a/examples/vetted_hmac/CMakeLists.txt +++ b/examples/vetted_hmac/CMakeLists.txt @@ -31,24 +31,22 @@ if(MORPH_BUILD_HMAC_EXAMPLE_LIBSODIUM) # build (same rationale as examples/bank/CMakeLists.txt's ORM headers). if(MORPH_BUILD_TESTS) - # This directory is add_subdirectory()'d from the MORPH_BUILD_EXAMPLES - # block, which runs *before* the top-level MORPH_BUILD_TESTS block that - # find_package()s/FetchContent's Catch2 (see root CMakeLists.txt), so - # Catch2's `Catch` CMake module (needed by include(Catch) below) is not - # yet on CMAKE_MODULE_PATH here even though the Catch2::Catch2WithMain - # *target* itself resolves fine (CMake defers target_link_libraries() - # resolution). Same fix as examples/bank/CMakeLists.txt's tests block: - # look up Catch2 independently and extend CMAKE_MODULE_PATH from here. - find_package(Catch2 3 CONFIG QUIET) - if(NOT Catch2_FOUND) - message(WARNING "Catch2 not found; libsodium HMAC adapter tests will not be built.") + # Catch2 is fetched by the root CMakeLists.txt above every + # add_subdirectory() in the tree (morph#674), so this is a + # build-system invariant rather than an environment question. It used + # to be a `find_package(Catch2 3 CONFIG QUIET)` with a + # message(WARNING) fallback, which meant a machine without the distro + # package configured fine and silently built no tests here at all. + if(NOT TARGET Catch2::Catch2WithMain) + message(FATAL_ERROR + "Catch2::Catch2WithMain does not exist; the root CMakeLists.txt's " + "Catch2 section must run before add_subdirectory(examples/vetted_hmac).") else() add_executable(morph_vetted_hmac_libsodium_tests test_libsodium_adapter.cpp) target_link_libraries(morph_vetted_hmac_libsodium_tests PRIVATE morph::morph PkgConfig::SODIUM morph_test_main) apply_bigobj(morph_vetted_hmac_libsodium_tests) - list(APPEND CMAKE_MODULE_PATH ${Catch2_DIR}) include(Catch) catch_discover_tests(morph_vetted_hmac_libsodium_tests DISCOVERY_MODE PRE_TEST) endif() @@ -67,20 +65,17 @@ if(MORPH_BUILD_HMAC_EXAMPLE_OPENSSL) # build (same rationale as examples/bank/CMakeLists.txt's ORM headers). if(MORPH_BUILD_TESTS) - # See the matching comment in the libsodium block above: this - # directory is processed before the top-level MORPH_BUILD_TESTS block - # resolves Catch2, so its `Catch` CMake module isn't on - # CMAKE_MODULE_PATH yet here. - find_package(Catch2 3 CONFIG QUIET) - if(NOT Catch2_FOUND) - message(WARNING "Catch2 not found; OpenSSL HMAC adapter tests will not be built.") + # See the matching comment in the libsodium block above. + if(NOT TARGET Catch2::Catch2WithMain) + message(FATAL_ERROR + "Catch2::Catch2WithMain does not exist; the root CMakeLists.txt's " + "Catch2 section must run before add_subdirectory(examples/vetted_hmac).") else() add_executable(morph_vetted_hmac_openssl_tests test_openssl_adapter.cpp) target_link_libraries(morph_vetted_hmac_openssl_tests PRIVATE morph::morph OpenSSL::Crypto morph_test_main) apply_bigobj(morph_vetted_hmac_openssl_tests) - list(APPEND CMAKE_MODULE_PATH ${Catch2_DIR}) include(Catch) catch_discover_tests(morph_vetted_hmac_openssl_tests DISCOVERY_MODE PRE_TEST) endif() diff --git a/scripts/check_catch2_pin.sh b/scripts/check_catch2_pin.sh deleted file mode 100755 index b1a8197d4..000000000 --- a/scripts/check_catch2_pin.sh +++ /dev/null @@ -1,218 +0,0 @@ -#!/usr/bin/env bash -# Usage: bash scripts/check_catch2_pin.sh [REPO_ROOT] [--strict] -# -# Two halves, both about the same fact: which Catch2 the clang-tidy-diff job -# analyses against. -# -# A. Textual. Every line in the tree asserting `CI pins catch2 ` must name -# the version .github/workflows/ci.yml records in CATCH2_VERSION, and any -# other line naming a Catch2 version beside a CI reference is rejected as -# a phrasing this gate cannot check. -# B. Behavioural. The Catch2 headers actually installed here are read and -# compared against that same pin. Under `--strict` (how the CI job runs -# it) a mismatch, or no Catch2 at all, fails. Without it -- a workstation -# run -- a mismatch prints a divergence notice instead, because a -# workstation is not required to carry the runner's package, only to know -# that it does not. -# -# Why this gate exists (morph#666): a local `clang-tidy-diff` over the same -# diff, with the same clang-tidy version and the same job flags, can exit 0 on -# a diff the CI job fails -- silently, reporting nothing rather than reporting -# less. The variable is the Catch2 on the include path: -# `readability-function-cognitive-complexity` computes the *same* score under -# both, and what differs is whether `ClangTidyDiagnosticConsumer` classifies -# the finding as user code, which depends on which notes a given Catch2 -# release's `TEST_CASE` expansion produces and where it puts them. That is how -# morph#656's branch shipped a NOLINT reason asserting a neighbouring TEST_CASE -# "scores under the threshold" while it scored 87 against a threshold of 25: -# the local gate agreed with it. -# -# What this gate does and does not do, stated plainly, because the distinction -# is the whole point of the ticket: -# -# * It makes CI's own Catch2 a *decision* rather than an accident. `apt-get -# install -y catch2` is unpinned; if the runner image's package moves, the -# clang-tidy job's measurement changes with nothing anywhere saying so. -# Half B under `--strict` turns that silent move into a failed job. -# * It does not make a local clang-tidy-diff agree with CI's. Nothing short -# of the job not depending on the runner's Catch2 at all does that -- -# morph#666's first closing condition, filed separately as a follow-up -# because it needs a CMake change in a file this change does not own. -# What half B does on a workstation is report, every time it is run, that -# the two measurements are not the same one. -# -# Requires git and grep; compiles nothing. -set -euo pipefail - -repo_root="" -strict=0 -for arg in "$@"; do - case "$arg" in - --strict) strict=1 ;; - *) repo_root="$arg" ;; - esac -done -if [ -z "$repo_root" ]; then - repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -fi -readonly repo_root strict - -readonly ci_workflow=".github/workflows/ci.yml" -readonly self="scripts/check_catch2_pin.sh" -readonly self_test="scripts/test_check_catch2_pin.sh" - -# `catch2 3.4.0`, `Catch2-3.4.0`, `Catch2 v3.8.1`. Not `Catch2.git` (no -# separator), and not a bare `3.4.0` with no product name on the line. -readonly catch2_version_re='[Cc]atch2[ -]v?[0-9]+\.[0-9]+(\.[0-9]+)?' -readonly ci_ref_re='(\bCI\b|ci\.yml|CATCH2_VERSION)' -readonly canonical_re='CI pins catch2 [0-9]+\.[0-9]+\.[0-9]+' -readonly historical_marker='catch2-pin: historical' - -failures=0 -canonical_sites=0 - -note() { printf 'ok: %s\n' "$*"; } -fail() { printf 'error: %s\n' "$*" >&2; failures=$((failures + 1)); } - -cd "$repo_root" - -# -- The source of truth ----------------------------------------------------- -if [ ! -f "$ci_workflow" ]; then - printf 'error: %s not found under %s\n' "$ci_workflow" "$repo_root" >&2 - exit 1 -fi - -pinned="$(sed -nE 's/^[[:space:]]*CATCH2_VERSION:[[:space:]]*"?([0-9]+\.[0-9]+\.[0-9]+)"?[[:space:]]*$/\1/p' \ - "$ci_workflow" | head -n 1)" - -if [ -z "$pinned" ]; then - printf 'error: no `CATCH2_VERSION: ""` found in %s -- this gate reads\n' \ - "$ci_workflow" >&2 - printf ' its expected value from there and cannot check anything without it\n' >&2 - exit 1 -fi - -note "${ci_workflow} pins catch2 ${pinned}" - -# -- A. The textual half ----------------------------------------------------- -mapfile -t files < <(git ls-files \ - | grep -vFx "$ci_workflow" \ - | grep -vFx "$self" \ - | grep -vFx "$self_test" \ - | { grep -v '^$' || true; }) - -if [ "${#files[@]}" -eq 0 ]; then - printf 'error: git ls-files returned nothing under %s\n' "$repo_root" >&2 - exit 1 -fi - -while IFS= read -r hit; do - [ -n "$hit" ] || continue - location="${hit%%:*}" - rest="${hit#*:}" - lineno="${rest%%:*}" - text="${rest#*:}" - - case "$text" in - *"$historical_marker"*) - note "${location}:${lineno}: marked historical, not checked" - continue - ;; - esac - - if printf '%s' "$text" | grep -qE "$canonical_re"; then - while IFS= read -r stated; do - canonical_sites=$((canonical_sites + 1)) - if [ "$stated" = "$pinned" ]; then - note "${location}:${lineno}: states catch2 ${stated}" - else - fail "${location}:${lineno}: states 'CI pins catch2 ${stated}', but ${ci_workflow} pins catch2 ${pinned}: - ${text}" - fi - done < <(printf '%s' "$text" | grep -oE "$canonical_re" \ - | grep -oE '[0-9]+\.[0-9]+\.[0-9]+') - continue - fi - - fail "${location}:${lineno}: names a Catch2 version beside a CI reference in a - phrasing this gate cannot check. Write it as 'CI pins catch2 ${pinned}', or - append the marker '${historical_marker}' if it is a dated record rather - than a claim about the pin now: - ${text}" -done < <(grep -nHIE "$catch2_version_re" -- "${files[@]}" 2>/dev/null \ - | grep -E "$ci_ref_re" || true) - -if [ "$canonical_sites" -eq 0 ]; then - fail "no 'CI pins catch2 ' assertion found anywhere in the tree. Either - the documentation stopped saying which Catch2 the clang-tidy job analyses - against, or it was reworded out of the shape this gate reads -- both leave - the gate checking nothing while still exiting 0, so it fails instead." -fi - -# -- B. The behavioural half ------------------------------------------------- -# Read the version out of the headers that are actually on this machine's -# include path, the same ones clang-tidy would expand TEST_CASE from. -# -# MORPH_CATCH2_INCLUDE_DIR, when set, *replaces* the default search rather than -# preceding it: the self-test needs a run in which no Catch2 is found, and a -# fallback to /usr/include would make that case pass or fail depending on what -# the machine running the self-test happens to have installed. -installed="" -installed_dir="" -if [ -n "${MORPH_CATCH2_INCLUDE_DIR:-}" ]; then - search_prefixes="${MORPH_CATCH2_INCLUDE_DIR}" -else - search_prefixes="/usr/include /usr/local/include" -fi -for prefix in $search_prefixes; do - header="${prefix}/catch2/catch_version_macros.hpp" - [ -f "$header" ] || continue - major="$(sed -nE 's/^#define CATCH_VERSION_MAJOR ([0-9]+).*$/\1/p' "$header" | head -n 1)" - minor="$(sed -nE 's/^#define CATCH_VERSION_MINOR ([0-9]+).*$/\1/p' "$header" | head -n 1)" - patch="$(sed -nE 's/^#define CATCH_VERSION_PATCH ([0-9]+).*$/\1/p' "$header" | head -n 1)" - if [ -n "$major" ] && [ -n "$minor" ] && [ -n "$patch" ]; then - installed="${major}.${minor}.${patch}" - installed_dir="$prefix" - break - fi -done - -if [ -z "$installed" ]; then - if [ "$strict" -eq 1 ]; then - fail "no Catch2 headers found on this machine, but --strict says this run *is* - the clang-tidy job's own environment. The job installs catch2 from apt - before this step; if that stopped happening, the measurement below this - step is no longer the one the pin describes." - else - note "no Catch2 headers found here -- nothing to compare against the pin" - fi -elif [ "$installed" = "$pinned" ]; then - note "installed Catch2 ${installed} (${installed_dir}) matches the pin" -elif [ "$strict" -eq 1 ]; then - fail "installed Catch2 is ${installed} (${installed_dir}) but ${ci_workflow} pins - catch2 ${pinned}. The runner image's package moved. Every clang-tidy-diff - result from this job is now a measurement against ${installed}, and the - nine examples/*/tests/.clang-tidy comments that explain why a local run - disagrees with CI describe ${pinned}. Update CATCH2_VERSION and those - comments together, having checked that the suppressions they argue for - still apply to ${installed}." -else - printf '\n' >&2 - printf 'WARNING: this machine'"'"'s Catch2 is %s (%s); CI pins catch2 %s.\n' \ - "$installed" "$installed_dir" "$pinned" >&2 - printf ' A local clang-tidy-diff run is therefore NOT the measurement the\n' >&2 - printf ' clang-tidy-diff job makes. For checks whose evidence lives inside\n' >&2 - printf ' Catch2 macro expansions -- readability-function-cognitive-complexity\n' >&2 - printf ' on a TEST_CASE body is the known one -- it can exit 0 on a diff CI\n' >&2 - printf ' fails, reporting nothing rather than reporting less (morph#666).\n' >&2 - printf ' A green local run is not evidence for those checks. It is still\n' >&2 - printf ' evidence for every check whose finding lands on a line you wrote.\n' >&2 - printf '\n' >&2 -fi - -if [ "$failures" -ne 0 ]; then - printf '\n%s catch2-pin check(s) failed\n' "$failures" >&2 - exit 1 -fi - -note "all ${canonical_sites} catch2-pin assertion(s) agree with ${ci_workflow}" diff --git a/scripts/test_check_catch2_pin.sh b/scripts/test_check_catch2_pin.sh deleted file mode 100755 index d1f35fefe..000000000 --- a/scripts/test_check_catch2_pin.sh +++ /dev/null @@ -1,226 +0,0 @@ -#!/usr/bin/env bash -# Usage: bash scripts/test_check_catch2_pin.sh -# -# Self-test for scripts/check_catch2_pin.sh, the gate that keeps the Catch2 the -# clang-tidy-diff job analyses against a recorded, checked fact rather than -# whatever apt shipped that morning. -# -# A lint gate nobody tests reports green whether or not it still detects -# anything, and this one is doubly exposed: the tree it guards is correct -# today, and the runner's package is the pinned one today, so the gate passes -# today whether or not either half is looking at anything. Both halves are -# therefore driven from the wrong side as well as the right one -- every drift -# the gate claims to catch is reintroduced into a scratch copy of the tree, or -# into a synthetic include directory, and must be caught for the stated reason -# rather than merely with a nonzero exit. -# -# One mutation at a time: applied together, a single detection would mask -# every other. -# -# The checker enumerates the tree with `git ls-files`, so each case runs -# against a throwaway git repository holding a copy of this one's tracked -# files rather than against a directory argument. -set -euo pipefail - -readonly repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -readonly checker="scripts/check_catch2_pin.sh" - -failures=0 - -note() { printf 'ok: %s\n' "$*"; } -fail() { printf 'error: %s\n' "$*" >&2; failures=$((failures + 1)); } - -scratch="$(mktemp -d)" -trap 'rm -rf "$scratch"' EXIT - -readonly pristine="${scratch}/pristine" -mkdir -p "$pristine" -while IFS= read -r -d '' tracked; do - mkdir -p "${pristine}/$(dirname "$tracked")" - cp "${repo_root}/${tracked}" "${pristine}/${tracked}" -done < <(cd "$repo_root" && git ls-files -z) -git -C "$pristine" init -q -git -C "$pristine" add -A - -make_tree() { - local dest="$1" - rm -rf "$dest" - mkdir -p "$dest" - cp -R "${pristine}/." "$dest" -} - -# `sed -i` is not portable between GNU and BSD sed; edit through a temp file. -edit() { - local file="$1"; shift - sed "$@" "$file" > "${file}.new" - mv "${file}.new" "$file" -} - -# A synthetic Catch2 include tree carrying exactly the three version macros the -# checker reads. This is what lets the behavioural half be driven from a -# version the machine running the self-test does not have installed. -make_catch2() { - local dir="$1" version="$2" - local major="${version%%.*}" rest="${version#*.}" - local minor="${rest%%.*}" patch="${rest#*.}" - rm -rf "$dir" - mkdir -p "${dir}/catch2" - { - printf '#define CATCH_VERSION_MAJOR %s\n' "$major" - printf '#define CATCH_VERSION_MINOR %s\n' "$minor" - printf '#define CATCH_VERSION_PATCH %s\n' "$patch" - } > "${dir}/catch2/catch_version_macros.hpp" -} - -# An include directory with no Catch2 in it at all. -readonly empty_include="${scratch}/no-catch2" -mkdir -p "$empty_include" - -# `$expected` is a substring the resulting diagnostic must contain; without it -# a mutation that broke the tree in some unrelated way -- a mangled sed, a file -# the mutator emptied -- would count as a detection, and this self-test would -# report a gate that no longer detects anything as fully working. -expect_caught() { - local description="$1" mutator="$2" expected="$3" extra_args="${4:-}" catch2_dir="${5:-}" - local tree="${scratch}/case" output - make_tree "$tree" - if ! ( cd "$tree" && eval "$mutator" ); then - fail "mutator failed to apply: ${description}" - return - fi - if output="$( cd "$tree" && MORPH_CATCH2_INCLUDE_DIR="$catch2_dir" \ - bash "$checker" . $extra_args 2>&1 )"; then - fail "NOT caught: ${description} -- the gate passed a tree it should reject" - printf '%s\n' "$output" >&2 - return - fi - if printf '%s' "$output" | grep -qF "$expected"; then - note "caught: ${description}" - else - fail "caught for the WRONG reason: ${description} -- no diagnostic containing '${expected}':" - printf '%s\n' "$output" >&2 - fi -} - -# The mirror, for the false positives this gate must not manufacture. A rule -# that rejected every mention of a Catch2 version anywhere would "catch" every -# case above while being useless. -expect_accepted() { - local description="$1" mutator="$2" expected="${3:-}" extra_args="${4:-}" catch2_dir="${5:-}" - local tree="${scratch}/case" output - make_tree "$tree" - if ! ( cd "$tree" && eval "$mutator" ); then - fail "mutator failed to apply: ${description}" - return - fi - if ! output="$( cd "$tree" && MORPH_CATCH2_INCLUDE_DIR="$catch2_dir" \ - bash "$checker" . $extra_args 2>&1 )"; then - fail "FALSE POSITIVE: ${description} -- the gate rejected a tree it should accept:" - printf '%s\n' "$output" >&2 - return - fi - if [ -z "$expected" ] || printf '%s' "$output" | grep -qF "$expected"; then - note "accepted: ${description}" - else - fail "accepted but SILENT: ${description} -- no output containing '${expected}':" - printf '%s\n' "$output" >&2 - fi -} - -readonly pinned_catch2="${scratch}/catch2-3.4.0" -readonly moved_catch2="${scratch}/catch2-3.5.3" -make_catch2 "$pinned_catch2" 3.4.0 -make_catch2 "$moved_catch2" 3.5.3 - -# -- The unmodified tree must pass ------------------------------------------- -make_tree "${scratch}/clean" -if output="$( cd "${scratch}/clean" && MORPH_CATCH2_INCLUDE_DIR="$pinned_catch2" \ - bash "$checker" . --strict 2>&1 )"; then - note "the unmodified tree passes against the pinned Catch2" -else - fail "the unmodified tree was rejected by the gate:" - printf '%s\n' "$output" >&2 -fi - -# -- A. The textual half ----------------------------------------------------- -expect_caught "a doc asserting CI pins catch2 3.5.3 while ci.yml pins 3.4.0" \ - "printf '%s\n' 'The lint leg reproduces because CI pins catch2 3.5.3 there.' \ - >> docs/spec/testing_charter.md" \ - "states 'CI pins catch2 3.5.3', but .github/workflows/ci.yml pins catch2 3.4.0" - -# The direction morph#666 will actually take: the runner image moves, someone -# updates CATCH2_VERSION, and the nine .clang-tidy copies stay where they are. -expect_caught "ci.yml bumped to 3.5.3 while the nine copies still say 3.4.0" \ - "edit .github/workflows/ci.yml -e 's/^ CATCH2_VERSION: \"3.4.0\"/ CATCH2_VERSION: \"3.5.3\"/'" \ - "but .github/workflows/ci.yml pins catch2 3.5.3" - -# Rule B: the rewording that would defeat rule A alone. The nine copies said -# the same sentence nine times; a tenth site wording it differently is exactly -# how the number went unchecked in the first place. -expect_caught "a CI Catch2 claim in an unrecognised phrasing, even with the right version" \ - "printf '%s\n' 'The CI lint job installs catch2 3.4.0 from apt.' \ - >> docs/spec/testing_charter.md" \ - "phrasing this gate cannot check" - -expect_caught "a CI Catch2 claim in an unrecognised phrasing with the wrong version" \ - "printf '%s\n' 'Measured against Catch2 3.16.0 while CI has catch2-3.5.3.' \ - >> docs/spec/testing_charter.md" \ - "phrasing this gate cannot check" - -expect_caught "every canonical assertion removed from the tree" \ - "for f in \$(git grep -lF 'CI pins catch2' -- . ':!scripts/check_catch2_pin.sh' \ - ':!scripts/test_check_catch2_pin.sh'); do - edit \"\$f\" -e 's/CI pins catch2 [0-9.]*/the pinned Catch2/g' - done" \ - "no 'CI pins catch2 ' assertion found anywhere in the tree" - -expect_caught "ci.yml with no CATCH2_VERSION to read" \ - "edit .github/workflows/ci.yml -e 's/^ CATCH2_VERSION: \"3.4.0\"/ UNRELATED_CATCH2: \"3.4.0\"/'" \ - 'no `CATCH2_VERSION: ""` found' - -# A Catch2 version discussed without invoking CI in the same breath is not this -# gate's business; rejecting it would make the gate unsatisfiable for any -# document recording a local measurement or a FetchContent tag. -expect_accepted "a Catch2 version named with no CI reference on the line" \ - "printf '%s\n' 'Reproduced against Catch2 3.16.0 on this workstation.' \ - >> docs/spec/testing_charter.md" - -expect_accepted "a historical record carrying the documented marker" \ - "printf '%s\n' 'Before noble, CI had catch2 2.13.10 (catch2-pin: historical).' \ - >> docs/spec/testing_charter.md" - -# -- B. The behavioural half ------------------------------------------------- -# The whole reason this gate is not just another prose checker: the runner's -# package moving under an unpinned `apt-get install -y catch2` must fail the -# job rather than quietly change what it measures. -expect_caught "--strict against a runner whose Catch2 package has moved" \ - "true" \ - "installed Catch2 is 3.5.3" \ - "--strict" "$moved_catch2" - -expect_caught "--strict with no Catch2 installed at all" \ - "true" \ - "no Catch2 headers found on this machine" \ - "--strict" "$empty_include" - -# Without --strict -- a workstation run -- a divergence is reported rather than -# failed, because a workstation is not required to carry the runner's package. -# But it must be *reported*: silence here is the defect morph#666 is about. -expect_accepted "a workstation whose Catch2 differs is warned, not failed" \ - "true" \ - "A local clang-tidy-diff run is therefore NOT the measurement" \ - "" "$moved_catch2" - -# And a workstation that does match must not be warned, or the notice becomes -# noise that gets filtered out. -expect_accepted "a workstation whose Catch2 matches the pin is not warned" \ - "true" \ - "matches the pin" \ - "" "$pinned_catch2" - -if [ "$failures" -ne 0 ]; then - printf '\n%s self-test check(s) failed\n' "$failures" >&2 - exit 1 -fi - -note "all catch2-pin checker self-tests passed" diff --git a/vcpkg.json b/vcpkg.json index ff6c3730f..457e6d853 100644 --- a/vcpkg.json +++ b/vcpkg.json @@ -4,7 +4,6 @@ "version": "0.1.0", "dependencies": [ "glaze", - "catch2", "yaml-cpp", "libzip", "openssl", From 41bbf115527889f6d0c80d940cd965d2eaee7bcd Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Tue, 22 Sep 2026 03:02:49 +0200 Subject: [PATCH 2/2] ci: the two sanitizer sweeps that enumerate ctest without the Test step's display setting (fixes #691, refs #690) `check_sanitizer_instrumentation.sh` starts with `ctest --show-only=json-v1`, and ctest re-enumerates any suite registered `DISCOVERY_MODE PRE_TEST` by running its binary with `--list-tests`. On a runner there is no display, so a Qt-linked binary aborts there -- and the abort does not cost one suite, it costs the whole listing: ctest exits 8 with zero bytes of stdout and the sweep reports only `ctest listed no tests`. That is #690, which cost three sessions, two of which failed to reproduce it locally because a workstation has DISPLAY set. #692 gave bank-sanitizers' sweep the variable. The same step in kanban-tsan and ladder-sanitizers still runs without it, and both build Qt-linked suites. They are green today only because every Qt-linked suite they build is registered POST_BUILD (cmake/morph_add_rung.cmake:532, examples/common/CMakeLists.txt:307), so the enumeration happened during Build, where the variable is set. Nothing about either sweep step protects them: one `POST_BUILD` changed to `PRE_TEST` in morph_add_rung.cmake turns two green jobs red with that same uninformative message. linux-sanitizers deliberately does not get the block. It configures no Qt, so the line would be inert, and an inert line invites the next reader to work out what it guards. The alternative #691's triage names -- a gate asserting "a Qt-linked target must not use DISCOVERY_MODE PRE_TEST" -- was considered and is not available without reversing #692. bank's three suites are Qt-linked *and* PRE_TEST (examples/bank/CMakeLists.txt), which is exactly the configuration #690 was about, and #692 chose to fix it with the environment rather than by moving bank to POST_BUILD. Such a gate would therefore fail on master's own tree today. It would also have to compute Qt linkage transitively through morph::qt and morph_ladder_gui, which a text gate cannot do honestly and a generate-time gate could only do by walking each target's link closure. Cost of the approach taken: two `env:` blocks that are inert until someone changes a discovery mode, and a third place the reasoning has to be kept true. Cost of the gate: undoing #692 and moving bank's suites to POST_BUILD, to buy an invariant enforced at the cause rather than three comments -- defensible, but it is a change to how the ladder registers tests, which belongs to whoever owns that, not to a CI ticket. Verified on this commit by re-deriving #691's four-job table from the parsed workflow: linux-sanitizers sweep-env=NO buildsQt=no kanban-tsan sweep-env=yes buildsQt=yes bank-sanitizers sweep-env=yes buildsQt=yes ladder-sanitizers sweep-env=yes buildsQt=yes Every workflow still parses as YAML, and check_workflow_job_banners.py and check_workflow_option_coverage.py pass. Not verified: that either job *would* fail without this, which needs a PRE_TEST Qt suite on a headless runner. That remains inferred from #690's reproduced mechanism, as #691 states. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW --- .github/workflows/ci.yml | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 66f26b4f2..614003129 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -943,7 +943,24 @@ jobs: # claim while proving nothing. Keyed on `__tsan_`, not `__asan_` — an # ASan-only assertion on a TSan leg is itself a control that measures # nothing. + # + # QT_QPA_PLATFORM=offscreen for the reason bank-sanitizers' copy of this + # step spells out at length (morph#690, morph#691). The sweep's first + # act is `ctest --show-only=json-v1`, and ctest re-enumerates any suite + # registered with `DISCOVERY_MODE PRE_TEST` by running the binary with + # `--list-tests` -- here, headless. Today every Qt-linked suite this job + # builds is POST_BUILD (cmake/morph_add_rung.cmake), so that enumeration + # already happened in the Build step above where the variable is set, + # and this step is green without it. That is a property of one CMake + # keyword rather than of this job: flip POST_BUILD to PRE_TEST there and + # the headless `--list-tests` aborts, which takes out the *entire* ctest + # listing and surfaces as `ctest listed no tests` with nothing else to + # go on. linux-sanitizers deliberately does not get this block -- it + # configures no Qt, so the line would protect nothing and only invite + # the next reader to work out what it was for. - name: Every ctest binary is instrumented + env: + QT_QPA_PLATFORM: offscreen run: bash scripts/check_sanitizer_instrumentation.sh build/clang-tsan tsan # Every ladder ctest case only ever carries the "ladder"/"ladder-" @@ -1943,8 +1960,19 @@ jobs: # reaches (morph#542). The shared script walks what ctest will actually # run instead of a pattern, so a suite added tomorrow is covered by # having been added. + # + # QT_QPA_PLATFORM=offscreen for the reason kanban-tsan's copy of this + # step gives, and bank-sanitizers' gives at length (morph#690, + # morph#691): the sweep starts with `ctest --show-only=json-v1`, which + # re-runs `--list-tests` on every suite registered `DISCOVERY_MODE + # PRE_TEST`, and a headless Qt binary aborts there. This job's Qt-linked + # suites are all POST_BUILD today, so it is green without this -- on the + # strength of a keyword in cmake/morph_add_rung.cmake, not of anything + # this step or this job does. - name: Every ctest binary is instrumented if: steps.filter.outputs.run == 'true' + env: + QT_QPA_PLATFORM: offscreen run: bash scripts/check_sanitizer_instrumentation.sh build/clang-asan asan # detect_leaks=0: LeakSanitizer reports allocations the Qt platform