feat(migration): fully replace Go binary app-builder-bin with TS implementation#9829
Merged
Conversation
🦋 Changeset detectedLatest commit: 87f5e49 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
This PR completes the removal of the app-builder-bin (Go) native binary dependency by replacing its remaining responsibilities with TypeScript implementations and separately downloaded toolsets (7-Zip, Wine, snap templates), reducing native binary surface area across electron-builder.
Changes:
- Removed
executeAppBuilder/app-builder-binand migrated remaining call sites to TS implementations and toolset downloads. - Introduced downloadable toolsets for 7-Zip and Wine, and updated packaging flows (snap, NSIS/Wine, Proton/LibUI, extraction paths) to use them.
- Added/updated Vitest coverage for the new behaviors (archive args, snap helpers, wine toolset, stream collector race fix, extraction security).
Reviewed changes
Copilot reviewed 40 out of 42 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| we-need-to-remove-app-builder-bin.md | New migration plan/inventory doc for removing app-builder-bin. |
| test/src/node-module-collector/streamCollectorTest.ts | Updates mocks + adds test for outStream error handling. |
| test/src/mac/wineToolsetSuite.ts | Adds integration tests for getWineToolset() on macOS. |
| test/src/linux/snapCoreLegacyTest.ts | Adds unit tests for shellQuote() helper. |
| test/src/libUiFrameworkTest.ts | Adds tests for LibUI helper mapping + shell-embeddable validation. |
| test/src/ksuidTest.ts | Removes parity tests that depended on executeAppBuilder(["ksuid"]). |
| test/src/helpers/winHelper.ts | Updates Wine-based installer verification wiring to use the new toolset. |
| test/src/helpers/wine.ts | Allows injecting wine path/env; uses toolset env and derived wineboot path. |
| test/src/extractArchiveTest.ts | Adds ZIP traversal/symlink guard tests for extractArchive(). |
| test/src/certInfoTest.ts | Removes parity tests that depended on executeAppBuilder(["certificate-info"]). |
| test/src/blockmapTest.ts | Removes parity comparisons to app-builder-bin golden output. |
| test/src/archiveUtilTest.ts | Adds tests for compute7zCompressArgs and archive guards. |
| test/package.json | Removes app-builder-bin dev dependency from tests. |
| pnpm-lock.yaml | Removes app-builder-bin and 7zip-bin lock entries. |
| packages/electron-builder-squirrel-windows/src/SquirrelWindowsTarget.ts | Switches wine execution to WineVmManager (toolset-based). |
| packages/builder-util/src/util.ts | Removes executeAppBuilder and 7za exports (API change). |
| packages/builder-util/src/7za.ts | Deletes old 7zip-bin wrapper. |
| packages/builder-util/package.json | Drops 7zip-bin and app-builder-bin dependencies. |
| packages/app-builder-lib/templates/snap/desktop-init.sh | Adds desktop integration script template. |
| packages/app-builder-lib/templates/snap/desktop-gnome-specific.sh | Adds GNOME-specific desktop integration script template. |
| packages/app-builder-lib/templates/snap/desktop-common.sh | Adds common desktop integration script template. |
| packages/app-builder-lib/src/wine.ts | Deletes old execWine wrapper around app-builder. |
| packages/app-builder-lib/src/vm/WineVm.ts | Updates WineVmManager to use getWineToolset(). |
| packages/app-builder-lib/src/vm/vm.ts | Minor tweak in Parallels VM selection logic. |
| packages/app-builder-lib/src/util/electronGet.ts | Adds .tar.xz extraction path and increases lock retries. |
| packages/app-builder-lib/src/toolsets/wine.ts | New Wine toolset downloader/env builder. |
| packages/app-builder-lib/src/toolsets/7zip.ts | New 7-Zip toolset downloader + cached resolver. |
| packages/app-builder-lib/src/targets/snap/coreLegacy.ts | Rewrites legacy snap builder to TS/template download + snapcraft flow. |
| packages/app-builder-lib/src/targets/nsis/NsisTarget.ts | Removes last direct 7za listing usage; uses unpackedSize and toolset wine VM. |
| packages/app-builder-lib/src/targets/MsiTarget.ts | Updates Wine VM construction to accept toolset config. |
| packages/app-builder-lib/src/targets/archive.ts | Refactors compression args; removes zip-specific helper; adds path guard. |
| packages/app-builder-lib/src/ProtonFramework.ts | Passes launchUiVersion through to LibUI. |
| packages/app-builder-lib/src/packager.ts | Respects string launchUiVersion (not just boolean). |
| packages/app-builder-lib/src/node-module-collector/nodeModulesCollector.ts | Fixes stream flush race; expands retry conditions. |
| packages/app-builder-lib/src/frameworks/LibUiFramework.ts | Replaces proton-native app-builder command with TS downloads + shell safety validation. |
| packages/app-builder-lib/src/electron/ElectronFramework.ts | Uses extractArchive() instead of direct 7za unzip path. |
| packages/app-builder-lib/src/configuration.ts | Adds toolsets.wine config option. |
| packages/app-builder-lib/scheme.json | Adds schema support for toolsets.wine. |
| packages/app-builder-lib/package.json | Minor ordering change (no functional change). |
| .changeset/remove-app-builder-bin-snap-wine.md | Changeset for snap/wine migration + full removal. |
| .changeset/remove-app-builder-bin-proton-native.md | Changeset for proton-native migration. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… equivalents; `executeAppBuilder` and `app-builder-bin` package fully removed from codebase; snap templates now downloaded via `downloadBuilderToolset` with pinned checksums; Wine is downloaded automatically on macOS (legacy 4.0.1 bundle by default, Wine 11 bundle available via `toolsets.wine: "1.0.1"`); on Linux with legacy config and when `USE_SYSTEM_WINE=true`, system wine is used instead feat: replace app-builder-bin `proton-native` command with pure-TS equivalents; Node.js and LaunchUI binaries are now downloaded via `downloadBuilderToolset`; `config.launchUiVersion` string is now respected
fd61254 to
bbf67e9
Compare
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
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.
Summary
app-builder-binfully removed —executeAppBuilderand its entire call surface deleted frombuilder-utilandapp-builder-lib. Theapp-builder-binnpm package is no longer a dependency.packages/app-builder-lib/src/toolsets/7zip.ts) — New pure-TS downloader for7zafromelectron-userland/electron-builder-binaries. Replacesbuilder-util/src/7za.ts.getPath7za()now resets its memoized promise on failure so callers can retry on transient errors.packages/app-builder-lib/src/toolsets/wine.ts) — Replaceswine.tsand the oldexecWine()helper. Supports legacywine-4.0.1-mac(default),wine@1.0.1(Wine 11), andELECTRON_BUILDER_WINE_TOOLSET_DIRlocal override. Env var merging fixed:DYLD_FALLBACK_LIBRARY_PATHandLD_LIBRARY_PATHnow correctly merge with existing process env without duplication. Env-override path probes bothbin/wineandbin/wine64to handle legacy bundles.packages/app-builder-lib/src/vm/WineVm.ts) —exec()now spreadsprocess.envas the base soPATH,HOME, andTMPDIRare preserved when invoking Wine.packages/app-builder-lib/src/targets/snap/coreLegacy.ts):buildWithTemplate: downloads snap template tarball with pinned SHA-256 checksums; callsmksquashfsdirectly.buildWithoutTemplate: stages desktop-integration shell scripts fromtemplates/snap/;command.shnow correctly references$SNAP/scripts/desktop-*.sh(not$SNAP/desktop-*.sh) in no-template builds.executableNamevalidated viavalidateShellEmbeddablebefore embedding in the generatedcommand.shto prevent shell injection via crafted app names.desktop-common.shfixed:LIBVA_DRIVERS_PATHnow uses$SNAP_DESKTOP_ARCH_TRIPLET(exported) instead of$ARCH(local only).packages/app-builder-lib/src/node-module-collector/nodeModulesCollector.ts):outStreamerror:child.kill()andoutStream.destroy()are now called (best-effort) before rejecting, preventing orphaned processes."No JSON content found in output".-EncodedCommandwrapping on Windows to avoid.cmdshim spawn issues.app-builder proton-nativecommand withdownloadBuilderToolset.unpackedSize— Replaces7za larchive listing with theunpackedSizevalue already computed during packaging.amd64andarmhftemplates;downloadBuilderToolsetsignature restored to accept optional checksums (callers without pre-known hashes remain compatible viaunsafelyDisableChecksums).