From 2b27797f42cfa6ab711bbafd0d1a70668802d5f6 Mon Sep 17 00:00:00 2001 From: Scott Lamb Date: Mon, 13 Feb 2023 21:19:55 -0800 Subject: [PATCH] tweak bpaf usage message As discussed here: https://github.com/pacak/bpaf/discussions/165#discussioncomment-4967176 I also snuck in a conversion from `lazy_static` to `once_cell`, rather than adding another usage of the former. --- server/Cargo.lock | 13 +++--- server/Cargo.toml | 2 +- server/base/Cargo.toml | 1 - server/db/Cargo.toml | 2 +- server/db/auth.rs | 6 +-- server/src/cmds/check.rs | 6 +++ server/src/cmds/config/mod.rs | 6 +++ server/src/cmds/init.rs | 6 +++ server/src/cmds/login.rs | 14 ++++-- server/src/cmds/run/mod.rs | 6 +++ server/src/cmds/sql.rs | 13 ++++-- server/src/cmds/ts.rs | 6 +++ server/src/cmds/upgrade/mod.rs | 6 +++ server/src/main.rs | 82 ++++++++++++++++------------------ server/src/mp4.rs | 8 ++-- server/src/slices.rs | 23 +++++----- server/src/web/mod.rs | 5 +-- 17 files changed, 120 insertions(+), 85 deletions(-) diff --git a/server/Cargo.lock b/server/Cargo.lock index add9272..d64e6b6 100644 --- a/server/Cargo.lock +++ b/server/Cargo.lock @@ -146,8 +146,8 @@ dependencies = [ [[package]] name = "bpaf" -version = "0.7.8" -source = "git+https://github.com/pacak/bpaf.git?branch=exit_code#77b8efc5d33adcf57901a03514a3ce6183144f8d" +version = "0.7.9" +source = "git+https://github.com/pacak/bpaf.git?branch=exit_code#5238a6069abca0d61cf1c000e56bbac946e7ee18" dependencies = [ "bpaf_derive", "owo-colors", @@ -155,8 +155,8 @@ dependencies = [ [[package]] name = "bpaf_derive" -version = "0.3.3" -source = "git+https://github.com/pacak/bpaf.git?branch=exit_code#77b8efc5d33adcf57901a03514a3ce6183144f8d" +version = "0.3.4" +source = "git+https://github.com/pacak/bpaf.git?branch=exit_code#5238a6069abca0d61cf1c000e56bbac946e7ee18" dependencies = [ "proc-macro2", "quote", @@ -1101,7 +1101,6 @@ version = "0.0.1" dependencies = [ "failure", "futures", - "lazy_static", "libc", "log", "nom", @@ -1126,7 +1125,6 @@ dependencies = [ "h264-reader", "hashlink", "itertools", - "lazy_static", "libc", "log", "moonfire-base", @@ -1134,6 +1132,7 @@ dependencies = [ "nix", "num-rational", "odds", + "once_cell", "pretty-hex", "protobuf", "protobuf-codegen", @@ -1168,7 +1167,6 @@ dependencies = [ "http-serve", "hyper", "itertools", - "lazy_static", "libc", "log", "memchr", @@ -1179,6 +1177,7 @@ dependencies = [ "nix", "nom", "num-rational", + "once_cell", "password-hash", "protobuf", "reffers", diff --git a/server/Cargo.toml b/server/Cargo.toml index 30d9a09..f9f6ebd 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -37,7 +37,6 @@ http = "0.2.3" http-serve = { version = "0.3.1", features = ["dir"] } hyper = { version = "0.14.2", features = ["http1", "server", "stream", "tcp"] } itertools = "0.10.0" -lazy_static = "1.0" libc = "0.2" log = { version = "0.4" } memchr = "2.0.2" @@ -62,6 +61,7 @@ toml = "0.5" tracing = { version = "0.1", features = ["log"] } url = "2.1.1" uuid = { version = "1.1.2", features = ["serde", "std", "v4"] } +once_cell = "1.17.0" [dev-dependencies] mp4 = { git = "https://github.com/scottlamb/mp4-rust", branch = "moonfire" } diff --git a/server/base/Cargo.toml b/server/base/Cargo.toml index 305a7f7..2993c06 100644 --- a/server/base/Cargo.toml +++ b/server/base/Cargo.toml @@ -15,7 +15,6 @@ path = "lib.rs" [dependencies] failure = "0.1.1" futures = "0.3" -lazy_static = "1.0" libc = "0.2" log = "0.4" nom = "7.0.0" diff --git a/server/db/Cargo.toml b/server/db/Cargo.toml index 73868d3..92ddbca 100644 --- a/server/db/Cargo.toml +++ b/server/db/Cargo.toml @@ -24,7 +24,6 @@ fnv = "1.0" futures = "0.3" h264-reader = "0.6.0" hashlink = "0.8.1" -lazy_static = "1.0" libc = "0.2" log = "0.4" mylog = { git = "https://github.com/scottlamb/mylog" } @@ -46,6 +45,7 @@ tokio = { version = "1.24", features = ["macros", "rt-multi-thread", "sync"] } url = { version = "2.1.1", features = ["serde"] } uuid = { version = "1.1.2", features = ["serde", "std", "v4"] } itertools = "0.10.0" +once_cell = "1.17.0" [build-dependencies] protobuf-codegen = "3.0" diff --git a/server/db/auth.rs b/server/db/auth.rs index 8e73894..560b57c 100644 --- a/server/db/auth.rs +++ b/server/db/auth.rs @@ -9,7 +9,6 @@ use crate::schema::Permissions; use base::{bail_t, format_err_t, strutil, ErrorKind, ResultExt as _}; use failure::{bail, format_err, Error, Fail, ResultExt as _}; use fnv::FnvHashMap; -use lazy_static::lazy_static; use log::info; use protobuf::Message; use ring::rand::{SecureRandom, SystemRandom}; @@ -21,9 +20,8 @@ use std::net::IpAddr; use std::str::FromStr; use std::sync::Mutex; -lazy_static! { - static ref PARAMS: Mutex = Mutex::new(scrypt::Params::recommended()); -} +static PARAMS: once_cell::sync::Lazy> = + once_cell::sync::Lazy::new(|| Mutex::new(scrypt::Params::recommended())); /// For testing only: use fast but insecure hashes. /// Call via `testutil::init()`. diff --git a/server/src/cmds/check.rs b/server/src/cmds/check.rs index a5e98c9..296c6f0 100644 --- a/server/src/cmds/check.rs +++ b/server/src/cmds/check.rs @@ -9,7 +9,9 @@ use db::check; use failure::Error; use std::path::PathBuf; +/// Checks database integrity (like fsck). #[derive(Bpaf, Debug)] +#[bpaf(options)] pub struct Args { /// Directory holding the SQLite3 index database. /// @@ -40,6 +42,10 @@ pub struct Args { trash_corrupt_rows: bool, } +pub fn subcommand() -> impl bpaf::Parser { + crate::subcommand(args(), "check") +} + pub fn run(args: Args) -> Result { let (_db_dir, mut conn) = super::open_conn(&args.db_dir, super::OpenMode::ReadWrite)?; check::run( diff --git a/server/src/cmds/config/mod.rs b/server/src/cmds/config/mod.rs index f078f6d..d3a6d3b 100644 --- a/server/src/cmds/config/mod.rs +++ b/server/src/cmds/config/mod.rs @@ -19,7 +19,9 @@ mod cameras; mod dirs; mod users; +/// Interactively edits configuration. #[derive(Bpaf, Debug)] +#[bpaf(options)] pub struct Args { /// Directory holding the SQLite3 index database. /// @@ -28,6 +30,10 @@ pub struct Args { db_dir: PathBuf, } +pub fn subcommand() -> impl bpaf::Parser { + crate::subcommand(args(), "config") +} + pub fn run(args: Args) -> Result { let (_db_dir, conn) = super::open_conn(&args.db_dir, super::OpenMode::ReadWrite)?; let clocks = clock::RealClocks {}; diff --git a/server/src/cmds/init.rs b/server/src/cmds/init.rs index adc766e..e91e3b6 100644 --- a/server/src/cmds/init.rs +++ b/server/src/cmds/init.rs @@ -7,7 +7,9 @@ use failure::Error; use log::info; use std::path::PathBuf; +/// Initializes a database. #[derive(Bpaf, Debug)] +#[bpaf(options)] pub struct Args { /// Directory holding the SQLite3 index database. /// @@ -16,6 +18,10 @@ pub struct Args { db_dir: PathBuf, } +pub fn subcommand() -> impl bpaf::Parser { + crate::subcommand(args(), "init") +} + pub fn run(args: Args) -> Result { let (_db_dir, mut conn) = super::open_conn(&args.db_dir, super::OpenMode::Create)?; diff --git a/server/src/cmds/login.rs b/server/src/cmds/login.rs index 26dea87..12ed74c 100644 --- a/server/src/cmds/login.rs +++ b/server/src/cmds/login.rs @@ -24,7 +24,14 @@ fn parse_flags(flags: String) -> Result, Error> { .collect() } +/// Logs in a user, returning the session cookie. +/// +/// +/// This is a privileged command that directly accesses the database. It doesn't check the +/// user's password and even can be used to create sessions with permissions the user doesn't +/// have. #[derive(Bpaf, Debug, PartialEq, Eq)] +#[bpaf(options)] pub struct Args { /// Directory holding the SQLite3 index database. /// @@ -64,6 +71,10 @@ pub struct Args { username: String, } +pub fn subcommand() -> impl bpaf::Parser { + crate::subcommand(args(), "login") +} + pub fn run(args: Args) -> Result { let clocks = clock::RealClocks {}; let (_db_dir, conn) = super::open_conn(&args.db_dir, super::OpenMode::ReadWrite)?; @@ -148,12 +159,9 @@ fn curl_cookie(cookie: &str, flags: i32, domain: &str) -> String { mod tests { use super::*; - use bpaf::Parser; - #[test] fn parse_args() { let args = args() - .to_options() .run_inner(bpaf::Args::from(&[ "--permissions", "{\"viewVideo\": true}", diff --git a/server/src/cmds/run/mod.rs b/server/src/cmds/run/mod.rs index c8ef04f..fd09b6e 100644 --- a/server/src/cmds/run/mod.rs +++ b/server/src/cmds/run/mod.rs @@ -25,7 +25,9 @@ use self::config::ConfigFile; mod config; +/// Runs the server, saving recordings and allowing web access. #[derive(Bpaf, Debug)] +#[bpaf(options)] pub struct Args { /// Path to configuration file. /// @@ -40,6 +42,10 @@ pub struct Args { read_only: bool, } +pub fn subcommand() -> impl bpaf::Parser { + crate::subcommand(args(), "run") +} + // These are used in a hack to get the name of the current time zone (e.g. America/Los_Angeles). // They seem to be correct for Linux and macOS at least. const LOCALTIME_PATH: &str = "/etc/localtime"; diff --git a/server/src/cmds/sql.rs b/server/src/cmds/sql.rs index 5ef1ca9..917d176 100644 --- a/server/src/cmds/sql.rs +++ b/server/src/cmds/sql.rs @@ -12,7 +12,13 @@ use std::os::unix::process::CommandExt; use std::path::PathBuf; use std::process::Command; +/// Runs a SQLite3 shell on Moonfire NVR's index database. +/// +/// +/// Note this locks the database to prevent simultaneous access with a running server. The +/// server maintains cached state which could be invalidated otherwise. #[derive(Bpaf, Debug, PartialEq, Eq)] +#[bpaf(options)] pub struct Args { /// Directory holding the SQLite3 index database. /// @@ -33,6 +39,10 @@ pub struct Args { arg: Vec, } +pub fn subcommand() -> impl bpaf::Parser { + crate::subcommand(args(), "sql") +} + pub fn run(args: Args) -> Result { let mode = if args.read_only { OpenMode::ReadOnly @@ -59,12 +69,9 @@ pub fn run(args: Args) -> Result { mod tests { use super::*; - use bpaf::Parser; - #[test] fn parse_args() { let args = args() - .to_options() .run_inner(bpaf::Args::from(&[ "--db-dir", "/foo/bar", diff --git a/server/src/cmds/ts.rs b/server/src/cmds/ts.rs index 0b4ea6f..30e693b 100644 --- a/server/src/cmds/ts.rs +++ b/server/src/cmds/ts.rs @@ -5,7 +5,9 @@ use bpaf::Bpaf; use failure::Error; +/// Translates between integer and human-readable timestamps. #[derive(Bpaf, Debug)] +#[bpaf(options)] pub struct Args { /// Timestamp(s) to translate. /// @@ -17,6 +19,10 @@ pub struct Args { timestamps: Vec, } +pub fn subcommand() -> impl bpaf::Parser { + crate::subcommand(args(), "ts") +} + pub fn run(args: Args) -> Result { for timestamp in &args.timestamps { let t = db::recording::Time::parse(timestamp)?; diff --git a/server/src/cmds/upgrade/mod.rs b/server/src/cmds/upgrade/mod.rs index 6813d97..9dda2bb 100644 --- a/server/src/cmds/upgrade/mod.rs +++ b/server/src/cmds/upgrade/mod.rs @@ -8,7 +8,9 @@ use bpaf::Bpaf; /// See `guide/schema.md` for more information. use failure::Error; +/// Upgrades to the latest database schema. #[derive(Bpaf, Debug)] +#[bpaf(options)] pub struct Args { /// Directory holding the SQLite3 index database. /// @@ -35,6 +37,10 @@ pub struct Args { no_vacuum: bool, } +pub fn subcommand() -> impl bpaf::Parser { + crate::subcommand(args(), "upgrade") +} + pub fn run(args: Args) -> Result { let (_db_dir, mut conn) = super::open_conn(&args.db_dir, super::OpenMode::ReadWrite)?; diff --git a/server/src/main.rs b/server/src/main.rs index 324b167..79749f0 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -19,50 +19,43 @@ mod stream; mod streamer; mod web; +/// The program name, taken from the OS-provided arguments if available. +/// +/// E.g. if invoked as `target/debug/nvr`, should return `nvr`. +static PROGNAME: once_cell::sync::Lazy<&'static str> = once_cell::sync::Lazy::new(|| { + std::env::args_os() + .next() + .and_then(|p| { + let p = std::path::PathBuf::from(p); + p.file_name().and_then(|f| { + f.to_str() + .map(|s| &*Box::leak(s.to_owned().into_boxed_str())) + }) + }) + .unwrap_or(env!("CARGO_PKG_NAME")) +}); + +fn subcommand( + parser: bpaf::OptionParser, + cmd: &'static str, +) -> impl bpaf::Parser { + let usage = format!("Usage: {progname} {cmd} {{usage}}", progname = *PROGNAME); + parser.usage(Box::leak(usage.into_boxed_str())).command(cmd) +} + /// Moonfire NVR: security camera network video recorder. #[derive(Bpaf, Debug)] #[bpaf(options, version)] enum Args { - /// Checks database integrity (like fsck). - #[bpaf(command)] - Check(#[bpaf(external(cmds::check::args))] cmds::check::Args), - - /// Interactively edits configuration. - #[bpaf(command)] - Config(#[bpaf(external(cmds::config::args))] cmds::config::Args), - - /// Initializes a database. - #[bpaf(command)] - Init(#[bpaf(external(cmds::init::args))] cmds::init::Args), - - /// Logs in a user, returning the session cookie. - /// - /// - /// This is a privileged command that directly accesses the database. It doesn't check the - /// user's password and even can be used to create sessions with permissions the user doesn't - /// have. - #[bpaf(command)] - Login(#[bpaf(external(cmds::login::args))] cmds::login::Args), - - /// Runs the server, saving recordings and allowing web access. - #[bpaf(command)] - Run(#[bpaf(external(cmds::run::args))] cmds::run::Args), - - /// Runs a SQLite3 shell on Moonfire NVR's index database. - /// - /// - /// Note this locks the database to prevent simultaneous access with a running server. The - /// server maintains cached state which could be invalidated otherwise. - #[bpaf(command)] - Sql(#[bpaf(external(cmds::sql::args))] cmds::sql::Args), - - /// Translates between integer and human-readable timestamps. - #[bpaf(command)] - Ts(#[bpaf(external(cmds::ts::args))] cmds::ts::Args), - - /// Upgrades to the latest database schema. - #[bpaf(command)] - Upgrade(#[bpaf(external(cmds::upgrade::args))] cmds::upgrade::Args), + // See docstrings of `cmds::*::Args` structs for a description of the respective subcommands. + Check(#[bpaf(external(cmds::check::subcommand))] cmds::check::Args), + Config(#[bpaf(external(cmds::config::subcommand))] cmds::config::Args), + Init(#[bpaf(external(cmds::init::subcommand))] cmds::init::Args), + Login(#[bpaf(external(cmds::login::subcommand))] cmds::login::Args), + Run(#[bpaf(external(cmds::run::subcommand))] cmds::run::Args), + Sql(#[bpaf(external(cmds::sql::subcommand))] cmds::sql::Args), + Ts(#[bpaf(external(cmds::ts::subcommand))] cmds::ts::Args), + Upgrade(#[bpaf(external(cmds::upgrade::subcommand))] cmds::upgrade::Args), } impl Args { @@ -141,12 +134,15 @@ fn main() { .build(); h.clone().install().unwrap(); + let args = args().usage(Box::leak( + format!("Usage: {progname} {{usage}}", progname = *PROGNAME).into_boxed_str(), + )); + // TODO: remove this when bpaf adds more direct support for defaulting to `--help`. // See discussion: . if std::env::args_os().len() < 2 { std::process::exit( - args() - .run_inner(bpaf::Args::from(&["--help"])) + args.run_inner(bpaf::Args::from(&["--help"])) .unwrap_err() .exit_code(), ); @@ -159,7 +155,7 @@ fn main() { std::panic::set_hook(Box::new(&panic_hook)); } - let args = args().run(); + let args = args.run(); log::trace!("Parsed command-line arguments: {args:#?}"); let r = { diff --git a/server/src/mp4.rs b/server/src/mp4.rs index 75f7ce7..e308ddc 100644 --- a/server/src/mp4.rs +++ b/server/src/mp4.rs @@ -2979,11 +2979,10 @@ mod bench { use futures::future; use http_serve; use hyper; - use lazy_static::lazy_static; use url::Url; /// An HTTP server for benchmarking. - /// It's used as a singleton via `lazy_static!` so that when getting a CPU profile of the + /// It's used as a singleton so that when getting a CPU profile of the /// benchmark, more of the profile focuses on the HTTP serving rather than the setup. /// /// Currently this only serves a single `.mp4` file but we could set up variations to benchmark @@ -3029,9 +3028,8 @@ mod bench { } } - lazy_static! { - static ref SERVER: BenchServer = BenchServer::new(); - } + static SERVER: once_cell::sync::Lazy = + once_cell::sync::Lazy::new(BenchServer::new); #[bench] fn build_index(b: &mut test::Bencher) { diff --git a/server/src/slices.rs b/server/src/slices.rs index 063a85f..262fbe8 100644 --- a/server/src/slices.rs +++ b/server/src/slices.rs @@ -183,7 +183,6 @@ mod tests { use crate::body::BoxedError; use db::testutil; use futures::stream::{self, Stream, TryStreamExt}; - use lazy_static::lazy_static; use std::ops::Range; use std::pin::Pin; @@ -224,18 +223,16 @@ mod tests { } } - lazy_static! { - #[rustfmt::skip] - static ref SLICES: Slices = { - let mut s = Slices::new(); - s.append(FakeSlice { end: 5, name: "a" }).unwrap(); - s.append(FakeSlice { end: 5 + 13, name: "b" }).unwrap(); - s.append(FakeSlice { end: 5 + 13 + 7, name: "c" }).unwrap(); - s.append(FakeSlice { end: 5 + 13 + 7 + 17, name: "d" }).unwrap(); - s.append(FakeSlice { end: 5 + 13 + 7 + 17 + 19, name: "e" }).unwrap(); - s - }; - } + #[rustfmt::skip] + static SLICES: once_cell::sync::Lazy> = once_cell::sync::Lazy::new(|| { + let mut s = Slices::new(); + s.append(FakeSlice { end: 5, name: "a" }).unwrap(); + s.append(FakeSlice { end: 5 + 13, name: "b" }).unwrap(); + s.append(FakeSlice { end: 5 + 13 + 7, name: "c" }).unwrap(); + s.append(FakeSlice { end: 5 + 13 + 7 + 17, name: "d" }).unwrap(); + s.append(FakeSlice { end: 5 + 13 + 7 + 17 + 19, name: "e" }).unwrap(); + s + }); async fn get_range(r: Range) -> Vec { Pin::from(SLICES.get_range(&&*SLICES, r)) diff --git a/server/src/web/mod.rs b/server/src/web/mod.rs index 1dde9d0..bc1cfc9 100644 --- a/server/src/web/mod.rs +++ b/server/src/web/mod.rs @@ -728,7 +728,6 @@ mod bench { use db::testutil::{self, TestDb}; use hyper; - use lazy_static::lazy_static; use std::sync::Arc; use uuid::Uuid; @@ -786,9 +785,7 @@ mod bench { } } - lazy_static! { - static ref SERVER: Server = Server::new(); - } + static SERVER: once_cell::sync::Lazy = once_cell::sync::Lazy::new(Server::new); #[bench] fn serve_stream_recordings(b: &mut test::Bencher) {