radicle: Handle `GIT_DIR` more uniformly

Pull specification of the working directory into `radicle::git::run`,
to unify how higher level wrappers pass it down.
This commit is contained in:
Lorenz Leutgeb 2025-09-11 21:35:43 +02:00 committed by Fintan Halpenny
parent 4e67467966
commit 354565c579
7 changed files with 100 additions and 94 deletions

View File

@ -125,14 +125,26 @@ fn find_patch_commit<'a>(
working: &'a git::raw::Repository, working: &'a git::raw::Repository,
) -> anyhow::Result<git::raw::Commit<'a>> { ) -> anyhow::Result<git::raw::Commit<'a>> {
let head = *revision.head(); let head = *revision.head();
let workdir = working
.workdir()
.ok_or(anyhow::anyhow!("repository is a bare git repository "))?;
match working.find_commit(head) { match working.find_commit(head) {
Ok(commit) => Ok(commit), Ok(commit) => Ok(commit),
Err(e) if git::ext::is_not_found_err(&e) => { 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()) working.find_commit(head).map_err(|e| e.into())
} }
Err(e) => Err(e.into()), Err(e) => Err(e.into()),

View File

@ -137,7 +137,7 @@ pub fn git<S: AsRef<std::ffi::OsStr>>(
repo: &std::path::Path, repo: &std::path::Path,
args: impl IntoIterator<Item = S>, args: impl IntoIterator<Item = S>,
) -> anyhow::Result<std::process::Output> { ) -> anyhow::Result<std::process::Output> {
let output = radicle::git::run::<_, _, &str, &str>(repo, args, [])?; let output = radicle::git::run(Some(repo), args)?;
if !output.status.success() { if !output.status.success() {
anyhow::bail!( anyhow::bail!(

View File

@ -1,5 +1,4 @@
use std::io; use std::io;
use std::path::Path;
use std::str::FromStr; use std::str::FromStr;
use thiserror::Error; use thiserror::Error;
@ -28,7 +27,6 @@ pub enum Error {
/// Run a git fetch command. /// Run a git fetch command.
pub fn run<R: ReadRepository>( pub fn run<R: ReadRepository>(
mut refs: Vec<(git::Oid, git::RefString)>, mut refs: Vec<(git::Oid, git::RefString)>,
working: &Path,
stored: R, stored: R,
stdin: &io::Stdin, stdin: &io::Stdin,
verbosity: Verbosity, verbosity: Verbosity,
@ -54,6 +52,9 @@ pub fn run<R: ReadRepository>(
// Verify them and prepare the final refspecs. // Verify them and prepare the final refspecs.
let oids = refs.into_iter().map(|(oid, _)| oid); 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 // N.b. we shell out to `git`, avoiding using `git2`. This is to
// avoid an issue where somewhere within the fetch there is an // avoid an issue where somewhere within the fetch there is an
// attempt to lookup a `rad/sigrefs` object, which says that the // attempt to lookup a `rad/sigrefs` object, which says that the

View File

@ -243,14 +243,7 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> {
let oid = git::Oid::from_str(oid)?; let oid = git::Oid::from_str(oid)?;
let refstr = git::RefString::try_from(*refstr)?; let refstr = git::RefString::try_from(*refstr)?;
// N.b. `working` is the `.git` folder and `fetch::run` return fetch::run(vec![(oid, refstr)], stored, &stdin, opts.verbosity)
// 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)
.map_err(Error::from); .map_err(Error::from);
} }
["push", refspec] => { ["push", refspec] => {

View File

@ -502,7 +502,7 @@ where
// //
// In case the reference is not properly deleted, the next attempt to open a patch should // 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. // 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 (_, target) = stored.canonical_head()?;
let base = if let Some(base) = opts.base { let base = if let Some(base) = opts.base {
@ -616,7 +616,7 @@ where
let commit = *src; let commit = *src;
let dst = dst.with_namespace(nid.into()); 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 { let Ok(Some(patch)) = patches.get(&patch_id) else {
return Err(Error::NotFound(patch_id)); 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. // 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(); 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 { if let Some(old) = old {
let proj = stored.project()?; let proj = stored.project()?;
@ -877,7 +877,6 @@ fn push_ref(
src: &git::Oid, src: &git::Oid,
dst: &git::Namespaced, dst: &git::Namespaced,
force: bool, force: bool,
working: &git::raw::Repository,
stored: &git::raw::Repository, stored: &git::raw::Repository,
verbosity: Verbosity, verbosity: Verbosity,
) -> Result<(), Error> { ) -> Result<(), Error> {
@ -885,7 +884,6 @@ fn push_ref(
// Nb. The *force* indicator (`+`) is processed by Git tooling before we even reach this code. // Nb. The *force* indicator (`+`) is processed by Git tooling before we even reach this code.
// This happens during the `list for-push` phase. // This happens during the `list for-push` phase.
let refspec = git::Refspec { src, dst, force }; let refspec = git::Refspec { src, dst, force };
let repo = working.workdir().unwrap_or_else(|| working.path());
let mut args = vec![ let mut args = vec![
"push".to_string(), "push".to_string(),
@ -904,7 +902,10 @@ fn push_ref(
args.extend([url.to_string(), refspec.to_string()]); 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() { if !output.status.success() {
return Err(Error::InternalPushFailed { return Err(Error::InternalPushFailed {

View File

@ -756,23 +756,24 @@ pub fn head_refname(repo: &git2::Repository) -> Result<Option<String>, git2::Err
} }
} }
/// Execute a git command by spawning a child process. /// Execute a `git` command by spawning a child process and collect its output.
pub fn run<P, S, K, V>( /// If `working` is [`Some`], the command is run as if `git` was started in
repo: P, /// `working` instead of the current working directory, by prepending
/// `-C <working>` to the command line.
pub fn run<S>(
working: Option<&std::path::Path>,
args: impl IntoIterator<Item = S>, args: impl IntoIterator<Item = S>,
envs: impl IntoIterator<Item = (K, V)>,
) -> io::Result<std::process::Output> ) -> io::Result<std::process::Output>
where where
P: AsRef<Path>,
S: AsRef<std::ffi::OsStr>, S: AsRef<std::ffi::OsStr>,
K: AsRef<std::ffi::OsStr>,
V: AsRef<std::ffi::OsStr>,
{ {
Command::new("git") let mut cmd = Command::new("git");
.current_dir(repo)
.envs(envs) if let Some(working) = working {
.args(args) cmd.arg("-C").arg(dunce::canonicalize(working)?);
.output() }
cmd.args(args).output()
} }
/// Functions that call to the `git` CLI instead of `git2`. /// 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 /// `oids` are the set of [`Oid`]s that are being fetched from the
/// `storage`. /// `storage`.
pub fn fetch_local<R>( pub fn fetch_local<R>(
working: &Path, working: Option<&Path>,
storage: &R, storage: &R,
oids: impl IntoIterator<Item = Oid>, oids: impl IntoIterator<Item = Oid>,
verbosity: Verbosity, verbosity: Verbosity,
@ -797,16 +798,15 @@ pub mod process {
where where
R: ReadRepository, R: ReadRepository,
{ {
let mut fetch = vec![ let mut args = vec![
"fetch".to_string(), "fetch".to_string(),
// 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(), "--no-write-fetch-head".to_string(),
]; ];
fetch.extend(verbosity.into_flag()); args.extend(verbosity.into_flag());
fetch.push(url::File::new(storage.path()).to_string()); args.push(url::File::new(storage.path()).to_string());
fetch.extend(oids.into_iter().map(|oid| oid.to_string())); args.extend(oids.into_iter().map(|oid| oid.to_string()));
// N.b. `.` is used since we're fetching within the working copy run(working, args)
run::<_, _, &str, &str>(working, fetch, [])
} }
} }

View File

@ -1,6 +1,6 @@
#![allow(clippy::let_unit_value)] #![allow(clippy::let_unit_value)]
use std::io; use std::io;
use std::path::{Path, PathBuf}; use std::path::Path;
use std::str::FromStr; use std::str::FromStr;
use std::sync::LazyLock; use std::sync::LazyLock;
@ -32,10 +32,6 @@ pub static PATCHES_REFNAME: LazyLock<git::RefString> =
#[derive(Error, Debug)] #[derive(Error, Debug)]
pub enum InitError { pub enum InitError {
#[error(
"the Git repository found at {path:?} is a bare repository, expected a working directory"
)]
BareRepository { path: PathBuf },
#[error("doc: {0}")] #[error("doc: {0}")]
Doc(#[from] DocError), Doc(#[from] DocError),
#[error("repository: {0}")] #[error("repository: {0}")]
@ -115,23 +111,22 @@ where
git::configure_repository(repo)?; git::configure_repository(repo)?;
git::configure_remote(repo, &REMOTE_NAME, url, &url.clone().with_namespace(*pk))?; git::configure_remote(repo, &REMOTE_NAME, url, &url.clone().with_namespace(*pk))?;
let branch = git::Qualified::from(git::fmt::lit::refs_heads(default_branch)); 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(), // Push branch to storage.
dst: branch.with_namespace(git::Component::from(pk)), let stored = dunce::canonicalize(stored.path())?.display().to_string();
force: false,
}; // Pushes to default branch to the namespace of the `signer`.
git::run::<_, _, &str, &str>( let pushspec = git::Refspec {
repo.workdir().ok_or(InitError::BareRepository { src: branch.clone(),
path: repo.path().to_path_buf(), dst: branch.with_namespace(git::Component::from(pk)),
})?, force: false,
[ }
"push", .to_string();
&format!("{}", dunce::canonicalize(stored.path())?.display()),
&pushspec.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 // N.b. we need to create the remote branch for the default branch
let rad_remote = let rad_remote =
git::Qualified::from(git::lit::refs_remotes(&*REMOTE_COMPONENT)).join(default_branch); git::Qualified::from(git::lit::refs_remotes(&*REMOTE_COMPONENT)).join(default_branch);
@ -231,12 +226,14 @@ where
#[derive(Error, Debug)] #[derive(Error, Debug)]
pub enum CheckoutError { pub enum CheckoutError {
#[error( #[error("failed to fetch to working copy: {0}")]
"the Git repository found at {path:?} is a bare repository, expected a working directory" FetchIo(#[source] std::io::Error),
)] #[error("internal fetch failed with exit status {status}, stderr and stdout follow:\n{stderr}\n{stdout}")]
BareRepository { path: PathBuf }, FetchGit {
#[error("failed to fetch to working copy")] status: std::process::ExitStatus,
Fetch(#[source] std::io::Error), stderr: String,
stdout: String,
},
#[error("git: {0}")] #[error("git: {0}")]
Git(#[from] git2::Error), Git(#[from] git2::Error),
#[error("payload: {0}")] #[error("payload: {0}")]
@ -277,33 +274,35 @@ pub fn checkout<P: AsRef<Path>, S: storage::ReadStorage>(
&url, &url,
&url.clone().with_namespace(*remote), &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 remote head to working copy.
[
"fetch", let fetchspec = git::Refspec {
&format!( src: git::refspec::pattern!("refs/heads/*"),
"{}", dst: git::Qualified::from(git::lit::refs_remotes(&*REMOTE_NAME))
dunce::canonicalize(stored.path()) .to_pattern(git::refspec::STAR)
.map_err(CheckoutError::Fetch)? .into_patternstring(),
.display() force: false,
), }
&fetchspec.to_string(), .to_string();
],
[], let stored = dunce::canonicalize(storage.repository(proj)?.path())
) .map_err(CheckoutError::FetchIo)?
.map_err(CheckoutError::Fetch)?; .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. // Setup default branch.