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

Fix NSIS installer continuing install before uninstall completes #1299

Merged
merged 1 commit into from Feb 9, 2017

Conversation

Vezzra
Copy link
Member

@Vezzra Vezzra commented Feb 9, 2017

This is an attempt to fix an issue with the NSIS installer, brought up on the forum here:

http://freeorion.org/forum/viewtopic.php?f=28&t=10402

The problem is, the fix proposed in this PR doesn't work completely satisfactorily, as it causes the previous version of FO not being uninstalled completely, the FreeOrion folder in the Windows programs directory and the uninstall.exe within don't get deleted (all the other contents of FreeOrion do though).

That might not be a big issue with the "uninstall before install" feature this PR tries to fix, as FO gets installed again, most likely into the same location so the left over folder and the uninstall.exe will get overwritten anyway. But e.g. in case the user decides to install to a different location, these bits of the old installation would be left behind.

Which probably is still better than the issue described in the forum thread above, but I didn't want to make assumptions, so I'm putting this up here for review. And maybe someone has a better solution.

The approach implemented by this PR has been suggested by @dbenage-cx in the thread linked above, it has one small issue though mentioned by @dbenage-cx there (referring to the filename of the uninstall executable):

The issue with this approach is separating the filename from the directory name, as the previous versions stored the one value.

Considering that the filename of the uninstall executable hasn't been changed ever, I consider this issue negligible.

@Vezzra Vezzra added category:bug The Issue/PR describes or solves a perceived malfunction within the game. component:deployment The Issue/PR deals with the deployment process or distribution packages. status:help wanted Other developers or external contributors are welcome to support developing this Issue/PR. status:work in progress The PR contains some implementation but isn't ready for merging onto the main development branch. labels Feb 9, 2017
@Vezzra Vezzra added this to the Next Release milestone Feb 9, 2017
@Vezzra Vezzra force-pushed the fix-nsis-uninstall-before-install branch from 5468136 to 0a6848f Compare February 9, 2017 16:00
@Vezzra Vezzra force-pushed the fix-nsis-uninstall-before-install branch from 0a6848f to d97fc20 Compare February 9, 2017 16:02
@Vezzra
Copy link
Member Author

Vezzra commented Feb 9, 2017

After an error with the second solution suggested on the forum thread linked in the OP has been addressed, I changed the implementation of the fix to use the approach of that second solution.

This approach, although still having a minor problem, seems simple and good enough, so I'll go ahead and merge.

@Vezzra Vezzra removed status:help wanted Other developers or external contributors are welcome to support developing this Issue/PR. status:work in progress The PR contains some implementation but isn't ready for merging onto the main development branch. labels Feb 9, 2017
@Vezzra Vezzra merged commit 55a5f7a into freeorion:master Feb 9, 2017
@Vezzra Vezzra deleted the fix-nsis-uninstall-before-install branch February 9, 2017 16:36
@Vezzra Vezzra added the status:merged All relevant commits of this PR were merged into the master development branch. label Feb 9, 2017
@Vezzra Vezzra self-assigned this Feb 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:bug The Issue/PR describes or solves a perceived malfunction within the game. component:deployment The Issue/PR deals with the deployment process or distribution packages. status:merged All relevant commits of this PR were merged into the master development branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant