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): silent upgrade of per-machine install not elevated (#5468) #6438

Conversation

krisdages
Copy link

Without elevation, app is uninstalled, but new files can't be extracted
and no error message is displayed.

…electron-userland#5468)

Without elevation, app is uninstalled, but new files can't be extracted
 and no error message is displayed.
@changeset-bot
Copy link

changeset-bot bot commented Nov 20, 2021

⚠️ No Changeset found

Latest commit: 1fc0806

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@mmaietta
Copy link
Collaborator

Can you please rebase off latest master? That should resolve the test failures

Copy link
Contributor

@robertpatrick robertpatrick left a comment

Choose a reason for hiding this comment

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

While I think this change is probably ok, I am concerned that the change added in PR #6073 already is elevating all Windows "per machine" installs. In debugging my issue #6425, it seems that we need to limit the change made by PR #6073 to only use elevate.exe when the install is running in silent mode (maybe by rolling back #6073 and merging this PR?).

In electron-updater@4.6.2 (which includes #6073), the autoInstallOnAppQuit feature (which run the installer in silent mode) is already working on a per machine basis using the NSIS installer so it's not clear to me that this change will do something not already being done by #6073. As I mentioned above, maybe the PR #6073 change should have been accomplished using this PR instead so that #6425 is resolved by not trying to elevate installation when isSilent is set to false? Am I missing something?

@mmaietta
Copy link
Collaborator

Closing PR in favor of #6450
Thank you @krisdages for your contribution, we've utilized it as part of #6450 for the full solution and credit to you has been given as well :)

@mmaietta mmaietta closed this Nov 30, 2021
mmaietta pushed a commit that referenced this pull request Nov 30, 2021
…nstall/updates (#6450)

* fix(nsis): fix per-machine installs

Add @krisdages changes from PR #6438 to elevate silent per-machine installs.
Remove PR #6073 that incorrectly elevates all per-machine installs, breaking interactive per-machine installs.

Closes #6425, #5468

Co-authored-by: Robert Patrick
Co-authored-by: Kris Dages
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.

None yet

3 participants