Opened 8 years ago

Closed 4 years ago

#249 closed Bug / Defect (fixed)

Installer script bugs

Reported by: aerDNA Owned by: Samuli Seppänen
Priority: minor Milestone: release 2.4
Component: Installation Version: OpenVPN git master branch (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:
Cc:

Description

I found two problems with the v2.3 installer:

1) '\OpenVPN' is not appended to path when install dir is changed on MUI_PAGE_INSTFILES (dir selection).

2) specifying the install dir on the command line (/D=) has no consequence, making it impossible to silently install OpenVPN to a non-default directory.

The cause of problem No.1 is that the InstallDir? attribute was omitted from the script.
Current script uses the following code to set default install dir with regard to architecture:

	Function .onInit

	...

	${If} "${ARCH}" == "x86_64"
		SetRegView 64
		StrCpy $INSTDIR "$PROGRAMFILES64\${PACKAGE_NAME}"
	${Else}
		StrCpy $INSTDIR "$PROGRAMFILES\${PACKAGE_NAME}"
	${EndIf}

	...

	FunctionEnd

This works fine but it doesn't make the InstallDir? attribute redundant. InstallDir? shouldn't have been removed because it serves another purpose - the part of the string after the last '\' is what gets appended when the user changes the install dir.

Problem No.2 is directly tied to the above code. The current script always sets $INSTDIR to default in .onInit and thus overwrites the value possibly set by the /D switch.

And the solution to both problems is:

	InstallDir "$PROGRAMFILES\${PACKAGE_NAME}"

	...

	Function .onInit

	...

	${If} "${ARCH}" == "x86_64"
		SetRegView 64
		StrCmp $INSTDIR "$PROGRAMFILES\${PACKAGE_NAME}" 0 +2
		StrCpy $INSTDIR "$PROGRAMFILES64\${PACKAGE_NAME}"
	${EndIf}

	...

	FunctionEnd

Sorry if my report doesn't conform to standards, but better to report bugs some way than no way I suppose. Anyway, I know my NSIS and what I wrote is definitely correct.

Attachments (1)

0002-Fix-to-Trac-ticket-249.patch (1.3 KB) - added by Samuli Seppänen 8 years ago.
Fix to Trac ticket #249

Download all attachments as: .zip

Change History (25)

comment:1 Changed 8 years ago by Samuli Seppänen

Owner: set to Samuli Seppänen
Status: newaccepted

Thanks for reporting this. I'll fix this and attach a new installer to this bug report so that you can test it.

comment:2 Changed 8 years ago by Samuli Seppänen

I applied the fix you suggested and built new Windows installers. Please test and report back:

I tested the installers interactively and they seemed to work correctly. Could you provide a silent-install command-line that failed previously, and which (hopefully) works with the new installer?

A patch against the latest code in the openvpn-build repository is attached to this ticket.

Changed 8 years ago by Samuli Seppänen

Fix to Trac ticket #249

comment:3 in reply to:  2 Changed 8 years ago by aerDNA

Tested new installers on Win7 32/64 and yep, everything is fine now.

Cmd example, as requested:

openvpn-install-2.3.0-I002-i686 /S /D=C:\TestDir

Notes:

  • NSIS switches are case-sensitive
  • /D must be the last param
  • you can test without /S, installer doesn't have to be silent for /D to take effect

comment:4 Changed 8 years ago by aerDNA

Sorry, I didn't tetst thoroughly, there's a problem after all.

When I install to a non-default dir and start OpenVPN GUI, I get:
"CreateProcess? failed, exe='C:\Program Files\OpenVPN\bin\openvpn.exe' cmdline='openvpn --version' dir='C:\Program Files\OpenVPN\bin'"

But it turns out that this was not introduced by your new installer, the release version (2.3.0-I001) produces the same problem.

comment:5 Changed 8 years ago by aerDNA

Located the problem:
HKEY_LOCAL_MACHINE\SOFTWARE\OpenVPN-GUI is not removed by uninstaller. This key is created by openvpn-gui.exe on first run, unless the key already exists. So in the case of a subsequent OpenVPN install to a different folder, openvpn-gui.exe reads values, such as "exe_path", that wrongly point to previous install location.

comment:6 Changed 8 years ago by aerDNA

It's a one-line fix, just add

