This repository has been archived by the owner on Apr 22, 2023. It is now read-only.
win,msi: change InstallScope to perMachine #25640
Closed
Conversation
This file contains 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
The MSI install scope was set to the WiX default, which is per-user. However, with UAC, it could not be installed by a standard user because InstallPrivileges is elevated by default, hence the install scope should be set to per-machine. Furthermore, the default install path is a per-machine location and setting the system path requires administrator privileges. By changing the InstallScope to perMachine, Start Menu shortcuts are placed in ProgramData and not the installing user's AppData folder, making the shortcuts available to other users. This also fixes the installation when AppData is a network folder. The custom action is necessary to allow upgrades. Since a per-machine MSI cannot upgrade an application installed per-user, the custom action checks if there is going to be an upgrade to a previous version installed per-user and sets the installation as per-user to allow upgrading. Hence, the advantages of installing per-machine will only apply in fresh installations. Fixes #5849 Fixes #7629
Since install is per machine only, installation path should be stored in local machine instead of current user. The registry stores HKLM in different places for 32 and 64 bit applications, so the installer will not suggest the old path when upgrading from 32 to 64 bit version. Fixes #5592 Fixes #25087
|
LGTM, but would still be good if someone else (@piscisaureus ?) could take a look. |
|
The problem I see with this is the following: In order for NPM to work, |
|
@piscisaureus Thanks for reviewing. In all my tests, the |
@piscisaureus , thanks for reviewing! As @joaocgreis said, %appdata%\npm still gets added to the user path as before. OK to land this? |
If that's true, then yes. |
|
Triple-checked that it does. Will land shortly... |
joaocgreis
added a commit
that referenced
this pull request
Jul 22, 2015
The MSI install scope was set to the WiX default, which is per-user. However, with UAC, it could not be installed by a standard user because InstallPrivileges is elevated by default, hence the install scope should be set to per-machine. Furthermore, the default install path is a per-machine location and setting the system path requires administrator privileges. By changing the InstallScope to perMachine, Start Menu shortcuts are placed in ProgramData and not the installing user's AppData folder, making the shortcuts available to other users. This also fixes the installation when AppData is a network folder. The custom action is necessary to allow upgrades. Since a per-machine MSI cannot upgrade an application installed per-user, the custom action checks if there is going to be an upgrade to a previous version installed per-user and sets the installation as per-user to allow upgrading. Hence, the advantages of installing per-machine will only apply in fresh installations. Fixes #5849 Fixes #7629 PR-URL: #25640 Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Bert Belder <[email protected]>
joaocgreis
added a commit
that referenced
this pull request
Jul 22, 2015
Since install is per machine only, installation path should be stored in local machine instead of current user. The registry stores HKLM in different places for 32 and 64 bit applications, so the installer will not suggest the old path when upgrading from 32 to 64 bit version. Fixes #5592 Fixes #25087 PR-URL: #25640 Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Bert Belder <[email protected]>
This test just failed on Ubuntu in Jenkins, for a change that is 100% Windows-specific.
|
FYI, adding a commit to this PR mark test-signal-unregister as flaky. It failed on Ubuntu and that's obviously unrelated to this change: |
|
Actually, I will mark that test as flaky in a separate PR. |
|
Landed in 14db629. |
|
This should probably go in the converged repo. @jasnell what do we need to do to make it so? |
joaocgreis
added a commit
to JaneaSystems/node
that referenced
this pull request
Aug 28, 2015
This is an adaptation of 8e80528. Original commit message: The MSI install scope was set to the WiX default, which is per-user. However, with UAC, it could not be installed by a standard user because InstallPrivileges is elevated by default, hence the install scope should be set to per-machine. Furthermore, the default install path is a per-machine location and setting the system path requires administrator privileges. By changing the InstallScope to perMachine, Start Menu shortcuts are placed in ProgramData and not the installing user's AppData folder, making the shortcuts available to other users. This also fixes the installation when AppData is a network folder. The custom action is necessary to allow upgrades. Since a per-machine MSI cannot upgrade an application installed per-user, the custom action checks if there is going to be an upgrade to a previous version installed per-user and sets the installation as per-user to allow upgrading. Hence, the advantages of installing per-machine will only apply in fresh installations. Fixes nodejs/node-v0.x-archive#5849 Fixes nodejs/node-v0.x-archive#7629 PR-URL: nodejs/node-v0.x-archive#25640 Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Bert Belder <[email protected]> The original commit was adapted to search all upgrade codes listed in the upgrade table, as the current installer tries to upgrade from two different upgrade codes. PR-URL: nodejs#2565 Reviewed-By: Alexis Campailla <[email protected]>
joaocgreis
added a commit
to JaneaSystems/node
that referenced
this pull request
Aug 28, 2015
This is a port of 14db629. Original commit message: Since install is per machine only, installation path should be stored in local machine instead of current user. The registry stores HKLM in different places for 32 and 64 bit applications, so the installer will not suggest the old path when upgrading from 32 to 64 bit version. Fixes nodejs/node-v0.x-archive#5592 Fixes nodejs/node-v0.x-archive#25087 PR-URL: nodejs/node-v0.x-archive#25640 Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Bert Belder <[email protected]> PR-URL: nodejs#2565 Reviewed-By: Alexis Campailla <[email protected]>
rvagg
pushed a commit
to rvagg/io.js
that referenced
this pull request
Aug 29, 2015
This is an adaptation of 8e80528. Original commit message: The MSI install scope was set to the WiX default, which is per-user. However, with UAC, it could not be installed by a standard user because InstallPrivileges is elevated by default, hence the install scope should be set to per-machine. Furthermore, the default install path is a per-machine location and setting the system path requires administrator privileges. By changing the InstallScope to perMachine, Start Menu shortcuts are placed in ProgramData and not the installing user's AppData folder, making the shortcuts available to other users. This also fixes the installation when AppData is a network folder. The custom action is necessary to allow upgrades. Since a per-machine MSI cannot upgrade an application installed per-user, the custom action checks if there is going to be an upgrade to a previous version installed per-user and sets the installation as per-user to allow upgrading. Hence, the advantages of installing per-machine will only apply in fresh installations. Fixes nodejs/node-v0.x-archive#5849 Fixes nodejs/node-v0.x-archive#7629 PR-URL: nodejs/node-v0.x-archive#25640 Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Bert Belder <[email protected]> The original commit was adapted to search all upgrade codes listed in the upgrade table, as the current installer tries to upgrade from two different upgrade codes. PR-URL: nodejs#2565 Reviewed-By: Alexis Campailla <[email protected]>
rvagg
pushed a commit
to rvagg/io.js
that referenced
this pull request
Aug 29, 2015
This is a port of 14db629. Original commit message: Since install is per machine only, installation path should be stored in local machine instead of current user. The registry stores HKLM in different places for 32 and 64 bit applications, so the installer will not suggest the old path when upgrading from 32 to 64 bit version. Fixes nodejs/node-v0.x-archive#5592 Fixes nodejs/node-v0.x-archive#25087 PR-URL: nodejs/node-v0.x-archive#25640 Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Bert Belder <[email protected]> PR-URL: nodejs#2565 Reviewed-By: Alexis Campailla <[email protected]>
joaocgreis
added a commit
to nodejs/node
that referenced
this pull request
Aug 30, 2015
This is a port of 14db629. Original commit message: Since install is per machine only, installation path should be stored in local machine instead of current user. The registry stores HKLM in different places for 32 and 64 bit applications, so the installer will not suggest the old path when upgrading from 32 to 64 bit version. Fixes nodejs/node-v0.x-archive#5592 Fixes nodejs/node-v0.x-archive#25087 PR-URL: nodejs/node-v0.x-archive#25640 Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Bert Belder <[email protected]> PR-URL: #2565 Reviewed-By: Alexis Campailla <[email protected]> Cherry picked to v3.x by @rvagg, PR: #2608
jBarz
pushed a commit
to ibmruntimes/node
that referenced
this pull request
Nov 4, 2016
The MSI install scope was set to the WiX default, which is per-user. However, with UAC, it could not be installed by a standard user because InstallPrivileges is elevated by default, hence the install scope should be set to per-machine. Furthermore, the default install path is a per-machine location and setting the system path requires administrator privileges. By changing the InstallScope to perMachine, Start Menu shortcuts are placed in ProgramData and not the installing user's AppData folder, making the shortcuts available to other users. This also fixes the installation when AppData is a network folder. The custom action is necessary to allow upgrades. Since a per-machine MSI cannot upgrade an application installed per-user, the custom action checks if there is going to be an upgrade to a previous version installed per-user and sets the installation as per-user to allow upgrading. Hence, the advantages of installing per-machine will only apply in fresh installations. Fixes nodejs#5849 Fixes nodejs#7629 PR-URL: nodejs#25640 Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Bert Belder <[email protected]>
jBarz
pushed a commit
to ibmruntimes/node
that referenced
this pull request
Nov 4, 2016
Since install is per machine only, installation path should be stored in local machine instead of current user. The registry stores HKLM in different places for 32 and 64 bit applications, so the installer will not suggest the old path when upgrading from 32 to 64 bit version. Fixes nodejs#5592 Fixes nodejs#25087 PR-URL: nodejs#25640 Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Bert Belder <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Currently, installing with the MSI always asks for Administrator permissions, as mentioned in #7629 . Explicitly installing per machine only adds the benefit that shortcuts are placed in ProgramData thus being accessible to all users and also solving #5849 .
The second commit moves the installation path in the registry to Local Machine. The registry stores HKLM in different places for 32 and 64 bit applications, so the installer will not suggest the old path when upgrading from 32 to 64 bit version, solving #5592 and #25087 .
/cc @piscisaureus