Skip to content

fix(ci): guard check-abi-manifest's empty abidiff_args for bash 3.2 - #1326

Merged
justinjoy merged 1 commit into
mainfrom
fix/1324-abi-manifest-empty-array
Sep 3, 2026
Merged

fix(ci): guard check-abi-manifest's empty abidiff_args for bash 3.2#1326
justinjoy merged 1 commit into
mainfrom
fix/1324-abi-manifest-empty-array

Conversation

@justinjoy

Copy link
Copy Markdown
Collaborator

Closes #1324. Based on main — independent of every other open PR.

The bug

abidiff_args=()
if [ -f "$suppr" ]; then abidiff_args+=(--suppr "$suppr"); fi
...
abidiff "${abidiff_args[@]}" "$baseline" "$lib" 2>&1

abi/libwirelog-1.0.suppr does not exist in this repository, so the array is empty in the
default configuration — not an edge case. Expanding an empty array under set -u is an
unbound-variable error on bash 3.2; bash 4.4 changed it.

Same class as #1320 (PR #1325), in a different file, found while reviewing it.

Verified against real bash 3.2.57

Built from the GNU tarball with all 57 official patches — the version macOS ships — and
driven through the real script with a stub abidiff:

unfixed, suppr absent, 3.2.57 → line 132: abidiff_args[@]: unbound variable
unfixed, suppr absent, 5.3    → works (does not reproduce)
fixed,   suppr absent         → abidiff argc=2   both shells
fixed,   suppr present        → abidiff argc=4, --suppr and path as two words, both shells

The argc check is the point: the guard must pass zero extra arguments when empty, not
one empty string. The reviewer additionally proved a suppression path containing spaces stays
one argument, and that the unquoted outer ${...+...} neither globs nor word-splits.

What this is not

It is not a latent macOS CI break, and my first version of this comment said it was. I
wrote that libabigail's absence on macOS is what stops the line being reached. That names the
wrong guard. meson builds libwirelog.dylib there (darwin_versions, meson.build:490), so
the libwirelog.so not found SKIP fires two checks earlier — confirmed by running the
unfixed script against a macOS-shaped build directory with abidiff present on PATH.
macos-latest is also arm64, which the x86_64 guard stops independently.

So the live exposure is a Linux x86_64 host running bash 3.2, which is near-nobody. This is a
defensive fix that removes an incidental dependency on guards that happen to fire first — not
something to prioritise as though a brew install would trip it. The issue body has been
corrected the same way.

Sweep

Required by the issue's acceptance criteria, done as a runtime check rather than a grep: every
suite-registered script parse-checked under 3.2.57 (zero failures), and every "${arr[@]}"
expansion assessed. PUBLIC_HEADERS and SENTINELS are literal non-empty assignments;
violations in check-shared-library-closure.sh is already guarded by ((${#violations[@]}));
check-threading-doc.sh's two sites are PR #1325. ${#arr[@]} on an assigned-empty array is
safe at 3.2, so the existing length guards are sound.

My sweep method was incomplete and the reviewer's was not. Grepping tests/meson.build
finds only meson-registered scripts; two of my hits were comment prose, and registered tests
transitively execute several scripts that appear in no meson file. The reviewer swept all 52
scripts under scripts/, and additionally tested ${arr[*]} — a shape I never considered,
which fails identically on 3.2 (six sites, all safe). The conclusion held; the derivation
didn't. Both are recorded in #1321, whose acceptance criteria specified the same incomplete
derivation.

Local: 304 Ok / 0 Fail / 12 Skipped, serialized, dedicated build dir.

abi/libwirelog-1.0.suppr does not exist in this repository, so abidiff_args is
empty in the default configuration -- not an edge case. Expanding an empty array
under `set -u` is an unbound-variable error on bash 3.2; bash 4.4 changed it.

Use ${abidiff_args[@]+"${abidiff_args[@]}"}. Verified on GNU bash 3.2.57 (built
from source with all 57 official patches, the version macOS ships) and on 5.3:
empty passes zero extra arguments rather than one empty string, and populated
passes --suppr and the path as two words, including when the path contains
spaces. The unquoted outer expansion neither globs nor word-splits, because the
inner expansion is quoted.

macOS does not reach this line, and not for the reason first written here:
meson builds libwirelog.dylib there (darwin_versions, meson.build:490), so the
`libwirelog.so not found` SKIP fires two checks earlier -- confirmed with
abidiff present on PATH. macos-latest is also arm64, which the x86_64 guard
stops independently. So this is a defensive fix for a Linux x86_64 host running
bash 3.2, not a latent CI break.

Sweep for the same shape across every script a suite-registered test can reach:
PUBLIC_HEADERS and SENTINELS are literal non-empty assignments, violations in
check-shared-library-closure.sh is already guarded by ((${#violations[@]})), and
check-threading-doc.sh's two sites are fixed in #1320. ${#arr[@]} on an
assigned-empty array is safe at 3.2, so the existing length guards are sound.

Refs #1324
@justinjoy
justinjoy merged commit a6b5dd6 into main Sep 3, 2026
26 checks passed
@justinjoy
justinjoy deleted the fix/1324-abi-manifest-empty-array branch September 3, 2026 12:10
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.

check-abi-manifest.sh expands an empty array under set -u, which aborts on bash 3.2

1 participant