Merge pull request 'fix(416): docs-only skip never fired — detect against FETCH_HEAD (shallow-checkout safe)' (#429) from fix/416-shallow-detect into main
Build ErsatzTV Image / CI image pin matches docker/ci (push) Has been skipped
Build ErsatzTV Image / Docs update reminder (push) Has been skipped
Build ErsatzTV Image / decisions.md append-only (push) Has been skipped
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 7m12s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Has been skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Has been skipped
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m2s
Build ErsatzTV Image / Functional E2E (curl contracts) (push) Has started running
Build ErsatzTV Image / Build & push image (amd64) (push) Has been cancelled
Build ErsatzTV Image / CI image pin matches docker/ci (push) Has been skipped
Build ErsatzTV Image / Docs update reminder (push) Has been skipped
Build ErsatzTV Image / decisions.md append-only (push) Has been skipped
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 7m12s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Has been skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Has been skipped
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m2s
Build ErsatzTV Image / Functional E2E (curl contracts) (push) Has started running
Build ErsatzTV Image / Build & push image (amd64) (push) Has been cancelled
This commit was merged in pull request #429.
This commit is contained in:
@@ -316,6 +316,15 @@ files) has nothing for the heavy jobs to validate. Before this, such a change ra
|
||||
is gated `if: steps.detect.outputs.docs_only != 'true'`. On a docs-only change the job runs only
|
||||
checkout + detect and **reports `success` in seconds**.
|
||||
|
||||
**Shallow-checkout safe (the change-set diff).** `test`/`migrations` check out `fetch-depth: 1`, and
|
||||
a shallow clone has **no `origin/<base>` tracking ref and no merge-base** — so a three-dot
|
||||
`origin/main...HEAD` diff *errors*, the fail-safe returns `docs_only=false`, and the skip silently
|
||||
never fires (the first cut shipped this bug — every docs-only PR still ran the full matrix; caught by
|
||||
#416's "verify on a real PR" box). The script therefore fetches the base and diffs against
|
||||
**`FETCH_HEAD`** (always written by `git fetch`, resolves in a shallow clone) with a **two-dot** tree
|
||||
diff (`git diff --no-renames FETCH_HEAD HEAD`) — no merge-base required. (`api-docs`/`format` avoided
|
||||
the bug only because they check out `fetch-depth: 0`.)
|
||||
|
||||
The jobs are **not** `if:`-skipped. That is deliberate and it is the whole trap of this issue:
|
||||
|
||||
- `main`'s branch protection requires two checks **by name** — `Build ErsatzTV Image / Build & test
|
||||
|
||||
@@ -1522,3 +1522,18 @@ part — is skipped.
|
||||
Two adjacent redundancies are deliberately **out of scope**: the whole matrix re-running on a PR and
|
||||
again on the merge-to-`main` over identical code (#420), and the within-run triple `dotnet build`
|
||||
(#398). Full mechanism in `ci-cd.md` → "Docs-only skip".
|
||||
## 2026-07-17 — Docs-only detect must be shallow-checkout safe: FETCH_HEAD + two-dot, not origin/main + three-dot (#416 follow-up)
|
||||
|
||||
The #416 docs-only skip shipped (#422) safe but **ineffective**: every docs-only PR still ran the full
|
||||
matrix. Root cause — `test`/`migrations` check out `fetch-depth: 1`, and in a shallow clone
|
||||
`origin/<base>` has no remote-tracking ref and there is no merge-base, so the detect script's
|
||||
three-dot `git diff origin/main...HEAD` errored → `|| true` → empty diff → the fail-safe returned
|
||||
`docs_only=false` → full matrix. Confirmed in a real shallow `file://` clone (`origin/main` did not
|
||||
resolve; three-dot errored; `git diff FETCH_HEAD HEAD` returned the changed files correctly).
|
||||
|
||||
Decision: the detect diffs against **`FETCH_HEAD`** (always written by `git fetch`, resolves in a
|
||||
shallow clone) with a **two-dot** tree diff (no merge-base). `api-docs`/`format` were unaffected only
|
||||
because they use `fetch-depth: 0` — a difference the first cut missed. Meta-lesson reinforced: a CI
|
||||
gating change can pass every local test and merge green while being a complete no-op in CI; only
|
||||
real-PR verification that **measures the effect** (job durations, not just a green check) catches it —
|
||||
which is exactly what #416's Done-when demanded. Fixed in the #416 follow-up PR.
|
||||
|
||||
@@ -37,7 +37,15 @@ if [ "${GITHUB_REF_TYPE:-}" = "tag" ]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
range=""
|
||||
# Resolve a concrete BASE COMMIT to diff HEAD against. SHALLOW-CHECKOUT SAFE (ersatztv#416
|
||||
# follow-up): the `test`/`migrations` jobs check out `fetch-depth: 1`, and a shallow clone has NO
|
||||
# `origin/<base>` remote-tracking ref and no merge-base — so the old `origin/<base>...HEAD`
|
||||
# (three-dot) errored, the diff came back empty, and EVERY docs-only PR silently ran the full
|
||||
# matrix (safe but useless). `git fetch` ALWAYS writes FETCH_HEAD (the fetched base tip), which
|
||||
# resolves even in a shallow clone; we diff against it with a TWO-dot tree diff below (no merge-base
|
||||
# required). Verified in a real shallow `file://` clone. (`api-docs`/`format` only worked because
|
||||
# they use `fetch-depth: 0`.)
|
||||
base_rev=""
|
||||
case "$event" in
|
||||
pull_request)
|
||||
base="${GITHUB_BASE_REF:-}"
|
||||
@@ -46,19 +54,23 @@ case "$event" in
|
||||
emit false
|
||||
exit 0
|
||||
fi
|
||||
git fetch --no-tags --depth=200 origin "$base" || true
|
||||
range="origin/${base}...HEAD"
|
||||
if ! git fetch --no-tags --depth=200 origin "$base" 2>/dev/null; then
|
||||
echo "fetch of base '$base' failed; running full matrix (safe default)"
|
||||
emit false
|
||||
exit 0
|
||||
fi
|
||||
base_rev="$(git rev-parse --verify -q FETCH_HEAD || true)"
|
||||
;;
|
||||
push)
|
||||
# Only branch pushes reach here (tags handled above). We skip the heavy matrix on a docs-only
|
||||
# push only when the range is UNAMBIGUOUS — i.e. HEAD is a merge commit, which is how every
|
||||
# update to main lands (PR merge). Its first parent is the pre-merge branch tip, so
|
||||
# HEAD^1...HEAD is exactly the merged delta. A non-merge (direct/multi-commit) push is rare and
|
||||
# its true range is ambiguous here, so we fall back to running the full matrix.
|
||||
git fetch --no-tags --depth=200 origin "${GITHUB_REF_NAME:-main}" || true
|
||||
nfields="$(git rev-list --parents -n1 HEAD | wc -w | tr -d ' ')" # 1 (self) + parent count
|
||||
# Only branch pushes reach here (tags handled above). Skip only when the range is UNAMBIGUOUS —
|
||||
# HEAD is a merge commit (how every update to main lands): its first parent is the pre-merge
|
||||
# tip, so HEAD^1 is the base. Fetch depth 2 so the first-parent tree exists in the shallow
|
||||
# checkout; if it still can't be resolved/diffed we fall through to the full matrix. A non-merge
|
||||
# (direct/multi-commit) push is ambiguous here -> full matrix.
|
||||
git fetch --no-tags --depth=2 origin "${GITHUB_REF_NAME:-main}" 2>/dev/null || true
|
||||
nfields="$(git rev-list --parents -n1 HEAD 2>/dev/null | wc -w | tr -d ' ')" # 1 (self) + parents
|
||||
if [ "${nfields:-0}" -ge 3 ]; then
|
||||
range="HEAD^1...HEAD"
|
||||
base_rev="$(git rev-parse --verify -q 'HEAD^1' || true)"
|
||||
else
|
||||
echo "non-merge push (parents=$(( nfields - 1 ))); running full matrix (safe default)"
|
||||
emit false
|
||||
@@ -72,13 +84,20 @@ case "$event" in
|
||||
;;
|
||||
esac
|
||||
|
||||
# --no-renames is load-bearing: with rename detection ON (git's default) a code->docs rename
|
||||
# (e.g. Foo.cs -> docs/Foo.md) shows ONLY the destination `docs/Foo.md`, hiding that a source file
|
||||
# left the build -> misclassified as docs-only -> required tests skipped on a code change.
|
||||
# --no-renames surfaces the deletion (`Foo.cs`, non-docs) so it correctly forces the full matrix,
|
||||
# keeping the "any code path => run everything" invariant total.
|
||||
changed="$(git diff --no-renames --name-only "$range" 2>/dev/null || true)"
|
||||
echo "Range: $range"
|
||||
if [ -z "$base_rev" ]; then
|
||||
echo "could not resolve a base revision (shallow/offline); running full matrix (safe default)"
|
||||
emit false
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# TWO-dot tree diff `git diff <base> HEAD` (NOT three-dot) so no merge-base is needed — that is what
|
||||
# makes this work in a shallow checkout. `--no-renames` is load-bearing: with rename detection ON
|
||||
# (git's default) a code->docs rename (e.g. Foo.cs -> docs/Foo.md) shows ONLY the destination
|
||||
# `docs/Foo.md`, hiding that a source file left the build -> misclassified as docs-only -> required
|
||||
# tests skipped on a code change. --no-renames surfaces the deletion (`Foo.cs`, non-docs) so it
|
||||
# correctly forces the full matrix, keeping the "any code path => run everything" invariant total.
|
||||
changed="$(git diff --no-renames --name-only "$base_rev" HEAD 2>/dev/null || true)"
|
||||
echo "Base: $base_rev (event=$event)"
|
||||
echo "Changed files:"
|
||||
printf '%s\n' "$changed"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user