D10090: rhg: Make configuration available as early as possible in main()
SimonSapin
phabricator at mercurial-scm.org
Tue Mar 2 19:58:27 UTC 2021
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.
REPOSITORY
rHG Mercurial
BRANCH
default
REVISION DETAIL
https://phab.mercurial-scm.org/D10090
AFFECTED FILES
rust/rhg/src/blackbox.rs
rust/rhg/src/commands/config.rs
rust/rhg/src/main.rs
CHANGE DETAILS
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 <https://github.com/clap-rs/clap/discussions/2366>.
+///
+/// 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<Vec<u8>>,
+ /// Value of the `-R` or `--repository` argument, if any.
+ repo: Option<Vec<u8>>,
+}
+
+impl EarlyArgs {
+ fn parse(args: impl IntoIterator<Item = OsString>) -> 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 }
}
}
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/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<Self, HgError> {
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),
})
To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
More information about the Mercurial-devel
mailing list