This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12543

rhg: refactor to pass argv down, instead of caling args_os()
AcceptedPublic

Authored by aalekseyev on Tue, Apr 12, 3:02 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Summary

This refactoring makes it easy to patch some command-line preprocessing into rhg.
We use this to support using rhg as a shebang interpreter, for example.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

aalekseyev created this revision.Tue, Apr 12, 3:02 PM
Alphare added a subscriber: Alphare.Wed, Apr 13, 4:49 AM
Alphare added inline comments.
rust/rhg/src/main.rs
390–391

Nitpicking: why is this "b? This implies some other 'a lifetime by convention.

aalekseyev updated this revision to Diff 33146.Wed, Apr 13, 5:41 AM
aalekseyev marked an inline comment as done.Wed, Apr 13, 5:44 AM
aalekseyev added inline comments.
rust/rhg/src/main.rs
390–391

Changed it now.
(I guess the name 'a must have been taken when I wrote it. \shrug)

Alphare accepted this revision.Thu, Apr 14, 4:36 AM
This revision is now accepted and ready to land.Thu, Apr 14, 4:36 AM

Revision Contents
Changeset List

PathPackages
Mrust/rhg/src/blackbox.rs (21 lines)
Mrust/rhg/src/main.rs (30 lines)

Diff 33146

rust/rhg/src/blackbox.rs

//! Logging for repository events, including commands run in the repository. //! Logging for repository events, including commands run in the repository.
use crate::CliInvocation; use crate::CliInvocation;
use format_bytes::format_bytes; use format_bytes::format_bytes;
use hg::errors::HgError; use hg::errors::HgError;
use hg::repo::Repo; use hg::repo::Repo;
use hg::utils::{files::get_bytes_from_os_str, shell_quote}; use hg::utils::{files::get_bytes_from_os_str, shell_quote};
use std::ffi::OsString;
const ONE_MEBIBYTE: u64 = 1 << 20; const ONE_MEBIBYTE: u64 = 1 << 20;
// TODO: somehow keep defaults in sync with `configitem` in `hgext/blackbox.py` // TODO: somehow keep defaults in sync with `configitem` in `hgext/blackbox.py`
const DEFAULT_MAX_SIZE: u64 = ONE_MEBIBYTE; const DEFAULT_MAX_SIZE: u64 = ONE_MEBIBYTE;
const DEFAULT_MAX_FILES: u32 = 7; const DEFAULT_MAX_FILES: u32 = 7;
// Python does not support %.3f, only %f // Python does not support %.3f, only %f
None None
}; };
Ok(Self { Ok(Self {
process_start_time, process_start_time,
configured, configured,
}) })
} }
pub fn log_command_start(&self) { pub fn log_command_start<'arg>(
&self,
argv: impl Iterator<Item = &'arg OsString>,
) {
if let Some(configured) = &self.configured { if let Some(configured) = &self.configured {
let message = format_bytes!(b"(rust) {}", format_cli_args()); let message = format_bytes!(b"(rust) {}", format_cli_args(argv));
configured.log(&self.process_start_time.calendar_based, &message); configured.log(&self.process_start_time.calendar_based, &message);
} }
} }
pub fn log_command_end(&self, exit_code: i32) { pub fn log_command_end<'arg>(
&self,
argv: impl Iterator<Item = &'arg OsString>,
exit_code: i32,
) {
if let Some(configured) = &self.configured { if let Some(configured) = &self.configured {
let now = chrono::Local::now(); let now = chrono::Local::now();
let duration = self let duration = self
.process_start_time .process_start_time
.monotonic_clock .monotonic_clock
.elapsed() .elapsed()
.as_secs_f64(); .as_secs_f64();
let message = format_bytes!( let message = format_bytes!(
b"(rust) {} exited {} after {} seconds", b"(rust) {} exited {} after {} seconds",
format_cli_args(), format_cli_args(argv),
exit_code, exit_code,
format_bytes::Utf8(format_args!("{:.03}", duration)) format_bytes::Utf8(format_args!("{:.03}", duration))
); );
configured.log(&now, &message); configured.log(&now, &message);
} }
} }
} }
Ok(()) => {} Ok(()) => {}
Err(_io_error) => { Err(_io_error) => {
// TODO: log a non-fatal warning to stderr // TODO: log a non-fatal warning to stderr
} }
} }
} }
} }
fn format_cli_args() -> Vec<u8> { fn format_cli_args<'a>(
let mut args = std::env::args_os(); mut args: impl Iterator<Item = &'a OsString>,
) -> Vec<u8> {
let _ = args.next(); // Skip the first (or zeroth) arg, the name of the `rhg` executable let _ = args.next(); // Skip the first (or zeroth) arg, the name of the `rhg` executable
let mut args = args.map(|arg| shell_quote(&get_bytes_from_os_str(arg))); let mut args = args.map(|arg| shell_quote(&get_bytes_from_os_str(arg)));
let mut formatted = Vec::new(); let mut formatted = Vec::new();
if let Some(arg) = args.next() { if let Some(arg) = args.next() {
formatted.extend(arg) formatted.extend(arg)
} }
for arg in args { for arg in args {
formatted.push(b' '); formatted.push(b' ');
formatted.extend(arg) formatted.extend(arg)
} }
formatted formatted
} }

rust/rhg/src/main.rs

mod color; mod color;
mod error; mod error;
mod ui; mod ui;
pub mod utils { pub mod utils {
pub mod path_utils; pub mod path_utils;
} }
fn main_with_result( fn main_with_result(
argv: Vec<OsString>,
process_start_time: &blackbox::ProcessStartTime, process_start_time: &blackbox::ProcessStartTime,
ui: &ui::Ui, ui: &ui::Ui,
repo: Result<&Repo, &NoRepoInCwdError>, repo: Result<&Repo, &NoRepoInCwdError>,
config: &Config, config: &Config,
) -> Result<(), CommandError> { ) -> Result<(), CommandError> {
check_unsupported(config, repo)?; check_unsupported(config, repo)?;
let app = App::new("rhg") let app = App::new("rhg")
.long("--color") .long("--color")
.value_name("TYPE") .value_name("TYPE")
.takes_value(true) .takes_value(true)
.global(true), .global(true),
) )
.version("0.0.1"); .version("0.0.1");
let app = add_subcommand_args(app); let app = add_subcommand_args(app);
let matches = app.clone().get_matches_safe()?; let matches = app.clone().get_matches_from_safe(argv.iter())?;
let (subcommand_name, subcommand_matches) = matches.subcommand(); let (subcommand_name, subcommand_matches) = matches.subcommand();
// Mercurial allows users to define "defaults" for commands, fallback // Mercurial allows users to define "defaults" for commands, fallback
// if a default is detected for the current command // if a default is detected for the current command
let defaults = config.get_str(b"defaults", subcommand_name.as_bytes()); let defaults = config.get_str(b"defaults", subcommand_name.as_bytes());
if defaults?.is_some() { if defaults?.is_some() {
let msg = "`defaults` config set"; let msg = "`defaults` config set";
let msg = "subrepos (.hgsub is present)"; let msg = "subrepos (.hgsub is present)";
return Err(CommandError::unsupported(msg)); return Err(CommandError::unsupported(msg));
} }
} }
if config.is_extension_enabled(b"blackbox") { if config.is_extension_enabled(b"blackbox") {
let blackbox = let blackbox =
blackbox::Blackbox::new(&invocation, process_start_time)?; blackbox::Blackbox::new(&invocation, process_start_time)?;
blackbox.log_command_start(); blackbox.log_command_start(argv.iter());
let result = run(&invocation); let result = run(&invocation);
blackbox.log_command_end(exit_code( blackbox.log_command_end(argv.iter(), exit_code(
&result, &result,
// TODO: show a warning or combine with original error if // TODO: show a warning or combine with original error if
// `get_bool` returns an error // `get_bool` returns an error
config config
.get_bool(b"ui", b"detailed-exit-code") .get_bool(b"ui", b"detailed-exit-code")
.unwrap_or(false), .unwrap_or(false),
)); ));
result result
} else { } else {
run(&invocation) run(&invocation)
} }
} }
fn main() { fn rhg_main(argv: Vec<OsString>) -> ! {
// Run this first, before we find out if the blackbox extension is even // Run this first, before we find out if the blackbox extension is even
// enabled, in order to include everything in-between in the duration // enabled, in order to include everything in-between in the duration
// measurements. Reading config files can be slow if they’re on NFS. // measurements. Reading config files can be slow if they’re on NFS.
let process_start_time = blackbox::ProcessStartTime::now(); let process_start_time = blackbox::ProcessStartTime::now();
env_logger::init(); env_logger::init();
let early_args = EarlyArgs::parse(std::env::args_os()); let early_args = EarlyArgs::parse(&argv);
let initial_current_dir = early_args.cwd.map(|cwd| { let initial_current_dir = early_args.cwd.map(|cwd| {
let cwd = get_path_from_bytes(&cwd); let cwd = get_path_from_bytes(&cwd);
std::env::current_dir() std::env::current_dir()
.and_then(|initial| { .and_then(|initial| {
std::env::set_current_dir(cwd)?; std::env::set_current_dir(cwd)?;
Ok(initial) Ok(initial)
}) })
.unwrap_or_else(|error| { .unwrap_or_else(|error| {
exit( exit(
&argv,
&None, &None,
&Ui::new_infallible(&Config::empty()), &Ui::new_infallible(&Config::empty()),
OnUnsupported::Abort, OnUnsupported::Abort,
Err(CommandError::abort(format!( Err(CommandError::abort(format!(
"abort: {}: '{}'", "abort: {}: '{}'",
error, error,
cwd.display() cwd.display()
))), ))),
false, false,
) )
}) })
}); });
let mut non_repo_config = let mut non_repo_config =
Config::load_non_repo().unwrap_or_else(|error| { Config::load_non_repo().unwrap_or_else(|error| {
// Normally this is decided based on config, but we don’t have that // Normally this is decided based on config, but we don’t have that
// available. As of this writing config loading never returns an // available. As of this writing config loading never returns an
// "unsupported" error but that is not enforced by the type system. // "unsupported" error but that is not enforced by the type system.
let on_unsupported = OnUnsupported::Abort; let on_unsupported = OnUnsupported::Abort;
exit( exit(
&argv,
&initial_current_dir, &initial_current_dir,
&Ui::new_infallible(&Config::empty()), &Ui::new_infallible(&Config::empty()),
on_unsupported, on_unsupported,
Err(error.into()), Err(error.into()),
false, false,
) )
}); });
non_repo_config non_repo_config
.load_cli_args(early_args.config, early_args.color) .load_cli_args(early_args.config, early_args.color)
.unwrap_or_else(|error| { .unwrap_or_else(|error| {
exit( exit(
&argv,
&initial_current_dir, &initial_current_dir,
&Ui::new_infallible(&non_repo_config), &Ui::new_infallible(&non_repo_config),
OnUnsupported::from_config(&non_repo_config), OnUnsupported::from_config(&non_repo_config),
Err(error.into()), Err(error.into()),
non_repo_config non_repo_config
.get_bool(b"ui", b"detailed-exit-code") .get_bool(b"ui", b"detailed-exit-code")
.unwrap_or(false), .unwrap_or(false),
) )
}); });
if let Some(repo_path_bytes) = &early_args.repo { if let Some(repo_path_bytes) = &early_args.repo {
lazy_static::lazy_static! { lazy_static::lazy_static! {
static ref SCHEME_RE: regex::bytes::Regex = static ref SCHEME_RE: regex::bytes::Regex =
// Same as `_matchscheme` in `mercurial/util.py` // Same as `_matchscheme` in `mercurial/util.py`
regex::bytes::Regex::new("^[a-zA-Z0-9+.\\-]+:").unwrap(); regex::bytes::Regex::new("^[a-zA-Z0-9+.\\-]+:").unwrap();
} }
if SCHEME_RE.is_match(&repo_path_bytes) { if SCHEME_RE.is_match(&repo_path_bytes) {
exit( exit(
&argv,
&initial_current_dir, &initial_current_dir,
&Ui::new_infallible(&non_repo_config), &Ui::new_infallible(&non_repo_config),
OnUnsupported::from_config(&non_repo_config), OnUnsupported::from_config(&non_repo_config),
Err(CommandError::UnsupportedFeature { Err(CommandError::UnsupportedFeature {
message: format_bytes!( message: format_bytes!(
b"URL-like --repository {}", b"URL-like --repository {}",
repo_path_bytes repo_path_bytes
), ),
let repo_result = match Repo::find(&non_repo_config, repo_path.to_owned()) let repo_result = match Repo::find(&non_repo_config, repo_path.to_owned())
{ {
Ok(repo) => Ok(repo), Ok(repo) => Ok(repo),
Err(RepoError::NotFound { at }) if repo_path.is_none() => { Err(RepoError::NotFound { at }) if repo_path.is_none() => {
// Not finding a repo is not fatal yet, if `-R` was not given // Not finding a repo is not fatal yet, if `-R` was not given
Err(NoRepoInCwdError { cwd: at }) Err(NoRepoInCwdError { cwd: at })
} }
Err(error) => exit( Err(error) => exit(
&argv,
&initial_current_dir, &initial_current_dir,
&Ui::new_infallible(&non_repo_config), &Ui::new_infallible(&non_repo_config),
OnUnsupported::from_config(&non_repo_config), OnUnsupported::from_config(&non_repo_config),
Err(error.into()), Err(error.into()),
// TODO: show a warning or combine with original error if // TODO: show a warning or combine with original error if
// `get_bool` returns an error // `get_bool` returns an error
non_repo_config non_repo_config
.get_bool(b"ui", b"detailed-exit-code") .get_bool(b"ui", b"detailed-exit-code")
.unwrap_or(false), .unwrap_or(false),
), ),
}; };
let config = if let Ok(repo) = &repo_result { let config = if let Ok(repo) = &repo_result {
repo.config() repo.config()
} else { } else {
&non_repo_config &non_repo_config
}; };
let ui = Ui::new(&config).unwrap_or_else(|error| { let ui = Ui::new(&config).unwrap_or_else(|error| {
exit( exit(
&argv,
&initial_current_dir, &initial_current_dir,
&Ui::new_infallible(&config), &Ui::new_infallible(&config),
OnUnsupported::from_config(&config), OnUnsupported::from_config(&config),
Err(error.into()), Err(error.into()),
config config
.get_bool(b"ui", b"detailed-exit-code") .get_bool(b"ui", b"detailed-exit-code")
.unwrap_or(false), .unwrap_or(false),
) )
}); });
let on_unsupported = OnUnsupported::from_config(config); let on_unsupported = OnUnsupported::from_config(config);
let result = main_with_result( let result = main_with_result(
argv.iter().map(|s| s.to_owned()).collect(),
&process_start_time, &process_start_time,
&ui, &ui,
repo_result.as_ref(), repo_result.as_ref(),
config, config,
); );
exit( exit(
&argv,
&initial_current_dir, &initial_current_dir,
&ui, &ui,
on_unsupported, on_unsupported,
result, result,
// TODO: show a warning or combine with original error if `get_bool` // TODO: show a warning or combine with original error if `get_bool`
// returns an error // returns an error
config config
.get_bool(b"ui", b"detailed-exit-code") .get_bool(b"ui", b"detailed-exit-code")
.unwrap_or(false), .unwrap_or(false),
) )
} }
fn main() -> ! {
rhg_main(std::env::args_os().collect())
}
fn exit_code( fn exit_code(
result: &Result<(), CommandError>, result: &Result<(), CommandError>,
use_detailed_exit_code: bool, use_detailed_exit_code: bool,
) -> i32 { ) -> i32 {
match result { match result {
Ok(()) => exit_codes::OK, Ok(()) => exit_codes::OK,
Err(CommandError::Abort { Err(CommandError::Abort {
message: _, message: _,
// Exit with a specific code and no error message to let a potential // Exit with a specific code and no error message to let a potential
// wrapper script fallback to Python-based Mercurial. // wrapper script fallback to Python-based Mercurial.
Err(CommandError::UnsupportedFeature { .. }) => { Err(CommandError::UnsupportedFeature { .. }) => {
exit_codes::UNIMPLEMENTED exit_codes::UNIMPLEMENTED
} }
} }
} }
fn exit( fn exit<'a>(
original_args: &'a [OsString],
AlphareUnsubmitted
Not Done

Nitpicking: why is this "b? This implies some other 'a lifetime by convention.

Alphare: Nitpicking: why is this `"b`? This implies some other `'a` lifetime by convention.
aalekseyevAuthorUnsubmitted
Done

Changed it now.
(I guess the name 'a must have been taken when I wrote it. \shrug)

aalekseyev: Changed it now. (I guess the name `'a` must have been taken when I wrote it. \shrug)
initial_current_dir: &Option<PathBuf>, initial_current_dir: &Option<PathBuf>,
ui: &Ui, ui: &Ui,
mut on_unsupported: OnUnsupported, mut on_unsupported: OnUnsupported,
result: Result<(), CommandError>, result: Result<(), CommandError>,
use_detailed_exit_code: bool, use_detailed_exit_code: bool,
) -> ! { ) -> ! {
if let ( if let (
OnUnsupported::Fallback { executable }, OnUnsupported::Fallback { executable },
Err(CommandError::UnsupportedFeature { message }), Err(CommandError::UnsupportedFeature { message }),
) = (&on_unsupported, &result) ) = (&on_unsupported, &result)
{ {
let mut args = std::env::args_os(); let mut args = original_args.iter();
let executable = match executable { let executable = match executable {
None => { None => {
exit_no_fallback( exit_no_fallback(
ui, ui,
OnUnsupported::Abort, OnUnsupported::Abort,
Err(CommandError::abort( Err(CommandError::abort(
"abort: 'rhg.on-unsupported=fallback' without \ "abort: 'rhg.on-unsupported=fallback' without \
'rhg.fallback-executable' set.", 'rhg.fallback-executable' set.",
color: Option<Vec<u8>>, color: Option<Vec<u8>>,
/// Value of the `-R` or `--repository` argument, if any. /// Value of the `-R` or `--repository` argument, if any.
repo: Option<Vec<u8>>, repo: Option<Vec<u8>>,
/// Value of the `--cwd` argument, if any. /// Value of the `--cwd` argument, if any.
cwd: Option<Vec<u8>>, cwd: Option<Vec<u8>>,
} }
impl EarlyArgs { impl EarlyArgs {
fn parse(args: impl IntoIterator<Item = OsString>) -> Self { fn parse<'a>(args: impl IntoIterator<Item = &'a OsString>) -> Self {
let mut args = args.into_iter().map(get_bytes_from_os_str); let mut args = args.into_iter().map(get_bytes_from_os_str);
let mut config = Vec::new(); let mut config = Vec::new();
let mut color = None; let mut color = None;
let mut repo = None; let mut repo = None;
let mut cwd = None; let mut cwd = None;
// Use `while let` instead of `for` so that we can also call // Use `while let` instead of `for` so that we can also call
// `args.next()` inside the loop. // `args.next()` inside the loop.
while let Some(arg) = args.next() { while let Some(arg) = args.next() {