diff --git a/radicle/src/git.rs b/radicle/src/git.rs index fca7f7e8..e5c064cc 100644 --- a/radicle/src/git.rs +++ b/radicle/src/git.rs @@ -12,7 +12,9 @@ use crate::storage; use crate::storage::refs::Refs; use crate::storage::RemoteId; +pub use ext::is_not_found_err; pub use ext::Error; +pub use ext::NotFound; pub use ext::Oid; pub use git2 as raw; pub use git_ref_format as fmt; diff --git a/radicle/src/identity/project.rs b/radicle/src/identity/project.rs index 60ea9119..f5a1deb1 100644 --- a/radicle/src/identity/project.rs +++ b/radicle/src/identity/project.rs @@ -60,6 +60,7 @@ impl DocError { match self { Self::NotFound(_) => true, Self::Git(git::Error::NotFound(_)) => true, + Self::Git(git::Error::Git(e)) if git::is_not_found_err(e) => true, _ => false, } } @@ -358,8 +359,7 @@ impl Doc { impl Doc { pub fn head(remote: &RemoteId, repo: &R) -> Result { let head = &git::refname!("heads").join(&*git::refs::IDENTITY_BRANCH); - repo.reference_oid(remote, head)? - .ok_or_else(|| DocError::NotFound(head.to_owned())) + repo.reference_oid(remote, head).map_err(DocError::from) } } @@ -483,12 +483,29 @@ mod test { use crate::rad; use crate::storage::git::Storage; use crate::storage::{ReadStorage, WriteStorage}; + use crate::test::arbitrary; use crate::test::fixtures; use crate::test::signer::MockSigner; use super::*; use quickcheck_macros::quickcheck; + #[test] + fn test_not_found() { + let tempdir = tempfile::tempdir().unwrap(); + let storage = Storage::open(tempdir.path().join("storage")).unwrap(); + let remote = arbitrary::gen::(1); + let proj = arbitrary::gen::(1); + let repo = storage.repository(proj).unwrap(); + let oid = git2::Oid::from_str("2d52a53ce5e4f141148a5f770cfd3ead2d6a45b8").unwrap(); + + let err = Doc::::head(&remote, &repo).unwrap_err(); + assert!(dbg!(err).is_not_found()); + + let err = Doc::load_at(oid.into(), &repo).unwrap_err(); + assert!(err.is_not_found()); + } + #[test] fn test_valid_identity() { let tempdir = tempfile::tempdir().unwrap(); diff --git a/radicle/src/rad.rs b/radicle/src/rad.rs index 8f85e3ca..416ac938 100644 --- a/radicle/src/rad.rs +++ b/radicle/src/rad.rs @@ -89,8 +89,12 @@ pub fn init( #[derive(Error, Debug)] pub enum ForkError { + #[error("ref string: {0}")] + RefString(#[from] git::fmt::Error), #[error("git: {0}")] - Git(#[from] git2::Error), + GitRaw(#[from] git2::Error), + #[error("git: {0}")] + Git(#[from] git::Error), #[error("storage: {0}")] Storage(#[from] storage::Error), #[error("project `{0}` was not found in storage")] @@ -168,12 +172,8 @@ pub fn fork( let mut heads = Vec::new(); for delegate in project.delegates.iter() { let name = format!("heads/{}", &project.default_branch); - let refname = git::RefString::try_from(name.as_str()).unwrap(); - let r = repository - .reference(&delegate.id, &refname)? - .unwrap() - .target() - .unwrap(); + let refname = git::RefString::try_from(name.as_str())?; + let r = repository.reference_oid(&delegate.id, &refname)?.into(); heads.push(r); } diff --git a/radicle/src/storage.rs b/radicle/src/storage.rs index 0cca4fa1..5956f03f 100644 --- a/radicle/src/storage.rs +++ b/radicle/src/storage.rs @@ -246,14 +246,10 @@ pub trait ReadRepository { &self, remote: &RemoteId, reference: &RefStr, - ) -> Result, git2::Error>; + ) -> Result; fn commit(&self, oid: Oid) -> Result, git2::Error>; fn revwalk(&self, head: Oid) -> Result; - fn reference_oid( - &self, - remote: &RemoteId, - reference: &RefStr, - ) -> Result, git2::Error>; + fn reference_oid(&self, remote: &RemoteId, reference: &RefStr) -> Result; fn references(&self, remote: &RemoteId) -> Result; fn remote(&self, remote: &RemoteId) -> Result, refs::Error>; fn remotes(&self) -> Result, refs::Error>; diff --git a/radicle/src/storage/git.rs b/radicle/src/storage/git.rs index f63590ed..6be8e5ed 100644 --- a/radicle/src/storage/git.rs +++ b/radicle/src/storage/git.rs @@ -417,16 +417,11 @@ impl ReadRepository for Repository { &self, remote: &RemoteId, name: &git::RefStr, - ) -> Result, git2::Error> { + ) -> Result { let name = name.strip_prefix(git::refname!("refs")).unwrap_or(name); let name = format!("refs/remotes/{remote}/{name}"); - self.backend.find_reference(&name).map(Some).or_else(|e| { - if git::ext::is_not_found_err(&e) { - Ok(None) - } else { - Err(e) - } - }) + + self.backend.find_reference(&name).map_err(git::Error::from) } fn commit(&self, oid: Oid) -> Result, git2::Error> { @@ -446,13 +441,13 @@ impl ReadRepository for Repository { Ok(revwalk) } - fn reference_oid( - &self, - remote: &RemoteId, - reference: &git::RefStr, - ) -> Result, git2::Error> { - let reference = self.reference(remote, reference)?; - Ok(reference.and_then(|r| r.target().map(|o| o.into()))) + fn reference_oid(&self, remote: &RemoteId, reference: &git::RefStr) -> Result { + let oid = self + .reference(remote, reference)? + .target() + .ok_or(git::Error::NotFound(git::NotFound::NoRefTarget))?; + + Ok(oid.into()) } fn remote(&self, remote: &RemoteId) -> Result, refs::Error> { @@ -752,10 +747,10 @@ mod tests { for remote in remotes { let (id, _) = remote.unwrap(); let alice_repo = alice.repository(proj).unwrap(); - let alice_oid = alice_repo.reference(&id, &refname).unwrap().unwrap(); + let alice_oid = alice_repo.reference(&id, &refname).unwrap(); let bob_repo = bob.repository(proj).unwrap(); - let bob_oid = bob_repo.reference(&id, &refname).unwrap().unwrap(); + let bob_oid = bob_repo.reference(&id, &refname).unwrap(); assert_eq!(alice_oid.target(), bob_oid.target()); } @@ -805,7 +800,7 @@ mod tests { // Bob's storage is updated. let bob_repo = bob.repository(proj_id).unwrap(); - let bob_master = bob_repo.reference(alice_id, &refname).unwrap().unwrap(); + let bob_master = bob_repo.reference(alice_id, &refname).unwrap(); assert_eq!(bob_master.target().unwrap(), alice_head); } diff --git a/radicle/src/storage/refs.rs b/radicle/src/storage/refs.rs index 36c833fc..196ae43b 100644 --- a/radicle/src/storage/refs.rs +++ b/radicle/src/storage/refs.rs @@ -45,8 +45,17 @@ pub enum Error { Git(#[from] git2::Error), #[error(transparent)] GitExt(#[from] git_ext::Error), - #[error("refs were not found")] - NotFound, +} + +impl Error { + /// Whether this error is caused by a reference not being found. + pub fn is_not_found(&self) -> bool { + match self { + Self::GitExt(git::Error::NotFound(_)) => true, + Self::GitExt(git::Error::Git(e)) if git::is_not_found_err(e) => true, + _ => false, + } + } } /// The published state of a local repository. @@ -217,11 +226,9 @@ impl SignedRefs { where S: ReadRepository, { - if let Some(oid) = repo.reference_oid(remote, &SIGNATURE_REF)? { - Self::load_at(oid, remote, repo) - } else { - Err(Error::NotFound) - } + let oid = repo.reference_oid(remote, &SIGNATURE_REF)?; + + SignedRefs::load_at(oid, remote, repo) } pub fn load_at(oid: Oid, remote: &RemoteId, repo: &S) -> Result @@ -255,10 +262,12 @@ impl SignedRefs { repo: &S, ) -> Result { let sigref = &*SIGNATURE_REF; - let parent: Option = repo - .reference(remote, sigref)? - .map(|r| r.peel_to_commit()) - .transpose()?; + let parent = match repo.reference(remote, sigref).map_err(|e| dbg!(e)) { + Ok(r) => Some(r.peel_to_commit()?), + Err(git_ext::Error::Git(e)) if git_ext::is_not_found_err(&e) => None, + Err(git_ext::Error::NotFound(_)) => None, + Err(e) => return Err(e.into()), + }; let tree = { let raw = repo.raw(); diff --git a/radicle/src/test/storage.rs b/radicle/src/test/storage.rs index 248fd6d2..58457965 100644 --- a/radicle/src/test/storage.rs +++ b/radicle/src/test/storage.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; use git_url::Url; +use radicle_git_ext as git_ext; use crate::crypto::{Signer, Verified}; use crate::identity::project::Doc; @@ -106,9 +107,9 @@ impl ReadRepository for MockRepository { fn blob_at<'a>( &'a self, - _oid: radicle_git_ext::Oid, + _oid: git_ext::Oid, _path: &'a std::path::Path, - ) -> Result, radicle_git_ext::Error> { + ) -> Result, git_ext::Error> { todo!() } @@ -116,7 +117,7 @@ impl ReadRepository for MockRepository { &self, _remote: &RemoteId, _reference: &git::RefStr, - ) -> Result, git2::Error> { + ) -> Result { todo!() } @@ -124,7 +125,7 @@ impl ReadRepository for MockRepository { &self, _remote: &RemoteId, _reference: &git::RefStr, - ) -> Result, git2::Error> { + ) -> Result { todo!() }