fix(linux): bundle libgomp in the AppImage, the one format that cannot declare it - #326
Merged
Merged
Conversation
Contributor
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughLinux Whisper packaging now stages and validates ChangesWhisper OpenMP packaging
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
…t declare it 1.9.2 declared the three sonames its packages needed and never asked for, which fixed three formats out of four. The AppImage has no dependency mechanism at all, so declaring anything does nothing for it: on a machine without libgomp.so.1 its STT stack still dies in ld.so before main(), and transcription still shows an end user a developer error. Of those three, exactly one may be bundled, and it happens to be that one. The AppImage project's excludelist names the other two and gives the reason: libgbm.so.1 is "part of mesa" and talks to the host's DRM stack, libasound.so.2 loads the host's ALSA plugins and configuration. A bundled copy of either is worse than none. libgomp is a self-contained runtime and is absent from that list. Nothing else was needed to make it resolve, which is worth recording: whisper-stt-server and every libggml/libwhisper/libparakeet beside it already carry `RUNPATH=$ORIGIN:$ORIGIN/bin`. A copy in that directory is found ahead of the system one, so this needs no patchelf pass and no custom AppRun — both of which the first sketch of this change assumed were unavoidable. WHERE the copy is made matters more than that it is made. It happens in build-whisper-stt.sh, on the machine that compiles these binaries, whose Linux leg build-whisper-stt.yml pins to ubuntu-22.04 — the same floor before-pack.cjs enforces. So the library that ships comes from the same machine and the same glibc as the binaries that load it, and it travels inside the whisper artifact to every consumer whatever their own distro. The first version of this change copied it during packaging instead, and that was wrong in a way worth naming: it took the library from whoever ran the build, so a developer on 24.04 staged a libgomp needing GLIBC_2.38, before-pack.cjs correctly refused it, and `npm run build:linux` stopped working locally for a change that was supposed to be invisible there. Provenance was the fix, not a version check. stage-whisper-stt.sh now only asserts the library arrived, and names the workflow to re-run when an older artifact does not carry it. It checks on the early-exit path too: that is the branch a developer with a locally built binary takes, and skipping it there is how the AppImage would keep shipping without libgomp on exactly the builds nobody re-checks. Resolved through the binary rather than a hardcoded /usr/lib path so arm64 needs no second case, and copied under its soname because that is the DT_NEEDED the loader looks for; the file on disk is libgomp.so.1.0.0. The libgomp1/libgomp entries added to deb, rpm and pacman in 1.9.2 stay. They are redundant now that the copy travels with the binaries, and they cost nothing: if this staging ever regresses silently, three formats out of four still resolve. Verified: with the copy in place, ldd resolves libgomp through $ORIGIN to the bundled file rather than the system one, and the STT binary loads on an ubuntu:24.04 container that has no libgomp1 installed. Remove the staged copy in that same container and it dies on "libgomp.so.1: cannot open shared object file".
EtienneLescot
force-pushed
the
fix/appimage-bundle-libgomp
branch
from
August 10, 2026 11:14
fc8ef1c to
d3f1437
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@technical-documentation/engineering/build-and-packaging.md`:
- Line 132: Update the documentation reference to use the repository-relative
path scripts/stage-whisper-stt.sh, and verify any related source-code file paths
or line ranges still point to the intended staging implementation.
- Around line 130-134: Correct the AppImage dependency explanation in the
surrounding documentation: replace the claim that every system soname is missing
by construction with the statement that the AppImage has no host-dependency
manifest for this check. Preserve the existing descriptions of bundled
libgomp.so.1 and libvulkan.so.1 and the host-supplied libraries.
🪄 Autofix
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: ee33f046-0974-4975-b84e-b64ac54e4101
📒 Files selected for processing (2)
scripts/stage-whisper-stt.shtechnical-documentation/engineering/build-and-packaging.md
Review catch, and a fair one. The paragraph said every system soname is missing by construction for the AppImage, then described two that ship inside it — libgomp.so.1 from this change, libvulkan.so.1 from electron-builder. Both cannot be true. What is actually true is narrower: the check verifies that everything a package needs is either declared or shipped, and the AppImage declares nothing, so it would report every library the format legitimately expects from the host. That is the model working as intended rather than a defect, which is the reason the check skips the format — not that its sonames are absent. Both scripts also take the repository-relative path on first mention, which is what the rest of this file does (`scripts/before-pack.cjs` then `before-pack.cjs`).
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
1 participant
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.
1.9.2 declared the three sonames its Linux packages needed and never asked for.
That fixed three formats out of four. The AppImage has no dependency mechanism,
so declaring anything does nothing for it — on a machine without
libgomp.so.1its STT stack still dies in
ld.sobeforemain(), and transcription stillshows an end user a developer error.
Why this one and not the other two
libgomp.so.1libgbm.so.1libasound.so.2It needed less than expected
Every STT binary already carries
RUNPATH=$ORIGIN:$ORIGIN/bin, so a copy inthat directory is found ahead of the system one. No
patchelfpass, no customAppRun— both of which the first sketch of this change assumed wereunavoidable.
Where the copy is made matters more than that it is made
It happens in
build-whisper-stt.sh, on the machine that compiles thesebinaries, whose Linux leg
build-whisper-stt.ymlpins toubuntu-22.04— thesame floor
before-pack.cjsenforces. The library that ships therefore comesfrom the same machine and the same glibc as the binaries that load it, and it
travels inside the whisper artifact to every consumer whatever their own distro.
The first version of this PR copied it during packaging instead, and that was
wrong in a way worth naming: it took the library from whoever ran the build, so
a developer on 24.04 staged a
libgompneedingGLIBC_2.38,before-pack.cjscorrectly refused it, and
npm run build:linuxstopped working locally for achange meant to be invisible there. Provenance was the fix, not a version check.
stage-whisper-stt.shnow only asserts the library arrived, and names theworkflow to re-run when an older artifact does not carry it — including on the
early-exit path a developer with a locally built binary takes.
The
libgomp1/libgompentries added in 1.9.2 stay: redundant now, and freeinsurance if this staging ever regresses silently.
Verification
lddresolveslibgomp.so.1through$ORIGINto the bundled file rather than the system one.ubuntu:24.04container with nolibgomp1installed.libgomp.so.1: cannot open shared object file.Note for whoever merges
The whisper artifact currently published predates this change, so it does not
carry the library yet.
stage-whisper-stt.shwill fail loudly and say so —re-run
build-whisper-stt.ymlbefore the next release build. That is theintended behaviour, not a regression.
Not in scope
The rest of what the AppImage takes from the host is the GTK/GLib/NSS stack,
which no AppImage bundles — theme engines, GIO modules and pixbuf loaders all
resolve against the host.
libvulkan.so.1is already bundled at the AppImageroot by electron-builder itself.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
libgomp.so.1runtime with the artifact.Documentation