From f8cd0e3ed40d3044ba354783035170c36eee263f Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 21 Sep 2026 00:46:11 +0200 Subject: [PATCH 1/2] ci: name the option-coverage job by its job id, not its script (fixes #637) linux-all-features' MORPH_BUILD_BANK_GUI rationale referred the reader to "the check-workflow-option-coverage job below". No job by that name exists in .github/workflows/: the job is `option-coverage`, and check_workflow_option_coverage.py is the script it runs. A reader who searches ci.yml for the name the comment gives finds nothing, and the sentence's whole purpose is to point at the standing guard. Measured on c55ea5b7, before the change: $ grep -n 'check-workflow-option-coverage' .github/workflows/ci.yml 1838: # fetch requires. The check-workflow-option-coverage job below is $ grep -rn '^ check-workflow-option-coverage:' .github/workflows/ $ grep -n '^ option-coverage:' .github/workflows/ci.yml 2449: option-coverage: The replacement names both -- the job id a reader can jump to, and the script it runs -- so the next reader does not have to guess which of the two the sentence meant. Swept the rest of the tree for the same defect: over every `# ...` comment line in .github/workflows/*.yml, the hyphenated lowercase tokens immediately preceding the word "job" are automoc-include-lint, scenario-coverage, ladder-tests, linux-all-features, linux-coverage, option-coverage and dependency-free. All but the last resolve to a real job id in some workflow; "dependency-free job" is an adjective, not a reference. This was the only stale one. Nothing gates such a reference -- #638's check_workflow_job_banners.py pairs banners to jobs, and a job id inside a comment body is not a banner. That residual is accepted here and filed separately rather than folded in: closing it needs a new script plus the self-test this repository requires of a gate, which is its own change. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW --- .github/workflows/ci.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7fdcbc709..5fd004dcf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1835,8 +1835,9 @@ jobs: # GUI needs Qml/Quick/QuickControls2 at 6.5+, which the distro Qt the # other bank jobs use cannot give) and already installs the # ODBC/SQLite/yaml-cpp/libzip set the bank example's Lightweight - # fetch requires. The check-workflow-option-coverage job below is - # what keeps the next option from repeating this. + # fetch requires. The option-coverage job below, which runs + # scripts/check_workflow_option_coverage.py, is what keeps the next + # option from repeating this. # No -DCMAKE_..._COMPILER_LAUNCHER=sccache: see linux-compilers' # Configure steps for why leaving it unset is what lets # fastcache-cc be selected. From 4b34bfd55c2162459d46f240e5822a512923c18c Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 21 Sep 2026 01:01:54 +0200 Subject: [PATCH 2/2] ci: generate the two self-included .moc headers before clang-tidy-diff (fixes #624) examples/common/testkit/test_qml_surface.cpp and src/qt/forms/tests/ tst_main.cpp both end with `#include ".moc"` -- the AUTOMOC idiom for a Q_OBJECT declared in a .cpp. AUTOMOC writes that header at build time, the clang-tidy job configures and never builds, so every PR that touches either file failed the job on a parse error before a single changed line was analysed. Reproduced locally on c55ea5b7, this job's configure flags, clang-tidy 22.1.8, one line added to each of the two files: /.../examples/common/testkit/test_qml_surface.cpp:757:10: error: 'test_qml_surface.moc' file not found [clang-diagnostic-error] /.../src/qt/forms/tests/tst_main.cpp:70:10: error: 'tst_main.moc' file not found [clang-diagnostic-error] exit=1 ## Why generating, and not suppressing The issue offered two fixes. Suppressing the diagnostic turns out not to be one of them: clang-tidy does not let a compiler error be filtered. Measured on tst_main.cpp with the moc absent, all three still exit 1 on the same line -- --- baseline (no filter) : exit=1 'tst_main.moc' file not found --- -checks=-clang-diagnostic-error : exit=1 'tst_main.moc' file not found --- -checks=-clang-diagnostic-* : exit=1 'tst_main.moc' file not found --- -warnings-as-errors= empty : exit=1 'tst_main.moc' file not found so "filter it" would have meant grepping clang-tidy-diff.py's output and overriding its exit code -- morph#479's defect, a gate that cannot fail, rebuilt on purpose. That is what the rejected option would have cost. Generating them costs a partial build, measured locally with USE_COMPILER_CACHE=OFF on 12 cores: 100 ninja edges, 67 compilations, 7 links, 9 moc runs, 31s. 39 of the 67 are the vendored Lightweight ORM, pulled in because a _autogen target depends on its target's link dependencies. Building *every* autogen target instead was measured at 264 objects and 179s, so the step names the two targets it needs. Compiler caching is still not wired into this job: 67 objects against the 703 a real build of this configure would compile. ## The comment the issue was really about ci.yml's "No Build step" paragraph already reasoned about which generated files exist without a build, answered it for configure_file() output, and read as though it had settled the question. It now says which kind it covers, and points at the AUTOMOC step for the kind it does not. The -Wno-missing-include-dirs comment made the matching over-broad claim -- that no source includes a generated header -- when what its guard covers is an ascending `moc_.h`, not a same-name `.moc`; corrected too. ## Verification Measured locally on this commit's tree, from a clean configure-only build directory (`rm -rf build/clang-debug`, configure, 0 .moc files on disk), running the step body extracted from ci.yml rather than a retyped copy: === 2. NEGATIVE CONTROL: the step's own check, run before the build === ::error::examples/common/testkit/test_qml_surface.cpp: test_qml_surface.moc was not generated -- ... ::error::src/qt/forms/tests/tst_main.cpp: tst_main.moc was not generated -- ... ok: 2 self-included .moc header(s), 2 missing check-before-build exit=1 (must be 1) === 3. VACUITY CONTROL: same check with the scan finding nothing === ::error::no tracked source self-includes a .moc -- this step's scan has stopped detecting the idiom it exists for empty-scan exit=1 (must be 1) === 4. the step as written === ok: examples/common/testkit/test_qml_surface.cpp -> test_qml_surface.moc ok: src/qt/forms/tests/tst_main.cpp -> tst_main.moc ok: 2 self-included .moc header(s), 0 missing step exit=0 A green clang-tidy-diff would prove nothing on its own, so the job was made to report a finding on the changed line in each of the two files. A deliberate `int laneProbeReachMarker() { return 0; }` inserted above each `.moc` include, with the job's own clang-tidy-diff arguments: --- violation: clang-tidy-diff exit=1 /.../src/qt/forms/tests/tst_main.cpp:69:5: error: function 'laneProbeReachMarker' can be made static or moved into an anonymous namespace [misc-use-internal-linkage,...] /.../examples/common/testkit/test_qml_surface.cpp:756:5: error: function 'laneProbeReachMarker' can be made static or moved into an anonymous namespace [misc-use-internal-linkage,...] clang-diagnostic-error count: 0 --- harmless: clang-tidy-diff exit=0 clang-diagnostic-error count: 0 Both translation units are now analysed to completion and the changed line is reached; the same diff before this change produced two clang-diagnostic-errors and no findings at all. Not verified: any of this on a GitHub hosted runner, or against aqtinstall's Qt ${QT_VERSION} rather than the local distro Qt 6.11.2 and apt.llvm.org's clang-tidy-22 rather than local 22.1.8. The CI timings will be worse than 31s on four cores. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW --- .github/workflows/ci.yml | 143 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 133 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5fd004dcf..7390f4c90 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2245,16 +2245,25 @@ jobs: # The prerequisites examples/common/CMakeLists.txt enforces with a # FATAL_ERROR are already met above: MORPH_BUILD_QT=ON is passed, and # MORPH_BUILD_TESTS defaults ON. - # No Build step: clang-tidy-diff.py reads compile_commands.json (a + # No full Build step: clang-tidy-diff.py reads compile_commands.json (a # configure-time artifact, CMAKE_EXPORT_COMPILE_COMMANDS=ON in the base # preset) and runs clang-tidy itself per translation unit -- it neither # needs the project actually compiled nor linked. pinned_facts.cmake's # generated header is likewise a configure_file() (configure-time), not - # a build-time add_custom_command, so it's already on disk too. Compiler - # caching (sccache/fastcache-cc) accordingly has nothing to do in this - # job, unlike every other Linux job here -- so those steps, and the - # Build step that was their only reason to run, are gone rather than - # merely skipped. + # a build-time add_custom_command, so it's already on disk too. + # + # That paragraph answers "which generated headers exist without a + # build?" for configure_file() output and *only* for configure_file() + # output, while reading as though it had settled the question. It has + # not: AUTOMOC's output is the other kind -- a build-time + # add_custom_command -- and two sources #include it by name. See the + # AUTOMOC step below (morph#624), which is why this now says "no full + # Build step" rather than "no Build step". + # + # Compiler caching (sccache/fastcache-cc) is still not set up here. That + # step compiles 67 objects; a real build of this configure would compile + # the database's 703, so the caching steps would cost more setup than + # they could save. - name: Configure (generates compile_commands.json over every optional feature) run: | cmake --preset clang-debug \ @@ -2270,6 +2279,112 @@ jobs: -DCMAKE_C_COMPILER=clang-${{ env.CLANG_VERSION }} \ -DCMAKE_CXX_COMPILER=clang++-${{ env.CLANG_VERSION }} + # Two sources end with `#include ".moc"` -- the AUTOMOC + # idiom for a Q_OBJECT declared inside a .cpp rather than in a header: + # examples/common/testkit/test_qml_surface.cpp and + # src/qt/forms/tests/tst_main.cpp. AUTOMOC writes that header at *build* + # time into _autogen/include/, so after a configure-only run it + # does not exist and clang-tidy dies on the whole translation unit -- + # `error: 'tst_main.moc' file not found [clang-diagnostic-error]` -- with + # WarningsAsErrors:"*" turning it into a failed job, before a single + # changed line is analysed (morph#624). Reproduced locally on c55ea5b7 + # with this job's own configure flags and clang-tidy 22.1.8: one line + # added to each of the two files, and clang-tidy-diff.py exits 1 with + # exactly those two diagnostics and nothing else. + # + # Suppressing the diagnostic instead is not available, not merely + # unattractive. -Wno-missing-include-dirs below covers an include + # *directory* that does not exist, not a #include that resolves to + # nothing; and clang-tidy does not let a compiler error be filtered at + # all. Measured locally with clang-tidy 22.1.8 on tst_main.cpp with the + # moc absent: `-checks=-clang-diagnostic-error`, + # `-checks=-clang-diagnostic-*` and `--warnings-as-errors=''` each still + # exit 1 reporting the same line. The only remaining way to "filter" it + # would be to grep clang-tidy-diff.py's output and override its exit + # code, which is morph#479's defect -- a gate that cannot fail -- rebuilt + # deliberately. + # + # So the headers get generated, for these two targets only. Not `cmake + # --build` over everything, and not every *_autogen target either: + # building all 91 of them was measured locally at 264 objects and 179s, + # because a _autogen target depends on that target's link + # dependencies. Just these two, with USE_COMPILER_CACHE=OFF on a 12-core + # Linux box: 100 ninja edges, 67 compilations, 7 links, 9 moc runs, 31s. + # 39 of the 67 are the vendored Lightweight ORM, which + # ladder_common_tests depends on transitively -- that, not moc, is what + # this step costs. A four-core hosted runner pays more. + # + # The target list is written out rather than derived, so that it is + # greppable and reviewable; the *check* underneath is derived from the + # tree, so the list cannot go quietly stale. It rescans every tracked + # C++ source for the self-include idiom and fails if a .moc that a + # source names is still absent -- which is what a third source adopting + # the idiom looks like. It also fails when it finds no self-include at + # all: a step that generates nothing and reports green is this + # repository's standing failure mode, and an empty scan is exactly how + # this one would produce it. + - name: Generate the AUTOMOC headers two sources include by name + run: | + cmake --build build/clang-debug --target \ + ladder_common_tests_autogen \ + morph_forms_qml_tests_autogen + + python3 - <<'PY' + import json + import pathlib + import re + import subprocess + import sys + + root = pathlib.Path().resolve() + database = json.loads( + pathlib.Path("build/clang-debug/compile_commands.json").read_text()) + command_of = { + pathlib.Path(entry["file"]).resolve(): entry["command"] + for entry in database + } + + tracked = subprocess.run( + ["git", "ls-files", "-z", "*.cpp", "*.cc", "*.cxx"], + capture_output=True, text=True, check=True).stdout.split("\0") + self_include = re.compile( + r'^[ \t]*#[ \t]*include[ \t]+"([^"]+\.moc)"', re.M) + + found = 0 + missing = 0 + for name in filter(None, tracked): + source = pathlib.Path(name) + for moc in self_include.findall(source.read_text(errors="replace")): + found += 1 + command = command_of.get((root / source).resolve()) + if command is None: + # Not in this configure's database: clang-tidy cannot + # analyse the file at all, which is a different problem + # (morph#481) and not one this step can fix. + print(f"::warning::{name} self-includes {moc} but has no " + f"compile command in this configure") + continue + directories = re.findall(r'-I(\S+_autogen/include)', command) + if not directories: + print(f"::error::{name} self-includes {moc} but its compile " + f"command names no *_autogen/include directory") + missing += 1 + continue + if not any((pathlib.Path(d) / moc).is_file() for d in directories): + print(f"::error::{name}: {moc} was not generated -- add this " + f"source's _autogen to the cmake --build above") + missing += 1 + continue + print(f"ok: {name} -> {moc}") + + if found == 0: + print("::error::no tracked source self-includes a .moc -- this " + "step's scan has stopped detecting the idiom it exists for") + sys.exit(1) + print(f"ok: {found} self-included .moc header(s), {missing} missing") + sys.exit(1 if missing else 0) + PY + - name: Run clang-tidy-diff on changed lines run: | if [ "${{ github.event_name }}" = "pull_request" ]; then @@ -2329,12 +2444,20 @@ jobs: # distinct directories, none of which exists after a configure-only # run), up from 203 of 261 -- and -Weverything + WarningsAsErrors:"*" # turns each into a clang-diagnostic error that says nothing about the - # changed lines. Suppressing it is sufficient, not merely convenient: + # changed lines. Suppressing it is sufficient *for the directories*: # over the last ten commits' changed lines against the ladder database - # this run reports 48 findings and *zero* clang-diagnostic-error, and - # no source under examples/, include/ or src/ includes a generated - # moc_/ui_ header (the "Application ladder" job's own "Check no + # this run reports 48 findings and zero clang-diagnostic-error from a + # missing autogen directory, and no source under examples/, include/ + # or src/ includes a generated `moc_.h`/`ui_.h` from + # another directory (the "Application ladder" job's own "Check no # generated moc include ascends" step is the standing guard on that). + # + # It is not sufficient for a generated header that a source includes + # *by name*. This flag says nothing about `#include "tst_main.moc"`: + # the directory it would be found in is one of the 90, but the error + # raised is the file not being there, not the directory. That is what + # the AUTOMOC step above generates, and why this job now builds two + # targets (morph#624). if ! git diff -U0 "$BASE_SHA" | \ python3 "$CLANG_TIDY_DIFF" \ -path build/clang-debug \