From 3a25b11aaa7b9626c7c2820bc1aab4d547ffde26 Mon Sep 17 00:00:00 2001 From: Matt Keeter Date: Fri, 15 May 2026 12:30:49 -0400 Subject: [PATCH] Construct ExecutionContext in humility-bin --- humility-bin/src/main.rs | 212 ++++++++++++++++++++++++++++++++++----- humility-cli/src/lib.rs | 188 +--------------------------------- 2 files changed, 191 insertions(+), 209 deletions(-) diff --git a/humility-bin/src/main.rs b/humility-bin/src/main.rs index f116589d..9b62ad28 100644 --- a/humility-bin/src/main.rs +++ b/humility-bin/src/main.rs @@ -2,9 +2,12 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use clap::{ArgGroup, CommandFactory, FromArgMatches, Parser}; +use clap::{ + ArgGroup, CommandFactory, FromArgMatches, Parser, parser::ValueSource, +}; mod cmd; -use humility_cli::{Cli, ExecutionContext}; +use humility::{msg, warn}; +use humility_cli::{Cli, ExecutionContext, env::Environment}; /// Main CLI entry point /// @@ -35,21 +38,195 @@ fn main() -> std::process::ExitCode { return std::process::ExitCode::FAILURE; } }; - let OuterCli { cli, cmd } = outer_cli; - if let Some(s) = version(&cli) { - println!("{s}"); + let OuterCli { mut cli, cmd } = outer_cli; + + // The --version (-V) and --list-targets operations are implemented as + // flags, rather than subcommands, so we'll special-case them here. + if cli.version { + println!( + "{} {} {}", + env!("CARGO_BIN_NAME"), + env!("CARGO_PKG_VERSION"), + PROBES_ENABLED + ); + if cmd.is_some() { + warn!("ignoring subcommand after printing version"); + } return std::process::ExitCode::SUCCESS; - }; + } else if cli.list_targets { + let env = cli.environment.as_ref().expect("checked by clap"); + let targets = match Environment::targets(env) { + Ok(targets) => targets, + Err(err) => { + msg!("failed to parse environment: {err:?}"); + return std::process::ExitCode::FAILURE; + } + }; + + if cli.terse { + println!( + "{}", + targets + .iter() + .map(|(t, _)| &**t) + .collect::>() + .join(", ") + ); + } else { + println!("{:15} DESCRIPTION", "TARGET"); + + for (target, description) in &targets { + println!( + "{:15} {}", + target, + match description { + Some(d) => d, + None => "-", + } + ); + } + } + + if cmd.is_some() { + warn!("ignoring subcommand after listing targets"); + } + return std::process::ExitCode::SUCCESS; + } let log_level = if cli.verbose { "trace" } else { "warn" }; - let mut context = match ExecutionContext::new(cli, &m) { - Ok(ctx) => ctx, - Err(e) => { - eprintln!("failed to build execution context: {e:#}"); - return std::process::ExitCode::FAILURE; + + // Before we do our processing, we need to check for the presence of + // environment variables on our mutually exclusive attach options + // (i.e., dump/probe/ip/target). Clap has support for options being + // set as environment variables, so why are we doing it here? We do + // this manually because Clap's behavior when an option has a + // specified environment variable is to treat a variable that is in + // the environment just as if the option had been set on the command + // line -- but that isn't actually what we want here: if an option is + // set on the command line, we want to ignore the fact that a + // different mutually exclusive option may be in the environment. So + // we only look at the environment if none of these is set (Clap + // assures that at most one is set on the command line), and we + // examine the environment in an order of precence: probe, ip, target, + // dump. If the user has specified no command line options but + // multiple of these in the environment, they will get the first in + // this ordering that we find. + if cli.dump.is_none() + && cli.probe.is_none() + && cli.ip.is_none() + && cli.target.is_none() + { + use std::env; + + if let Ok(e) = env::var("HUMILITY_PROBE") { + cli.probe = Some(e); + } else if let Ok(e) = env::var("HUMILITY_IP") { + cli.ip = Some(e.parse().unwrap_or_else(|x| match x {})); + } else if let Ok(e) = env::var("HUMILITY_TARGET") { + cli.target = Some(e); + } else if let Ok(e) = env::var("HUMILITY_DUMP") { + cli.dump = Some(e); + } + } + + let environment = match (&cli.environment, &cli.target) { + (Some(env), Some(target)) => { + let env = match Environment::from_file(env, target) { + Ok(e) => e, + Err(err) => { + msg!("failed to match environment: {err:?}"); + return std::process::ExitCode::FAILURE; + } + }; + + // Cannot specify a dump/probe/IP address and also an + // environment and target (enforced by clap) + assert!(cli.dump.is_none()); + assert!(cli.probe.is_none()); + assert!(cli.ip.is_none()); + + cli.probe = Some(env.probe.clone()); + + // + // If we have an archive on the command-line or in an environment + // variable, we want to prefer that over whatever is in the + // environment file -- but we also want to warn the user about + // what is going on. + // + if cli.archive.is_some() { + let msg = if m.value_source("archive") + == Some(clap::parser::ValueSource::CommandLine) + { + "archive on command-line" + } else { + "archive in environment variable" + }; + + warn!("{msg} overriding archive in environment file"); + } else { + cli.archive = match env.archive(&cli.archive_name) { + Ok(a) => Some(a), + Err(e) => { + msg!("Failed to get archive: {e}"); + return std::process::ExitCode::FAILURE; + } + } + } + + Some(env) + } + + (Some(env), None) => { + if let Err(err) = Environment::validate(env) { + eprintln!("failed to parse environment: {err:?}"); + return std::process::ExitCode::FAILURE; + } + + None } + + _ => None, }; + // + // Check to see if we have both a dump and an archive. Because these + // conflict with one another but because we allow both of them to be + // set with an environment variable, we need to manually resolve this: + // we want to allow an explicitly set value (that is, via the command + // line) to win the conflict. + // + if cli.dump.is_some() && cli.archive.is_some() { + match ( + m.value_source("dump") == Some(ValueSource::CommandLine), + m.value_source("archive") == Some(ValueSource::CommandLine), + ) { + (true, true) => { + msg!("cannot specify both a dump and an archive"); + return std::process::ExitCode::FAILURE; + } + + (false, false) => { + msg!( + "both dump and archive have been set via environment \ + variables; unset one of them, or use a command-line \ + option to override" + ); + return std::process::ExitCode::FAILURE; + } + + (true, false) => { + warn!("dump on command-line overriding archive in environment"); + cli.archive = None; + } + + (false, true) => { + warn!("archive on command-line overriding dump in environment"); + cli.dump = None; + } + } + } + + let mut context = ExecutionContext { environment, cli }; let Some(cmd) = cmd else { eprintln!("humility failed: subcommand expected (--help to list)"); return std::process::ExitCode::FAILURE; @@ -74,19 +251,6 @@ static PROBES_ENABLED: &str = ""; #[cfg(not(feature = "probes"))] static PROBES_ENABLED: &str = "probeless"; -pub(crate) fn version(cli: &Cli) -> Option { - if cli.version { - Some(format!( - "{} {} {}", - env!("CARGO_BIN_NAME"), - env!("CARGO_PKG_VERSION"), - PROBES_ENABLED - )) - } else { - None - } -} - #[test] fn validate_clap() { OuterCli::command().debug_assert() diff --git a/humility-cli/src/lib.rs b/humility-cli/src/lib.rs index 7ed9893e..49d86c46 100644 --- a/humility-cli/src/lib.rs +++ b/humility-cli/src/lib.rs @@ -4,13 +4,13 @@ pub mod env; -use anyhow::{Context, Result, anyhow, bail}; -use clap::{ArgMatches, Parser, parser::ValueSource}; +use anyhow::{Result, anyhow, bail}; +use clap::Parser; use env::Environment; use humility::{ core::Core, hubris::{HubrisArchive, HubrisValidate}, - msg, net, warn, + net, }; use std::time::Duration; @@ -351,185 +351,3 @@ pub struct ExecutionContext { pub environment: Option, pub cli: Cli, } - -impl ExecutionContext { - pub fn new(mut cli: Cli, m: &ArgMatches) -> Result { - // - // Before we do our processing, we need to check for the presence of - // environment variables on our mutually exclusive attach options - // (i.e., dump/probe/ip/target). Clap has support for options being - // set as environment variables, so why are we doing it here? We do - // this manually because Clap's behavior when an option has a - // specified environment variable is to treat a variable that is in - // the environment just as if the option had been set on the command - // line -- but that isn't actually what we want here: if an option is - // set on the command line, we want to ignore the fact that a - // different mutually exclusive option may be in the environment. So - // we only look at the environment if none of these is set (Clap - // assures that at most one is set on the command line), and we - // examine the environment in an order of precence: probe, ip, target, - // dump. If the user has specified no command line options but - // multiple of these in the environment, they will get the first in - // this ordering that we find. - // - if cli.dump.is_none() - && cli.probe.is_none() - && cli.ip.is_none() - && cli.target.is_none() - { - use std::env; - - if let Ok(e) = env::var("HUMILITY_PROBE") { - cli.probe = Some(e); - } else if let Ok(e) = env::var("HUMILITY_IP") { - cli.ip = - Some(e.parse().context("could not parse HUMILITY_IP")?); - } else if let Ok(e) = env::var("HUMILITY_TARGET") { - cli.target = Some(e); - } else if let Ok(e) = env::var("HUMILITY_DUMP") { - cli.dump = Some(e); - } - } - - let environment = match (&cli.environment, &cli.target) { - (Some(env), Some(target)) => { - let env = match Environment::from_file(env, target) { - Ok(e) => e, - Err(err) => { - msg!("failed to match environment: {err:?}"); - std::process::exit(1); - } - }; - - // - // Cannot specify a dump/probe/IP address and also an - // environment and target - // - assert!(cli.dump.is_none()); - assert!(cli.probe.is_none()); - assert!(cli.ip.is_none()); - - cli.probe = Some(env.probe.clone()); - - // - // If we have an archive on the command-line or in an environment - // variable, we want to prefer that over whatever is in the - // environment file -- but we also want to warn the user about - // what is going on. - // - if cli.archive.is_some() { - let msg = if m.value_source("archive") - == Some(ValueSource::CommandLine) - { - "archive on command-line" - } else { - "archive in environment variable" - }; - - warn!("{} overriding archive in environment file", msg); - } else { - cli.archive = match env.archive(&cli.archive_name) { - Ok(a) => Some(a), - Err(e) => { - msg!("Failed to get archive: {e}"); - std::process::exit(1); - } - } - } - - Some(env) - } - - (Some(env), None) => { - if cli.list_targets { - let targets = match Environment::targets(env) { - Ok(targets) => targets, - Err(err) => { - msg!("failed to parse environment: {err:?}"); - std::process::exit(1); - } - }; - - if cli.terse { - println!( - "{}", - targets - .iter() - .map(|(t, _)| &**t) - .collect::>() - .join(", ") - ); - } else { - println!("{:15} DESCRIPTION", "TARGET"); - - for (target, description) in &targets { - println!( - "{:15} {}", - target, - match description { - Some(d) => d, - None => "-", - } - ); - } - } - - std::process::exit(0); - } - - if let Err(err) = Environment::validate(env) { - eprintln!("failed to parse environment: {:?}", err); - std::process::exit(1); - } - - None - } - - _ => None, - }; - - // - // Check to see if we have both a dump and an archive. Because these - // conflict with one another but because we allow both of them to be - // set with an environment variable, we need to manually resolve this: - // we want to allow an explicitly set value (that is, via the command - // line) to win the conflict. - // - if cli.dump.is_some() && cli.archive.is_some() { - match ( - m.value_source("dump") == Some(ValueSource::CommandLine), - m.value_source("archive") == Some(ValueSource::CommandLine), - ) { - (true, true) => { - msg!("cannot specify both a dump and an archive"); - std::process::exit(1); - } - - (false, false) => { - msg!( - "both dump and archive have been set via environment \ - variables; unset one of them, or use a command-line \ - option to override" - ); - std::process::exit(1); - } - - (true, false) => { - warn!( - "dump on command-line overriding archive in environment" - ); - cli.archive = None; - } - - (false, true) => { - warn!( - "archive on command-line overriding dump in environment" - ); - cli.dump = None; - } - } - } - - Ok(ExecutionContext { environment, cli }) - } -}