Skip to content

Commit

Permalink
Update clap to 4-series (and async-recursion, async-stream, serde, op…
Browse files Browse the repository at this point in the history
…enssl-macros, syn) (#8974)

This is something we need to upgrade the toolchain. Largely the update was uneventful, but it also pulls in other new crates, such as syn 2.0, which leads to many duplicated crates while the ecosystem is moving over to the new version.

One observation is that with the new API it is somewhat easy to get things wrong when using the manual clap API, rather than the derive version. Now that it stores the exact type it parsed, you can no longer rely on clap converting stuff for you. You are kind-of required to get types right for yourself, and any mismatch will result in a runtime panic, which is iffy.
  • Loading branch information
nagisa committed Apr 27, 2023
1 parent cff3fab commit 1183b61
Show file tree
Hide file tree
Showing 18 changed files with 430 additions and 208 deletions.
461 changes: 332 additions & 129 deletions Cargo.lock

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ anyhow = "1.0.62"
arbitrary = { version = "1.2.3", features = ["derive"] }
arc-swap = "1.5"
assert_matches = "1.5.0"
async-recursion = "0.3.2"
async-recursion = "1.0.4"
async-trait = "0.1.58"
atty = "0.2"
awc = { version = "3", features = ["openssl"] }
Expand All @@ -110,7 +110,7 @@ cargo_metadata = "0.14.1"
cc = "1.0"
cfg-if = "1.0"
chrono = { version = "0.4.19", features = ["serde"] }
clap = { version = "3.1.6", features = ["derive", "env"] }
clap = { version = "4.2.0", features = ["derive", "env", "string"] }
conqueue = "0.4.0"
cpu-time = "1.0"
criterion = { version = "0.3.5", default_features = false, features = ["html_reports", "cargo_bench_support"] }
Expand Down Expand Up @@ -277,7 +277,7 @@ smartstring = "1.0.1"
strum = { version = "0.24", features = ["derive"] }
stun = "0.4"
subtle = "2.2"
syn = { version = "1.0.54", features = ["extra-traits", "full"] }
syn = { version = "2.0.4", features = ["extra-traits", "full"] }
sysinfo = "0.24.5"
tar = "0.4.38"
target-lexicon = { version = "0.12.2", default-features = false }
Expand Down
8 changes: 4 additions & 4 deletions core/o11y/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ pub struct DefaultSubscriberGuard<S> {
}

// Doesn't define WARN and ERROR, because the highest verbosity of spans is INFO.
#[derive(Copy, Clone, Debug, Default, clap::ArgEnum, serde::Serialize, serde::Deserialize)]
#[derive(Copy, Clone, Debug, Default, clap::ValueEnum, serde::Serialize, serde::Deserialize)]
pub enum OpenTelemetryLevel {
#[default]
OFF,
Expand All @@ -124,11 +124,11 @@ pub enum OpenTelemetryLevel {
#[derive(Debug, Default, clap::Parser)]
pub struct Options {
/// Enables export of span data using opentelemetry exporters.
#[clap(long, arg_enum, default_value = "off")]
#[clap(long, value_enum, default_value = "off")]
opentelemetry: OpenTelemetryLevel,

/// Whether the log needs to be colored.
#[clap(long, arg_enum, default_value = "auto")]
#[clap(long, value_enum, default_value = "auto")]
color: ColorOutput,

/// Enable logging of spans. For instance, this prints timestamps of entering and exiting a span,
Expand Down Expand Up @@ -171,7 +171,7 @@ impl<S: tracing::Subscriber + Send + Sync> DefaultSubscriberGuard<S> {
/// Whether to use colored log format.
/// Option `Auto` enables color output only if the logging is done to a terminal and
/// `NO_COLOR` environment variable is not set.
#[derive(clap::ArgEnum, Debug, Clone, Default)]
#[derive(clap::ValueEnum, Debug, Clone, Default)]
pub enum ColorOutput {
#[default]
Always,
Expand Down
11 changes: 11 additions & 0 deletions deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,21 @@ skip = [
# wasmer 0.18
{ name = "nix", version = "=0.15.0" },

# many crates haven't updated to syn 2.0 yet.
{ name = "syn", version = "=1.0.103" },

# old version of tokio, parking_lot
{ name = "windows-sys", version = "=0.36.1" },
{ name = "windows_x86_64_msvc", version = "=0.36.1" },

# old version of rustix, wasmtime, is-terminal, etc.
{ name = "rustix", version = "0.36.6" },
{ name = "linux-raw-sys", version = "0.1.4" },
{ name = "windows-sys", version = "=0.42.0" },
{ name = "windows-sys", version = "=0.45.0" },
{ name = "windows_x86_64_msvc", version = "=0.42.1" },
{ name = "windows-targets", version = "=0.42.1" },

# chrono uses old time crate
{ name = "time", version = "=0.1.44" },

Expand Down
18 changes: 9 additions & 9 deletions genesis-tools/genesis-csv-to-json/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,35 @@ use clap::{Arg, Command};
use near_primitives::types::ShardId;
use nearcore::get_default_home;
use std::collections::HashSet;
use std::path::Path;
use std::path::PathBuf;

pub mod csv_parser;
pub mod csv_to_json_configs;
pub mod serde_with;

fn main() {
let default_home = get_default_home();
let matches = Command::new("Genesis CSV to JSON.")
.about("Converts accounts listed in CSV format to JSON configs")
.arg(
Arg::new("home")
.long("home")
.default_value_os(default_home.as_os_str())
.default_value(get_default_home().into_os_string())
.value_parser(clap::value_parser!(PathBuf))
.help("Directory for config and data (default \"~/.near\")")
.takes_value(true),
.action(clap::ArgAction::Set),
)
.arg(Arg::new("chain-id").long("chain-id").takes_value(true))
.arg(Arg::new("chain-id").long("chain-id").action(clap::ArgAction::Set))
.arg(
Arg::new("tracked-shards")
.long("tracked-shards")
.takes_value(true)
.action(clap::ArgAction::Set)
.help("Set of shards that this node wants to track (default empty)"),
)
.get_matches();

let home_dir = matches.value_of("home").map(|dir| Path::new(dir)).unwrap();
let chain_id = matches.value_of("chain-id").expect("Chain id is requried");
let tracked_shards: HashSet<ShardId> = match matches.value_of("tracked-shards") {
let home_dir = matches.get_one::<PathBuf>("home").unwrap();
let chain_id = matches.get_one::<String>("chain-id").expect("Chain id is requried");
let tracked_shards: HashSet<ShardId> = match matches.get_one::<String>("tracked-shards") {
Some(s) => {
if s.is_empty() {
HashSet::default()
Expand Down
23 changes: 16 additions & 7 deletions genesis-tools/genesis-populate/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,33 @@ use clap::{Arg, Command};
use genesis_populate::GenesisBuilder;
use near_chain_configs::GenesisValidationMode;
use nearcore::{get_default_home, load_config};
use std::path::Path;
use std::path::PathBuf;

fn main() {
let default_home = get_default_home();
let matches = Command::new("Genesis populator")
.arg(
Arg::new("home")
.long("home")
.default_value_os(default_home.as_os_str())
.default_value(get_default_home().into_os_string())
.value_parser(clap::value_parser!(PathBuf))
.help("Directory for config and data (default \"~/.near\")")
.takes_value(true),
.action(clap::ArgAction::Set),
)
.arg(
Arg::new("additional-accounts-num")
.long("additional-accounts-num")
.required(true)
.action(clap::ArgAction::Set)
.help(
"Number of additional accounts per shard to add directly to the trie \
(TESTING ONLY)",
),
)
.arg(Arg::new("additional-accounts-num").long("additional-accounts-num").required(true).takes_value(true).help("Number of additional accounts per shard to add directly to the trie (TESTING ONLY)"))
.get_matches();

let home_dir = matches.value_of("home").map(|dir| Path::new(dir)).unwrap();
let home_dir = matches.get_one::<PathBuf>("home").unwrap();
let additional_accounts_num = matches
.value_of("additional-accounts-num")
.get_one::<String>("additional-accounts-num")
.map(|x| x.parse::<u64>().expect("Failed to parse number of additional accounts."))
.unwrap();
let near_config = load_config(home_dir, GenesisValidationMode::Full)
Expand Down
22 changes: 11 additions & 11 deletions genesis-tools/keypair-generator/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::fs;
use std::path::{Path, PathBuf};
use std::path::PathBuf;

use clap::{Arg, Command};

Expand All @@ -12,34 +12,34 @@ fn generate_key_to_file(account_id: &str, key: SecretKey, path: &PathBuf) -> std
}

fn main() {
let default_home = get_default_home();
let matches = Command::new("Key-pairs generator")
.subcommand_required(true)
.arg_required_else_help(true)
.about("Generates: access key-pairs, validation key-pairs, network key-pairs")
.arg(
Arg::new("home")
.long("home")
.default_value_os(default_home.as_os_str())
.default_value(get_default_home().into_os_string())
.value_parser(clap::value_parser!(PathBuf))
.help("Directory for config and data (default \"~/.near\")")
.takes_value(true),
.action(clap::ArgAction::Set)
)
.arg(
Arg::new("account-id")
.long("account-id")
.takes_value(true),
.action(clap::ArgAction::Set)
)
.arg(
Arg::new("generate-config")
.long("generate-config")
.help("Whether to generate a config file when generating keys. Requires account-id to be specified.")
.takes_value(false),
.action(clap::ArgAction::SetTrue),
)
.subcommand(
Command::new("signer-keys").about("Generate signer keys.").arg(
Arg::new("num-keys")
.long("num-keys")
.takes_value(true)
.action(clap::ArgAction::Set)
.help("Number of signer keys to generate. (default 3)"),
),
)
Expand All @@ -49,15 +49,15 @@ fn main() {
.subcommand(Command::new("validator-key").about("Generate staking key."))
.get_matches();

let home_dir = matches.value_of("home").map(|dir| Path::new(dir)).unwrap();
let home_dir = matches.get_one::<PathBuf>("home").unwrap();
fs::create_dir_all(home_dir).expect("Failed to create directory");
let account_id = matches.value_of("account-id");
let generate_config = matches.is_present("generate-config");
let account_id = matches.get_one::<String>("account-id");
let generate_config = matches.get_flag("generate-config");

match matches.subcommand() {
Some(("signer-keys", args)) => {
let num_keys = args
.value_of("num-keys")
.get_one::<String>("num-keys")
.map(|x| x.parse().expect("Failed to parse number keys."))
.unwrap_or(3usize);
let keys: Vec<SecretKey> =
Expand Down
4 changes: 2 additions & 2 deletions neard/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ struct NeardOpts {
#[clap(long, name = "target")]
verbose: Option<Option<String>>,
/// Directory for config and data.
#[clap(long, parse(from_os_str), default_value_os = crate::DEFAULT_HOME.as_os_str())]
#[clap(long, value_parser, default_value_os = crate::DEFAULT_HOME.as_os_str())]
home: PathBuf,
/// Skips consistency checks of genesis.json (and records.json) upon startup.
/// Let's you start `neard` slightly faster.
Expand Down Expand Up @@ -263,7 +263,7 @@ pub(super) struct InitCmd {
#[clap(long)]
account_id: Option<String>,
/// Chain ID, by default creates new random.
#[clap(long, forbid_empty_values = true)]
#[clap(long, value_parser(clap::builder::NonEmptyStringValueParser::new()))]
chain_id: Option<String>,
/// Specify a custom download URL for the genesis file.
#[clap(long)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub(crate) struct CheckConfig {
#[clap(long)]
zulip_user: Option<u64>,
/// Checks have to be done on one specific metric.
#[clap(long, arg_enum)]
#[clap(long, value_enum)]
metric: Metric,
/// First git commit hash used for comparisons, used as base to calculate
/// the relative changes. If left unspecified, the two commits that were
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub(crate) struct EstimateConfig {
#[clap(long)]
pub home: Option<String>,
/// Comma separated list of metrics to use in estimation.
#[clap(long, default_value = "icount,time", possible_values = &["icount", "time"], use_value_delimiter = true)]
#[clap(long, default_value = "icount,time", value_parser(["icount", "time"]), use_value_delimiter = true)]
pub metrics: Vec<String>,
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ fn main() -> anyhow::Result<()> {
Ok(())
}

#[derive(Debug, Clone, Copy, PartialEq, clap::ArgEnum)]
#[derive(Debug, Clone, Copy, PartialEq, clap::ValueEnum)]
enum Metric {
#[clap(name = "icount")]
ICount,
Expand Down
2 changes: 1 addition & 1 deletion runtime/runtime-params-estimator/src/cost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::str::FromStr;
/// Kinds of things we measure in parameter estimator and charge for in runtime.
///
/// TODO: Deduplicate this enum with `ExtCosts` and `ActionCosts`.
#[derive(Copy, Clone, PartialEq, Eq, Debug, PartialOrd, Ord, clap::ArgEnum)]
#[derive(Copy, Clone, PartialEq, Eq, Debug, PartialOrd, Ord, clap::ValueEnum)]
#[repr(u8)]
pub enum Cost {
// Every set of actions in a transaction needs to be transformed into a
Expand Down
4 changes: 2 additions & 2 deletions runtime/runtime-params-estimator/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ struct CliArgs {
/// using qemu instrumentation.
/// Note that `icount` measurements are not accurate when translating to gas. The main purpose of it is to
/// have a stable output that can be used to detect performance regressions.
#[clap(long, default_value = "time", possible_values = &["icount", "time"])]
#[clap(long, default_value = "time", value_parser(["icount", "time"]))]
metric: String,
/// Which VM to test.
#[clap(long, possible_values = &["wasmer", "wasmer2", "wasmtime", "near-vm"])]
#[clap(long, value_parser(["wasmer", "wasmer2", "wasmtime", "near-vm"]))]
vm_kind: Option<String>,
/// Render existing `costs.txt` as `RuntimeConfig`.
#[clap(long)]
Expand Down
16 changes: 7 additions & 9 deletions test-utils/store-validator/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,32 +1,30 @@
use std::path::Path;
use std::process;
use std::sync::Arc;

use ansi_term::Color::{Green, Red, White, Yellow};
use clap::{Arg, Command};

use near_chain::store_validator::StoreValidator;
use near_chain::RuntimeWithEpochManagerAdapter;
use near_chain_configs::GenesisValidationMode;
use near_o11y::testonly::init_integration_logger;
use nearcore::{get_default_home, load_config};
use std::path::PathBuf;
use std::process;
use std::sync::Arc;

fn main() {
init_integration_logger();

let default_home = get_default_home();
let matches = Command::new("store-validator")
.arg(
Arg::new("home")
.long("home")
.default_value_os(default_home.as_os_str())
.default_value(get_default_home().into_os_string())
.value_parser(clap::value_parser!(PathBuf))
.help("Directory for config and data (default \"~/.near\")")
.takes_value(true),
.action(clap::ArgAction::Set),
)
.subcommand(Command::new("validate"))
.get_matches();

let home_dir = matches.value_of("home").map(Path::new).unwrap();
let home_dir = matches.get_one::<PathBuf>("home").unwrap();
let near_config = load_config(home_dir, GenesisValidationMode::Full)
.unwrap_or_else(|e| panic!("Error loading config: {:#}", e));

Expand Down
Loading

0 comments on commit 1183b61

Please sign in to comment.