diff --git a/crates/radicle-cli/src/commands/patch/checkout.rs b/crates/radicle-cli/src/commands/patch/checkout.rs index dd9aeffb..08d03691 100644 --- a/crates/radicle-cli/src/commands/patch/checkout.rs +++ b/crates/radicle-cli/src/commands/patch/checkout.rs @@ -125,14 +125,26 @@ fn find_patch_commit<'a>( working: &'a git::raw::Repository, ) -> anyhow::Result> { let head = *revision.head(); - let workdir = working - .workdir() - .ok_or(anyhow::anyhow!("repository is a bare git repository "))?; 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::Verbosity::default())?; + let output = git::process::fetch_local( + Some(working.path()), + stored, + [head.into()], + git::Verbosity::default(), + )?; + + if !output.status.success() { + anyhow::bail!( + "`git fetch` exited with status {}, stderr and stdout follow:\n{}\n{}\n", + output.status, + String::from_utf8_lossy(&output.stderr), + String::from_utf8_lossy(&output.stdout) + ); + } + working.find_commit(head).map_err(|e| e.into()) } Err(e) => Err(e.into()), diff --git a/crates/radicle-cli/src/git.rs b/crates/radicle-cli/src/git.rs index ca8ffdbe..ec4ffcea 100644 --- a/crates/radicle-cli/src/git.rs +++ b/crates/radicle-cli/src/git.rs @@ -137,7 +137,7 @@ pub fn git>( repo: &std::path::Path, args: impl IntoIterator, ) -> anyhow::Result { - let output = radicle::git::run::<_, _, &str, &str>(repo, args, [])?; + let output = radicle::git::run(Some(repo), args)?; if !output.status.success() { anyhow::bail!( diff --git a/crates/radicle-remote-helper/src/fetch.rs b/crates/radicle-remote-helper/src/fetch.rs index 7c2feb4c..3715402d 100644 --- a/crates/radicle-remote-helper/src/fetch.rs +++ b/crates/radicle-remote-helper/src/fetch.rs @@ -1,5 +1,4 @@ use std::io; -use std::path::Path; use std::str::FromStr; use thiserror::Error; @@ -28,7 +27,6 @@ pub enum Error { /// Run a git fetch command. pub fn run( mut refs: Vec<(git::Oid, git::RefString)>, - working: &Path, stored: R, stdin: &io::Stdin, verbosity: Verbosity, @@ -54,6 +52,9 @@ pub fn run( // Verify them and prepare the final refspecs. let oids = refs.into_iter().map(|(oid, _)| oid); + // Rely on the environment variable `GIT_DIR` pointing at the repository. + let working = None; + // N.b. we shell out to `git`, avoiding using `git2`. This is to // avoid an issue where somewhere within the fetch there is an // attempt to lookup a `rad/sigrefs` object, which says that the diff --git a/crates/radicle-remote-helper/src/main.rs b/crates/radicle-remote-helper/src/main.rs index 1b3ad4ba..f26ab417 100644 --- a/crates/radicle-remote-helper/src/main.rs +++ b/crates/radicle-remote-helper/src/main.rs @@ -243,14 +243,7 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { let oid = git::Oid::from_str(oid)?; let refstr = git::RefString::try_from(*refstr)?; - // N.b. `working` is the `.git` folder and `fetch::run` - // requires the working directory. - let working = dunce::canonicalize(working.map_err(|_| Error::NoGitDir)?)?; - let working = working.parent().ok_or_else(|| Error::NoWorkingCopy { - path: working.clone(), - })?; - - return fetch::run(vec![(oid, refstr)], working, stored, &stdin, opts.verbosity) + return fetch::run(vec![(oid, refstr)], 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 df8c3d4e..d5354929 100644 --- a/crates/radicle-remote-helper/src/push.rs +++ b/crates/radicle-remote-helper/src/push.rs @@ -502,7 +502,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(), opts.verbosity)?; + push_ref(src, &dst, false, stored.raw(), opts.verbosity)?; let (_, target) = stored.canonical_head()?; let base = if let Some(base) = opts.base { @@ -616,7 +616,7 @@ where let commit = *src; let dst = dst.with_namespace(nid.into()); - push_ref(src, &dst, force, working, stored.raw(), opts.verbosity)?; + push_ref(src, &dst, force, stored.raw(), opts.verbosity)?; let Ok(Some(patch)) = patches.get(&patch_id) else { return Err(Error::NotFound(patch_id)); @@ -696,7 +696,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(), verbosity)?; + push_ref(src, &dst, force, stored.raw(), verbosity)?; if let Some(old) = old { let proj = stored.project()?; @@ -877,7 +877,6 @@ fn push_ref( src: &git::Oid, dst: &git::Namespaced, force: bool, - working: &git::raw::Repository, stored: &git::raw::Repository, verbosity: Verbosity, ) -> Result<(), Error> { @@ -885,7 +884,6 @@ fn push_ref( // 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(), @@ -904,7 +902,10 @@ fn push_ref( args.extend([url.to_string(), refspec.to_string()]); - let output = radicle::git::run::<_, _, &str, &str>(repo, args, [])?; + // Rely on the environment variable `GIT_DIR`. + let working = None; + + let output = radicle::git::run(working, args)?; if !output.status.success() { return Err(Error::InternalPushFailed { diff --git a/crates/radicle/src/git.rs b/crates/radicle/src/git.rs index 5b7ba0d9..7a3db866 100644 --- a/crates/radicle/src/git.rs +++ b/crates/radicle/src/git.rs @@ -756,23 +756,24 @@ pub fn head_refname(repo: &git2::Repository) -> Result, git2::Err } } -/// Execute a git command by spawning a child process. -pub fn run( - repo: P, +/// Execute a `git` command by spawning a child process and collect its output. +/// If `working` is [`Some`], the command is run as if `git` was started in +/// `working` instead of the current working directory, by prepending +/// `-C ` to the command line. +pub fn run( + working: Option<&std::path::Path>, args: impl IntoIterator, - envs: impl IntoIterator, ) -> io::Result where - P: AsRef, S: AsRef, - K: AsRef, - V: AsRef, { - Command::new("git") - .current_dir(repo) - .envs(envs) - .args(args) - .output() + let mut cmd = Command::new("git"); + + if let Some(working) = working { + cmd.arg("-C").arg(dunce::canonicalize(working)?); + } + + cmd.args(args).output() } /// Functions that call to the `git` CLI instead of `git2`. @@ -789,7 +790,7 @@ pub mod process { /// `oids` are the set of [`Oid`]s that are being fetched from the /// `storage`. pub fn fetch_local( - working: &Path, + working: Option<&Path>, storage: &R, oids: impl IntoIterator, verbosity: Verbosity, @@ -797,16 +798,15 @@ pub mod process { where R: ReadRepository, { - let mut fetch = vec![ + let mut args = vec![ "fetch".to_string(), // 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, []) + args.extend(verbosity.into_flag()); + args.push(url::File::new(storage.path()).to_string()); + args.extend(oids.into_iter().map(|oid| oid.to_string())); + run(working, args) } } diff --git a/crates/radicle/src/rad.rs b/crates/radicle/src/rad.rs index 63a931f5..21581ecc 100644 --- a/crates/radicle/src/rad.rs +++ b/crates/radicle/src/rad.rs @@ -1,6 +1,6 @@ #![allow(clippy::let_unit_value)] use std::io; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::str::FromStr; use std::sync::LazyLock; @@ -32,10 +32,6 @@ pub static PATCHES_REFNAME: LazyLock = #[derive(Error, Debug)] pub enum InitError { - #[error( - "the Git repository found at {path:?} is a bare repository, expected a working directory" - )] - BareRepository { path: PathBuf }, #[error("doc: {0}")] Doc(#[from] DocError), #[error("repository: {0}")] @@ -115,23 +111,22 @@ where git::configure_repository(repo)?; git::configure_remote(repo, &REMOTE_NAME, url, &url.clone().with_namespace(*pk))?; let branch = git::Qualified::from(git::fmt::lit::refs_heads(default_branch)); - // Pushes to default branch to the namespace of the `signer` - let pushspec = git::Refspec { - src: branch.clone(), - dst: branch.with_namespace(git::Component::from(pk)), - force: false, - }; - git::run::<_, _, &str, &str>( - repo.workdir().ok_or(InitError::BareRepository { - path: repo.path().to_path_buf(), - })?, - [ - "push", - &format!("{}", dunce::canonicalize(stored.path())?.display()), - &pushspec.to_string(), - ], - [], - )?; + + { + // Push branch to storage. + let stored = dunce::canonicalize(stored.path())?.display().to_string(); + + // Pushes to default branch to the namespace of the `signer`. + let pushspec = git::Refspec { + src: branch.clone(), + dst: branch.with_namespace(git::Component::from(pk)), + force: false, + } + .to_string(); + + git::run(Some(repo.path()), ["push".to_string(), stored, pushspec])?; + } + // N.b. we need to create the remote branch for the default branch let rad_remote = git::Qualified::from(git::lit::refs_remotes(&*REMOTE_COMPONENT)).join(default_branch); @@ -231,12 +226,14 @@ where #[derive(Error, Debug)] pub enum CheckoutError { - #[error( - "the Git repository found at {path:?} is a bare repository, expected a working directory" - )] - BareRepository { path: PathBuf }, - #[error("failed to fetch to working copy")] - Fetch(#[source] std::io::Error), + #[error("failed to fetch to working copy: {0}")] + FetchIo(#[source] std::io::Error), + #[error("internal fetch failed with exit status {status}, stderr and stdout follow:\n{stderr}\n{stdout}")] + FetchGit { + status: std::process::ExitStatus, + stderr: String, + stdout: String, + }, #[error("git: {0}")] Git(#[from] git2::Error), #[error("payload: {0}")] @@ -277,33 +274,35 @@ pub fn checkout, S: storage::ReadStorage>( &url, &url.clone().with_namespace(*remote), )?; - let fetchspec = git::Refspec { - src: git::refspec::pattern!("refs/heads/*"), - dst: git::Qualified::from(git::lit::refs_remotes(&*REMOTE_NAME)) - .to_pattern(git::refspec::STAR) - .into_patternstring(), - force: false, - }; - let stored = storage.repository(proj)?; - let workdir = repo.workdir().ok_or(CheckoutError::BareRepository { - path: repo.path().to_path_buf(), - })?; - git::run::<_, _, &str, &str>( - workdir, - [ - "fetch", - &format!( - "{}", - dunce::canonicalize(stored.path()) - .map_err(CheckoutError::Fetch)? - .display() - ), - &fetchspec.to_string(), - ], - [], - ) - .map_err(CheckoutError::Fetch)?; + { + // Fetch remote head to working copy. + + let fetchspec = git::Refspec { + src: git::refspec::pattern!("refs/heads/*"), + dst: git::Qualified::from(git::lit::refs_remotes(&*REMOTE_NAME)) + .to_pattern(git::refspec::STAR) + .into_patternstring(), + force: false, + } + .to_string(); + + let stored = dunce::canonicalize(storage.repository(proj)?.path()) + .map_err(CheckoutError::FetchIo)? + .display() + .to_string(); + + let output = git::run(Some(repo.path()), ["fetch", &stored, &fetchspec]) + .map_err(CheckoutError::FetchIo)?; + + if !output.status.success() { + return Err(CheckoutError::FetchGit { + status: output.status, + stdout: String::from_utf8_lossy(&output.stdout).to_string(), + stderr: String::from_utf8_lossy(&output.stderr).to_string(), + }); + } + } { // Setup default branch.