fix(npm): run on glibc older than the builder's — route Ubuntu 22.04 / Debian 12 / RHEL 9 to the static build (#6298) - #6350
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughAdds Linux glibc-floor detection and musl fallback routing for the npm launcher and shell installer, updates installation and CI documentation, and introduces workflow coverage for platform detection, package resolution, launcher execution, and fallback compilation. ChangesLinux libc fallback
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Launcher as perry.js
participant Detect as detect.cjs
participant Package as Platform package
participant Binary as Perry binary
User->>Launcher: invoke perry
Launcher->>Detect: inspect host and detectPlatform
Detect-->>Launcher: ordered glibc or musl candidates
Launcher->>Package: resolve candidate binary
Package-->>Launcher: binary path
Launcher->>Binary: execute selected binary
Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/npm-launcher.yml (1)
29-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExercise installer routing rather than only its syntax.
sh -ncannot catch incorrect glibc parsing or artifact suffix selection, and the Ubuntu end-to-end job never invokesinstall.sh. Add a dry-run or mocked-command harness covering musl, glibc below/at the floor, and both architectures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/npm-launcher.yml around lines 29 - 31, Extend the workflow’s installer validation after the existing syntax check to execute packaging/install.sh through a dry-run or mocked-command harness. Cover musl, glibc below and at the supported floor, and both relevant architectures, asserting the detected libc and selected artifact suffix; ensure the Ubuntu end-to-end job actually invokes this harness.npm/perry/test/detect.test.cjs (1)
190-220: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlso enforce the installer’s glibc floor.
This test guards the release matrix against
GLIBC_BUILD_FLOOR, butpackaging/install.shduplicates the floor as two independent constants. Assert that those values compose toGLIBC_BUILD_FLOORso npm and shell installations cannot silently diverge.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@npm/perry/test/detect.test.cjs` around lines 190 - 220, Extend the existing glibc floor test around the release-matrix assertions to read packaging/install.sh and verify its two glibc floor constants compose to GLIBC_BUILD_FLOOR. Parse or otherwise identify both installer constants, assert their combined value equals GLIBC_BUILD_FLOOR, and retain the current matrix validation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/npm-launcher.yml:
- Around line 10-16: Harden both jobs in the npm-launcher workflow by setting
read-only workflow permissions and configuring each actions/checkout step with
credential persistence disabled. Update the workflow-level permissions and both
checkout invocations, preserving the existing triggers and job behavior.
In `@npm/perry/bin/perry.js`:
- Around line 127-155: Update the recovery messages in the glibc-too-old branch
and the generic fallback around the launcher’s console.error calls to
distinguish local and global installations: retain the existing local npm
command, add the corresponding npm install -g --force command for globally
installed Perry, and clearly label both paths.
---
Nitpick comments:
In @.github/workflows/npm-launcher.yml:
- Around line 29-31: Extend the workflow’s installer validation after the
existing syntax check to execute packaging/install.sh through a dry-run or
mocked-command harness. Cover musl, glibc below and at the supported floor, and
both relevant architectures, asserting the detected libc and selected artifact
suffix; ensure the Ubuntu end-to-end job actually invokes this harness.
In `@npm/perry/test/detect.test.cjs`:
- Around line 190-220: Extend the existing glibc floor test around the
release-matrix assertions to read packaging/install.sh and verify its two glibc
floor constants compose to GLIBC_BUILD_FLOOR. Parse or otherwise identify both
installer constants, assert their combined value equals GLIBC_BUILD_FLOOR, and
retain the current matrix validation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 44a6ec7f-7f32-46b0-a99a-79b7741d4992
📒 Files selected for processing (8)
.github/workflows/npm-launcher.ymlREADME.mddocs/src/getting-started/installation.mdnpm/perry/README.mdnpm/perry/bin/detect.cjsnpm/perry/bin/perry.jsnpm/perry/test/detect.test.cjspackaging/install.sh
…lder than the builder's (#6298) The prebuilt Linux glibc binaries are built on ubuntu-24.04 (glibc 2.39), so they die in the dynamic loader with "GLIBC_2.39 not found" on Ubuntu 22.04 (2.35), Debian 12 (2.36) and RHEL 9 / AL2023 (2.34). The launcher only ever appended -musl for musl hosts; there was no glibc *version* check anywhere. bin/detect.cjs resolves a host to an ordered list of platform packages: glibc older than GLIBC_BUILD_FLOOR routes to the fully-static musl build, which has no PT_INTERP and runs on any Linux. The floor is a named constant tied to the builder image, and the self-test fails if the release matrix moves the linux-gnu legs off ubuntu-24.04 without revisiting it. node npm/perry/test/detect.test.cjs covers 2.31 / 2.34 / 2.35 / 2.36 / 2.39 / 2.41 / musl / no-report hosts and cross-checks that the musl packages the fallback names are real optionalDependencies built by the release matrix.
…n the builder's (#6298) install.sh had no libc detection at all: it always downloaded perry-linux-<arch>.tar.gz, so Alpine got a binary it cannot load and Ubuntu 22.04 / Debian 12 / RHEL 9 got GLIBC_2.39-not-found. Route both to the fully-static perry-linux-<arch>-musl.tar.gz, which already ships.
Also corrects the release-matrix section in README.md, which still claimed the Linux legs build on ubuntu-22.04 — they moved to ubuntu-24.04 in v0.5.860, which is what raised the floor to glibc 2.39 in the first place.
Jammy ships clang 14 by default; perry's LLVM backend emits `ptr` and clang 14 rejects it with "expected type". Orthogonal to the glibc floor — it would fail the same way with the glibc binary — but it has to be out of the way for the job to prove the musl fallback actually compiles and links.
Found while verifying the #6298 fallback on a real Ubuntu 22.04 runner: once the binary loads, jammy's default clang 14 rejects perry's `ptr` IR with "error: expected type". `perry doctor` reports clang as OK without checking its version, so the failure reads as a compiler bug.
The old-glibc fallback already warns that the static build cannot produce perry/ui GTK4 apps, but the Alpine/musl path did not. Those users have no glibc alternative, so it is not a downgrade — but perry/ui still will not link, and they should hear that at install time rather than at first build.
… the launcher workflow Two review findings. 1. The musl-fallback recovery message said `npm install --force <pkg>`, which only fixes a PROJECT-local install. Perry is usually installed globally (`npm i -g @perryts/perry`), and for those users that command changes nothing — they would run it, see no improvement, and be stuck. Both the glibc-too-old message and the generic missing-package fallback now spell out the global and project forms. 2. npm-launcher.yml installs and executes published registry content (the platform packages), so it must not carry a writable token or leave a git credential on disk for that content to find: workflow-level `permissions: contents: read`, and `persist-credentials: false` on both checkouts. Found by review.
a3fea0a to
be074a4
Compare
…file (#7967) * ci: put every Node the project chooses on 26, and assert it from one file .node-version (26.5.1) is the authoritative oracle, but the pin has leaked twice since #6367 made it single: CLAUDE.md's prose drifted off the file (#7599), and npm-launcher.yml was created by #6350 on the SAME DAY #6367 converted every existing workflow, keeping that day's ambient "22.23.1" literal by omission rather than by decision. - npm-launcher.yml (x2): "22.23.1" -> node-version-file: .node-version. It runs npm/perry/test/detect.test.cjs, which exercises the shipped launcher logic every installing user hits, so its Node is a behavioural input. Safe on the ubuntu-22.04 job: Node 26 needs glibc >= 2.28 and jammy has 2.35. - release-packages.yml: "20" -> "26". Node 20 reached EOL on 2026-04-30 and this is the repo's most privileged job (id-token: write, OIDC-publishes every platform package). - release-hono-server.yml: "24" -> "26". Both release workflows stay pinned to a bare MAJOR rather than node-version-file: they are publishing toolchains, and a gap-suite oracle bump must never be able to move the runtime that publishes releases. New scripts/check_node_version_consistency.py, wired as a lint step (a required context). It re-derives every restatement of a Node version from the file it quotes, and requires every literal node-version: in a workflow to be a registered exemption with a reason. Exemptions are asserted against the tree, so one that stops matching FAILS and must be updated or deleted. Reverting npm-launcher.yml to "22.23.1" reproduces the historical bug as two named failures. --self-test proves each rule can fail; both vacuity floors can fail too. Not changed, deliberately: test-compat/node-core/pinned-version.txt (v22.x runs Node's own corpus) and benchmarks/public-baseline-config.json (v22.23.1). The latter is in public_baseline.HARNESS_PATHS, so editing it alone reddens the required lint job -- measured, ci_public_baseline_check.py exits 2 with "benchmark harness changed". The pin and its ~2 h measurement are atomic by design (#7282/#7958) and the regeneration needs the quiet M1 mini, so it is registered as a self-clearing exemption carrying the runbook instead. Node 26 is faster than Node 22, so that regeneration is expected to reduce Perry's published advantage. Also: CLAUDE.md said "Two workflows are deliberately exempt" and then listed three, which is probably why the fourth pin read as accounted-for; and external-tools.json told readers to bump a NODE_PIN constant that does not exist in node_compat_matrix.mjs (it reads external-tools.json). Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 * docs: changelog fragment for #7967 Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2 --------- Co-authored-by: Ralph Küpper <ralph3@skelpo.com>
Closes #6298.
The hole
release-packages.ymlbuilds every glibc Linux target onubuntu-24.04/ubuntu-24.04-arm— glibc 2.39. The npm launcher (npm/perry/bin/perry.js) only ever appended-muslwhen it detected a musl host; there was no glibc version check anywhere innpm/, andpackaging/install.shhad no libc detection at all.So every glibc distro below 2.39 gets a binary its dynamic loader refuses:
GLIBC_2.39 not foundGLIBC_2.39 not foundGLIBC_2.39 not foundGLIBC_2.39 not foundinstall.sh)Perry already publishes exactly the artifact these hosts need:
perry-linux-{x86_64,aarch64}-muslis a fully-static PIE — I checked the shipped ELF, it has noPT_INTERPand no libc dependency at all.What this PR does
Routes old-glibc hosts to that static build.
npm/perry/bin/detect.cjs(new) — resolution rules, split out so they can be tested without the machine under them.GLIBC_BUILD_FLOOR = "2.39"is a named constant documented against the builder image; below it, Linux hosts resolve to@perryts/perry-linux-{x64,arm64}-musl.npm/perry/bin/perry.js— walks candidate packages in preference order, prints a one-time notice when it lands on the static build (stamped in tmpdir;PERRY_NO_FALLBACK_NOTICE=1silences), and when the musl package isn't installed replaces the loader error with the exact command to fix it.packaging/install.sh— detects musl and old glibc, downloadsperry-linux-<arch>-musl.tar.gz. This path needs no user action at all..github/workflows/npm-launcher.yml(new) — path-filtered CI: the hermetic self-test, plus an end-to-end job on a realubuntu-22.04runner.installation.mdand the npm README. Also corrects the README release-matrix section, which still claimed the Linux legs build onubuntu-22.04(they moved toubuntu-24.04in v0.5.860 — that move is what raised the floor).Bonus robustness: a musl-detected host now falls back to the glibc package if the musl one is missing, which un-breaks the empty-
glibcVersionRuntimecase from #116.Verification
Self-test (
node npm/perry/test/detect.test.cjs) — 2.31 / 2.34 / 2.35 / 2.36 / 2.39 / 2.41 / musl / no-report hosts, both arches, plus a check that the packages it names really are optionalDependencies built by the release matrix:The floor is pinned to reality: the self-test parses the release matrix and fails if the
-unknown-linux-gnulegs move offubuntu-24.04withoutGLIBC_BUILD_FLOORbeing revisited. Bumping the builder can't silently re-break this.End-to-end on a real Ubuntu 22.04 runner (
npm-launcherworkflow, this PR): installs the published packages the way a user does, reproduces the bug, then runs the patched launcher against them.Run: https://github.com/PerryTS/perry/actions/runs/29233234338 —
Ubuntu 22.04 (glibc 2.35) end-to-end, green. Verbatim:Two things worth calling out from that run:
--versioninvocation and not on the subsequentcompileinvocation.libperry_runtime.a/libperry_stdlib.alink cleanly under the host's glibccc, and the output is a normal dynamic glibc executable — the static-ness is confined to the compiler, it does not leak into what users ship. I checked why before trusting it: those archives bundle no musl libc objects (only Rust CGUs + compiler-builtins), and all 380 of their libc-facing undefined symbols are plain POSIX/ISO-C names (__errno_location,__xpg_strerror_r,__cxa_thread_atexit_impl,__tls_get_addr,accept4, …) that glibc exports. Nothing musl-internal to resolve.Local verification of the launcher itself (
perry.js, with fake platform packages and a stubbedprocess.report) covers the paths CI doesn't: notice-once + suppression, the--forceinstruction when the musl package is absent (exit 1, no loader error), and the #116 rescue.What the fallback costs the user
perry/ui(GTK4) desktop apps. The musl package doesn't shiplibperry_ui_gtk4.a(GTK4 is glibc-only), and the musl target hard-errors onperry/uiby design. Everything else — server, CLI, native compile, stdlib — is the same compiler.libc: ["musl"]on a glibc machine, so the launcher can't silently pull it in. It now printsnpm install --force @perryts/perry-linux-x64-musl@<version>instead of a loader error. If you'd rather it be zero-touch, dropping thelibcfield from the two muslpackage.json.tmpls makes npm install the static build alongside the glibc one on every Linux host — that costs every Linux user ~67 MB of unpacked package they may not need, which is why I didn't do it unilaterally. Say the word and it's a two-line change.mallocisn't in the hot path of the programs you build.Separate blocker found on 22.04 (not a libc issue)
Once the binary loads, jammy's default clang is 14, which rejects Perry's opaque-pointer IR:
Perry's LLVM backend emits
ptrand shells out toclang -c, so Linux needs clang ≥ 15.perry doctorreports clang as OK without ever checking its version, so this surfaces as a mysterious codegen error. Documented here (apt install clang-15+PERRY_LLVM_CLANG); the doctor gate is filed separately.Is a native glibc-2.35 build leg worth adding? Not in this PR — and it wouldn't remove the launcher logic
I looked at it properly; it is not a clean matrix addition:
ubuntu-22.04is blocked byperry-ui-gtk4. Jammy has neitherlibwebkitgtk-6.0-dev(WebView, perry/ui WebView component: cross-platform native WebView for auth / payments / embedded pages #658) norlibshumate-dev(MapView, Provide a widget for showing a map #517) — that is precisely why v0.5.860 moved the runners 22.04 → 24.04. Going back means feature-gating both widgets out of the Linux desktop build, and it still leaves RHEL 9 / AL2023 (2.34) broken.--forceor an unconditional ~110 MB optional dep. It doubles the slowest part of the release matrix (dist-profile LTO Linux builds) and buys nothing over the musl fallback.clangat runtime rather than linkingllvm-sys). The blocker isperry-ui-gtk4again.So the proper fix is a
perry-ui-gtk4feature-gating project plus a release-matrix change, not a packaging tweak — filed as a follow-up. Whatever we do there, a launcher that understands glibc versions is still required, because npm's selector cannot express one.Still open (not fixable from the launcher)
The apt
.debis built from theperry-linux-x86_64glibc artifact, soapt install perrystays broken below glibc 2.39 — a deb can't fall back at runtime. It should either declareDepends: libc6 (>= 2.39)so it fails cleanly at install time, or ship the static binary. Folded into the follow-up issue.Notes
conformance-smokeshards are known-flaky and unrelated to anything here.Summary by CodeRabbit
PERRY_NO_FALLBACK_NOTICE=1).clang ≥ 15.