fix(build): stop VMAF_VERSION degrading to a bare commit abbreviation

[lusoris]

Summary

test_output::test_vmaf_version failed the Intel LLVM and macOS Clang legs of build.yml on #1223 — a PR that does not touch test_output.c, does not touch any version surface, and whose only meson change is a CUDA include path. The test is byte-identical on master. The failure is not #1223's.

core/include/meson.build derives VMAF_VERSION from git describe --tags --long --match 'v*.*.*' --always. With --always, git exits 0 even when no matching tag is reachable and prints a bare abbreviated object name, which meson substitutes into vcs_version.h verbatim. build.yml checks out at the actions/checkout default fetch-depth: 1, which fetches no tags — so every build on that workflow has been stamping a commit abbreviation where a version belongs, and vmaf --version, the JSON/XML version field and vmaf_version() all reported it.

That is silent until the abbreviation happens to contain no ASCII digit, which is exactly what test_vmaf_version asserts against. #1223's merge commit is abafdfcc3c8ef40c369b4bb776c14188729ceada → abafdfc. No digit. About one commit in a thousand — (6/16)^7 — so the defect sat in tree and then failed an unrelated PR.

The fix

Dropping --always makes git exit non-zero in precisely those cases, and mesonbuild/scripts/vcstagger.py substitutes the vcs_tag fallback on any exception from the subprocess. The fallback is now spelled out rather than left implicit, because it is the entire tagless path. When a tag is reachable the --long form still embeds the commit, so no provenance is lost. build.yml moves to fetch-depth: 0, matching libvmaf-build-matrix.yml, so CI exercises the tagged path rather than the fallback.

Master is not exempt — it has been lucky

Master's build.yml does run the Intel LLVM and macOS legs and they are green. That is luck, not coverage: all 20 most recent master commits abbreviate with a digit. The defect is latent on master and fires on whichever commit first abbreviates to all letters. PR merge commits are fresh objects that change whenever the PR or its base moves, so they simply roll the dice more often — which is why a PR surfaced it first.

(An earlier revision of this description claimed the CI impact planner had skipped master's Intel LLVM legs. That was wrong — it conflated build.yml with libvmaf-build-matrix.yml. Master's build.yml legs ran and passed.)

One real coverage gap, closed here

The Windows job — the third leg — runs an explicit whitelist that omitted test_output. It now runs it; the test already carries a GetTempPathA/GetTempFileNameA path for that platform.

While there: that Windows for loop gained || exit /b 1. GitHub runs shell: cmd as %ComSpec% /D /E:ON /V:OFF /S /C "CALL ...", so the step result was the errorlevel of the last executable alone — a failure in any earlier test was discarded outright. /V:OFF rules out !ERRORLEVEL!, so || is the portable form.

Type

• fix — bug fix
• build / ci — tooling / infra

Checklist

• Commits follow Conventional Commits (the commit-msg hook enforces this).
• make format && make lint is green locally.
• Unit tests pass: meson test -C build.
• If I touched any SIMD/GPU code path, I ran /cross-backend-diff and the worst ULP is ≤ 2. — n/a, no numeric code touched.
• If I touched a feature extractor with SIMD/GPU twins, I either updated every twin or listed the gap under "Known follow-ups" below. — n/a.
• If I added a new .c / .cpp / .cu / .h / .hpp, it has the appropriate license header. — n/a, no new C/C++ files.
• If this is a breaking change, the commit message uses ! or BREAKING CHANGE:. — n/a, not breaking.
• If this PR adds an ADR, the ADR row lives in docs/adr/_index_fragments/. — n/a, no ADR (bug fix, CLAUDE.md §12 r8).

Bug-status hygiene (ADR-0165)

• docs/state.md updated in this PR with a row.

Netflix golden-data gate (ADR-0024)

• I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests.

Deep-dive deliverables (ADR-0108)

• Research digest — no digest needed: trivial. Root cause is a single git flag, proven by direct reproduction rather than investigation of alternatives.
• Decision matrix — no alternatives: only-one-way fix. The only other candidate was relaxing the test's digit assertion, which would keep shipping a commit SHA as the version — i.e. hide the defect rather than fix it.
• AGENTS.md invariant note — scripts/ci/AGENTS.md gains a check-vcs-version-not-bare-sha.sh section covering all three enforced properties and the fetch-depth: 0 coupling.
• Reproducer / smoke-test command — below.
• CHANGELOG fragment — changelog.d/fixed/vcs-version-bare-sha.md, changelog.d/fixed/windows-ci-swallowed-test-failures.md.
• Rebase note — docs/rebase-notes.md, fix/vcs-version-bare-sha. Upstream carries the --always form (verified against upstream/master:libvmaf/include/meson.build), so this file conflicts on every sync; the new gate turns a careless "take theirs" into a build failure.

Reproducer

Reproduce the exact CI condition — a depth-1, tagless fetch of #1223's merge commit:

git init -q repo && cd repo
git remote add origin https://github.com/VMAFx/vmafx.git
git fetch -q --depth 1 --no-tags origin abafdfcc3c8ef40c369b4bb776c14188729ceada
git checkout -q FETCH_HEAD

git describe --tags --long --match 'v*.*.*' --always   # -> abafdfc   (becomes VMAF_VERSION; no digit -> test fails)
git describe --tags --long --match 'v*.*.*'            # -> exit 128  (meson uses the fallback -> "3.2.1")

Verify the built surface and the gate:

meson setup core core/build -Denable_cuda=false -Denable_sycl=false
ninja -C core/build
grep define core/build/include/vcs_version.h     # v3.1.0-2417-g1ee6ebde42
./core/build/tools/vmaf --version                # v3.1.0-2417-g1ee6ebde42
./core/build/test/test_output                    # 13 tests run, 13 passed
scripts/ci/check-vcs-version-not-bare-sha.sh     # OK

Verification performed

Check	Result
Before/after through mesonbuild.scripts.vcstagger on the failing commit	abafdfc (fails) → 3.2.1 (passes)
CPU build	clean
meson test --suite=fast	106 Ok / 0 Fail
test_output	13/13
Gate negative tests (restore --always, drop fallback, drop --match)	3/3 fire
Gate positive tests (restored file; prose mentioning --always outside the call)	2/2 silent
shellcheck on the new gate	clean
pre-commit on all touched files	clean

Known follow-ups

• Adding test_output to the Windows list and making the loop fail-fast may surface pre-existing Windows failures in the other 14 tests that were being swallowed. That would be a genuine finding, not a regression from this PR.

🤖 Generated with Claude Code