diff --git a/rust/rhg/src/blackbox.rs b/rust/rhg/src/blackbox.rs --- a/rust/rhg/src/blackbox.rs +++ b/rust/rhg/src/blackbox.rs @@ -52,20 +52,22 @@ process_start_time: &'a ProcessStartTime, ) -> Result { let configured = if let Ok(repo) = invocation.repo { - let config = invocation.config(); - if config.get(b"extensions", b"blackbox").is_none() { + if invocation.config.get(b"extensions", b"blackbox").is_none() { // The extension is not enabled None } else { Some(ConfiguredBlackbox { repo, - max_size: config + max_size: invocation + .config .get_byte_size(b"blackbox", b"maxsize")? .unwrap_or(DEFAULT_MAX_SIZE), - max_files: config + max_files: invocation + .config .get_u32(b"blackbox", b"maxfiles")? .unwrap_or(DEFAULT_MAX_FILES), - date_format: config + date_format: invocation + .config .get_str(b"blackbox", b"date-format")? .unwrap_or(DEFAULT_DATE_FORMAT), }) diff --git a/rust/rhg/src/commands/config.rs b/rust/rhg/src/commands/config.rs --- a/rust/rhg/src/commands/config.rs +++ b/rust/rhg/src/commands/config.rs @@ -29,7 +29,7 @@ .split_2(b'.') .ok_or_else(|| HgError::abort(""))?; - let value = invocation.config().get(section, name).unwrap_or(b""); + let value = invocation.config.get(section, name).unwrap_or(b""); invocation.ui.write_stdout(&format_bytes!(b"{}\n", value))?; Ok(()) diff --git a/rust/rhg/src/main.rs b/rust/rhg/src/main.rs --- a/rust/rhg/src/main.rs +++ b/rust/rhg/src/main.rs @@ -7,7 +7,10 @@ use format_bytes::format_bytes; use hg::config::Config; use hg::repo::{Repo, RepoError}; -use std::path::{Path, PathBuf}; +use hg::utils::files::{get_bytes_from_os_str, get_path_from_bytes}; +use hg::utils::SliceExt; +use std::ffi::OsString; +use std::path::PathBuf; mod blackbox; mod error; @@ -16,10 +19,11 @@ use error::CommandError; fn main_with_result( + process_start_time: &blackbox::ProcessStartTime, ui: &ui::Ui, - process_start_time: &blackbox::ProcessStartTime, + repo: Result<&Repo, &NoRepoInCwdError>, + config: &Config, ) -> Result<(), CommandError> { - env_logger::init(); let app = App::new("rhg") .global_setting(AppSettings::AllowInvalidUtf8) .setting(AppSettings::SubcommandRequired) @@ -57,29 +61,11 @@ let subcommand_args = subcommand_matches .expect("no subcommand arguments from clap despite AppSettings::SubcommandRequired"); - let config_args = matches - .values_of_os("config") - // Turn `Option::None` into an empty iterator: - .into_iter() - .flatten() - .map(hg::utils::files::get_bytes_from_os_str); - let non_repo_config = &hg::config::Config::load(config_args)?; - - let repo_path = matches.value_of_os("repository").map(Path::new); - let repo = match Repo::find(non_repo_config, repo_path) { - Ok(repo) => Ok(repo), - Err(RepoError::NotFound { at }) if repo_path.is_none() => { - // Not finding a repo is not fatal yet, if `-R` was not given - Err(NoRepoInCwdError { cwd: at }) - } - Err(error) => return Err(error.into()), - }; - let invocation = CliInvocation { ui, subcommand_args, - non_repo_config, - repo: repo.as_ref(), + config, + repo, }; let blackbox = blackbox::Blackbox::new(&invocation, process_start_time)?; blackbox.log_command_start(); @@ -94,17 +80,36 @@ // measurements. Reading config files can be slow if they’re on NFS. let process_start_time = blackbox::ProcessStartTime::now(); + env_logger::init(); let ui = ui::Ui::new(); - let result = main_with_result(&ui, &process_start_time); - if let Err(CommandError::Abort { message }) = &result { - if !message.is_empty() { - // Ignore errors when writing to stderr, we’re already exiting - // with failure code so there’s not much more we can do. - let _ = ui.write_stderr(&format_bytes!(b"abort: {}\n", message)); + let early_args = EarlyArgs::parse(std::env::args_os()); + let non_repo_config = Config::load(early_args.config) + .unwrap_or_else(|error| exit(&ui, Err(error.into()))); + + let repo_path = early_args.repo.as_deref().map(get_path_from_bytes); + let repo_result = match Repo::find(&non_repo_config, repo_path) { + Ok(repo) => Ok(repo), + Err(RepoError::NotFound { at }) if repo_path.is_none() => { + // Not finding a repo is not fatal yet, if `-R` was not given + Err(NoRepoInCwdError { cwd: at }) } - } - std::process::exit(exit_code(&result)) + Err(error) => exit(&ui, Err(error.into())), + }; + + let config = if let Ok(repo) = &repo_result { + repo.config() + } else { + &non_repo_config + }; + + let result = main_with_result( + &process_start_time, + &ui, + repo_result.as_ref(), + config, + ); + exit(&ui, result) } fn exit_code(result: &Result<(), CommandError>) -> i32 { @@ -118,6 +123,17 @@ } } +fn exit(ui: &Ui, result: Result<(), CommandError>) -> ! { + if let Err(CommandError::Abort { message }) = &result { + if !message.is_empty() { + // Ignore errors when writing to stderr, we’re already exiting + // with failure code so there’s not much more we can do. + let _ = ui.write_stderr(&format_bytes!(b"abort: {}\n", message)); + } + } + std::process::exit(exit_code(&result)) +} + macro_rules! subcommands { ($( $command: ident )+) => { mod commands { @@ -157,7 +173,7 @@ pub struct CliInvocation<'a> { ui: &'a Ui, subcommand_args: &'a ArgMatches<'a>, - non_repo_config: &'a Config, + config: &'a Config, /// References inside `Result` is a bit peculiar but allow /// `invocation.repo?` to work out with `&CliInvocation` since this /// `Result` type is `Copy`. @@ -168,12 +184,45 @@ cwd: PathBuf, } -impl CliInvocation<'_> { - fn config(&self) -> &Config { - if let Ok(repo) = self.repo { - repo.config() - } else { - self.non_repo_config +/// CLI arguments to be parsed "early" in order to be able to read +/// configuration before using Clap. Ideally we would also use Clap for this, +/// see . +/// +/// These arguments are still declared when we do use Clap later, so that Clap +/// does not return an error for their presence. +struct EarlyArgs { + /// Values of all `--config` arguments. (Possibly none) + config: Vec>, + /// Value of the `-R` or `--repository` argument, if any. + repo: Option>, +} + +impl EarlyArgs { + fn parse(args: impl IntoIterator) -> Self { + let mut args = args.into_iter().map(get_bytes_from_os_str); + let mut config = Vec::new(); + let mut repo = None; + // Use `while let` instead of `for` so that we can also call + // `args.next()` inside the loop. + while let Some(arg) = args.next() { + if arg == b"--config" { + if let Some(value) = args.next() { + config.push(value) + } + } else if let Some(value) = arg.drop_prefix(b"--config=") { + config.push(value.to_owned()) + } + + if arg == b"--repository" || arg == b"-R" { + if let Some(value) = args.next() { + repo = Some(value) + } + } else if let Some(value) = arg.drop_prefix(b"--repository=") { + repo = Some(value.to_owned()) + } else if let Some(value) = arg.drop_prefix(b"-R") { + repo = Some(value.to_owned()) + } } + Self { config, repo } } }