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

fix(nsis): Windows on ARM support #4228

Merged
merged 5 commits into from
Sep 23, 2019

Conversation

dangeredwolf
Copy link
Contributor

@dangeredwolf dangeredwolf commented Sep 11, 2019

Checklist:

@develar develar merged commit d738644 into electron-userland:master Sep 23, 2019
@lutzroeder
Copy link
Contributor

lutzroeder commented Oct 31, 2019

This change causes NSIS installers built on macOS to crash on Windows:

return getBinFromUrl("nsis", "3.0.4", "FVF4HClUCsTZ32vYOIC7rTo1qb2pvt2nZwzb86MbKbORukMH0rS3SGBYg/MHmT58PqvRCSlEFai6impEYDYp2Q==")

#4374 @dangeredwolf @devlar

@dangeredwolf
Copy link
Contributor Author

dangeredwolf commented Nov 2, 2019

@lutzroeder I've gotta be honest, he merged my PR on this project before I properly validated and debugged the updated scripts.

I wasn't able to validate the scripts without electron-builder-binaries being updated, and I made a PR there too. Both PRs were approved essentially at the same time, so by the time I had the ability to validate it, my PR was already merged.

I ran the built in tests after making my changes, and they passed, meaning the scripts are valid, but because of the reason above, I haven't been able to actually fully vet it before merging.

There was a reason why not all the checkboxes were checked, lol.

Anywayyyyy...

Are there any error messages, or logs, or anything that could help us diagnose the issue?

@lutzroeder
Copy link
Contributor

lutzroeder commented Nov 3, 2019

Crash log running the installer on Windows is included in #4374. Should the change be reverted while this is getting tested and investigated as it will produce invalid installers that get pushed out to users?

@dangeredwolf
Copy link
Contributor Author

Sorry I didn't respond. My life has been all over the place lately. Yes, that's fine, and I noticed you already did it. At least until we can figure out what change broke it on macOS.

@quanglam2807
Copy link
Contributor

Is this change ready? The PR that tried to revert this is closed: #4416 but when I tried to add arm64 support to my app, the universal-arch NSIS doesn't seem to work correctly on my x64 system. It only installs the Uninstall {...}.exe and ignores everything else.

Capture

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants