Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use electron-winstaller instead of self module #8344

Open
wants to merge 52 commits into
base: master
Choose a base branch
from

Conversation

beyondkmp
Copy link
Collaborator

@beyondkmp beyondkmp commented Jul 17, 2024

  1. Add customSquirrelExePath option, which allows using custom squirrel windows
  2. Use official electron-winstaller
  3. Support Windows ARM
  4. Fix some bugs([Security] Hijacking DLL problem electron-builder-binaries#33, Squirrel.Windows does not support ARM64 electron-builder-binaries#52)

Copy link

changeset-bot bot commented Jul 17, 2024

🦋 Changeset detected

Latest commit: 3d30f3f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
app-builder-lib Patch
electron-builder-squirrel-windows Patch
dmg-builder Patch
electron-builder Patch
electron-forge-maker-appimage Patch
electron-forge-maker-nsis-web Patch
electron-forge-maker-nsis Patch
electron-forge-maker-snap Patch

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

@Stanzilla
Copy link

Is this package actually still supported by the electron team? Many packages in that org have been dead for a long time.

@mmaietta
Copy link
Collaborator

Is this package actually still supported by the electron team? Many packages in that org have been dead for a long time.

That's a great point. @beyondkmp what bug fixes is this solving for? (Can't tell from the description)

@mmaietta
Copy link
Collaborator

@Stanzilla looks like updates are still being pushed to electron-winstaller, last update was 3mo ago. https://www.npmjs.com/package/electron-winstaller

Copy link
Contributor

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Sep 29, 2024
@github-actions github-actions bot removed the windows label Nov 1, 2024
* The custom squirrel.windows.exe path. Defaults to `https://github.com/electron-userland/electron-builder-binaries/tree/master/Squirrel.Windows`.
*/
readonly vendorDirectory?: string

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update doc comment

/**
*  The custom file path to `squirrel.windows.exe` for usage with [electron-winstaller](https://github.com/electron/windows-installer) `vendorDirectory`.
*/

Defaults to https://github.com/electron-userland/electron-builder-binaries/tree/master/Squirrel.Windows

Also, is this true with the current setup? I don't see any default value being set for vendorDirectory. As this is currently coded, it looks like it'll default to the vendor files that come with electron/windows-installer

Copy link
Collaborator Author

@beyondkmp beyondkmp Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated.

/**
   * The custom squirrel.windows.exe path. If not specified will use the Squirrel.Windows that is shipped with electron-installer(https://github.com/electron/windows-installer/tree/main/vendor).
   * After https://github.com/electron-userland/electron-builder-binaries/pull/56 merged, will add `electron-builder-binaries` to get the latest version of squirrel.
   */

The squirrel windows in this electron-userland/electron-builder-binaries#56 is the same as the one in https://github.com/electron/windows-installer/tree/main/vendor, so when this value is empty, it defaults to using the squirrel windows from windows-installer.

@electron-userland electron-userland deleted a comment from netlify bot Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants