Skip to content

fix(gh-r): detect musl from its loader, not from curl (#576) - #581

Merged
ss-o merged 2 commits into
nextfrom
bug-576-musl-detection
Sep 30, 2026
Merged

ss-o merged 2 commits into
nextfrom
bug-576-musl-detection

Conversation

@ss-o

@ss-o ss-o commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Description

.zi-get-latest-gh-r-url-part decides whether to prefer musl release assets with HAS_MUSL. That value was set to linux-musl on almost every host, for two reasons:

  • (( ${+commands[curl]} )) made any host with curl count as musl, as fix(gh-r): musl detection is true whenever curl is installed #576 reports.
  • The fallback, find /lib/ -maxdepth 1 -name '*musl*', can never be false: find exits 0 whether or not anything matches. Measured on a glibc host with no /lib/*musl*, it returns 0 with empty output.

So removing only the curl operand would not have fixed the bug. With both causes present, a gh-r install on glibc picked the musl build of every release that publishes both. The fix detects musl through a small helper, .zi-has-musl-loader, which globs /lib for the musl loader (ld-musl-<arch>.so.1). That file is absent on glibc and macOS unless a musl runtime is installed alongside (Debian's musl package, for example, installs /lib/ld-musl-<arch>.so.1); such a host then prefers musl builds, which run there because the loader exists. Detecting the C library the shell itself uses would be a separate change. It globs ld-musl-* rather than *musl*, so a musl toolchain directory on a glibc host is not mistaken for a musl host.

Without a musl loader, the musl-preference filter no longer applies. When two builds both match the host, the asset order decides between them, as it does for every other filter in the function. Actively preferring glibc would be a separate change.

The issue also asks whether the anchored filter at install.zsh:1710 (*/$~HAS_MUSL) matches a real asset. Measured: it only matches an asset whose whole file name is linux-musl or the bare $MACHTYPE, so it is a no-op on real archive names. It is harmless, because it only narrows the list when something matches, and it is left unchanged here.

tests/gh-r-musl-detection.zsh runs offline and makes three checks:

  • The helper detects a loader, and rejects a toolchain directory and an empty directory.
  • With a stubbed download serving a gnu and a musl asset, putting a curl on PATH does not change the chosen asset, and a Linux host without a musl loader does not narrow to the musl build.
  • A musl host picks the musl build even when the glibc build is listed first.

It is registered in zsh-n.yml, as tests/ci-registration.zsh requires.

Related issues

Closes #576

Type of change

  • fix - bug fix (non-breaking)
  • feat - new feature (non-breaking)
  • feat! / fix! - breaking change
  • perf - performance improvement
  • refactor - code change with no functional impact
  • docs - documentation only
  • test - test addition or correction
  • build - build system or dependency change
  • ci - CI/workflow change
  • style - formatting with no behavior change
  • chore - maintenance / dependency bump
  • revert - revert of an earlier change

Checklist

  • Ordinary work targets next; only same-repository hotfixes target main
  • A next to main promotion uses a merge commit and records both parent SHAs (not a promotion)
  • Commit messages follow Conventional Commits format
  • Any Co-authored-by trailer credits a real human, never a bot, AI agent, or automation (none)
  • I have read the contribution guidelines
  • Existing tests pass (zsh -n zi.zsh / Trunk checks)
  • Documentation updated if needed (no user-facing documentation describes musl detection)

Verification

