Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a config file format like ssh's #25

Merged
merged 8 commits into from
Dec 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 0 additions & 28 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ derive-deftly = "0.14.2"
dirs = "5.0.1"
dns-lookup = "2.0.4"
expanduser = "1.2.2"
figment = { version = "0.10.19", features = ["toml"] }
figment = { version = "0.10.19" }
futures-util = { version = "0.3.31", default-features = false }
gethostname = "0.5.0"
glob = "0.3.1"
Expand Down Expand Up @@ -74,7 +74,6 @@ rand = "0.8.5"
serde_json = "1.0.133"
serde_test = "1.0.177"
tempfile = "3.14.0"
toml = "0.8.19"

[lints.rust]
dead_code = "warn"
Expand Down
29 changes: 19 additions & 10 deletions src/cli/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use clap::{ArgAction::SetTrue, Args as _, FromArgMatches as _, Parser};
use crate::{config::Manager, util::AddressFamily};

/// Options that switch us into another mode i.e. which don't require source/destination arguments
pub(crate) const MODE_OPTIONS: &[&str] = &["server", "help_buffers", "show_config", "config_files"];
pub(crate) const MODE_OPTIONS: &[&str] = &["server", "help_buffers", "config_files", "show_config"];

#[derive(Debug, Parser, Clone)]
#[command(
Expand Down Expand Up @@ -44,7 +44,14 @@ pub(crate) struct CliArgs {
)]
pub server: bool,

/// Outputs the configuration, then exits
/// Outputs the configuration, then exits.
///
/// If a remote `SOURCE` or `DESTINATION` argument is given, outputs the configuration we would use
/// for operations to that host.
///
/// If not, outputs only global settings from configuration, which may be overridden in
/// `Host` blocks in configuration files.
///
#[arg(long, help_heading("Configuration"))]
pub show_config: bool,
/// Outputs the paths to configuration file(s), then exits
Expand Down Expand Up @@ -93,20 +100,22 @@ impl CliArgs {
CliArgs::from_arg_matches(&cli.get_matches_from(std::env::args_os())).unwrap();
// Custom logic: '-4' and '-6' convenience aliases
if args.ipv4_alias__ {
args.config.address_family = Some(AddressFamily::V4);
args.config.address_family = Some(AddressFamily::Inet);
} else if args.ipv6_alias__ {
args.config.address_family = Some(AddressFamily::V6);
args.config.address_family = Some(AddressFamily::Inet6);
}
args
}
}

impl From<&CliArgs> for Manager {
/// Merge options from the CLI into the structure.
/// Any new option packs (_Optional structs) need to be added here.
fn from(value: &CliArgs) -> Self {
let mut mgr = Manager::new();
impl TryFrom<&CliArgs> for Manager {
type Error = anyhow::Error;

fn try_from(value: &CliArgs) -> Result<Self, Self::Error> {
let host = value.client_params.remote_host_lossy()?;

let mut mgr = Manager::standard(host.as_deref());
mgr.merge_provider(&value.config);
mgr
Ok(mgr)
}
}
28 changes: 11 additions & 17 deletions src/cli/cli_main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@

use std::process::ExitCode;

use super::{
args::CliArgs,
styles::{ERROR_S, INFO_S},
};
use super::{args::CliArgs, styles::ERROR_S};
use crate::{
client::{client_main, Parameters as ClientParameters, MAX_UPDATE_FPS},
config::{Configuration, Manager},
Expand Down Expand Up @@ -58,19 +55,19 @@ pub async fn cli() -> anyhow::Result<ExitCode> {
}

// Now fold the arguments in with the CLI config (which may fail)
let config_manager = Manager::from(&args);
let config_manager = match Manager::try_from(&args) {
Ok(m) => m,
Err(err) => {
eprintln!("{}: {err}", "ERROR".style(*ERROR_S));
return Ok(ExitCode::FAILURE);
}
};

let config = match config_manager.get::<Configuration>() {
Ok(c) => c,
Err(err) => {
println!("{}: Failed to parse configuration", "ERROR".style(*ERROR_S));
if err.count() == 1 {
println!("{err}");
} else {
for (i, e) in err.into_iter().enumerate() {
println!("{}: {e}", (i + 1).style(*INFO_S));
}
}
eprintln!("{}: Failed to parse configuration", "ERROR".style(*ERROR_S));
err.into_iter().for_each(|e| eprintln!("{e}"));
return Ok(ExitCode::FAILURE);
}
};
Expand All @@ -84,10 +81,7 @@ pub async fn cli() -> anyhow::Result<ExitCode> {
.inspect_err(|e| eprintln!("{e:?}"))?;

if args.show_config {
println!(
"{}",
config_manager.to_display_adapter::<Configuration>(true)
);
println!("{}", config_manager.to_display_adapter::<Configuration>());
Ok(ExitCode::SUCCESS)
} else if args.server {
let _span = error_span!("REMOTE").entered();
Expand Down
30 changes: 26 additions & 4 deletions src/client/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ pub struct Parameters {
///
/// Exactly one of source and destination must be remote.
#[arg(
conflicts_with_all(crate::cli::MODE_OPTIONS),
required = true,
required_unless_present_any(crate::cli::MODE_OPTIONS),
value_name = "SOURCE"
)]
pub source: Option<FileSpec>,
Expand All @@ -66,8 +65,7 @@ pub struct Parameters {
///
/// Exactly one of source and destination must be remote.
#[arg(
conflicts_with_all(crate::cli::MODE_OPTIONS),
required = true,
required_unless_present_any(crate::cli::MODE_OPTIONS),
value_name = "DESTINATION"
)]
pub destination: Option<FileSpec>,
Expand Down Expand Up @@ -98,3 +96,27 @@ impl TryFrom<&Parameters> for CopyJobSpec {
})
}
}

impl Parameters {
/// A best-effort attempt to extract a single remote host string from the parameters.
///
/// # Output
/// If neither source nor dest are present, Ok("")
/// If at most one of source and dest contains a remote host, Ok(<host>)
///
/// # Errors
/// If both source and dest contain a remote host, Err("Only one remote file argument is supported")
pub(crate) fn remote_host_lossy(&self) -> anyhow::Result<Option<String>> {
let src_host = self.source.as_ref().and_then(|fs| fs.host.as_ref());
let dst_host = self.destination.as_ref().and_then(|fs| fs.host.as_ref());
Ok(if let Some(src_host) = src_host {
if dst_host.is_some() {
anyhow::bail!("Only one remote file argument is supported");
}
Some(src_host.to_string())
} else {
// Destination without source would be an exotic situation, but do our best anyway:
dst_host.map(std::string::ToString::to_string)
})
}
}
4 changes: 2 additions & 2 deletions src/client/ssh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl ConfigFile {
}
return None;
}
let mut parser = match Parser::for_path(path, self.user) {
let parser = match Parser::for_path(path, self.user) {
Ok(p) => p,
Err(e) => {
// file permissions issue?
Expand All @@ -52,7 +52,7 @@ impl ConfigFile {
}
};
let data = match parser
.parse_file_for(host)
.parse_file_for(Some(host))
.with_context(|| format!("reading configuration file {path:?}"))
{
Ok(data) => data,
Expand Down
Loading
Loading