Strip unused-arch native prebuilts from packaged installers#3371
Conversation
gavande1
changed the title
There was a problem hiding this comment.
Pull request overview
Adjusts Studio’s Electron Forge prePackage hook to strip unused-architecture native prebuilts from the bundled CLI node_modules, reducing installer size and avoiding cross-platform binary/code-signing issues.
Changes:
- Make koffi prebuilt cleanup arch-aware by keeping only
${platform}_${arch}. - Add arch-aware cleanup for
fs-ext-extra-prebuiltbinaries by keeping onlyfs-ext-${platform}-${arch}-*. - Add defensive “target must exist” guards that skip cleanup if expected target artifacts are missing.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const koffiDirs = fs.readdirSync( koffiBuildDir ); | ||
| // Refuse to delete anything if the target arch dir is missing — guards | ||
| // against a koffi naming change silently nuking every prebuilt. | ||
| if ( ! koffiDirs.includes( koffiTarget ) ) { | ||
| console.warn( | ||
| `Skipping koffi cleanup: no directory named ${ koffiTarget } in ${ koffiBuildDir }` | ||
| ); | ||
| } else { |
There was a problem hiding this comment.
Good catch — applied in 909eace. Now stat-checks the target before deleting siblings, mirroring the existing isDirectory() check inside the loop.
| fs.unlinkSync( path.join( fsExtBinDir, file ) ); | ||
| console.log( `Removed fs-ext-extra-prebuilt/binaries/${ file }` ); |
There was a problem hiding this comment.
Applied in 84cd9b2. Wrapped the unlink in try/catch and downgraded failures to a warning, matching the prior art in scripts/remove-fs-ext-other-platform-binaries.mjs. Cleanup is purely a size optimization — not worth aborting packaging over a stuck file.
…com:Automattic/studio into stu-1677-strip-unused-arch-native-prebuilts
ivan-ottinger
left a comment
ivan-ottinger
left a comment
There was a problem hiding this comment.
Thank you for the work on the clean-up, Rahul!
The changes look good and work as expected. 👍🏼 I did not observe any regressions. Heres are several screenshots from tests I run.
Filtering of fs-ext-linux-x64-node on arm64 Linux:
Filtering of fs-ext-darwin-x64-node on arm64 macOS:
Reduction in size of the .deb installer (before / after):
I have also run the installed app on all three platforms (all arm64) and did not observe any issues. Also found the installer is slightly smaller (as expected).
📊 Performance Test ResultsComparing 529a9dd vs trunk app-size
site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
Related issues
How AI was used in this PR
Used Claude to walk through the existing
forge.config.tsafterCopy hook, draft the arch-aware filter, and add a defensive guard that refuses to delete prebuilts unless the target arch is present. I reviewed the diff, verified the koffi/fs-ext naming conventions against the actualnode_moduleslayout, and checked that thearchvalue passed by electron-forge matches the directory/file naming used by both packages (x64,arm64).Proposed Changes
apps/studio/forge.config.tsto filter by${platform}_${arch}instead of just${platform}_. A Linux x64 DEB no longer shipslinux_arm64/armhf/ia32/loong64/riscv64d; same shape for Mac/Windows.fs-ext-extra-prebuiltcleanup in the same prePackage hook. The rootpostinstallscript still filters by platform only (intentional — at install time we don't know the target arch); the new hook strips the unused arch oncearchis in scope.Estimated savings per installer (from the issue):
fs-ext-extra-prebuiltadds a few hundred KB on top.Testing Instructions
${platform}_${arch}; fs-ext only contains files starting withfs-ext-${platform}-${arch}-.koffiTarget = 'bogus_arch'in the hook, run packaging, confirm the warning fires and koffi prebuilts are untouched. Revert before merge.CI should cover the cross-platform builds (Buildkite Linux dev build group + macOS + Windows).
Pre-merge Checklist