From 0a8317c35f8f0153b851e1fb214d87662be638b8 Mon Sep 17 00:00:00 2001 From: Lorenz Leutgeb Date: Thu, 11 Sep 2025 10:02:51 +0200 Subject: [PATCH] remote-helper: Interpret verbosity option The `verbosity` option is ignored, which makes it more difficult to obtain debugging information if the "internal" push, i.e., the `git push` invocation by `git-remote-rad` fails. Provide two types for verbosity, and wire them up. --- .../src/commands/patch/checkout.rs | 2 +- crates/radicle-remote-helper/src/fetch.rs | 5 +- crates/radicle-remote-helper/src/lib.rs | 49 +++++++++++++++++-- crates/radicle-remote-helper/src/push.rs | 40 +++++++++------ crates/radicle/src/git.rs | 40 +++++++++++++-- 5 files changed, 111 insertions(+), 25 deletions(-) diff --git a/crates/radicle-cli/src/commands/patch/checkout.rs b/crates/radicle-cli/src/commands/patch/checkout.rs index 03c35a5f..dd9aeffb 100644 --- a/crates/radicle-cli/src/commands/patch/checkout.rs +++ b/crates/radicle-cli/src/commands/patch/checkout.rs @@ -132,7 +132,7 @@ fn find_patch_commit<'a>( match working.find_commit(head) { Ok(commit) => Ok(commit), Err(e) if git::ext::is_not_found_err(&e) => { - git::process::fetch_local(workdir, stored, [head.into()])?; + git::process::fetch_local(workdir, stored, [head.into()], git::Verbosity::default())?; working.find_commit(head).map_err(|e| e.into()) } Err(e) => Err(e.into()), diff --git a/crates/radicle-remote-helper/src/fetch.rs b/crates/radicle-remote-helper/src/fetch.rs index d8b625ac..7c2feb4c 100644 --- a/crates/radicle-remote-helper/src/fetch.rs +++ b/crates/radicle-remote-helper/src/fetch.rs @@ -7,7 +7,7 @@ use thiserror::Error; use radicle::git; use radicle::storage::ReadRepository; -use crate::read_line; +use crate::{read_line, Verbosity}; #[derive(Debug, Error)] pub enum Error { @@ -31,6 +31,7 @@ pub fn run( working: &Path, stored: R, stdin: &io::Stdin, + verbosity: Verbosity, ) -> Result<(), Error> { // Read all the `fetch` lines. let mut line = String::new(); @@ -62,7 +63,7 @@ pub fn run( // used in the working copy, this will always result in the object // missing. This seems to only be an issue with `libgit2`/`git2` // and not `git` itself. - git::process::fetch_local(working, &stored, oids)?; + git::process::fetch_local(working, &stored, oids, verbosity.into())?; // Nb. An empty line means we're done. println!(); diff --git a/crates/radicle-remote-helper/src/lib.rs b/crates/radicle-remote-helper/src/lib.rs index 7f6a87cd..817e25bd 100644 --- a/crates/radicle-remote-helper/src/lib.rs +++ b/crates/radicle-remote-helper/src/lib.rs @@ -70,6 +70,40 @@ pub enum Error { List(#[from] list::Error), } +/// Models values for the `verbosity` option, see +/// . +#[derive(Copy, Clone, Debug)] +struct Verbosity(u8); + +impl From for radicle::git::Verbosity { + /// Converts the verbosity option passed to a Git remote helper to + /// one that can be passed to other Git commands via command line. + /// Note that these scales are one off: While the default verbosity + /// for remote helpers is 1, the default verbosity via command line + /// (omitting the flag) is 0. + /// This implementation also cuts off verbosities greater than [`i8::MAX`]. + fn from(val: Verbosity) -> Self { + radicle::git::Verbosity::from(i8::try_from(val.0).unwrap_or(i8::MAX) - 1) + } +} + +/// The documentation on Git remote helpers, see +/// +/// says: "1 is the default level of verbosity". +impl Default for Verbosity { + fn default() -> Self { + Self(1) + } +} + +impl FromStr for Verbosity { + type Err = std::num::ParseIntError; + + fn from_str(s: &str) -> Result { + u8::from_str(s).map(Self) + } +} + #[derive(Debug, Default, Clone)] pub struct Options { /// Don't sync after push. @@ -84,6 +118,7 @@ pub struct Options { base: Option, /// Patch message. message: cli::patch::Message, + verbosity: Verbosity, } /// Run the radicle remote helper using the given profile. @@ -139,9 +174,15 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { println!("fetch"); println!(); } - ["option", "verbosity"] => { - println!("ok"); - } + ["option", "verbosity", verbosity] => match verbosity.parse::() { + Ok(verbosity) => { + opts.verbosity = verbosity; + println!("ok"); + } + Err(err) => { + println!("error {err}"); + } + }, ["option", "push-option", args @ ..] => { // Nb. Git documentation says that we can print `error ` or `unsupported` // for options that are not supported, but this results in Git saying that @@ -167,7 +208,7 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { path: working.clone(), })?; - return fetch::run(vec![(oid, refstr)], working, stored, &stdin) + return fetch::run(vec![(oid, refstr)], working, stored, &stdin, opts.verbosity) .map_err(Error::from); } ["push", refspec] => { diff --git a/crates/radicle-remote-helper/src/push.rs b/crates/radicle-remote-helper/src/push.rs index c41770a8..c280bb20 100644 --- a/crates/radicle-remote-helper/src/push.rs +++ b/crates/radicle-remote-helper/src/push.rs @@ -31,7 +31,7 @@ use radicle::{git, rad}; use radicle_cli as cli; use radicle_cli::terminal as term; -use crate::{hint, read_line, Options}; +use crate::{hint, read_line, Options, Verbosity}; #[derive(Debug, Error)] pub enum Error { @@ -346,8 +346,17 @@ pub fn run( let rules = crefs.rules(); let me = Did::from(nid); - let explorer = - push(src, &dst, *force, &nid, &working, stored, patches, &signer)?; + let explorer = push( + src, + &dst, + *force, + &nid, + &working, + stored, + patches, + &signer, + opts.verbosity, + )?; // If we're trying to update the canonical head, make sure // we don't diverge from the current head. This only applies // to repos with more than one delegate. @@ -487,7 +496,7 @@ where // // In case the reference is not properly deleted, the next attempt to open a patch should // not fail, since the reference will already exist with the correct OID. - push_ref(src, &dst, false, working, stored.raw())?; + push_ref(src, &dst, false, working, stored.raw(), opts.verbosity)?; let (_, target) = stored.canonical_head()?; let base = if let Some(base) = opts.base { @@ -601,7 +610,7 @@ where let commit = *src; let dst = dst.with_namespace(nid.into()); - push_ref(src, &dst, force, working, stored.raw())?; + push_ref(src, &dst, force, working, stored.raw(), opts.verbosity)?; let Ok(Some(patch)) = patches.get(&patch_id) else { return Err(Error::NotFound(patch_id)); @@ -671,6 +680,7 @@ fn push( cob::cache::StoreWriter, >, signer: &Device, + verbosity: Verbosity, ) -> Result, Error> where G: crypto::signature::Signer, @@ -680,7 +690,7 @@ where // It's ok for the destination reference to be unknown, eg. when pushing a new branch. let old = stored.backend.find_reference(dst.as_str()).ok(); - push_ref(src, &dst, force, working, stored.raw())?; + push_ref(src, &dst, force, working, stored.raw(), verbosity)?; if let Some(old) = old { let proj = stored.project()?; @@ -863,22 +873,22 @@ fn push_ref( force: bool, working: &git::raw::Repository, stored: &git::raw::Repository, + verbosity: Verbosity, ) -> Result<(), Error> { let url = git::url::File::new(stored.path()).to_string(); // Nb. The *force* indicator (`+`) is processed by Git tooling before we even reach this code. // This happens during the `list for-push` phase. let refspec = git::Refspec { src, dst, force }; let repo = working.workdir().unwrap_or_else(|| working.path()); + + let mut args = vec!["push".to_string()]; - radicle::git::run::<_, _, &str, &str>( - repo, - [ - "push", - url.to_string().as_str(), - refspec.to_string().as_str(), - ], - [], - ) + let verbosity: git::Verbosity = verbosity.into(); + args.extend(verbosity.into_flag()); + + args.extend([url.to_string(), refspec.to_string()]); + + radicle::git::run::<_, _, &str, &str>(repo, args, []) .map_err(|err| { Error::Io(std::io::Error::other(format!( "failed to run `git push {url} {refspec}` in {:?}: {err}", diff --git a/crates/radicle/src/git.rs b/crates/radicle/src/git.rs index 89465cf9..17c0c1a8 100644 --- a/crates/radicle/src/git.rs +++ b/crates/radicle/src/git.rs @@ -54,6 +54,38 @@ impl std::fmt::Display for Version { } } +/// Verbosity level for Git commands. +#[derive(Default, Clone, Copy)] +pub struct Verbosity(i8); + +impl Verbosity { + /// Transform into a command line flag, helpful for passing to invocations + /// of `git`. + /// + /// See + pub fn into_flag(&self) -> Option { + const FLAG_PREFIX: &str = "-"; + const FLAG_QUIET: &str = "q"; + const FLAG_VERBOSE: &str = "v"; + + let repetitions = self.0.unsigned_abs() as usize; + + if repetitions == 0 { + return None; + } + + let flag = if self.0 > 0 { FLAG_VERBOSE } else { FLAG_QUIET }; + + Some(FLAG_PREFIX.to_string() + &flag.repeat(repetitions)) + } +} + +impl From for Verbosity { + fn from(v: i8) -> Self { + Self(v) + } +} + #[derive(thiserror::Error, Debug)] pub enum VersionError { #[error("malformed git version string")] @@ -761,7 +793,7 @@ pub mod process { use crate::storage::ReadRepository; - use super::{run, url, Oid}; + use super::{run, url, Oid, Verbosity}; /// Perform a local fetch, i.e. `file://`. /// @@ -771,16 +803,18 @@ pub mod process { working: &Path, storage: &R, oids: impl IntoIterator, + verbosity: Verbosity, ) -> Result<(), io::Error> where R: ReadRepository, { let mut fetch = vec![ "fetch".to_string(), - url::File::new(storage.path()).to_string(), - // N.b. avoid writing fetch head since we're only fetching objects + // Avoid writing fetch head since we're only fetching objects "--no-write-fetch-head".to_string(), ]; + fetch.extend(verbosity.into_flag()); + fetch.push(url::File::new(storage.path()).to_string()); fetch.extend(oids.into_iter().map(|oid| oid.to_string())); // N.b. `.` is used since we're fetching within the working copy run::<_, _, &str, &str>(working, fetch, [])?;