Skip to content

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

Merged
beyondkmp merged 74 commits into
electron-userland:masterfrom
beyondkmp:win-installer
Feb 3, 2025
Merged

feat: use electron-winstaller instead of self module#8344
beyondkmp merged 74 commits into
electron-userland:masterfrom
beyondkmp:win-installer

Conversation

@beyondkmp

@beyondkmp beyondkmp commented Jul 17, 2024

Copy link
Copy Markdown
Contributor
  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)

@changeset-bot

changeset-bot Bot commented Jul 17, 2024

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 11f53ee

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

Comment thread patches/electron-winstaller@5.3.1.patch Outdated
@Stanzilla

Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Collaborator

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

@beyondkmp

Copy link
Copy Markdown
Contributor Author

@mmaietta Resolved the code conflict now, but this UT keeps failing, running locally is no problem. It looks like there is a problem with this UT on the master, help me out if you have time.
image

@mmaietta

Copy link
Copy Markdown
Collaborator

I'll take a look at this today. There is no problem on master though...as the commit test history is clean: https://github.com/electron-userland/electron-builder/commits/master/

@mmaietta

Copy link
Copy Markdown
Collaborator

I pushed your branch to origin/win-installer within the electron-builder repo and the tests all pass correctly: https://github.com/electron-userland/electron-builder/actions/runs/13076539224

My best guess is that running the job from your fork is not able to access that repo secret specifically?

@beyondkmp

Copy link
Copy Markdown
Contributor Author

@mmaietta Yes, that's correct. The reason is the same as what you mentioned. The conflicts in this MR have been resolved. When you have time, please help review it again. If there are no issues, it can be merged.

Comment thread packages/electron-builder-squirrel-windows/src/SquirrelWindowsTarget.ts Outdated
Comment thread packages/electron-builder-squirrel-windows/src/SquirrelWindowsTarget.ts Outdated
Comment thread packages/electron-builder-squirrel-windows/src/squirrelPack.ts
Comment thread packages/electron-builder-squirrel-windows/src/squirrelPack.ts
@mmaietta mmaietta removed the Stale label Feb 3, 2025
Comment thread test/snapshots/filesTest.js.snap
@beyondkmp
beyondkmp requested a review from mmaietta February 3, 2025 06:26
mmaietta
mmaietta previously approved these changes Feb 3, 2025

@mmaietta mmaietta left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great work!

Comment thread packages/electron-builder-squirrel-windows/src/SquirrelWindowsTarget.ts Outdated
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