diff --git a/crates/radicle-remote-helper/src/fetch.rs b/crates/radicle-remote-helper/src/fetch.rs index 1c3829dc..e0caab57 100644 --- a/crates/radicle-remote-helper/src/fetch.rs +++ b/crates/radicle-remote-helper/src/fetch.rs @@ -4,8 +4,8 @@ use std::{io, process::ExitStatus}; use thiserror::Error; use radicle::git; -use radicle::storage::ReadRepository; +use crate::service::GitService; use crate::{read_line, Verbosity}; #[derive(Debug, Error)] @@ -33,9 +33,10 @@ pub enum Error { } /// Run a git fetch command. -pub fn run( +pub(super) fn run( mut refs: Vec<(git::Oid, git::fmt::RefString)>, - stored: R, + stored: radicle::storage::git::Repository, + git: &G, stdin: &io::Stdin, verbosity: Verbosity, ) -> Result<(), Error> { @@ -58,7 +59,7 @@ pub fn run( } // Verify them and prepare the final refspecs. - let oids = refs.into_iter().map(|(oid, _)| oid); + let oids = refs.into_iter().map(|(oid, _)| oid).collect(); // Rely on the environment variable `GIT_DIR` pointing at the repository. let working = None; @@ -72,7 +73,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. - let output = git::process::fetch_pack(working, &stored, oids, verbosity.into())?; + let output = git.fetch_pack(working, &stored, oids, verbosity.into())?; if !output.status.success() { return Err(Error::FetchPackFailed { @@ -82,8 +83,5 @@ pub fn run( }); } - // Nb. An empty line means we're done. - println!(); - Ok(()) } diff --git a/crates/radicle-remote-helper/src/main.rs b/crates/radicle-remote-helper/src/main.rs index 860c56ae..06f52d9a 100644 --- a/crates/radicle-remote-helper/src/main.rs +++ b/crates/radicle-remote-helper/src/main.rs @@ -19,6 +19,7 @@ mod fetch; mod list; mod push; +mod service; use std::path::PathBuf; use std::process; @@ -241,6 +242,7 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { let mut line = String::new(); let mut opts = Options::default(); let mut expected_refs = Vec::new(); + let git = service::RealGitService; if let Err(e) = radicle::io::set_file_limit(4096) { if debug { @@ -290,15 +292,15 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { let oid = git::Oid::from_str(oid)?; let refstr = git::fmt::RefString::try_from(*refstr)?; - return Ok(fetch::run( - vec![(oid, refstr)], - stored, - &stdin, - opts.verbosity, - )?); + fetch::run(vec![(oid, refstr)], stored, &git, &stdin, opts.verbosity)?; + + // Nb. An empty line means we're done + println!(); + + return Ok(()); } ["push", refspec] => { - return Ok(push::run( + let output = push::run( vec![refspec.to_string()], remote, url, @@ -307,7 +309,15 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { &stdin, opts, &expected_refs, - )?); + &git, + )?; + + for line in output { + println!("{line}"); + } + println!(); + + return Ok(()); } ["list"] => { let refs = list::for_fetch(&url, &profile, &stored)?; diff --git a/crates/radicle-remote-helper/src/push.rs b/crates/radicle-remote-helper/src/push.rs index 7de690f2..a75b4e4b 100644 --- a/crates/radicle-remote-helper/src/push.rs +++ b/crates/radicle-remote-helper/src/push.rs @@ -31,6 +31,7 @@ use radicle::{git, rad}; use radicle_cli as cli; use radicle_cli::terminal as term; +use crate::service::GitService; use crate::{hint, read_line, warn, Options, Verbosity}; #[derive(Debug, Error)] @@ -252,7 +253,8 @@ pub fn run( stdin: &io::Stdin, opts: Options, expected_refs: &[String], -) -> Result<(), Error> { + git: &impl GitService, +) -> Result, Error> { // Don't allow push if either of these conditions is true: // // 1. Our key is not in ssh-agent, which means we won't be able to sign the refs. @@ -268,6 +270,7 @@ pub fn run( let mut line = String::new(); let mut ok = HashMap::new(); let hints = opts.hints || profile.hints(); + let mut output = Vec::new(); assert_eq!(signer.public_key(), &nid); @@ -330,6 +333,7 @@ pub fn run( &signer, profile, opts.clone(), + git, ), PushAction::UpdatePatch { dst, patch } => patch_update( src, @@ -343,6 +347,7 @@ pub fn run( &signer, opts.clone(), expected_refs, + git, ), PushAction::PushRef { dst } => { let identity = stored.identity()?; @@ -364,6 +369,7 @@ pub fn run( &signer, opts.verbosity, expected_refs, + git, )?; // If we're trying to update the canonical head, make sure // we don't diverge from the current head. This only applies @@ -392,11 +398,11 @@ pub fn run( match result { // Let Git tooling know that this ref has been pushed. Ok(resource) => { - println!("ok {}", cmd.dst()); + output.push(format!("ok {}", cmd.dst())); ok.insert(spec, resource); } // Let Git tooling know that there was an error pushing the ref. - Err(e) => println!("error {} {e}", cmd.dst()), + Err(e) => output.push(format!("error {} {e}", cmd.dst())), } } @@ -470,10 +476,7 @@ pub fn run( } } - // Done. - println!(); - - Ok(()) + Ok(output) } fn patch_base( @@ -503,15 +506,25 @@ fn patch_base( /// /// We choose to push a temporary reference to storage, which gets deleted on /// [`Drop::drop`]. -struct TempPatchRef<'a> { +struct TempPatchRef<'a, G> { stored: &'a storage::git::Repository, reference: git::fmt::Namespaced<'a>, + git: &'a G, } -impl<'a> TempPatchRef<'a> { - fn new(stored: &'a storage::git::Repository, head: &git::Oid, nid: &NodeId) -> Self { +impl<'a, G: GitService> TempPatchRef<'a, G> { + fn new( + stored: &'a storage::git::Repository, + head: &git::Oid, + nid: &NodeId, + git: &'a G, + ) -> Self { let reference = git::refs::storage::staging::patch(nid, *head); - Self { stored, reference } + Self { + stored, + reference, + git, + } } fn push(&self, src: &git::Oid, verbosity: Verbosity) -> Result<(), Error> { @@ -522,11 +535,12 @@ impl<'a> TempPatchRef<'a> { self.stored.raw(), verbosity, &[], + self.git, ) } } -impl<'a> Drop for TempPatchRef<'a> { +impl<'a, G> Drop for TempPatchRef<'a, G> { fn drop(&mut self) { if let Err(err) = self .stored @@ -544,7 +558,7 @@ impl<'a> Drop for TempPatchRef<'a> { } /// Open a new patch. -fn patch_open( +fn patch_open( head: &git::Oid, upstream: &Option, nid: &NodeId, @@ -557,11 +571,13 @@ fn patch_open( signer: &Device, profile: &Profile, opts: Options, + git: &S, ) -> Result, Error> where G: crypto::signature::Signer, + S: GitService, { - let temp = TempPatchRef::new(stored, head, nid); + let temp = TempPatchRef::new(stored, head, nid, git); temp.push(head, opts.verbosity)?; let base = patch_base(head, &opts, stored)?; @@ -680,7 +696,7 @@ where /// Update an existing patch. #[allow(clippy::too_many_arguments)] -fn patch_update( +fn patch_update( head: &git::Oid, dst: &git::fmt::Qualified, force: bool, @@ -695,15 +711,17 @@ fn patch_update( signer: &Device, opts: Options, expected_refs: &[String], + git: &S, ) -> Result, Error> where G: crypto::signature::Signer, + S: GitService, { let Ok(Some(patch)) = patches.get(&patch_id) else { return Err(Error::NotFound(patch_id)); }; - let temp = TempPatchRef::new(stored, head, nid); + let temp = TempPatchRef::new(stored, head, nid, git); temp.push(head, opts.verbosity)?; let base = patch_base(head, &opts, stored)?; @@ -730,6 +748,7 @@ where stored.raw(), opts.verbosity, expected_refs, + git, )?; let mut patch_mut = patch::PatchMut::new(patch_id, patch, &mut patches); @@ -766,7 +785,7 @@ where Ok(Some(ExplorerResource::Patch { id: patch_id })) } -fn push( +fn push( src: &git::Oid, dst: &git::fmt::Qualified, force: bool, @@ -780,16 +799,26 @@ fn push( signer: &Device, verbosity: Verbosity, expected_refs: &[String], + git: &S, ) -> Result, Error> where G: crypto::signature::Signer, + S: GitService, { let head = *src; let dst = dst.with_namespace(nid.into()); // 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, stored.raw(), verbosity, expected_refs)?; + push_ref( + src, + &dst, + force, + stored.raw(), + verbosity, + expected_refs, + git, + )?; if let Some(old) = old { let proj = stored.project()?; @@ -973,6 +1002,7 @@ fn push_ref( stored: &git::raw::Repository, verbosity: Verbosity, expected_refs: &[String], + git: &impl GitService, ) -> Result<(), Error> { let path = dunce::canonicalize(stored.path())?.display().to_string(); // Nb. The *force* indicator (`+`) is processed by Git tooling before we even reach this code. @@ -996,7 +1026,7 @@ fn push_ref( // Rely on the environment variable `GIT_DIR`. let working = None; - let output = radicle::git::run(working, args)?; + let output = git.send_pack(working, &args)?; if !output.status.success() { return Err(Error::SendPackFailed { diff --git a/crates/radicle-remote-helper/src/service.rs b/crates/radicle-remote-helper/src/service.rs new file mode 100644 index 00000000..38c26154 --- /dev/null +++ b/crates/radicle-remote-helper/src/service.rs @@ -0,0 +1,40 @@ +use std::io; +use std::path::Path; +use std::process; + +use radicle::git; +use radicle::storage; + +/// Abstraction for Git subprocess calls. +pub(super) trait GitService { + /// Run `git fetch-pack`. + fn fetch_pack( + &self, + working: Option<&Path>, + stored: &storage::git::Repository, + oids: Vec, + verbosity: git::Verbosity, + ) -> io::Result; + + /// Run `git send-pack` (via `radicle::git::run`). + fn send_pack(&self, working: Option<&Path>, args: &[String]) -> io::Result; +} + +/// Production implementation using real Git subprocesses. +pub(super) struct RealGitService; + +impl GitService for RealGitService { + fn fetch_pack( + &self, + working: Option<&Path>, + stored: &storage::git::Repository, + oids: Vec, + verbosity: git::Verbosity, + ) -> io::Result { + git::process::fetch_pack(working, stored, oids, verbosity) + } + + fn send_pack(&self, working: Option<&Path>, args: &[String]) -> io::Result { + git::run(working, args) + } +}