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 Windows installer issues #2103

Merged
merged 3 commits into from Dec 12, 2018

Conversation

Projects
None yet
2 participants
@ripcurlx
Copy link
Member

ripcurlx commented Dec 10, 2018

Fixes #2066, #2037

ripcurlx added some commits Dec 10, 2018

@ripcurlx ripcurlx requested a review from ManfredKarrer as a code owner Dec 10, 2018

@ripcurlx ripcurlx requested a review from devinbileck Dec 10, 2018

function InitializeSetup(): Boolean;
begin
// Possible future improvements:
// if version less or same => just launch app
// if upgrade => check if same app is running and wait for it to exit
// Add pack200/unpack200 support?
DeleteOldAppDataDirectory;

This comment has been minimized.

@devinbileck

devinbileck Dec 11, 2018

Member

Slight problem with putting it here. It removes the folder prior to starting the install. So if you cancel the installer, your Bisq app folder will be gone. I think it might need to be done in PrepareToInstall() method?

This comment has been minimized.

@devinbileck

devinbileck Dec 11, 2018

Member

Or you may want to take a look at using an [InstallDelete] section: http://www.jrsoftware.org/ishelp/topic_installdeletesection.htm

This comment has been minimized.

@ripcurlx

ripcurlx Dec 11, 2018

Member

Good point - I've switched to the PrepareToInstall() trigger to prevent the deletion before the installer is actually run.

@ripcurlx ripcurlx changed the title [WIP] Fix Windows installer issues Fix Windows installer issues Dec 11, 2018

@ripcurlx

This comment has been minimized.

Copy link
Member

ripcurlx commented Dec 11, 2018

@devinbileck I've created a new installer (including tor cleanup) If you want to give it a try again: https://www.dropbox.com/s/rdecl0zyla7mr3q/Bisq-0.9.0-installer-with-tor-cleanup.exe?dl=0

@ManfredKarrer ManfredKarrer removed their request for review Dec 11, 2018

DirectoryCopy(hiddenServiceDir, hiddenServiceBackupDir);
DelTree(torDir, false, true, true);
CreateDir(hiddenServiceDir);
DirectoryCopy(hiddenServiceBackupDir, hiddenServiceDir);

This comment has been minimized.

@devinbileck

devinbileck Dec 11, 2018

Member

Should we remove hiddenServiceBackupDir once we are done with it?

This comment has been minimized.

@ripcurlx

ripcurlx Dec 11, 2018

Member

I left it there by intent, just in case there is an issue and someone has to recover their hiddenservices manually.

@devinbileck
Copy link
Member

devinbileck left a comment

NACK
Tor cleanup works, but might want to remove the backup once done with it. And this time I don't see the app folder being purged.

var
entry: String;
begin
entry := ExpandConstant('{localappdata}') + '\Bisq\';

This comment has been minimized.

@devinbileck

devinbileck Dec 11, 2018

Member

Is {localappdata} correct? I don't see it purging the app folder anymore with the latest exe, like it did in the previous exe. Did you use {app} previously?

This comment has been minimized.

@ripcurlx

ripcurlx Dec 11, 2018

Member

hmm - worked for me. I didn't change anything there. I put a test file in the app folder in it was gone afterwards. I can have a look again tomorrow. How did you test it?

This comment has been minimized.

@devinbileck

devinbileck Dec 11, 2018

Member

My apologies, it is deleting it. I confirmed using process monitor.
Previously there was a noticeable delay between the purge (when done in InitializeSetup) and install, that I could see the folder being removed. But now that the purge is done in PrepareToInstall, it is not noticeable.

@devinbileck
Copy link
Member

devinbileck left a comment

ACK

@ripcurlx ripcurlx merged commit f49b405 into bisq-network:master Dec 12, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ripcurlx ripcurlx referenced this pull request Dec 30, 2018

Closed

For December 2018 #195

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