From c63452e25d1def4371a0f619967c423e44a87ace Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Tue, 10 Jul 2018 03:36:04 +0800 Subject: [PATCH] Clean up deprecated options and add CHECK macro (#9036) * CHECK macro to replace hard-coded checks * Clean up deprecated options * typo: FlAG -> FLAG * Fix tests * Deprecated non-used ui params default is None * test: arg_ui_path is not deprecated * arg_ui_port should be None * Address grumbles * Fix tests --- parity/cli/mod.rs | 176 +++++++++++++++++++++++++------------------ parity/cli/usage.rs | 23 +++--- parity/deprecated.rs | 115 +++++++++++++++++++++++++--- 3 files changed, 220 insertions(+), 94 deletions(-) diff --git a/parity/cli/mod.rs b/parity/cli/mod.rs index 1f94deb3895..b67c728e5d6 100644 --- a/parity/cli/mod.rs +++ b/parity/cli/mod.rs @@ -449,6 +449,17 @@ usage! { "--reserved-peers=[FILE]", "Provide a file containing enodes, one per line. These nodes will always have a reserved slot on top of the normal maximum peers.", + CHECK |args: &Args| { + if let (Some(max_peers), Some(min_peers)) = (args.arg_max_peers, args.arg_min_peers) { + if min_peers > max_peers { + return Err(ArgsError::PeerConfiguration); + } + } + + Ok(()) + }, + + ["API and Console Options – HTTP JSON-RPC"] FLAG flag_no_jsonrpc: (bool) = false, or |c: &Config| c.rpc.as_ref()?.disable.clone(), "--no-jsonrpc", @@ -879,57 +890,66 @@ usage! { "Target size of the whisper message pool in megabytes.", ["Legacy Options"] - FLAG flag_warp: (bool) = false, or |_| None, - "--warp", - "Does nothing; warp sync is enabled by default. Use --no-warp to disable.", - - FLAG flag_dapps_apis_all: (bool) = false, or |_| None, - "--dapps-apis-all", - "Dapps server is merged with HTTP JSON-RPC server. Use --jsonrpc-apis.", + // Options that are hidden from config, but are still unique for its functionality. FLAG flag_geth: (bool) = false, or |_| None, "--geth", "Run in Geth-compatibility mode. Sets the IPC path to be the same as Geth's. Overrides the --ipc-path and --ipcpath options. Alters RPCs to reflect Geth bugs. Includes the personal_ RPC by default.", - FLAG flag_testnet: (bool) = false, or |_| None, - "--testnet", - "Testnet mode. Equivalent to --chain testnet. Overrides the --keys-path option.", - FLAG flag_import_geth_keys: (bool) = false, or |_| None, "--import-geth-keys", "Attempt to import keys from Geth client.", - FLAG flag_ipcdisable: (bool) = false, or |_| None, - "--ipcdisable", - "Equivalent to --no-ipc.", - - FLAG flag_ipc_off: (bool) = false, or |_| None, - "--ipc-off", - "Equivalent to --no-ipc.", + // Options that either do nothing, or are replaced by other options. + // FLAG Removed in 1.6 or before. - FLAG flag_nodiscover: (bool) = false, or |_| None, - "--nodiscover", - "Equivalent to --no-discovery.", + FLAG flag_warp: (bool) = false, or |_| None, + "--warp", + "Does nothing; warp sync is enabled by default. Use --no-warp to disable.", FLAG flag_jsonrpc: (bool) = false, or |_| None, "-j, --jsonrpc", "Does nothing; HTTP JSON-RPC is on by default now.", + FLAG flag_rpc: (bool) = false, or |_| None, + "--rpc", + "Does nothing; HTTP JSON-RPC is on by default now.", + FLAG flag_jsonrpc_off: (bool) = false, or |_| None, "--jsonrpc-off", "Equivalent to --no-jsonrpc.", FLAG flag_webapp: (bool) = false, or |_| None, "-w, --webapp", - "Does nothing; dapps server is on by default now.", + "Does nothing; dapps server has been removed.", FLAG flag_dapps_off: (bool) = false, or |_| None, "--dapps-off", "Equivalent to --no-dapps.", - FLAG flag_rpc: (bool) = false, or |_| None, - "--rpc", - "Does nothing; HTTP JSON-RPC is on by default now.", + FLAG flag_ipcdisable: (bool) = false, or |_| None, + "--ipcdisable", + "Equivalent to --no-ipc.", + + FLAG flag_ipc_off: (bool) = false, or |_| None, + "--ipc-off", + "Equivalent to --no-ipc.", + + FLAG flag_testnet: (bool) = false, or |_| None, + "--testnet", + "Testnet mode. Equivalent to --chain testnet. Overrides the --keys-path option.", + + FLAG flag_nodiscover: (bool) = false, or |_| None, + "--nodiscover", + "Equivalent to --no-discovery.", + + // FLAG Removed in 1.7. + + FLAG flag_dapps_apis_all: (bool) = false, or |_| None, + "--dapps-apis-all", + "Dapps server is merged with HTTP JSON-RPC server. Use --jsonrpc-apis.", + + // FLAG Removed in 1.11. FLAG flag_public_node: (bool) = false, or |_| None, "--public-node", @@ -947,41 +967,15 @@ usage! { "--ui-no-validation", "Does nothing; UI is now a separate project.", - ARG arg_ui_interface: (String) = "local", or |_| None, - "--ui-interface=[IP]", - "Does nothing; UI is now a separate project.", - - ARG arg_ui_hosts: (String) = "none", or |_| None, - "--ui-hosts=[HOSTS]", - "Does nothing; UI is now a separate project.", - - ARG arg_ui_port: (u16) = 8180u16, or |_| None, - "--ui-port=[PORT]", - "Does nothing; UI is now a separate project.", - - ARG arg_dapps_port: (Option) = None, or |c: &Config| c.dapps.as_ref()?.port.clone(), - "--dapps-port=[PORT]", - "Dapps server is merged with HTTP JSON-RPC server. Use --jsonrpc-port.", - - ARG arg_dapps_interface: (Option) = None, or |c: &Config| c.dapps.as_ref()?.interface.clone(), - "--dapps-interface=[IP]", - "Dapps server is merged with HTTP JSON-RPC server. Use --jsonrpc-interface.", - - ARG arg_dapps_hosts: (Option) = None, or |c: &Config| c.dapps.as_ref()?.hosts.as_ref().map(|vec| vec.join(",")), - "--dapps-hosts=[HOSTS]", - "Dapps server is merged with HTTP JSON-RPC server. Use --jsonrpc-hosts.", - - ARG arg_dapps_cors: (Option) = None, or |c: &Config| c.dapps.as_ref()?.cors.clone(), - "--dapps-cors=[URL]", - "Dapps server is merged with HTTP JSON-RPC server. Use --jsonrpc-cors.", + // ARG Removed in 1.6 or before. - ARG arg_dapps_user: (Option) = None, or |c: &Config| c.dapps.as_ref()?.user.clone(), - "--dapps-user=[USERNAME]", - "Dapps server authentication has been removed.", + ARG arg_etherbase: (Option) = None, or |_| None, + "--etherbase=[ADDRESS]", + "Equivalent to --author ADDRESS.", - ARG arg_dapps_pass: (Option) = None, or |c: &Config| c.dapps.as_ref()?.pass.clone(), - "--dapps-pass=[PASSWORD]", - "Dapps server authentication has been removed.", + ARG arg_extradata: (Option) = None, or |_| None, + "--extradata=[STRING]", + "Equivalent to --extra-data STRING.", ARG arg_datadir: (Option) = None, or |_| None, "--datadir=[PATH]", @@ -1027,23 +1021,55 @@ usage! { "--gasprice=[WEI]", "Equivalent to --min-gas-price WEI.", - ARG arg_etherbase: (Option) = None, or |_| None, - "--etherbase=[ADDRESS]", - "Equivalent to --author ADDRESS.", - - ARG arg_extradata: (Option) = None, or |_| None, - "--extradata=[STRING]", - "Equivalent to --extra-data STRING.", - ARG arg_cache: (Option) = None, or |_| None, "--cache=[MB]", "Equivalent to --cache-size MB.", - ARG arg_tx_queue_ban_count: (u16) = 1u16, or |c: &Config| c.mining.as_ref()?.tx_queue_ban_count.clone(), + // ARG Removed in 1.7. + + ARG arg_dapps_port: (Option) = None, or |c: &Config| c.dapps.as_ref()?.port.clone(), + "--dapps-port=[PORT]", + "Does nothing; dapps server has been removed.", + + ARG arg_dapps_interface: (Option) = None, or |c: &Config| c.dapps.as_ref()?.interface.clone(), + "--dapps-interface=[IP]", + "Does nothing; dapps server has been removed.", + + ARG arg_dapps_hosts: (Option) = None, or |c: &Config| c.dapps.as_ref()?.hosts.as_ref().map(|vec| vec.join(",")), + "--dapps-hosts=[HOSTS]", + "Does nothing; dapps server has been removed.", + + ARG arg_dapps_cors: (Option) = None, or |c: &Config| c.dapps.as_ref()?.cors.clone(), + "--dapps-cors=[URL]", + "Does nothing; dapps server has been removed.", + + ARG arg_dapps_user: (Option) = None, or |c: &Config| c.dapps.as_ref()?.user.clone(), + "--dapps-user=[USERNAME]", + "Dapps server authentication has been removed.", + + ARG arg_dapps_pass: (Option) = None, or |c: &Config| c.dapps.as_ref()?.pass.clone(), + "--dapps-pass=[PASSWORD]", + "Dapps server authentication has been removed.", + + // ARG removed in 1.11. + + ARG arg_ui_interface: (Option) = None, or |_| None, + "--ui-interface=[IP]", + "Does nothing; UI is now a separate project.", + + ARG arg_ui_hosts: (Option) = None, or |_| None, + "--ui-hosts=[HOSTS]", + "Does nothing; UI is now a separate project.", + + ARG arg_ui_port: (Option) = None, or |_| None, + "--ui-port=[PORT]", + "Does nothing; UI is now a separate project.", + + ARG arg_tx_queue_ban_count: (Option) = None, or |c: &Config| c.mining.as_ref()?.tx_queue_ban_count.clone(), "--tx-queue-ban-count=[C]", "Not supported.", - ARG arg_tx_queue_ban_time: (u16) = 180u16, or |c: &Config| c.mining.as_ref()?.tx_queue_ban_time.clone(), + ARG arg_tx_queue_ban_time: (Option) = None, or |c: &Config| c.mining.as_ref()?.tx_queue_ban_time.clone(), "--tx-queue-ban-time=[SEC]", "Not supported.", } @@ -1427,11 +1453,11 @@ mod tests { let args = Args::parse(&["parity", "--password", "~/.safe/1", "--password", "~/.safe/2", "--ui-port", "8123"]).unwrap(); assert_eq!(args.arg_password, vec!["~/.safe/1".to_owned(), "~/.safe/2".to_owned()]); - assert_eq!(args.arg_ui_port, 8123); + assert_eq!(args.arg_ui_port, Some(8123)); let args = Args::parse(&["parity", "--password", "~/.safe/1,~/.safe/2", "--ui-port", "8123"]).unwrap(); assert_eq!(args.arg_password, vec!["~/.safe/1".to_owned(), "~/.safe/2".to_owned()]); - assert_eq!(args.arg_ui_port, 8123); + assert_eq!(args.arg_ui_port, Some(8123)); } #[test] @@ -1583,9 +1609,9 @@ mod tests { flag_force_ui: false, flag_no_ui: false, - arg_ui_port: 8180u16, - arg_ui_interface: "local".into(), - arg_ui_hosts: "none".into(), + arg_ui_port: None, + arg_ui_interface: None, + arg_ui_hosts: None, arg_ui_path: "$HOME/.parity/signer".into(), flag_ui_no_validation: false, @@ -1692,8 +1718,8 @@ mod tests { arg_tx_queue_mem_limit: 4u32, arg_tx_queue_gas: "off".into(), arg_tx_queue_strategy: "gas_factor".into(), - arg_tx_queue_ban_count: 1u16, - arg_tx_queue_ban_time: 180u16, + arg_tx_queue_ban_count: Some(1u16), + arg_tx_queue_ban_time: Some(180u16), flag_remove_solved: false, arg_notify_work: Some("http://localhost:3001".into()), flag_refuse_service_transactions: false, diff --git a/parity/cli/usage.rs b/parity/cli/usage.rs index 704aa5d6f1d..caacd364f2c 100644 --- a/parity/cli/usage.rs +++ b/parity/cli/usage.rs @@ -135,6 +135,9 @@ macro_rules! usage { $( ARG $arg:ident : ($($arg_type_tt:tt)+) = $arg_default:expr, or $arg_from_config:expr, $arg_usage:expr, $arg_help:expr, )* + $( + CHECK $check:expr, + )* )* } ) => { @@ -318,13 +321,6 @@ macro_rules! usage { pub fn parse>(command: &[S]) -> Result { let raw_args = RawArgs::parse(command)?; - if let (Some(max_peers), Some(min_peers)) = (raw_args.arg_max_peers, raw_args.arg_min_peers) { - // Invalid configuration pattern `mix_peers` > `max_peers` - if min_peers > max_peers { - return Err(ArgsError::PeerConfiguration); - } - } - // Skip loading config file if no_config flag is specified if raw_args.flag_no_config { return Ok(raw_args.into_args(Config::default())); @@ -332,7 +328,8 @@ macro_rules! usage { let config_file = raw_args.arg_config.clone().unwrap_or_else(|| raw_args.clone().into_args(Config::default()).arg_config); let config_file = replace_home(&::dir::default_data_path(), &config_file); - match (fs::File::open(&config_file), raw_args.arg_config.clone()) { + + let args = match (fs::File::open(&config_file), raw_args.arg_config.clone()) { // Load config file (Ok(mut file), _) => { println_stderr!("Loading config file from {}", &config_file); @@ -349,7 +346,15 @@ macro_rules! usage { Err(e) => Err(ArgsError::Config(config_file, e)) } }, - } + }?; + + $( + $( + $check(&args)?; + )* + )* + + Ok(args) } #[cfg(test)] diff --git a/parity/deprecated.rs b/parity/deprecated.rs index f3e433d1389..24bd1638b1a 100644 --- a/parity/deprecated.rs +++ b/parity/deprecated.rs @@ -37,6 +37,8 @@ impl fmt::Display for Deprecated { pub fn find_deprecated(args: &Args) -> Vec { let mut result = vec![]; + // Removed in 1.6 or before. + if args.flag_warp { result.push(Deprecated::DoesNothing("--warp")); } @@ -77,21 +79,78 @@ pub fn find_deprecated(args: &Args) -> Vec { result.push(Deprecated::Replaced("--extradata", "--extra-data")); } - // Removed in 1.7 + if args.flag_testnet { + result.push(Deprecated::Replaced("--testnet", "--chain testnet")); + } + + if args.flag_nodiscover { + result.push(Deprecated::Replaced("--nodiscover", "--no-discovery")); + } + + if args.arg_datadir.is_some() { + result.push(Deprecated::Replaced("--datadir", "--base-path")); + } + + if args.arg_networkid.is_some() { + result.push(Deprecated::Replaced("--networkid", "--network-id")); + } + + if args.arg_peers.is_some() { + result.push(Deprecated::Replaced("--peers", "--min-peers")); + } + + if args.arg_nodekey.is_some() { + result.push(Deprecated::Replaced("--nodekey", "--node-key")); + } + + if args.arg_rpcaddr.is_some() { + result.push(Deprecated::Replaced("--rpcaddr", "--jsonrpc-interface")); + } + + if args.arg_rpcport.is_some() { + result.push(Deprecated::Replaced("--rpcport", "--jsonrpc-port")); + } + + if args.arg_rpcapi.is_some() { + result.push(Deprecated::Replaced("--rpcapi", "--jsonrpc-api")); + } + + if args.arg_rpccorsdomain.is_some() { + result.push(Deprecated::Replaced("--rpccorsdomain", "--jsonrpc-cors")); + } + + if args.arg_ipcapi.is_some() { + result.push(Deprecated::Replaced("--ipcapi", "--ipc-apis")); + } + + if args.arg_ipcpath.is_some() { + result.push(Deprecated::Replaced("--ipcpath", "--ipc-path")); + } + + if args.arg_gasprice.is_some() { + result.push(Deprecated::Replaced("--gasprice", "--min-gas-price")); + } + + if args.arg_cache.is_some() { + result.push(Deprecated::Replaced("--cache", "--cache-size")); + } + + // Removed in 1.7. + if args.arg_dapps_port.is_some() { - result.push(Deprecated::Replaced("--dapps-port", "--jsonrpc-port")); + result.push(Deprecated::Removed("--dapps-port")); } if args.arg_dapps_interface.is_some() { - result.push(Deprecated::Replaced("--dapps-interface", "--jsonrpc-interface")); + result.push(Deprecated::Removed("--dapps-interface")); } if args.arg_dapps_hosts.is_some() { - result.push(Deprecated::Replaced("--dapps-hosts", "--jsonrpc-hosts")); + result.push(Deprecated::Removed("--dapps-hosts")); } if args.arg_dapps_cors.is_some() { - result.push(Deprecated::Replaced("--dapps-cors", "--jsonrpc-cors")); + result.push(Deprecated::Removed("--dapps-cors")); } if args.arg_dapps_user.is_some() { @@ -106,7 +165,43 @@ pub fn find_deprecated(args: &Args) -> Vec { result.push(Deprecated::Replaced("--dapps-apis-all", "--jsonrpc-apis")); } - // Removed in 1.8 + // Removed in 1.11. + + if args.flag_public_node { + result.push(Deprecated::Removed("--public-node")); + } + + if args.flag_force_ui { + result.push(Deprecated::Removed("--force-ui")); + } + + if args.flag_no_ui { + result.push(Deprecated::Removed("--no-ui")); + } + + if args.flag_ui_no_validation { + result.push(Deprecated::Removed("--ui-no-validation")); + } + + if args.arg_ui_interface.is_some() { + result.push(Deprecated::Removed("--ui-interface")); + } + + if args.arg_ui_hosts.is_some() { + result.push(Deprecated::Removed("--ui-hosts")); + } + + if args.arg_ui_port.is_some() { + result.push(Deprecated::Removed("--ui-port")); + } + + if args.arg_tx_queue_ban_count.is_some() { + result.push(Deprecated::Removed("--tx-queue-ban-count")); + } + + if args.arg_tx_queue_ban_time.is_some() { + result.push(Deprecated::Removed("--tx-queue-ban-time")); + } result } @@ -150,10 +245,10 @@ mod tests { Deprecated::Replaced("--ipc-off", "--no-ipc"), Deprecated::Replaced("--etherbase", "--author"), Deprecated::Replaced("--extradata", "--extra-data"), - Deprecated::Replaced("--dapps-port", "--jsonrpc-port"), - Deprecated::Replaced("--dapps-interface", "--jsonrpc-interface"), - Deprecated::Replaced("--dapps-hosts", "--jsonrpc-hosts"), - Deprecated::Replaced("--dapps-cors", "--jsonrpc-cors"), + Deprecated::Removed("--dapps-port"), + Deprecated::Removed("--dapps-interface"), + Deprecated::Removed("--dapps-hosts"), + Deprecated::Removed("--dapps-cors"), Deprecated::Removed("--dapps-user"), Deprecated::Removed("--dapps-pass"), Deprecated::Replaced("--dapps-apis-all", "--jsonrpc-apis"),