Better command line in ethkey and eth by demon1999 · Pull Request #4635 · ethereum/aleth

demon1999 · 2017-10-28T15:01:54Z

Better comand line in ethkey and eth

codecov-io · 2017-10-28T15:34:42Z

Codecov Report

Merging #4635 into develop will decrease coverage by 2.43%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##           develop    #4635      +/-   ##
===========================================
- Coverage    60.88%   58.44%   -2.44%     
===========================================
  Files          343      338       -5     
  Lines        27111    25677    -1434     
  Branches      2781     3141     +360     
===========================================
- Hits         16506    15008    -1498     
+ Misses        9621     9616       -5     
- Partials       984     1053      +69

Impacted Files	Coverage Δ
eth/MinerAux.h	`0% <0%> (ø)`	⬆️
ethkey/main.cpp	`0% <0%> (ø)`	⬆️
eth/main.cpp	`0% <0%> (ø)`	⬆️
ethkey/KeyAux.h	`0% <0%> (ø)`	⬆️
test/unittests/libethereum/BlockQueue.cpp	`5.17% <0%> (-94.83%)`	⬇️
libethereum/StateImporter.h	`0% <0%> (-66.67%)`	⬇️
libethereum/VerifiedBlock.h	`7.14% <0%> (-52.86%)`	⬇️
libethereum/BlockQueue.h	`30.13% <0%> (-34.25%)`	⬇️
libethereum/SnapshotStorage.h	`0% <0%> (-25%)`	⬇️
libdevcore/TrieHash.h	`80% <0%> (-20%)`	⬇️
... and 202 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 117eeca...4b0bb1c. Read the comment docs.

gumb0 · 2017-10-30T19:06:54Z

eth/MinerAux.h

 	}

-	bool interpretOption(int& i, int argc, char** argv)
+	bool interpretOption(size_t& i, const vector<string>& argv)


const should be after type name according to our coding style, so should be vector<string> const&

gumb0 · 2017-10-30T19:08:01Z

eth/main.cpp

+#include <boost/program_options.hpp>
+#include <boost/program_options/options_description.hpp>
+
+/*


This comment block shouldn't be here

gumb0 · 2017-10-30T19:09:05Z

eth/main.cpp

+ * Ethereum client.
+ */
+
+#include <thread>


Some #includes are now duplicated in this file, remove it

gumb0 · 2017-10-30T19:10:30Z

eth/main.cpp

+	if (vm.count("client-name"))
+		clientName = vm["client-name"].as<string>();
+	if (vm.count("address"))
+		try {


braces should be on a separate line

gumb0 · 2017-10-30T19:16:25Z

ethkey/main.cpp

+			("version,V", "Show the version and exit.")
+			("help,h",  "Show this help message and exit.");
+	po::parsed_options parsed = po::command_line_parser(argc, argv).options(generalOptions).allow_unregistered().run();
+	vector<string> to_pass_further = collect_unrecognized(parsed.options, po::include_positional);


variable names should use camelCase, I'd name this unrecognisedOptions

demon1999 · 2017-10-31T05:22:59Z

fixed

gumb0

First some simple requests, mostly to fix formatting

gumb0 · 2017-11-27T16:20:35Z

eth/main.cpp

 	if (_interactive)
 		cout
-			<< "Type 'exit' to quit\n\n";
+				<< "Type 'exit' to quit\n\n";


Not sure why you changed it, but anyway better put it on the same line as cout

gumb0 · 2017-11-27T16:28:25Z

eth/main.cpp

+			("test", "Testing mode: Disable PoW and provide test rpc interface.")
+			("config", po::value<string>(), "<file> Configure specialised blockchain using given JSON information.")
+			("mode,o", po::value<string>(), "<full/peer>  Start a full node or a peer node (default: full).\n")
+			("json-rpc,j", "Enable JSON-RPC server (default: off).")


That was misleading description, let's change it to Enable the HTTP-RPC server (similar to geth)

gumb0 · 2017-11-27T16:32:01Z

eth/main.cpp

-	for (int i = 1; i < argc; ++i)
+	po::options_description clientDefaultMode("Client mode (default)");
+	clientDefaultMode.add_options()
+			("format", po::value<string>(), "<format> Set format.")


<binary/hex/human> Set the export format.

gumb0 · 2017-11-28T15:20:43Z

eth/main.cpp

-		else if (arg == "--upnp" && i + 1 < argc)
+	}
+#if ETH_EVMJIT
+	if (vm.count("vm"))


Don't indent the following line with brace (and if body)

gumb0 · 2017-11-28T15:22:12Z

eth/main.cpp

 			cerr << "provided genesis block description is not well formatted\n";
 			string genesisSample =
-			R"E(
+					R"E(


Don't change the indentation here

gumb0 · 2017-11-28T15:24:22Z

eth/main.cpp

-			rpc::NetFace, rpc::Web3Face, rpc::PersonalFace,
-			rpc::AdminEthFace, rpc::AdminNetFace, rpc::AdminUtilsFace,
-			rpc::DebugFace, rpc::TestFace
+				rpc::EthFace, rpc::WhisperFace,


