fix(release): test and cross-check the owned-soname derivation - #1333
Closed
justinjoy wants to merge 1 commit into
Closed
fix(release): test and cross-check the owned-soname derivation#1333justinjoy wants to merge 1 commit into
justinjoy wants to merge 1 commit into
Conversation
The upgrade matrix (#1272) derived its owned-soname set inline, so the derivation only ever executed on a release tag and had no test. Everything the closure gate asserts rests on that set being right: a library that drops out of it stops being checked, silently, while the gate keeps reporting PASS. Extract it to scripts/release/derive-owned-sonames.sh and exercise it from fixtures in milliseconds. Every property the derivation relies on is now falsifiable -- no -maxdepth, `! -type d` for meson's .p object directories, the suffix grep for .symbols by-products, sort -u, LC_ALL=C, and the empty-set exit -- and each is killed by a named case. The cross-check in run-upgrade-matrix.sh calls the same script rather than re-inlining its classifier. Keeping a second copy there would have been exactly the untested release-only duplicate this change exists to remove, and the two did not agree: the inline form used -maxdepth 1, so a library installed under a subdirectory of libdir was invisible to the check whose job is catching libraries the owned set does not name. Both derivation and comm failures now exit rather than being swallowed. Either one previously yielded an empty result that comm reported as a clean subset, so the cross-check passed having asserted nothing. An empty result is not merely tolerable there, it is impossible -- libdir is dirname of the libwirelog located above -- so the only way to reach it was a real failure. The headline assertion no longer uses a bare `[[ ]]` under errexit. That aborted the script before its own FAIL line and diagnostic dump could print, and before any remaining case ran: a regression produced an empty log. Worse on the platform that matters, since bash 3.2 does not apply errexit there and tests/meson.build gates this test to Linux. That gate is Linux-only for two reasons, only one of which was written down. Windows: MSYS `ln -s` copies, so the symlink assertions would pass for the wrong reason. macOS: the locale case asserts that en_US collation orders a hyphenated pair differently from C, which is glibc ignoring punctuation at the primary level; Darwin's en_US.UTF-8 is not known to, so the case would be a false failure. Widening needs that assertion turned into a skip-guard first. Refs #1285
justinjoy
force-pushed
the
fix/1285-owned-set
branch
from
September 3, 2026 03:10
68baaae to
12ea6e6
Compare
Collaborator
Author
|
Superseded. An independent implementation of #1285 merged as #1299 while this branch sat unpushed, so this PR now duplicates the extraction. Reviewing the merged version found four defects that are still on Those fixes are rebased onto |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1285. Based on
main— independent of every other open PR.The problem
The release upgrade matrix (#1272) derived its owned-soname set inline in
run-upgrade-matrix.sh, so the derivation only ever executed on a release tag and had notest. Everything the closure gate asserts rests on that set being right: a library that drops
out of it stops being checked, silently, while the gate keeps reporting PASS.
The change
Extract to
scripts/release/derive-owned-sonames.sh, exercise it from fixtures inmilliseconds, and have the cross-check call it rather than re-inlining its classifier.
Every property the derivation relies on is falsifiable and killed by a named case: no
-maxdepth,! -type dfor meson's.pobject directories, the suffix grep for.symbolsby-products,
sort -u,LC_ALL=C, and the empty-set exit.What review found
This was written in an earlier session, committed, and never pushed — no remote branch, no
PR. I picked it up with no review record and handed it over as unreviewed third-party code.
That was the right call; four substantive defects came out of it.
The headline assertion produced no diagnostic on the platform it runs. It used a bare
[[ ]]followed bycheck $?. Under errexit that aborts before its own FAIL line, before theexpected/got dump, and before any remaining case runs. Measured: a mutated derivation exited 1
having produced zero output. Worse in direction — bash 3.2 does not apply errexit there,
and this test is gated to Linux, so the diagnostics were dead exactly where they execute.
The extraction left a second copy of the classifier ten lines below the call — the very
untested, release-only duplicate the change exists to remove. And the two did not agree: the
inline form used
-maxdepth 1, so a library installed under a subdirectory oflibdirwasinvisible to the check whose job is catching libraries the owned set doesn't name.
My fix for that introduced a silent-pass route into the function whose purpose is preventing
silent passes. I wrote
2>/dev/null || trueand justified it with two false sentences. Botha derivation failure and a
commfailure previously yielded an empty result thatcommreported as a clean subset:
Empty there is not tolerable, it is impossible —
libdirisdirname "$lib"where$libwas located four lines earlier.
The "empty set makes the closure check vacuous" rationale was false in three places.
owned soname list is emptyexists atHEAD~1incheck-shared-library-closure.sh, and[[ -s "$owned" ]]sits directly below the|| exit 1whose comment described an unreachableoutcome.
The Linux gate
Recorded with both reasons, where only one was written down. Windows: MSYS
ln -scopies, sothe symlink assertions would pass for the wrong reason. macOS: the locale case asserts that
en_US collation orders a hyphenated pair differently from C — that is glibc ignoring
punctuation at the primary level, and Darwin's
en_US.UTF-8is not known to. Since thecandidate loop tries
en_US.UTF-8precisely because macOS has it, widening the gate wouldproduce a hard false failure. The prerequisite for ever widening (turn
fixture_discriminatesinto a skip-guard, confirm on a real macOS host) is stated. 17 lines ofWindows path handling that a Linux-only gate makes unreachable are deleted, with the divergence
from the sibling test explained.
The locale probe is also now pipeline-free. Not because the SIGPIPE was reachable — measured
headroom is large — but because it is an
ifcondition, so errexit doesn't apply: aSIGPIPE would silently take the else branch and print
ok locale (… skipped)while droppingboth assertions. Reporting PASS while having stopped asserting is the exact shape this change
exists to prevent.
Verified on real GNU bash 3.2.57 as well as 5.3. Local: 305 Ok / 0 Fail / 12 Skipped.