Skip to content

fix(release): test and cross-check the owned-soname derivation - #1333

Closed
justinjoy wants to merge 1 commit into
mainfrom
fix/1285-owned-set
Closed

fix(release): test and cross-check the owned-soname derivation#1333
justinjoy wants to merge 1 commit into
mainfrom
fix/1285-owned-set

Conversation

@justinjoy

Copy link
Copy Markdown
Collaborator

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 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.

The change

Extract to scripts/release/derive-owned-sonames.sh, exercise it from fixtures in
milliseconds, 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 d for meson's .p object directories, the suffix grep for .symbols
by-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 by check $?. Under errexit that aborts before its own FAIL line, before the
expected/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 of libdir was
invisible 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 || true and justified it with two false sentences. Both
a derivation failure and a comm failure previously yielded an empty result that comm
reported as a clean subset:

old form, derivation fails:  exit 0, missing=''   ← passes having asserted nothing
new form, derivation fails:  exit 1, diagnostic on stderr

Empty there is not tolerable, it is impossiblelibdir is dirname "$lib" where $lib
was located four lines earlier.

The "empty set makes the closure check vacuous" rationale was false in three places.
owned soname list is empty exists at HEAD~1 in check-shared-library-closure.sh, and
[[ -s "$owned" ]] sits directly below the || exit 1 whose comment described an unreachable
outcome.

The Linux gate

Recorded with both reasons, where only one 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 — that is glibc ignoring
punctuation at the primary level, and Darwin's en_US.UTF-8 is not known to.
Since the
candidate loop tries en_US.UTF-8 precisely because macOS has it, widening the gate would
produce a hard false failure. The prerequisite for ever widening (turn
fixture_discriminates into a skip-guard, confirm on a real macOS host) is stated. 17 lines of
Windows 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 if condition, so errexit doesn't apply: a
SIGPIPE would silently take the else branch and print ok locale (… skipped) while dropping
both 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.

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

Copy link
Copy Markdown
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 main: the headline assertion produces no diagnostic under errexit on the only platform it runs; the cross-check re-inlines the classifier it extracted, with a -maxdepth 1 that hides libraries in subdirectories; derivation and comm failures are swallowed into a silent pass; and the locale probe's SIGPIPE would drop both locale assertions while printing ok.

Those fixes are rebased onto main and opened separately, so the extraction is not re-landed.

@justinjoy justinjoy closed this Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

upgrade matrix: cross-check and unit-test the owned-soname derivation behind the closure gate

1 participant