No reason to change the indentation here

gumb0 · 2017-11-28T15:24:41Z

eth/main.cpp

-				adminEth, adminNet, adminUtils,
-				new rpc::Debug(*web3.ethereum()),
-				testEth
+					ethFace, new rpc::Whisper(web3, {}),


Don't change the indentation

gumb0 · 2017-11-28T15:24:47Z

eth/main.cpp

-				new rpc::AdminUtils(*sessionManager.get()),
-				new rpc::Debug(*web3.ethereum()),
-				testEth
+					ethFace, new rpc::Whisper(web3, {}), new rpc::Net(web3),


Don't change the indentation

gumb0 · 2017-11-28T15:25:05Z

ethkey/main.cpp

+	po::variables_map vm;
+	po::store(parsed, vm);
+	po::notify(vm);
+	if (vm.count("help")) {


Brace on a separate line

gumb0 · 2017-11-28T15:25:53Z

eth/main.cpp

-	for (int i = 1; i < argc; ++i)
+	po::options_description clientDefaultMode("Client mode (default)");
+	clientDefaultMode.add_options()
+			("format", po::value<string>(), "<format> Set format.")


I would prefer these lines to be one tab indented, not 2 tabs like you did ( I don't see the reason to make it unusual 2 tabs and it can lead to some additional line breaks, which makes it harder to read)

gumb0 · 2017-11-30T17:36:58Z

eth/main.cpp

+					new rpc::AdminEth(*web3.ethereum(), *gasPricer.get(), keyManager, *sessionManager.get()),
+					new rpc::AdminNet(web3, *sessionManager.get()),
+					new rpc::AdminUtils(*sessionManager.get()),
+					new rpc::Debug(*web3.ethereum()),


~~this doesn't compile, why did testeth disappear?~~ fixed

gumb0 · 2017-11-30T17:37:29Z

eth/main.cpp

 			jsonrpcIpcServer->addConnector(ipcConnector);
 			ipcConnector->StartListening();
-		}
+		}				testEth


ah, here it is, please fix this

gumb0 · 2017-11-30T17:39:27Z

I guess rlp/main.cpp shouldn't be part of this PR, as it's already in #4639 ? Remove it from here then

gumb0 · 2017-11-30T17:53:04Z

eth/main.cpp

+		Secret s(fromHex(vm["import-session-secret"].as<string>()));
+		toImport.emplace_back(s);
 	}
