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

.msi related cherry picks for v3.x #2608

Closed
wants to merge 2 commits into from
Closed

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Aug 29, 2015

This is for @joaocgreis to review, it's my attempt to cherry-pick 4cfe5eb and c6a54d0 but the differences are quite large and the conflicts non trivial.

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>
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>
@Fishrock123 Fishrock123 added build Issues and PRs related to build files or the CI. land-on-v3.x labels Aug 29, 2015
@rvagg rvagg mentioned this pull request Aug 29, 2015
@brendanashworth brendanashworth added the windows Issues and PRs related to the Windows platform. label Aug 29, 2015
@joaocgreis
Copy link
Member

@rvagg I would just apply this fixup to the first commit (752977b), to change the position of the line:

diff --git a/tools/msvs/msi/product.wxs b/tools/msvs/msi/product.wxs
index 841aeaa..9e7eadc 100755
--- a/tools/msvs/msi/product.wxs
+++ b/tools/msvs/msi/product.wxs
@@ -312,10 +312,10 @@
     </InstallUISequence>

     <InstallExecuteSequence>
+      <Custom Action='SetInstallScope' Before='FindRelatedProducts'/>
       <Custom Action="LinkNodeExeToIojsExe" After="InstallFiles">
         $NodeAlias = 3
       </Custom>
-      <Custom Action='SetInstallScope' Before='FindRelatedProducts'/>
       <Custom Action='BroadcastEnvironmentUpdate' After='InstallFinalize'/>
     </InstallExecuteSequence>

Other than that, LGTM.

joaocgreis added a commit 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
@rvagg rvagg closed this Aug 30, 2015
@rvagg rvagg deleted the msi-cherry-picks branch August 30, 2015 07:16
@rvagg
Copy link
Member Author

rvagg commented Aug 30, 2015

landed @ bb24c4a, thanks @joaocgreis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants