treewide: Upgrade to clap 4.2.1

Upgrade the clap crate everywhere to version 4.2.1, updating usage of
backwards-incompatible APIs along the way. Some highlights:

- Many enums are now just deriving ValueEnum, as the new version of the
  previous method of using `FromStr` and `possible_values` is much
  clunkier than before.
- Some values used in clap args now are required to derive Clone
- We have to either avoid automatically creating arg groups for structs
  that are `#[clap(flatten)]`ed, or give them a unique name with
  `#[group(id = ...)]`, or clap will give us an error that the groups
  are not unique.
- Arg names now use underscores instead of dashes when referenced from
  other args

Also, to keep the single-version-policy intact, a couple of other
dependencies had to update:
- rustyline in readyset_repl, to update linux-raw-sys
- console treewide
- indicatif treewide

Unfortunately the latest version of nperf-core depends on an older
version of the `syn` crate than clap uses, so there's nothing we can do
there and we just have to ignore it here.

Change-Id: I4f353294c85ebd1d494537baa16d3b2e797bec69
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/4660
Tested-by: Buildkite CI
Reviewed-by: Dan Wilbanks <[email protected]>

Author: glittershark

Cargo.lock

@@ -21,7 +21,7 @@ vec1 = "1.8.0"
 url = "2.2.2"
 hdrhistogram = "7.4"
 zipf = "7.0.0"
-clap = { version = "3.0", features = ["derive", "env"] }
+clap = { version = "4.2", features = ["derive", "env"] }
 reqwest = { version = "0.11", features = ["stream", "native-tls"] }
 chrono = "0.4"
 atomic-counter = "1.0.1"
@@ -38,7 +38,7 @@ lazy_static = "1.4.0"
 thiserror = "1.0.30"
 async-stream = "0.3.2"
 parking_lot = "0.11.2"
-indicatif = "0.16"
+indicatif = "0.17"
 prometheus-parse = "0.2.2"
 walkdir = "2.3"
 tokio-postgres = { workspace = true }

benchmarks/Cargo.toml

@@ -94,21 +94,11 @@ pub struct DeploymentParameters {
 
     /// Target database connection string. This is the database in the deployment
     /// we are benchmarking operations against.
-    #[clap(
-        long,
-        env = "TARGET_CONN_STR",
-        default_value = "",
-        required_unless_present("deployment")
-    )]
+    #[clap(long, env = "TARGET_CONN_STR", default_value = "")]
     pub target_conn_str: String,
 
     /// Setup database connection string.
-    #[clap(
-        long,
-        env = "SETUP_CONN_STR",
-        default_value = "",
-        required_unless_present("deployment")
-    )]
+    #[clap(long, env = "SETUP_CONN_STR", default_value = "")]
     pub setup_conn_str: String,
 
     #[clap(long)]

benchmarks/src/benchmark.rs

@@ -9,7 +9,12 @@ use readyset_data::Dialect;
 struct ExtendRecipe {
     #[clap(short, long, env("AUTHORITY_ADDRESS"), default_value("127.0.0.1:2181"))]
     authority_address: String,
-    #[clap(long, env("AUTHORITY"), default_value("zookeeper"), possible_values = &["consul", "zookeeper"])]
+    #[clap(
+        long,
+        env("AUTHORITY"),
+        default_value("zookeeper"),
+        value_parser = ["consul", "zookeeper"]
+    )]
     authority: AuthorityType,
     #[clap(short, long, env("DEPLOYMENT"))]
     deployment: String,

benchmarks/src/bin/extend_recipe.rs

@@ -1,3 +1,4 @@
+use clap::builder::NonEmptyStringValueParser;
 use clap::Parser;
 use readyset_client::consensus::AuthorityType;
 use readyset_client::recipe::changelist::ChangeList;
@@ -9,9 +10,14 @@ use readyset_data::Dialect;
 struct ExtendRecipeWritePropagation {
     #[clap(short, long, env("AUTHORITY_ADDRESS"), default_value("127.0.0.1:2181"))]
     authority_address: String,
-    #[clap(long, env("AUTHORITY"), default_value("zookeeper"), possible_values = &["consul", "zookeeper"])]
+    #[clap(
+        long,
+        env("AUTHORITY"),
+        default_value("zookeeper"),
+        value_parser = ["consul", "zookeeper"]
+    )]
     authority: AuthorityType,
-    #[clap(short, long, env("DEPLOYMENT"), forbid_empty_values = true)]
+    #[clap(short, long, env("DEPLOYMENT"), value_parser = NonEmptyStringValueParser::new())]
     deployment: String,
 }
 

benchmarks/src/bin/extend_recipe_write_prop.rs

@@ -10,7 +10,7 @@ use readyset_data::DfValue;
 #[derive(Parser, Debug)]
 struct Opts {
     /// SQL endpoint to connect to
-    #[clap(env = "DATABASE_URL", parse(try_from_str))]
+    #[clap(env = "DATABASE_URL")]
     database_url: DatabaseURL,
 
     /// Number of greenthreads to run hitting the database

benchmarks/src/bin/query_slammer.rs

@@ -4,7 +4,7 @@ use std::time::{Duration, Instant};
 use std::{env, fs, mem};
 
 use anyhow::Result;
-use clap::{ArgEnum, Parser};
+use clap::{Parser, ValueEnum};
 use database_utils::{DatabaseConnection, DatabaseURL};
 use rand::distributions::{Distribution, Uniform};
 use rand::prelude::*;
@@ -16,7 +16,7 @@ use vec1::Vec1;
 
 static THREAD_UPDATE_INTERVAL: Duration = Duration::from_secs(10);
 
-#[derive(Clone, ArgEnum)]
+#[derive(Clone, ValueEnum)]
 enum Dist {
     Uniform,
     Zipf,
@@ -33,7 +33,13 @@ struct NoriaClientOpts {
     )]
     authority_address: String,
 
-    #[clap(long, env("AUTHORITY"), required_if_eq("database-type", "noria"), default_value("zookeeper"), possible_values = &["consul", "zookeeper"])]
+    #[clap(
+        long,
+        env("AUTHORITY"),
+        required_if_eq("database-type", "noria"),
+        default_value("zookeeper"),
+        value_parser = ["consul", "zookeeper"]
+    )]
     authority: AuthorityType,
 
     #[clap(
@@ -72,7 +78,7 @@ struct MySqlOpts {
     nparams: Option<usize>,
 }
 
-#[derive(Clone, ArgEnum)]
+#[derive(Clone, Copy, ValueEnum)]
 enum DatabaseType {
     Noria,
     MySql,
@@ -104,7 +110,7 @@ struct Reader {
     ///
     /// 'zipf' - sample pattern is skewed such that 90% of accesses are for 10% of the ids (Zipf;
     /// α=1.15)
-    #[clap(arg_enum, default_value = "uniform")]
+    #[clap(default_value = "uniform")]
     distribution: Dist,
 
     /// Override the default alpha parameter for the zipf distribution
@@ -119,7 +125,7 @@ struct Reader {
     /// The type of database the client is connecting to. This determines
     /// the set of opts that should be populated. See `noria_opts` and
     /// `mysql_opts`.
-    #[clap(arg_enum, default_value = "noria")]
+    #[clap(default_value = "noria")]
     database_type: DatabaseType,
 
     /// The set of options to be specified by the user to connect via a

benchmarks/src/bin/reader.rs

@@ -23,6 +23,7 @@ use std::time::{Duration, Instant};
 
 use benchmarks::utils::generate::load_to_backend;
 use benchmarks::utils::spec::{DatabaseGenerationSpec, DatabaseSchema};
+use clap::builder::NonEmptyStringValueParser;
 use clap::{Parser, ValueHint};
 use nom_sql::Relation;
 use query_generator::ColumnGenerationSpec;
@@ -56,10 +57,10 @@ struct Writer {
     #[clap(short, long, env("AUTHORITY_ADDRESS"), default_value("127.0.0.1:2181"))]
     authority_address: String,
 
-    #[clap(long, env("AUTHORITY"), default_value("zookeeper"), possible_values = &["consul", "zookeeper"])]
+    #[clap(long, env("AUTHORITY"), default_value("zookeeper"), value_parser = ["consul", "zookeeper"])]
     authority: AuthorityType,
 
-    #[clap(short, long, env("DEPLOYMENT"), forbid_empty_values = true)]
+    #[clap(short, long, env("DEPLOYMENT"), value_parser = NonEmptyStringValueParser::new())]
     deployment: String,
 
     /// Path to the news app data model SQL schema.

benchmarks/src/bin/write_propagation.rs

@@ -64,7 +64,7 @@ pub struct EvictionBenchmark {
     /// The duration, specified as the number of seconds that the benchmark
     /// should be running. If `None` is provided, the benchmark will run
     /// until it is interrupted.
-    #[clap(long, parse(try_from_str = crate::utils::seconds_as_str_to_duration))]
+    #[clap(long, value_parser = crate::utils::seconds_as_str_to_duration)]
     run_for: Option<Duration>,
 
     /// Attempt to scale down query range from query_spec in order to hit the desired

benchmarks/src/eviction_benchmark.rs

@@ -46,7 +46,7 @@ pub struct FallbackBenchmark {
     /// The duration, specified as the number of seconds that the benchmark
     /// should be running. If `None` is provided, the benchmark will run
     /// until it is interrupted.
-    #[clap(long, parse(try_from_str = crate::utils::seconds_as_str_to_duration), default_value="30")]
+    #[clap(long, value_parser = crate::utils::seconds_as_str_to_duration, default_value="30")]
     run_for: Duration,
 
     /// Whether fallback cache should be enabled for this benchmark