DeleteRegKey HKLM "SOFTWARE\OpenVPN-GUI"

to the unistall section.

comment:7 Changed 8 years ago by Samuli Seppänen

Thanks for testing! I'll produce a fixed installer later this week.

comment:8 Changed 8 years ago by aerDNA

Great, I was worried my posts went unnoticed. Especially since unlike first two bugs, this one is not so minor. I'd just like to add that the reg key in question should be deleted by the installer as well as the uninstaller. Otherwise, the new installer will not fix the problem until an uninstall is performed.

comment:9 Changed 8 years ago by aerDNA

Just a follow-up on my previous reply: in the most common install scenario, the reg key will not exist and trying to delete it will set an error flag. Currently, it's not a problem since the script does no error checking, but if in some future revision IfErrors? is placed anywhere down the line, it will produce a bug that may not be too obvious when tracing. So i think it would be wise to elliminate the potential problem right away and have DeleteRegKey? followed by ClearErrors? in the installer part.

comment:10 Changed 8 years ago by Samuli Seppänen

Installers that should remove the OpenVPN-GUI registry key:

Removal is conditional, i.e. happens only if openvpn-gui is/was defined during installation. Did not add the removal to the install section yet.

comment:11 Changed 8 years ago by aerDNA

At least the uninstaller cleans up properly now.

comment:12 Changed 8 years ago by aerDNA

I presume that 90% of people install everything to %ProgramFiles?%, and those who don't will probably reinstall to the same folder as the first time. So in practice, what's remaining of this problem will manifest itself rarely.

comment:13 Changed 8 years ago by Samuli Seppänen

This is somewhat related to ticket #252. I'll fix both in one go.

comment:15 Changed 8 years ago by aerDNA

I only tested the x32 version, but it's no fix. All I see is a regression to not deleting the OpenVPN-GUI key, which reintroduces the problem - OpenVPN-GUI fails to launch when OpenVPN is subsequently installed to a different folder. You don't really have many options here. You either have to fix it on installer/uninstaller level by deleting the key, like you already did partially in I003, or you have to modify OpenVPN GUI source.
Leftovers after uninstall are considered a poor practice anyway (in case of user data, the user should be prompted on whether he wants to preserve it or not).

comment:16 Changed 8 years ago by Samuli Seppänen

Forgot to mention that the fix to the OpenVPN-GUI issue was removed from the installer intentionally. The rationale is described in full in ticket 252. Blindly deleting OpenVPN-GUI registry keys will certainly get us into trouble when people's OpenVPN-GUI proxy and language settings disappear after upgrading (i.e. uninstalling and installing) OpenVPN. I'll probably end up writing a separate installer and uninstaller for OpenVPN-GUI to solve this problem least intrusively.

Anyways, I here are new installers with more NSI-related changes:

I tested silent installation on WinXP 32-bit and Windows 7 64-bit and it worked fine. Not sure if the installer you tested had an issue, but these don't. Of course, the OpenVPN-GUI registry keys had to be deleted manually, but only if OpenVPN had been previously installed to a different directory.

comment:17 Changed 8 years ago by aerDNA

How about this - a piece of installer code that synchronizes the paths in OpenVPN-GUI key with those of the new installation:

ClearErrors

ReadRegStr $0 HKLM "SOFTWARE\OpenVPN-GUI" 'config_dir' ;get current path
IfErrors +3 ;error = reg value doesn't exist
StrCmp $0 "$INSTDIR\config" +2 ;is the path same as the new one?
WriteRegStr HKLM "SOFTWARE\OpenVPN-GUI" 'config_dir' '$INSTDIR\config' ;update

ReadRegStr $0 HKLM "SOFTWARE\OpenVPN-GUI" 'exe_path' ;this is the troublemaker
IfErrors +3
StrCmp $0 "$INSTDIR\bin\openvpn.exe" +2
WriteRegStr HKLM "SOFTWARE\OpenVPN-GUI" 'exe_path' '$INSTDIR\bin\openvpn.exe'

ReadRegStr $0 HKLM "SOFTWARE\OpenVPN-GUI" 'log_dir'
IfErrors +3
StrCmp $0 "$INSTDIR\log" +2
WriteRegStr HKLM "SOFTWARE\OpenVPN-GUI" 'log_dir' '$INSTDIR\log'

