From 0ca34d4892b2e3e681d69261255c05dcfa45fe45 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 10:02:41 -0400 Subject: [PATCH 1/3] submodule_pin: a submodule at its pin with a dirty work tree passes as "at their pinned commit" engine#2129 is already fixed on main by tools/ci/submodule_pin_check.py -- another lane's, registered in cmake/tests-submodule-pin.cmake as a ctest row -- and it reads `git submodule status`, which reports four states. This adds the fifth, which that command cannot report, and the wiring that runs the check before a build instead of only in CI. **The missing state: at the pin, not clean.** `git submodule status` prints ' ' for "the checkout IS the pinned commit" whatever the submodule's own work tree holds, and the build compiles that work tree. Measured on the box that motivated #2129: on top of the checkouts being off-pin, `third_party/llama.cpp` had 301 uncommitted paths, `xdna-driver` 32, `tokenizers` 20, and four others 4 each. Every one of those would read as "at their pinned commit" to the existing check. Reported always, with its count, and a failure only under the new `--require-clean`: a deliberate local patch in a submodule is a real thing a box does, and moving the pin is not always the caller's to do, so the default is to say it rather than to stop. Five self-test arms, including that a dirty '+' row is not double-counted (the commit is the failure there). **The wiring: nothing ran it before a build.** A ctest row runs in CI; the drift was measured on the box, where the release and the weekly build actually run. `scripts/weekly.sh`'s preflight now runs the check for the `bumps` and `build` stages through `preflight_submodules()` (a function of its own, testable against stub checkers like `preflight_runner_units`): exit 1 **blocks** the run -- the build would not be of the pinned sources -- and exit 2 (no submodule initialised, i.e. a fresh clone) only **warns**, saying that the check inspected nothing rather than implying a pass. tools/ci/submodule_pin_check.py --self-test 19 arms, 5 of them new tests/weekly_submodule_preflight_test.sh 6/6, the function cut out of weekly.sh, stub checkers tools/copyright.py --check clean tools/ci/tests_referenced_check.py 83/83 referenced `--require-clean` is deliberately not passed in the preflight: a dirty submodule is reported there and does not stop a build, while a wrong commit does. This supersedes #2177, which grew a parallel `scripts/check-submodules.sh` before main's tool landed. Two checks for one property is the failure mode the standing rules name, so that script is withdrawn rather than maintained beside it. --- cmake/tests-submodule-pin.cmake | 5 ++ scripts/weekly.sh | 28 ++++++++- tests/weekly_submodule_preflight_test.sh | 76 ++++++++++++++++++++++++ tools/ci/submodule_pin_check.py | 74 +++++++++++++++++++++-- 4 files changed, 177 insertions(+), 6 deletions(-) create mode 100755 tests/weekly_submodule_preflight_test.sh diff --git a/cmake/tests-submodule-pin.cmake b/cmake/tests-submodule-pin.cmake index 3a00b8433..f2c4cdbf0 100644 --- a/cmake/tests-submodule-pin.cmake +++ b/cmake/tests-submodule-pin.cmake @@ -25,3 +25,8 @@ # check's business); `--require-initialised` is the release-gate form, where a missing checkout is a failure too. add_python_test(submodule_pin_check ${CMAKE_SOURCE_DIR}/tools/ci/submodule_pin_check.py --check --root ${CMAKE_SOURCE_DIR}) add_python_test(submodule_pin_check_self ${CMAKE_SOURCE_DIR}/tools/ci/submodule_pin_check.py --self-test) + +# engine#2129: the release preflight's use of the check -- a build tree whose submodules are not the pinned commits +# BLOCKS the run, and "no checkout to compare" only WARNs so a fresh clone can still build. The function is cut out of +# scripts/weekly.sh and run against stub checkers: no submodules, no network, no build. +add_test(NAME weekly_submodule_preflight COMMAND ${CMAKE_SOURCE_DIR}/tests/weekly_submodule_preflight_test.sh) diff --git a/scripts/weekly.sh b/scripts/weekly.sh index d8ce116fc..4821a8336 100755 --- a/scripts/weekly.sh +++ b/scripts/weekly.sh @@ -658,6 +658,30 @@ preflight_runner_units() { return 0 } +# The submodule row of the preflight (engine#2129). The bumps and build stages build the tree in front of them, and a +# build compiles whatever the submodules' work trees hold: tools/ci/submodule_pin_check.py is the check for whether those +# checkouts ARE the commits this tree pins. It already exists as a ctest row, and a ctest row runs in CI -- nothing ran it +# before a build on the box, which is where the drift was measured (7 of 7 submodules off-pin). Exit 1 stops the run: the +# build would not be of the pinned sources. Exit 2 (nothing to compare -- no submodules initialised) only WARNS, because a +# fresh clone must still be able to build and the warning says the check inspected nothing rather than implying a pass. +# --require-clean is deliberately NOT passed here: it fails on a dirty submodule work tree, and a deliberate local patch +# is a real thing a box does; the check reports those with their counts either way. +# A function of its own so tests/weekly_submodule_preflight_test.sh can run it against a stub. +preflight_submodules() { + local check=$1 src=$2 out rc=0 + [ -f "$check" ] || return 0 + out=$(python3 "$check" --check --root "$src" 2>&1) || rc=$? + case "$rc" in + 0) printf '%s\n' "$out" | grep -v 'at their pinned commit' | sed 's/^/preflight: /' ;; + 2) printf '%s\n' "$out" | sed 's/^/preflight: /' + echo "preflight: WARNING, not blocking: no submodule checkout to compare against the pin, so this build follows whatever the work trees hold (tools/ci/submodule_pin_check.py exit 2; git submodule update --init fills it)" ;; + *) printf '%s\n' "$out" | sed 's/^/preflight: /' + echo "preflight: a submodule is not at the commit this tree pins, so the build would not be of the pinned sources (tools/ci/submodule_pin_check.py exit $rc; the line above names the submodule and what to do)" + return 1 ;; + esac + return 0 +} + # --- preflight ---------------------------------------------------------------------------- # What the stages need from the machine, checked before the first one runs, so that a missing # tool stops the run at the start rather than after an hour of build (2026-10-19 rehearsal). @@ -677,7 +701,9 @@ preflight() { 2> "$LOGS/hrx-toolchain.log"); then :; else echo "preflight: cannot fetch the pinned HRX toolchain (config/hrx-toolchain.config): see $LOGS/hrx-toolchain.log"; bad=1 fi ;; - esac ;; + esac + # engine#2129: this stage builds the tree in front of it, so the submodules have to BE the pinned tree + preflight_submodules "$SRC/tools/ci/submodule_pin_check.py" "$SRC" || bad=1 ;; package) # scripts/package-linux.sh rewrites RPATHs with patchelf command -v patchelf > /dev/null || diff --git a/tests/weekly_submodule_preflight_test.sh b/tests/weekly_submodule_preflight_test.sh new file mode 100755 index 000000000..35878dc28 --- /dev/null +++ b/tests/weekly_submodule_preflight_test.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash +# Copyright 2026 bong-water-water-bong +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# weekly_submodule_preflight_test.sh: the release preflight BLOCKS when a submodule is not the commit the tree pins and +# only WARNS when there is no checkout to compare against (engine#2129). The function is cut out of scripts/weekly.sh and +# run against stub checkers, so no submodules, no network and no build. +set -u +root=$(cd "$(dirname "$0")/.." && pwd) +tmp=$(mktemp -d); trap 'rm -rf "$tmp"' EXIT +fail=0 +ok() { printf 'ok %s\n' "$1"; } +bad() { printf 'FAIL %s\n' "$1"; fail=1; } + +sed -n '/^preflight_submodules() {$/,/^}$/p' "$root/scripts/weekly.sh" > "$tmp/fn.sh" +grep -q 'submodule_pin_check.py exit' "$tmp/fn.sh" || { bad "could not cut preflight_submodules() out of scripts/weekly.sh"; echo "weekly_submodule_preflight: FAIL"; exit 1; } +grep -q 'preflight_submodules "' "$root/scripts/weekly.sh" || bad "scripts/weekly.sh's preflight no longer calls preflight_submodules" +say() { echo "$*"; } +# shellcheck disable=SC1091 +. "$tmp/fn.sh" + +# A stub standing in for tools/ci/submodule_pin_check.py. It is a Python file because the function runs the real tool as +# `python3 `: printing what the tool prints and exiting with the code under test is the whole stub. +stub() { # + { + printf 'import sys\nprint(' + printf '%s' "$2" | sed 's/.*/"&"/' + printf ')\nsys.exit(%s)\n' "$1" + } > "$tmp/stub.py" + echo "$tmp/stub.py" +} +row() { # + local name=$1 want=$2 text=$3 rc=$4 line=$5 + local check out got=0 + check=$(stub "$rc" "$line") + out=$(preflight_submodules "$check" "$tmp" 2>&1) || got=$? + if [ "$got" = "$want" ] && printf '%s\n' "$out" | grep -q -- "$text"; then + ok "preflight: $name -> return $got" + else + bad "preflight: $name: want return $want with '$text', got $got: $out" + fi +} + +# A passing run prints nothing at all: the tool's summary line ("N submodule(s) at their pinned commit") is the one +# thing the function drops, because a preflight that always prints a line trains its reader to skip the lines. +check=$(stub 0 "submodule_pin: 7 submodule(s) at their pinned commit") +out=$(preflight_submodules "$check" "$tmp" 2>&1) +if [ -z "$out" ]; then ok "a tree whose submodules are their pins passes silently"; else bad "a passing run should print nothing, got: $out"; fi +row "a submodule at a different commit BLOCKS the run" 1 "not at the commit this tree pins" 1 "submodule_pin: third_party/x is checked out at abc, which is NOT the commit this tree pins" +row "no checkout to compare only WARNS" 0 "WARNING, not blocking" 2 "submodule_pin: no submodules reported - cannot tell, and that is not a pass" + +# A dirty submodule at its pin is reported and does NOT block: a deliberate local patch is a real thing a box does. +row "a dirty submodule at its pin is reported, not blocking" 0 "changed path" 0 "submodule_pin: third_party/x is at the pinned abc but its work tree has 301 changed path(s)" + +# A checker that is not there at all (an older tree): the row is skipped, not failed. +preflight_submodules "$tmp/no-such-check.py" "$tmp" >/dev/null 2>&1 && ok "a missing checker does not stop the run" \ + || bad "a missing checker should not stop the run" + +# The asymmetry, in one assertion: exit 2 must not block, exit 1 must. +check=$(stub 2 "submodule_pin: cannot tell") +if preflight_submodules "$check" "$tmp" >/dev/null 2>&1; then ok "exit 2 is not a block"; else bad "exit 2 should not block the release"; fi + +if [ "$fail" != 0 ]; then echo "weekly_submodule_preflight: FAIL"; exit 1; fi +echo "weekly_submodule_preflight: ok" diff --git a/tools/ci/submodule_pin_check.py b/tools/ci/submodule_pin_check.py index 9e3e45600..e36be0cdf 100644 --- a/tools/ci/submodule_pin_check.py +++ b/tools/ci/submodule_pin_check.py @@ -33,8 +33,13 @@ missing the source or using a system copy) -> fail with --require-initialised 'U' merge conflicts in the submodule -> fail +`git submodule status` cannot report one more state a build cares about: a submodule that IS at the pinned commit and +has uncommitted changes in its own work tree. It prints ' ' for that, and a build compiles the uncommitted files. On +2026-11-11 the same box that motivated this check had 301 such files in `third_party/llama.cpp`. Those are reported +always, and --require-clean turns them into a failure. + Usage: - tools/ci/submodule_pin_check.py --check [--root .] [--require-initialised] + tools/ci/submodule_pin_check.py --check [--root .] [--require-initialised] [--require-clean] tools/ci/submodule_pin_check.py --self-test """ from __future__ import annotations @@ -65,14 +70,30 @@ def parse_status(text: str) -> list[tuple[str, str, str, str]]: return rows -def verdict(rows: list[tuple[str, str, str, str]], require_initialised: bool) -> tuple[int, list[str]]: - """(rc, messages). rc 0 pass, 1 a submodule is not at its pin, 2 nothing readable.""" +def verdict(rows: list[tuple[str, str, str, str]], require_initialised: bool, + dirty: dict[str, int] | None = None, require_clean: bool = False) -> tuple[int, list[str]]: + """(rc, messages). rc 0 pass, 1 a submodule is not at its pin, 2 nothing readable. + + `dirty` maps a path to the number of paths its work tree has changed (git status --porcelain). A submodule at its + pin with a dirty work tree compiles code the pin does not name; it is always reported and fails only under + --require-clean, because a deliberate local patch is a real thing a box does and the fix for it (move the pin) is + not always the caller's to make.""" if not rows: return 2, ["submodule_pin: no submodules reported - cannot tell, and that is not a pass"] out: list[str] = [] - bad = unknown = uninit = 0 + bad = unknown = uninit = dirty_at_pin = 0 for flag, sha, path, describe in rows: if flag == " ": + n = (dirty or {}).get(path, 0) + if n: + dirty_at_pin += 1 + out.append( + f"submodule_pin: {path} is at the pinned {sha[:12]} but its work tree has {n} changed " + f"path(s), so the build compiles code the pin does not name. Repair: git -C {path} stash, or " + f"move the pin if the change is meant to stay (git add {path})" + ) + if require_clean: + bad += 1 continue if flag == "?": unknown += 1 @@ -102,6 +123,8 @@ def verdict(rows: list[tuple[str, str, str, str]], require_initialised: bool) -> return 1, out if unknown and not (len(rows) - unknown): return 2, out + if dirty_at_pin and not require_clean: + out.append(f"submodule_pin: WARNING, not a failure: {dirty_at_pin} submodule(s) at their pin with a dirty work tree (--require-clean makes this an error)") out.append( f"submodule_pin: {len(rows) - bad - unknown - uninit} submodule(s) at their pinned commit" + (f", {uninit} not initialised" if uninit else "") @@ -123,6 +146,28 @@ def run_git(root: pathlib.Path) -> tuple[str, int]: return p.stdout, 0 +def run_dirty(root: pathlib.Path, rows: list[tuple[str, str, str, str]]) -> dict[str, int]: + """{path: changed-path count} for the submodules that ARE at their pin. + + Only those: a '+' or 'U' row already fails and its dirty count would be noise. `git status --porcelain` inside the + submodule is the only thing that reports this state - `git submodule status` prints ' ' for it. + """ + out: dict[str, int] = {} + for flag, _sha, path, _describe in rows: + if flag != " ": + continue + try: + p = subprocess.run(["git", "-C", str(root / path), "status", "--porcelain"], + capture_output=True, text=True, check=False) + except OSError: + continue + if p.returncode == 0: + n = len([l for l in p.stdout.splitlines() if l.strip()]) + if n: + out[path] = n + return out + + def main(argv: list[str]) -> int: ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) g = ap.add_mutually_exclusive_group(required=True) @@ -134,6 +179,11 @@ def main(argv: list[str]) -> int: action="store_true", help="treat a submodule that is not checked out as a failure (a release build should use this)", ) + ap.add_argument( + "--require-clean", + action="store_true", + help="treat a submodule that is at its pin with a dirty work tree as a failure (CI uses this)", + ) args = ap.parse_args(argv) if args.self_test: @@ -143,7 +193,9 @@ def main(argv: list[str]) -> int: if rc != 0: print(f"submodule_pin: cannot tell: {text}") return 2 - rc, messages = verdict(parse_status(text), args.require_initialised) + rows = parse_status(text) + rc, messages = verdict(rows, args.require_initialised, run_dirty(pathlib.Path(args.root).resolve(), rows), + args.require_clean) for m in messages: print(m) return rc @@ -189,6 +241,18 @@ def check(ok: bool, what: str) -> None: any("not initialised" in m for m in verdict(parse_status(uninit), True)[1]), "... and so does the uninitialised one", ) + # engine#2129, the half `git submodule status` cannot report: at the pin, dirty work tree + check(verdict(parse_status(at_pin), False, {"third_party/llama.cpp": 301})[0] == 0, + "at the pin with 301 dirty paths -> pass by default (a deliberate local patch is a real thing)") + check(any("301 changed" in m for m in verdict(parse_status(at_pin), False, {"third_party/llama.cpp": 301})[1]), + "... but it IS reported, with its count") + check(verdict(parse_status(at_pin), False, {"third_party/llama.cpp": 301}, True)[0] == 1, + "... and FAILS with --require-clean") + check(verdict(parse_status(at_pin), False, {}, True)[0] == 0, + "a clean submodule at its pin still passes with --require-clean") + check(not any("changed" in m for m in verdict(parse_status(ahead), False, {"third_party/llama.cpp": 5})[1]), + "a dirty '+' row is not double-counted (the commit is the failure there)") + # one bad among several still fails: the check is not 'all lines' or 'the first line' check(verdict(parse_status(at_pin + ahead + at_pin), False)[0] == 1, "one '+' among three lines still fails") From fd40148027245d10c39b767cd4a9295eeb6143ed Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 10:31:48 -0400 Subject: [PATCH 2/3] weekly: the submodule gate runs in build(), not preflight() -- where it was a no-op that reported success Found by checking my own wiring against where the variables are set rather than against whether the tests pass. `SRC=$W/src` is created by `checkout`, which the `bumps` and `build` stages call, and `preflight()` runs before any stage -- so `[ -f "$SRC/tools/ci/submodule_pin_check.py" ]` was false on every real run, the function returned 0, and the gate inspected nothing while reporting success. A preflight row that cannot see the tree it is meant to inspect is worse than no row: it reads as verified. The gate now runs at the top of `build()`, immediately after `checkout` has updated `$SRC`'s submodules and before the first compile -- the tree that is about to be compiled, stopped before the compile rather than after it. `build()` returning non-zero fails the stage. And the missing-checker path now says so. An older ref does not carry the tool, so that build's submodule drift is not checked; it prints a WARNING naming the ref's reason instead of returning 0 silently. That was the same defect one level up: the function's first version treated "no file to run" as "fine", which is the reporting shape this whole check exists to remove. tests/weekly_submodule_preflight_test.sh 6/6, including that a missing checker warns rather than passing quietly tools/ci/submodule_pin_check.py --self-test 19 arms tools/copyright.py --check clean --- scripts/weekly.sh | 16 +++++++++++----- tests/weekly_submodule_preflight_test.sh | 6 +++--- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/scripts/weekly.sh b/scripts/weekly.sh index 4821a8336..2a0045f70 100755 --- a/scripts/weekly.sh +++ b/scripts/weekly.sh @@ -118,6 +118,11 @@ host_desc() { # --- build / test ------------------------------------------------------------------------- build() { + # engine#2129: what this builds is $SRC, whose submodules `checkout` has just updated, and a build compiles whatever + # those work trees hold. This cannot live in preflight(): preflight runs before $SRC exists, so the check would find + # no file to run, skip, and report success -- a check that inspects nothing and says it passed. Here it is the tree + # that is about to be compiled, and the run stops before the first compile instead of after it. + preflight_submodules "$SRC/tools/ci/submodule_pin_check.py" "$SRC" || return 1 # the private components compiled in (docs/releases.md, "Private components"): the NPU runtime # (into 1bit) and the HIP kernels (into the HRX llama.cpp), each a checkout at the commit its # config/*.config pins; the build stops here when one cannot be fetched at that commit @@ -658,7 +663,8 @@ preflight_runner_units() { return 0 } -# The submodule row of the preflight (engine#2129). The bumps and build stages build the tree in front of them, and a +# The submodule row of the build (engine#2129). It runs at the top of build(), not in preflight(): the tree to inspect is +# $SRC, which `checkout` creates, and preflight runs before that. A # build compiles whatever the submodules' work trees hold: tools/ci/submodule_pin_check.py is the check for whether those # checkouts ARE the commits this tree pins. It already exists as a ctest row, and a ctest row runs in CI -- nothing ran it # before a build on the box, which is where the drift was measured (7 of 7 submodules off-pin). Exit 1 stops the run: the @@ -669,7 +675,9 @@ preflight_runner_units() { # A function of its own so tests/weekly_submodule_preflight_test.sh can run it against a stub. preflight_submodules() { local check=$1 src=$2 out rc=0 - [ -f "$check" ] || return 0 + # An older ref does not carry the tool. That is not a failure, but it is not a pass either: say that the tree was not + # inspected, because a silent 0 here is a check that reports success for doing nothing. + [ -f "$check" ] || { echo "preflight: WARNING, not blocking: $src has no tools/ci/submodule_pin_check.py (a ref older than engine#2129), so submodule drift is not checked for this build"; return 0; } out=$(python3 "$check" --check --root "$src" 2>&1) || rc=$? case "$rc" in 0) printf '%s\n' "$out" | grep -v 'at their pinned commit' | sed 's/^/preflight: /' ;; @@ -701,9 +709,7 @@ preflight() { 2> "$LOGS/hrx-toolchain.log"); then :; else echo "preflight: cannot fetch the pinned HRX toolchain (config/hrx-toolchain.config): see $LOGS/hrx-toolchain.log"; bad=1 fi ;; - esac - # engine#2129: this stage builds the tree in front of it, so the submodules have to BE the pinned tree - preflight_submodules "$SRC/tools/ci/submodule_pin_check.py" "$SRC" || bad=1 ;; + esac ;; package) # scripts/package-linux.sh rewrites RPATHs with patchelf command -v patchelf > /dev/null || diff --git a/tests/weekly_submodule_preflight_test.sh b/tests/weekly_submodule_preflight_test.sh index 35878dc28..0e28554c6 100755 --- a/tests/weekly_submodule_preflight_test.sh +++ b/tests/weekly_submodule_preflight_test.sh @@ -64,9 +64,9 @@ row "no checkout to compare only WARNS" 0 "WARNING, not blocking" 2 "submodule_p # A dirty submodule at its pin is reported and does NOT block: a deliberate local patch is a real thing a box does. row "a dirty submodule at its pin is reported, not blocking" 0 "changed path" 0 "submodule_pin: third_party/x is at the pinned abc but its work tree has 301 changed path(s)" -# A checker that is not there at all (an older tree): the row is skipped, not failed. -preflight_submodules "$tmp/no-such-check.py" "$tmp" >/dev/null 2>&1 && ok "a missing checker does not stop the run" \ - || bad "a missing checker should not stop the run" +# A checker that is not there at all (an older ref): not a failure, but it must SAY that nothing was inspected -- +# a silent pass here is a check that reports success for doing nothing, which is what its first version did. +out=$(preflight_submodules "$tmp/no-such-check.py" "$tmp" 2>&1) && printf '%s' "$out" | grep -q 'is not checked for this build' && ok "a missing checker warns and does not stop the run" || bad "a missing checker should warn (and not stop the run): $out" # The asymmetry, in one assertion: exit 2 must not block, exit 1 must. check=$(stub 2 "submodule_pin: cannot tell") From ea3df25dbdedb165cfa18e72f09acece6feadac3 Mon Sep 17 00:00:00 2001 From: bong-water-water-bong Date: Wed, 11 Nov 2026 18:31:32 -0400 Subject: [PATCH 3/3] weekly_submodule_preflight_test: pin WHERE the gate runs, not merely that it exists (engine#2129) The previous commit moved the submodule gate from preflight() to build() because in preflight() it was a no-op that reported success: preflight runs before `checkout` creates $SRC, so the checker file was not there, the function returned 0, and a green run meant nothing had been inspected. The test it shipped with could not catch that regression. It asserted grep -q 'preflight_submodules "' scripts/weekly.sh -- "present anywhere in the file", which is true in exactly the broken state. The test and the bug agreed. It now extracts the two functions and asserts the call is inside build() and NOT inside preflight(), so moving it back fails with the reason rather than passing quietly: FAIL scripts/weekly.sh's build() no longer runs the submodule gate (a call elsewhere is the no-op this test exists for) FAIL the submodule gate is inside preflight(), which runs before $SRC exists: it would inspect nothing and report success Verified by making that move: with the call restored to preflight() both assertions fail and the row exits 1; with the call in build() the row passes. That is the negative control this test never had. --- tests/weekly_submodule_preflight_test.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/weekly_submodule_preflight_test.sh b/tests/weekly_submodule_preflight_test.sh index 0e28554c6..c66c3f5f3 100755 --- a/tests/weekly_submodule_preflight_test.sh +++ b/tests/weekly_submodule_preflight_test.sh @@ -26,7 +26,13 @@ bad() { printf 'FAIL %s\n' "$1"; fail=1; } sed -n '/^preflight_submodules() {$/,/^}$/p' "$root/scripts/weekly.sh" > "$tmp/fn.sh" grep -q 'submodule_pin_check.py exit' "$tmp/fn.sh" || { bad "could not cut preflight_submodules() out of scripts/weekly.sh"; echo "weekly_submodule_preflight: FAIL"; exit 1; } -grep -q 'preflight_submodules "' "$root/scripts/weekly.sh" || bad "scripts/weekly.sh's preflight no longer calls preflight_submodules" +# WHERE the call is, not merely that it exists: this gate was a silent no-op once because it lived in preflight(), which runs +# before `checkout` creates $SRC -- the file was not there to run, the function returned 0, and a green run meant nothing. A +# "present anywhere in weekly.sh" assertion passes in exactly that state, so pin the function it belongs to. +sed -n '/^build() {$/,/^}$/p' "$root/scripts/weekly.sh" > "$tmp/build_fn.sh" +sed -n '/^preflight() {$/,/^}$/p' "$root/scripts/weekly.sh" > "$tmp/preflight_fn.sh" +grep -q 'preflight_submodules "' "$tmp/build_fn.sh" || bad "scripts/weekly.sh's build() no longer runs the submodule gate (a call elsewhere is the no-op this test exists for)" +if grep -q 'preflight_submodules "' "$tmp/preflight_fn.sh"; then bad "the submodule gate is inside preflight(), which runs before \$SRC exists: it would inspect nothing and report success"; else ok "the submodule gate is in build(), where \$SRC exists"; fi say() { echo "$*"; } # shellcheck disable=SC1091 . "$tmp/fn.sh"