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

Allow to specify --datadir #1368

Merged
merged 3 commits into from May 1, 2019

Conversation

Projects
None yet
2 participants
@lontivero
Copy link
Contributor

commented Apr 25, 2019

Disclaimer: This PR is for validating the idea and the code was not tested enough.
Related to: #763


This allow us to specify --datadir for all the options (gui, deamon, password finder). For example, to lunch the Wasabi GUI client and use the /home/007/topsecret folder we use:

$ wassabee --datadir:/home/007/topsecret

To lunch the daemon:

$ wassabee mix --datadir:/home/007/topsecret --wallet:chaos --keepalive --mixall

And so on.

@@ -450,10 +450,6 @@ public async Task<int> RunAsync(IEnumerable<string> arguments)
{
return await Help.InvokeAsync(extra);
}
if (arguments.All(x => !x.Contains("version")))

This comment has been minimized.

Copy link
@nopara73

nopara73 Apr 25, 2019

Collaborator

What's this change?

This comment has been minimized.

Copy link
@lontivero

lontivero Apr 25, 2019

Author Contributor

This change removes a hack used to handle an especial case. In this PR --version is not the only available parameter because we are adding --datadir too. I have to see how Mono.Options handles this shared options.

This comment has been minimized.

Copy link
@nopara73

nopara73 Apr 26, 2019

Collaborator

I'm not worried about improperly working software when arguments are given inconsistently. I am worried about improperly working software when the arguments are correct. Get back to this issue when you checked that.

@@ -56,7 +58,7 @@ public static async Task<bool> ExecuteCommandsAsync(string[] args)
return false;
}

return false;

This comment has been minimized.

Copy link
@nopara73

nopara73 Apr 25, 2019

Collaborator

This is not good. Please don't modify the business logic around here. Last time you did this I had to spend days to fix everything up.

This comment has been minimized.

Copy link
@lontivero

lontivero Apr 25, 2019

Author Contributor

Currently (before this PR) the logic is "if no arguments are passed then run GUI", but that changes because now even the gui accepts the new --datadir option. I don't know if this change is correct or not, but the logic (code) has to change somewhere.

This comment has been minimized.

Copy link
@nopara73

nopara73 Apr 26, 2019

Collaborator

I don't know if this change is correct or not, but the logic (code) has to change somewhere.

I don't know either, I'm just saying it's dangerous, but I won't be able to get away with a code review here and I have to test this PR carefully, how it acts on different platforms from code and from the released binaries.

This comment has been minimized.

Copy link
@lontivero

lontivero Apr 27, 2019

Author Contributor

I tested it on Linux and Windows. Here are the outputs:

$ wassabee --help
Usage: wassabee [OPTIONS]+
Launches Wasabi Wallet.

  -v, --version              Displays Wasabi version and exit.
  -d, --datadir=VALUE        Directory path where store all the Wasabi data.

Available commands are:

        mix                  Start mixing without the GUI with the specified wallet.
        findpassword         Try to find typos in provided password.
$ wassabee help
Usage: wassabee [OPTIONS]+
Launches Wasabi Wallet.

  -v, --version              Displays Wasabi version and exit.
  -d, --datadir=VALUE        Directory path where store all the Wasabi data.

Available commands are:

        mix                  Start mixing without the GUI with the specified wallet.
        findpassword         Try to find typos in provided password.
$ wassabee --version
Wasabi Client Version: 1.1.4
Compatible Coordinator Version: 3
$ wassabee --unknown
wassabee: Unknown command: --unknown
wassabee: Use `wassabee help` for usage.
$ wassabee unknown
wassabee: Unknown command: unknown
wassabee: Use `wassabee help` for usage.
$ wassabee --mix
2019-04-27 17:08:25 INFO 5d154f8b-53ef-4d85-826c-0875617462b2: Wasabi Daemon is starting...
2019-04-27 17:08:25 CRITICAL Daemon: Wallet was not supplied. Add --wallet:WalletName
2019-04-27 17:08:25 INFO 5d154f8b-53ef-4d85-826c-0875617462b2: Wasabi Daemon stopped gracefully.
$ wassabee --mix --datadir:/home/lontivero/topsecret
2019-04-27 17:08:35 INFO 1cae7037-281b-4d4c-b3d9-493d7c0d134b: Wasabi Daemon is starting...
2019-04-27 17:08:38 CRITICAL Daemon: Wallet was not supplied. Add --wallet:WalletName
2019-04-27 17:08:38 INFO 1cae7037-281b-4d4c-b3d9-493d7c0d134b: Wasabi Daemon stopped gracefully.

It also works when all the arguments are correct on both: GUI and Daemon.
Same results on Windows.

@nopara73

This comment has been minimized.

Copy link
Collaborator

commented Apr 25, 2019

concept ACK

@nopara73

This comment has been minimized.

Copy link
Collaborator

commented Apr 26, 2019

I guess I won't be able to get away with just a code review here and testing this PR carefully. I'll wait until it becomes a non-draft.

@lontivero lontivero marked this pull request as ready for review Apr 27, 2019

@nopara73 nopara73 merged commit 033df1a into zkSNACKs:master May 1, 2019

1 check passed

CodeFactor No issues found.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.