+	if (vm.count("help")) {


brace on a separate line

gumb0 · 2017-11-30T17:57:19Z

ethkey/main.cpp

+			("verbosity,v", po::value<int>(), "<0 - 9>  Set the log verbosity from 0 to 9 (default: 8).")
+			("version,V", "Show the version and exit.")
+			("help,h",  "Show this help message and exit.");
+	po::parsed_options parsed = po::command_line_parser(argc, argv).options(generalOptions).allow_unregistered().run();


We should catch exceptions thrown from this, because parsing options can end with errors... See like I did it in #4639

… ethkey

gumb0 · 2017-12-08T15:39:36Z

Squashed everything into one commit and rebased on develop. Tried not to loose anything.

gumb0 · 2017-12-11T10:47:20Z

ethkey/main.cpp

+		("verbosity,v", po::value<int>(), "<0 - 9>  Set the log verbosity from 0 to 9 (default: 8).")
+		("version,V", "Show the version and exit.")
+		("help,h",  "Show this help message and exit.");
+	po::parsed_options parsed = po::command_line_parser(argc, argv).options(generalOptions).allow_unregistered().run();


We should catch exceptions here, too

chfast · 2017-12-11T10:46:18Z

ethkey/main.cpp

+			<< "Options:" << endl << endl;
+		KeyCLI::streamHelp(cout);
+		cout << generalOptions;
+		exit(0);


Use return 0 instead of exit().

…stead of exit()

…boost.program_options doesn't support them. Help output improvements.

gumb0 · 2017-12-11T18:44:14Z

For anyone interested here's the summary of non-backwards compatible changes affecting working command line options:

--testnet removed; use --ropsten instead;
undocumented --kill-blockchain removed; use --kill or -K instead;
--admin was not working, but undocumented --json-admin was working instead - this is fixed, now only --admin is working;
--author removed; use --address instead;
--public removed; use --public-ip instead;
--listen-port removed; use --listen instead;
--datadir and undocumented --path removed; use --db-path or -d instead.
import (without dashes) removed, use --import instead.
export (without dashes) removed, use --export instead.
undocumented --independent removed.

…mport" and ""--export" as other regular options

chfast · 2017-12-12T18:45:13Z

eth/main.cpp

-			listenIP = argv[++i];
-			listenSet = true;
+			cerr << "Invalid argument: " << unrecognisedOptions[i] << "\n";
+			exit(-1);


Missed one exit() here.

holiman · 2017-12-13T11:04:25Z

Note to self, need to update https://github.com/karalabe/hive/blob/master/clients/cpp-ethereum:develop/eth.sh#L142

gumb0 · 2017-12-13T11:20:33Z

@holiman Also this one https://github.com/karalabe/hive/blob/master/clients/cpp-ethereum:develop/eth.sh#L161
We recently removed HTTP server from eth, now instead of --json-rpc --json-rpc-port 8545 --admin-via-http you need to run scripts/jsonrpcproxy.py

holiman · 2017-12-13T12:54:34Z

Can that be started before eth, or does eth need to create the ipc file first?

holiman · 2017-12-13T13:39:58Z

@karalabe does hive use the HTTP endpoints, or the ipc endpoints when the simulator queries the nodes?

gumb0 · 2017-12-13T13:48:43Z

@holiman I think eth should be started first

chfast · 2017-12-13T14:17:34Z

It should not matter. In case it does let me know.

gumb0 · 2017-12-13T14:28:16Z

@chfast It looks like it doesn't find ipc socket when starting it wihout runnin eth:

> scripts/jsonrpcproxy.py
Traceback (most recent call last):
  File "scripts/jsonrpcproxy.py", line 156, in <module>
    run()
  File "scripts/jsonrpcproxy.py", line 151, in run
    proxy = Proxy(args.proxy_url, args.backend_path)
  File "scripts/jsonrpcproxy.py", line 113, in __init__
    self.sock = get_ipc_socket(self.backend_address)
  File "scripts/jsonrpcproxy.py", line 69, in get_ipc_socket
    sock.connect(ipc_path)
FileNotFoundError: [Errno 2] No such file or directory

chfast · 2017-12-13T15:29:51Z

Ok. I will fix it later on.

demon1999 changed the title ~~Develop2~~ Better comand line in ethkey Oct 28, 2017

demon1999 mentioned this pull request Oct 28, 2017

Better command line argument handling #3628 #4597

Closed

gumb0 requested review from chfast and gumb0 October 28, 2017 17:18

gumb0 suggested changes Oct 30, 2017

View reviewed changes

gumb0 added the needs review label Oct 30, 2017

demon1999 changed the title ~~Better comand line in ethkey~~ Better command line in ethkey Oct 31, 2017

demon1999 changed the title ~~Better command line in ethkey~~ Better command line in ethkey and eth Oct 31, 2017

gumb0 self-requested a review November 23, 2017 16:57

gumb0 suggested changes Nov 28, 2017

View reviewed changes

gumb0 reviewed Nov 30, 2017

View reviewed changes

gumb0 force-pushed the develop2 branch from 82e7904 to 12e0928 Compare December 8, 2017 15:34

Use boost.program_options to handle command line arguments in eth and…

d255eb9

… ethkey

gumb0 force-pushed the develop2 branch from 12e0928 to d255eb9 Compare December 8, 2017 15:38

gumb0 force-pushed the develop2 branch 2 times, most recently from 9011145 to f74b3ed Compare December 8, 2017 16:01

gumb0 reviewed Dec 11, 2017

View reviewed changes

chfast suggested changes Dec 11, 2017

View reviewed changes

gumb0 added 2 commits December 11, 2017 12:23

Fix formatting

d645a78

Catch exceptions from boost.program_options; user return from main in…

48da1c8

…stead of exit()

gumb0 force-pushed the develop2 branch from f74b3ed to 48da1c8 Compare December 11, 2017 11:25

Remove some deprecated options. Remove multiletter option aliases as …

4ba824f

…boost.program_options doesn't support them. Help output improvements.

gumb0 added 3 commits December 11, 2017 19:50

Remove unused functions.

cf5a788

Remove "import" and "export" options (without dashes) and handle "--i…

68be8c2

…mport" and ""--export" as other regular options

Remove some more unused stuff

2a537ab

gumb0 approved these changes Dec 12, 2017

View reviewed changes

chfast reviewed Dec 12, 2017

View reviewed changes

Replace exit() calls with return from main.

4b0bb1c

chfast approved these changes Dec 12, 2017

View reviewed changes

gumb0 merged commit 19bf103 into ethereum:develop Dec 13, 2017

This was referenced Dec 13, 2017

Make hive use IPC connection ethereum/hive#86

Closed

Fix renaming of cpp flags ethereum/hive#87

Merged

Conversation

demon1999 commented Oct 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Oct 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

demon1999 commented Oct 31, 2017

Uh oh!

gumb0 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gumb0 Nov 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gumb0 Nov 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gumb0 commented Nov 30, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gumb0 commented Dec 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gumb0 commented Dec 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

holiman commented Dec 13, 2017

Uh oh!

gumb0 commented Dec 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

holiman commented Dec 13, 2017

Uh oh!

holiman commented Dec 13, 2017

Uh oh!

demon1999 commented Oct 28, 2017 •

edited

Loading

codecov-io commented Oct 28, 2017 •

edited

Loading

gumb0 Nov 30, 2017 •

edited

Loading

gumb0 Nov 30, 2017 •

edited

Loading

gumb0 commented Dec 8, 2017 •

edited

Loading

gumb0 commented Dec 11, 2017 •

edited

Loading

gumb0 commented Dec 13, 2017 •

edited

Loading