It fixes the problem without deleting the key. The code takes effect only when reinstalling to a different location, otherwise it does nothing.

comment:18 Changed 7 years ago by Samuli Seppänen

Milestone: release 2.3release 2.4

I will take a closer look at this once we're preparing the 2.4 release. I wrote a separate NSIS installer for OpenVPN-GUI, which may have fixed this. Feel free to test it - it's available here:

Integration with the openvpn-build buildsystem is still lacking, but otherwise OpenVPN-GUI NSIS installer should work fine.

comment:19 Changed 7 years ago by Gert Döring

What's the state of this? Still "2.4"? Or has it been integrated into the recent installer improvements?

(If I read this correctly, the remaining issue is "if this is a second installation with a different install directory, and the gui has been run on the first installation, the keys in the registry are all wrong", right? I like the proposed installer fix from aerDNA "if the keys are already there, but pointing elsewhere, just update them on installation")

comment:20 Changed 7 years ago by Samuli Seppänen

The plan is to integrate the openvpn-gui.nsi installer (which fixes this bug) with the first 2.4 alphas. Changes to openvpn.nsi in openvpn-build are still needed. Additionally the registry key generation logic from the openvpn-gui application should be removed, as that's then handled by the installer in a much cleaner fashion.

That said, there's nothing blocking us from introducing these changes into 2.3.x either before or after first 2.4 alphas are out.

comment:21 Changed 6 years ago by Samuli Seppänen

Updated ticket #252 with more information plus a link to a new OpenVPN installer that bundles openvpn-gui-installer.exe.

comment:22 Changed 4 years ago by Samuli Seppänen

The plan to use a separate OpenVPN-GUI installer has been scrapped, because recent OpenVPN-GUI versions use the capabilities of the Interactive Service and write their registry keys to HKCU instead of HKLM. This also means that fixing any broken registry keys would be quite difficult, as each user who has started OpenVPN-GUI will have a outdated copy of the keys in his/her private registry (HKCU).

I'm inclined to just document this behavior to minimize the impact. Thoughts?

comment:23 Changed 4 years ago by Samuli Seppänen

Thinking about this a bit more... HKLM\SOFTWARE\OpenVPN already has config_dir, exe_path and log_dir keys. OpenVPN-GUI basically duplicates these keys, which causes the real paths and OpenVPN-GUI keys to get out of sync.

Why not just read the keys directly from HKLM\SOFTWARE\OpenVPN and scrap those keys from OpenVPN-GUI? Or, if we really need to allow users to (manually) tell OpenVPN-GUI where OpenVPN directories/executables are, we could probably use this three-level system:

  1. Use value from HKCU\SOFTWARE\OpenVPN-GUI, if it exists. If not, move down.
  2. Use value from HKLM\SOFTWARE\OpenVPN-GUI, if it exists. If not, move down.
  3. Use value from HKLM\SOFTWARE\OpenVPN (default, which should always be ok)

People who modify the keys manually are already a small minority, and those which also decide install OpenVPN to a different directory at some point are probably very few indeed.

comment:24 Changed 4 years ago by Samuli Seppänen

Resolution: fixed
Status: acceptedclosed

Afaics both of the the original issues have been fixed already. From openvpn-build/windows-nsis/openvpn.nsi:

;Remember install folder
InstallDirRegKey HKLM "SOFTWARE\${PACKAGE_NAME}" ""

And in .onInit function:

; Change the installation directory to C:\Program Files, but only if the
; user has not provided a custom install location.
${If} "$INSTDIR" == "$PROGRAMFILES\${PACKAGE_NAME}"
    StrCpy $INSTDIR "$PROGRAMFILES64\${PACKAGE_NAME}"
${EndIf}

In addition OpenVPN-GUI registry key handling has been improved a lot lately, with redundant/useless keys having been removed. This fixes the "OpenVPN-GUI cannot find OpenVPN that is installed to a non-standard location" problem, as OpenVPN-GUI gets the exe_path from HKLM\Software\OpenVPN now. It also no longer stores uncustomized configuration options in HKCU\Software\OpenVPN-GUI. For details, see GitHub pull request #62 and issue #61.

I'll close this ticket as fixed. If you disagree, please reopen this.

Note: See TracTickets for help on using tickets.