Skip to content
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
wants to merge 3 commits into from
Closed

win,msi: change InstallScope to perMachine #25640

wants to merge 3 commits into from

Conversation

joaocgreis
Copy link
Member

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

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
@orangemocha
Copy link
Contributor

LGTM, but would still be good if someone else (@piscisaureus ?) could take a look.

@piscisaureus
Copy link

The problem I see with this is the following:

In order for NPM to work, %appdata%\npm needs to be added to the path.
That path is user-specific.
Setting the installation scope to global would add the wrong path.

@joaocgreis
Copy link
Member Author

@piscisaureus Thanks for reviewing. In all my tests, the %appdata%\npm still gets added to the user path, but only for the user that installs. This is the same behavior that happens without these changes. Do you have a specific situation in mind in which the wrong path is added?

@orangemocha
Copy link
Contributor

In order for NPM to work, %appdata%\npm needs to be added to the path.

@piscisaureus , thanks for reviewing! As @joaocgreis said, %appdata%\npm still gets added to the user path as before. OK to land this?

@piscisaureus
Copy link

@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.

@orangemocha
Copy link
Contributor

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 <alexis@janeasystems.com>
Reviewed-By: Bert Belder <bertbelder@gmail.com>
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 <alexis@janeasystems.com>
Reviewed-By: Bert Belder <bertbelder@gmail.com>
This test just failed on Ubuntu in Jenkins, for a change that
is 100% Windows-specific.
@orangemocha
Copy link
Contributor

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:
https://jenkins-iojs.nodesource.com/job/node-test-commit-linux/28/nodes=ubuntu1504-64/tapTestReport/test.tap-576/

@orangemocha
Copy link
Contributor

Actually, I will mark that test as flaky in a separate PR.

@orangemocha
Copy link
Contributor

Landed in 14db629.

@orangemocha
Copy link
Contributor

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 <alexis@janeasystems.com>
  Reviewed-By: Bert Belder <bertbelder@gmail.com>

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 <alexis@janeasystems.com>
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 <alexis@janeasystems.com>
  Reviewed-By: Bert Belder <bertbelder@gmail.com>

PR-URL: nodejs#2565
Reviewed-By: Alexis Campailla <alexis@janeasystems.com>
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 <alexis@janeasystems.com>
  Reviewed-By: Bert Belder <bertbelder@gmail.com>

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 <alexis@janeasystems.com>
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 <alexis@janeasystems.com>
  Reviewed-By: Bert Belder <bertbelder@gmail.com>

PR-URL: nodejs#2565
Reviewed-By: Alexis Campailla <alexis@janeasystems.com>
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 <alexis@janeasystems.com>
  Reviewed-By: Bert Belder <bertbelder@gmail.com>

PR-URL: #2565
Reviewed-By: Alexis Campailla <alexis@janeasystems.com>

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 <alexis@janeasystems.com>
Reviewed-By: Bert Belder <bertbelder@gmail.com>
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 <alexis@janeasystems.com>
Reviewed-By: Bert Belder <bertbelder@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants