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

Windows installer always installs to C:/Program Files #6057

Closed
balane opened this issue Apr 5, 2016 · 12 comments
Closed

Windows installer always installs to C:/Program Files #6057

balane opened this issue Apr 5, 2016 · 12 comments
Labels
install Issues and PRs related to the installers. windows Issues and PRs related to the Windows platform.

Comments

@balane
Copy link

balane commented Apr 5, 2016

I prefer to install all of my dev apps in one directory (C:/apps) but when using the Node.js Windows installer it always gets installed in C:/Program Files regardless of what I put in the installer.

  • Version: 4.4.2
  • Platform: Windows 7 64-bit
    64-bit_
  • Subsystem: Windows Installer
@balane balane changed the title Node always installs to C:/Program Files Windows installer always installs to C:/Program Files Apr 5, 2016
@mscdex mscdex added windows Issues and PRs related to the Windows platform. install Issues and PRs related to the installers. labels Apr 5, 2016
@Fishrock123 Fishrock123 added the feature request Issues that request new features to be added to Node.js. label Apr 5, 2016
@eljefedelrodeodeljefe
Copy link
Contributor

I can confirm there is a bug (I consider it as such) at least on Windows 7. Windows 10 works fine though. @mscdex @Fishrock123 this would need re-labeling.

@eljefedelrodeodeljefe
Copy link
Contributor

@balane can you run msinfo32 and copy-paste the result here. I just did a fresh install after I bought a OEM Professional version and there it did work as expected. Before, I tested with modern_ie. So good news for us is it is not affecting everyone on windows...hopefully.

@Fishrock123 Fishrock123 removed the feature request Issues that request new features to be added to Node.js. label Apr 6, 2016
@balane
Copy link
Author

balane commented Apr 6, 2016

What exactly are you looking for from the msinfo32 result?

@eljefedelrodeodeljefe
Copy link
Contributor

Build number of Windows, Architecture etc. Before I go debugging I wanted to have as much information as possible. Minus security relevant stuff of course...

@balane
Copy link
Author

balane commented Apr 7, 2016

OS Name Microsoft Windows 7 Enterprise
Version 6.1.7601 Service Pack 1 Build 7601
Other OS Description Not Available
OS Manufacturer Microsoft Corporation
System Manufacturer Dell Inc.
System Model Latitude E6510
System Type x64-based PC
Processor Intel(R) Core(TM) i7 CPU Q 740 @ 1.73GHz, 1734 Mhz, 4 Core(s), 8 Logical Processor(s)
BIOS Version/Date Dell Inc. A16, 12/5/2013
SMBIOS Version 2.6
Windows Directory C:\Windows
System Directory C:\Windows\system32
Boot Device \Device\HarddiskVolume2
Locale United States
Hardware Abstraction Layer Version = "6.1.7601.17514"
Time Zone Central Daylight Time
Installed Physical Memory (RAM) 8.00 GB
Total Physical Memory 7.93 GB
Available Physical Memory 4.17 GB
Total Virtual Memory 15.9 GB
Available Virtual Memory 12.4 GB
Page File Space 7.93 GB

@orangemocha
Copy link
Contributor

cc @joaocgreis , our MSI expert.

@joaocgreis
Copy link
Member

I could not reproduce this. I tried to create the registry values that the installer reads (Software/Node.js under HKLM and HKCU) and those are read correctly but the path chosen by the user is always used.

@balane can you try to create a installation log? Use:

msiexec /i <filename>.msi /l*v InstallLog.txt

@eljefedelrodeodeljefe
Copy link
Contributor

Hmm. I couldn't reproduce (again) neither, even though it was reproducable earlier. It's likely related to Windows being under whatever circumstances. Just installing again then solves it though. I would vote for closing this until there is some logged reproduction. @joaocgreis msiexec /i C:\Users\myusername\Downloads\somepackage.msi /L*v install.txt would be logging sufficiently I assume?

@balane
Copy link
Author

balane commented Apr 7, 2016

InstallLog.txt

From a quick glance through the log file looks like it's saying that it correctly installed to C:/apps/nodejs but it absolutely did not.

@mcnameej
Copy link

The directory change is happening on the client side...

MSI (c) (F4:48) [16:08:46:758]: PROPERTY CHANGE: Modifying INSTALLDIR property. Its current value is 'C:\Program Files\nodejs\'. Its new value: 'C:\apps\nodejs\'.

...but never makes it to the server side...

MSI (s) (58:9C) [16:08:57:831]: Ignoring disallowed property INSTALLDIR

This results in the server-side using the default value...

MSI (s) (58:9C) [16:08:57:984]: PROPERTY CHANGE: Adding INSTALLDIR property. Its value is 'C:\Program Files\nodejs\'.

It's what happens on the server side that counts.

The problem is that INSTALLDIR needs to be added to the SecureCustomProperties MSI property. Something like this:

diff --git a/tools/msvs/msi/product.wxs b/tools/msvs/msi/product.wxs
index eed53e4..dcd6b90 100755
--- a/tools/msvs/msi/product.wxs
+++ b/tools/msvs/msi/product.wxs
@@ -45,6 +45,7 @@
     <Property Id="ARPPRODUCTICON" Value="NodeIcon"/>
     <Property Id="ApplicationFolderName" Value="nodejs"/>
     <Property Id="WIXUI_INSTALLDIR" Value="INSTALLDIR"/>
+    <Property Id="SecureCustomProperties" Value="INSTALLDIR"/>

     <Property Id="INSTALLDIR">
       <RegistrySearch Id="InstallPathRegistry"

(To answer the next logical question... Sorry, NO, I can't submit a PR for this right now.)

@eljefedelrodeodeljefe
Copy link
Contributor

eljefedelrodeodeljefe commented May 3, 2016

@joaocgreis does @mcnameej's patch seem reasonable (does to me)? I can make the PR and give it a try, but would need someone to review.

@joaocgreis
Copy link
Member

With the tips from above, I could finally reproduce this. On Windows 7 64-bit, node 4.4.2 64-bit:

  1. In the Administrator account, use the Group Policy Editor to enable Always install with elevated privileges, from both User and Computer Configuration under Administrative Template, All Settings.
  2. Switch to a user with a non-Administrator account, install node. This issue happens.

The patch above seems to be the way to fix this (thanks @mcnameej!). @eljefedelrodeodeljefe if you can make a PR, that'd be great! I'll review and can test in Windows 7.

There are two other things I'd investigate here. From the WiX documentation, there is a Secure attribute that could be used instead, something like <Property Id="INSTALLDIR" Secure="yes"> (@eljefedelrodeodeljefe can you give this a try first?). Also, some other properties are ignored but it seems that they are set correctly in the server side. I'd be happy if the fix installs with the MSI and INSTALLDIR in different drives, both non C:. I can test this when I review.

joaocgreis added a commit to JaneaSystems/node that referenced this issue Sep 26, 2016
Allows INSTALLDIR to be passed to the server MSIExec process during
installation.

Fixes: nodejs#6057
jasnell pushed a commit that referenced this issue Oct 10, 2016
Allows INSTALLDIR to be passed to the server MSIExec process during
installation.

PR-URL: #8795
Fixes: #6057
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 pushed a commit that referenced this issue Oct 11, 2016
Allows INSTALLDIR to be passed to the server MSIExec process during
installation.

PR-URL: #8795
Fixes: #6057
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Nov 18, 2016
Allows INSTALLDIR to be passed to the server MSIExec process during
installation.

PR-URL: #8795
Fixes: #6057
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
install Issues and PRs related to the installers. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

7 participants