All runs are local on next 85214aa plus this branch, with zsh 5.9.2 on glibc 2.44 Linux x86_64.

  • Failing first: before the fix, the new test failed with glibc host chose ...-linux-musl.tar.gz, and HAS_MUSL=linux-musl showed in a trace with no curl on PATH.
  • Mutants in scratch copies, each killed by the test: the curl operand restored; the find probe restored; the first head's *musl* glob; detection forced true; detection forced false; detection result ignored.
  • zsh -n on zi.zsh and lib/zsh/*.zsh: pass. rpm2cpio.zsh:54 prints redirection with no command with status 0, on next as well.
  • Every tests/*.zsh passes, ci-registration.zsh included (39 tests). self-update-reload.zsh and version-reporting.zsh need a Git checkout, so they ran in the worktree.
  • Trunk on the changed files: no issues.

Migration plan

Not applicable: no public contract changes.

.zi-get-latest-gh-r-url-part set HAS_MUSL to linux-musl whenever curl
was installed, and its fallback probe could never be false: find exits
0 whether or not anything matches. On a glibc host every release that
publishes both builds therefore resolved to the musl asset.

Detect musl by globbing /lib for its loader instead. A regression test
serves a gnu and a musl asset offline and checks that installing curl
does not change the choice, and that a host with no musl loader picks
the glibc build.

Closes #576

@ss-o ss-o left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fallback review under ADR-0026: Copilot request not registered on 15f1e70

Findings

Changes needed, minor only. The fix is correct and meets #576. There are no blockers and no majors. Four small findings on the test and on how precise the detection is will be fixed in one push. Each was rerun by the author's session on this head.

  1. Minor, tests/gh-r-musl-detection.zsh:57: the "glibc build must win" claim depends on asset order. On glibc, HAS_MUSL is now $MACHTYPE, which matches both assets, so the first listed asset wins. Rerun: gnu first picks gnu, musl first picks musl. The assertion still separates old from new code, because the old code narrowed to musl in either order. What it proves is that the musl filter no longer fires, not that glibc is preferred. Remedy: state that in the comment and the PR body. Actively preferring glibc is outside #576.
  2. Minor, lib/zsh/install.zsh:1656: /lib/*musl* is broader than its comment. It also matches a musl directory, such as a musl toolchain under /usr/lib reached through a symlinked /lib, which makes a glibc host look like musl. Rerun: a fake /lib holding musl/ and ld-musl-x86_64.so.1 gives 2 matches for *musl* and 1 for ld-musl-*. Remedy: glob ld-musl-*.
  3. Nit, tests/gh-r-musl-detection.zsh:16: the function under test is defined only as a side effect. .zi-prepare-home sources install.zsh only when the home is new. Remedy: source lib/zsh/install.zsh explicitly, as tests/hook-ownership.zsh does.
  4. Nit, tests/gh-r-musl-detection.zsh:28: stale comment. It says the function runs find, which it no longer does. Remedy: drop find from the comment and the linked tools.

Scope: commit 15f1e70, the complete diff against next (85214aa), for #576.

Note

This is the maintainer-elected fallback review of record, based on a separate read-only review session and on reruns by the author's session. It is not an independent human review, and it is not approval to merge.

Validation

  • Passed, hosted on this head: 142 of 142 check runs.
  • Passed, local on this head: the new test; all 39 tests/*.zsh; zsh -n on zi.zsh and lib/zsh/*.zsh; Trunk on the changed files; 3 of 3 hand mutants killed.
  • Measured: find /lib/ -maxdepth 1 -name '*musl*' exits 0 with no output on a glibc host, and exits 1 only when /lib is missing.
Checklist verdicts
  • Acceptance (#576): met. The curl operand is gone, the always-true find is replaced, and the :1710 question is answered by measurement (a harmless no-op on real names). Closes #576 links the Development sidebar; the issue is closed by hand at promotion.
  • Zsh quality: good. Runs under emulate -LR zsh with extended_glob; musl_libs is local; (N) keeps nomatch safe.
  • Public contract, loading, filesystem: not affected. Both callers read only reply; no new writes or network access; a subprocess is replaced by a glob.
  • Test hermeticity: good. mktemp root with a trap, a temporary HOME and ZI directories, a stubbed download, and a minimal PATH per case. On a musl host the glibc assertion is skipped; CI runs on glibc.

Limits and follow-up

Not measured on a real musl host or macOS. A new fallback review follows on the fixed head. Merging needs the maintainer's decision.

Comment thread tests/gh-r-musl-detection.zsh Outdated
Comment thread lib/zsh/install.zsh Outdated
Comment thread tests/gh-r-musl-detection.zsh
Comment thread tests/gh-r-musl-detection.zsh Outdated
Review of the first head found that /lib/*musl* also matched a musl
toolchain directory, which makes a glibc host with a musl toolchain look
like musl. Detection now globs ld-musl-* through a small helper,
.zi-has-musl-loader, so it can be tested against fixture directories.

The regression test sources install.zsh explicitly, checks the helper
against a loader, a toolchain directory and an empty directory, and
checks that a musl host picks the musl build even when the glibc build is
listed first. Its comments no longer claim that glibc is actively
preferred: without a musl loader the musl filter simply does not apply,
and the asset order decides between two matching builds.

Refs #576

@ss-o ss-o left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fallback review under ADR-0026: Copilot request not registered on d69ea82

Findings

No blocker or major findings. The change meets #576, Closes #576 is justified (closed by hand at promotion, per AGENTS.md), and the four round-1 threads are fixed on this head. One minor wording point is fixed in the PR body without a new commit; two nits are left with the dispositions below.

  1. Minor, fixed in the PR body: "absent on glibc" has an exception. A glibc host with a musl runtime installed (Debian's musl package installs /lib/ld-musl-<arch>.so.1) is detected as musl and prefers musl builds, which run there because the loader exists. The body now says so; changing what is detected (the shell's own C library) is outside #576. From a web source, not measured here.
  2. Nit, left as is: tests/gh-r-musl-detection.zsh:58 stubs the helper without checking its argument, so a caller probing the wrong directory would pass the musl-host case. The helper's own fixture checks and the glibc assertion still pin the behaviour on CI; tightening the stub is optional.
  3. Nit, left as is: the INT TERM trap cleans up without exiting. It is the pattern 31 other tests in tests/ use.

Scope: commit d69ea82, the complete diff against next (85214aa), for #576.

Note

This is the maintainer-elected fallback review of record, based on a separate read-only review session and on reruns by the author's session. It is not an independent human review, and it is not approval to merge.

Validation

  • Passed, hosted on this head: 142 check runs succeeded; 6 cancelled runs belong to the superseded push, and each of those checks also has a passing run.
  • Passed, local on this head: the new test; all 39 tests/*.zsh, including ci-registration.zsh and source-hygiene.zsh; zsh -n; Trunk on the changed files.
  • Mutants: 5 of 5 killed by the author's session, and 5 of 5 killed again by the reviewer (curl operand, find probe, *musl* helper, detection ignored, detection forced true).
  • Measured: find exits 0 with no match and 1 only for a missing directory; ld-musl-* ignores a musl/ toolchain directory and a bare libc.musl-*.
Checklist verdicts
  • Acceptance (#576): met, including the :1710 question (a no-op on real archive names).
  • Zsh quality: good. The helper runs under emulate -LR zsh with a local array and (N); safe under a caller's glob_subst; no global leaks.
  • Public contract, loading, filesystem: not affected. Callers read only reply; no writes or network access.
  • Test: hermetic and not vacuous on CI's glibc runner.

Limits and follow-up

Not measured on a real musl host or macOS. Merging needs the maintainer's decision.

@ss-o
ss-o merged commit 75d722c into next Sep 30, 2026
148 of 154 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant