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

Add an option in setup to use ProgramDataFolder for Vista and above #8

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@Goof-fr

Goof-fr commented Jul 9, 2014

Add an option to use ProgramData Folder to store configuration files and logs
It will help with Microsoft Vista and above compatibility

Add an option in setup to use ProgramDataFolder for Vista and above
Add an option to use ProgramData Folder to store configuration files and logs
It will help with Microsoft Vista and above
@mattock

This comment has been minimized.

Show comment
Hide comment
@mattock

mattock Oct 7, 2015

Member

Sorry for the delay, I didn't have notifications turned on so I missed this and many other pull requests. In general this looks fine. However, before merging this I'd need confirmation of what happens if

  1. The HKLM\Software\OpenVPN registry key is removed entirely (e.g. using regexit)
  2. OpenVPN is installed to PROGRAMDATA using this new option
  3. OpenVPN-GUI is launched as an admin

Does the first OpenVPN-GUI launch overwrite any of the registry keys your modified NSIS installer created? I'm not sure how OpenVPN-GUI behaves if some of its registry keys are present and some are missing.

Member

mattock commented Oct 7, 2015

Sorry for the delay, I didn't have notifications turned on so I missed this and many other pull requests. In general this looks fine. However, before merging this I'd need confirmation of what happens if

  1. The HKLM\Software\OpenVPN registry key is removed entirely (e.g. using regexit)
  2. OpenVPN is installed to PROGRAMDATA using this new option
  3. OpenVPN-GUI is launched as an admin

Does the first OpenVPN-GUI launch overwrite any of the registry keys your modified NSIS installer created? I'm not sure how OpenVPN-GUI behaves if some of its registry keys are present and some are missing.

@mattock

This comment has been minimized.

Show comment
Hide comment
@mattock

mattock Oct 9, 2015

Member

I had quick look at registry.c in openvpn-gui and it does not seem to overwrite existing registry keys. I will still have a look at what's happening on a real system.

Member

mattock commented Oct 9, 2015

I had quick look at registry.c in openvpn-gui and it does not seem to overwrite existing registry keys. I will still have a look at what's happening on a real system.

@Goof-fr

This comment has been minimized.

Show comment
Hide comment
@Goof-fr

Goof-fr Oct 11, 2015

This patch, does :

  • Create the directories into "ProgramData" folder for the "config" and "logs" folders
  • Create at installation the values in the registry that are loaded by OpenVPN and OpenVPN GUI to uses these directories instead of those created in the "Program files" folder.

The registry entries in "HKEY_LOCAL_MACHINE\SOFTWARE\OpenVPN" are mostly used by the OpenVPN Service when he had to look for Services .OVPN files to start automatically and for logs.

The registry entries in "HKEY_LOCAL_MACHINE\SOFTWARE\OpenVPN-GUI" are used by OpenVPN GUI to find the .OVPN configuration files and locate log files

Goof-fr commented Oct 11, 2015

This patch, does :

  • Create the directories into "ProgramData" folder for the "config" and "logs" folders
  • Create at installation the values in the registry that are loaded by OpenVPN and OpenVPN GUI to uses these directories instead of those created in the "Program files" folder.

The registry entries in "HKEY_LOCAL_MACHINE\SOFTWARE\OpenVPN" are mostly used by the OpenVPN Service when he had to look for Services .OVPN files to start automatically and for logs.

The registry entries in "HKEY_LOCAL_MACHINE\SOFTWARE\OpenVPN-GUI" are used by OpenVPN GUI to find the .OVPN configuration files and locate log files

Goof-fr added some commits Oct 12, 2015

Better integration of the changed option
Add the registry modifications in each sections
 - "SecOpenVPNGUI" for OpenVPN GUI registry entries
 - "SecService" for OpenVPN service registry entries
 - "SecProgData" for the creation of the directories.

Use of the "WriteRegStringIfUndef" macro to avoid configuration change on updates

The start menu shortcuts now lead to the right directories
Use ${PACKAGE_NAME} instead of "OpenVPN" in some directory strings. L…
…ocate README.txt files in the right dorectory

Use $APPDATA\${PACKAGE_NAME} instead of $APPDATA\OpenVPN

Create the file README.txt in the right folder depending on the configuration selected.
@mattock

This comment has been minimized.

Show comment
Hide comment
@mattock

mattock Nov 17, 2015

Member

I built experimental installers based on your latest patches:

I tested the latter installer on a Windows 8.1 64-bit VM and immediately ran into issues. It does not seem to matter whether SecProgData is checked or not: the registry keys will always end up pointing to the ProgramData folder. Can you reproduce this behavior yourself?

Besides this the code itself looks good. That said, would it be possible to create a prefix variable based on checked/unchecked value of the SecProgData, and use that to build the correct paths? This would make code slightly cleaner.

Member

mattock commented Nov 17, 2015

I built experimental installers based on your latest patches:

I tested the latter installer on a Windows 8.1 64-bit VM and immediately ran into issues. It does not seem to matter whether SecProgData is checked or not: the registry keys will always end up pointing to the ProgramData folder. Can you reproduce this behavior yourself?

Besides this the code itself looks good. That said, would it be possible to create a prefix variable based on checked/unchecked value of the SecProgData, and use that to build the correct paths? This would make code slightly cleaner.

@mattock

This comment has been minimized.

Show comment
Hide comment
@mattock

mattock Jan 11, 2016

Member

Any comments on the issues I encountered? If not, I'll close this pull request.

Member

mattock commented Jan 11, 2016

Any comments on the issues I encountered? If not, I'll close this pull request.

@mattock mattock closed this Jan 27, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment