ci(linux): guard the native payload, declare patchelf, drop the dead zsync glob - #298
Conversation
…zsync glob Three findings from an audit of the Linux build chain. None of them affects a published artifact today — they are the missing nets and one stale doc claim. before-pack.cjs asserted nothing on Linux. Its comment said "Linux ships no native addon of its own", written before the wgpu compositor addon and the PipeWire capture helper landed, and never revisited. macOS got a payload check for exactly this reason; Linux now gets the symmetric one, covering compositor_view.node, the five symbol-renamed ffmpeg .so files, the helper, whisper-stt-server and its ggml sidecars. The helper's ffmpeg/ subdirectory is checked separately, because a name match is not the property that matters: it has to be a directory holding the *unrenamed* libraries. That also catches a real collision — `fetch:ffmpeg` vendors the static ffmpeg binary to that exact path, so running it by hand replaces the directory with a file and produces a helper that cannot start. CI never sees it, `build:linux` only runs fetch:ffmpeg:sdk. Running `node scripts/before-pack.cjs` on Linux also fell through to the Windows branch and reported a missing D3D11 addon at a win32 path. patchelf is an unconditional dependency of build-linux-compositor-addon.mjs (resolvePatchelf throws without it) and was not in the apt line. It works because the ubuntu-24.04 image preinstalls it; declaring it stops the build depending on the runner image's contents. The release/**/*.zsync upload glob has matched nothing since the app-builder-lib 26.x bump — that version dropped zsync for an embedded block map, and there is no updater in this repo to consume one anyway. It stayed invisible because if-no-files-found: error evaluates the union of the patterns, so a dead glob alongside live ones never fails.
|
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 CI now installs ChangesLinux packaging validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant beforePack
participant NativePayloadChecker
participant LinuxArtifacts
beforePack->>NativePayloadChecker: validate Linux payload and artifact freshness
NativePayloadChecker->>LinuxArtifacts: inspect compositor and PipeWire helper files
LinuxArtifacts-->>NativePayloadChecker: return payload presence, shape, and freshness
NativePayloadChecker-->>beforePack: allow packaging or raise validation error
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 1
🤖 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 `@scripts/before-pack.cjs`:
- Around line 121-154: Update LINUX_REQUIRED by replacing the single combined
FFmpeg matcher with five separate requirements for libavcodec, libavformat,
libavutil, libswresample, and libswscale. Each requirement must match only its
library family, retain the existing compositor-linking description and Linux fix
guidance, and require at least one matching file so stale copies of another
family cannot satisfy validation.
🪄 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: 227b36d3-9d4a-443f-a73b-e7feb28d070f
📒 Files selected for processing (3)
.github/workflows/build.ymlscripts/before-pack.cjstechnical-documentation/engineering/ci-workflows.md
`atLeast: 5` portait sur le total de correspondances d'une regex combinée, pas sur la présence de chaque famille. Cinq copies versionnées d'une même bibliothèque — libavcodec.so.58 à .62 laissées par un build précédent — satisfaisaient le compte pendant qu'une autre manquait : le paquet passait la garde et le compositeur ne chargeait pas, exactement le mode de panne que cette garde existe pour attraper. Une exigence par famille, ce qui nomme aussi précisément celle qui manque dans le message d'erreur au lieu d'un « au moins 5 » opaque. Exercé contre des payloads fabriqués : le cas dégradé passait avec l'ancienne regex et est refusé avec la nouvelle, chacune des cinq familles est requise séparément, et le payload complet reste accepté.
Summary
Three findings from an audit of the Linux build chain. None affects a published artifact today — CI rebuilds everything on a fresh checkout, so these are the missing nets plus one stale doc claim.
1.
before-pack.cjsasserted nothing on LinuxThe hook branched on
win32anddarwin, then ended on:// Linux ships no native addon of its own; nothing to assert.That was true when it was written and stopped being true when the wgpu compositor addon (
d645a8ba) and the PipeWire capture helper (ef71c970) landed. macOS gotMAC_REQUIREDprecisely because "returned early on any non-win32 platform" had already let a broken bundle through once; Linux now gets the symmetric guard.LINUX_REQUIREDcoverscompositor_view.node, the five symbol-renamed ffmpeg.sofiles,openscreen-pipewire-helper,whisper-stt-serverand its ggml sidecars — everythinglinux.extraResourcesships viafilter: ["linux-*/**"].The helper's
ffmpeg/subdirectory is checked separately, because a name match is not the property that matters: it has to be a directory holding the unrenamed libraries. The.sofiles one level up have every symbol renamed toosff_*so the addon cannot bind to Chromium's bundled ffmpeg — the helper needs the originals.That separate check also caught a real collision worth naming:
npm run fetch:ffmpegvendors the static ffmpeg binary to exactlyelectron/native/bin/linux-x64/ffmpeg, the pathbuild:native:linuxwants as a directory. Whoever runs second loses. CI never sees it —build:linuxonly runsfetch:ffmpeg:sdk, which does not write that file — but it bites anyone who runs the full fetch by hand, and the resulting helper cannot start. The guard now says so in one line instead of failing at runtime.Also:
node scripts/before-pack.cjson Linux fell through to the Windows branch and reported a missing D3D11 addon at a win32 path.2.
patchelfis an undeclared hard dependencybuild-linux-compositor-addon.mjs:191callsresolvePatchelf()unconditionally, which throws when it is absent — and its own docstring says "it is not installed by default anywhere". The apt line installed onlylibarchive-tools. It works today because theubuntu-24.04runner image preinstalls patchelf 0.18.0; declaring it stops the build from depending on the image's contents.3.
release/**/*.zsynchas matched nothing for a whilezsync is electron-updater's delta format. This repo has no updater (
grep -rn 'electron-updater\|autoUpdater\|latest-linux'→ empty), and app-builder-lib 26.x contains no occurrence of "zsync" at all —appImageUtil.jscallsappendBlockmap, embedding the block map instead. The last green Linux build logsthere will be 3 files uploaded.It stayed invisible because
if-no-files-found: errorevaluates the union of the patterns, so one dead glob among live ones never fails.ci-workflows.md:104still advertised zsync as an output; corrected.Related issue
No issue — found while auditing the Linux build/packaging chain.
Type of change
Release impact
Desktop impact
Screenshots / video
n/a
Testing
scripts/before-pack.cjsexercised on Ubuntu 24.04 x86_64 against the realelectron/native/bin/linux-x64/payload, four cases:Linux native payload complete in electron/native/bin/linux-x64, addon up to date.— exit 0ffmpegis a file, not a directoryfetch:ffmpegcollision and the fix — exit 1openscreen-pipewire-helperremovednpm run build:native:linux— exit 1ffmpeg/present but empty.sofiles — exit 1patchelfrequirement reproduced:env -u PATCHELF PATH=/usr/bin:/bin node scripts/build-linux-compositor-addon.mjs→Error: patchelf not found.build.ymlre-parsed withyaml.safe_load;biome checkclean onbefore-pack.cjs.The macOS path is refactored, not changed:
checkMacNativePayloadnow delegates to a sharedcheckNativePayloadwith the same list, thresholds and message text.labelbecame a full noun ("Metal compositor addon"rather than"Metal") so the freshness check can also guard the capture helper, which is not a compositor addon.Not exercised here: a real
electron-builder --linuxpack, and the macOS branch.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation
Chores