diff --git a/radicle-git-ext/src/commit.rs b/radicle-git-ext/src/commit.rs index 9eaf772..95e6f2f 100644 --- a/radicle-git-ext/src/commit.rs +++ b/radicle-git-ext/src/commit.rs @@ -26,47 +26,9 @@ use trailers::{OwnedTrailer, Trailer, Trailers}; use crate::author::Author; -/// A git commit in its object description form, i.e. the output of -/// `git cat-file` for a commit object. -#[derive(Debug)] -pub struct Commit { - tree: Oid, - parents: Vec, - author: Author, - committer: Author, - headers: Headers, - message: String, - trailers: Vec, -} +pub type Commit = CommitData; impl Commit { - pub fn new( - tree: Oid, - parents: P, - author: Author, - committer: Author, - headers: Headers, - message: String, - trailers: I, - ) -> Self - where - P: IntoIterator, - I: IntoIterator, - OwnedTrailer: From, - { - let trailers = trailers.into_iter().map(OwnedTrailer::from).collect(); - let parents = parents.into_iter().collect(); - Self { - tree, - parents, - author, - committer, - headers, - message, - trailers, - } - } - /// Read the [`Commit`] from the `repo` that is expected to be found at /// `oid`. pub fn read(repo: &git2::Repository, oid: Oid) -> Result { @@ -83,14 +45,68 @@ impl Commit { Ok(odb.write(ObjectType::Commit, self.to_string().as_bytes())?) } - /// The tree [`Oid`] this commit points to. - pub fn tree(&self) -> Oid { - self.tree + fn verify_for_write(&self, odb: &git2::Odb) -> Result<(), error::Write> { + for parent in &self.parents { + verify_object(odb, parent, ObjectType::Commit)?; + } + verify_object(odb, &self.tree, ObjectType::Tree)?; + + Ok(()) + } +} + +/// A git commit in its object description form, i.e. the output of +/// `git cat-file` for a commit object. +#[derive(Debug)] +pub struct CommitData { + tree: Tree, + parents: Vec, + author: Author, + committer: Author, + headers: Headers, + message: String, + trailers: Vec, +} + +impl CommitData { + pub fn new( + tree: Tree, + parents: P, + author: Author, + committer: Author, + headers: Headers, + message: String, + trailers: I, + ) -> Self + where + P: IntoIterator, + I: IntoIterator, + OwnedTrailer: From, + { + let trailers = trailers.into_iter().map(OwnedTrailer::from).collect(); + let parents = parents.into_iter().collect(); + Self { + tree, + parents, + author, + committer, + headers, + message, + trailers, + } } - /// The parent [`Oid`]s of this commit. - pub fn parents(&self) -> impl Iterator + '_ { - self.parents.iter().copied() + /// The tree this commit points to. + pub fn tree(&self) -> &Tree { + &self.tree + } + + /// The parents of this commit. + pub fn parents(&self) -> impl Iterator + '_ + where + Parent: Clone, + { + self.parents.iter().cloned() } /// The author of this commit, i.e. the header corresponding to `author`. @@ -136,13 +152,49 @@ impl Commit { self.trailers.iter() } - fn verify_for_write(&self, odb: &git2::Odb) -> Result<(), error::Write> { - for parent in &self.parents { - verify_object(odb, parent, ObjectType::Commit)?; - } - verify_object(odb, &self.tree, ObjectType::Tree)?; + /// Convert the `CommitData::tree` into a value of type `U`. The + /// conversion function `f` can be fallible. + /// + /// For example, `map_tree` can be used to turn raw tree data into + /// an `Oid` by writing it to a repository. + pub fn map_tree(self, f: F) -> Result, E> + where + F: FnOnce(Tree) -> Result, + { + Ok(CommitData { + tree: f(self.tree)?, + parents: self.parents, + author: self.author, + committer: self.committer, + headers: self.headers, + message: self.message, + trailers: self.trailers, + }) + } - Ok(()) + /// Convert the `CommitData::parents` into a vector containing + /// values of type `U`. The conversion function `f` can be + /// fallible. + /// + /// For example, `map_parents` can be used to resolve the `Oid`s + /// to their respective `git2::Commit`s. + pub fn map_parents(self, f: F) -> Result, E> + where + F: FnMut(Parent) -> Result, + { + Ok(CommitData { + tree: self.tree, + parents: self + .parents + .into_iter() + .map(f) + .collect::, _>>()?, + author: self.author, + committer: self.committer, + headers: self.headers, + message: self.message, + trailers: self.trailers, + }) } } diff --git a/radicle-git-ext/src/commit/headers.rs b/radicle-git-ext/src/commit/headers.rs index b5467af..cf9e10c 100644 --- a/radicle-git-ext/src/commit/headers.rs +++ b/radicle-git-ext/src/commit/headers.rs @@ -10,6 +10,7 @@ const BEGIN_PGP: &str = "-----BEGIN PGP SIGNATURE-----\n"; pub struct Headers(pub(super) Vec<(String, String)>); /// A `gpgsig` signature stored in a [`crate::commit::Commit`]. +#[derive(Debug)] pub enum Signature<'a> { /// A PGP signature, i.e. starts with `-----BEGIN PGP SIGNATURE-----`. Pgp(Cow<'a, str>), diff --git a/radicle-git-ext/t/Cargo.toml b/radicle-git-ext/t/Cargo.toml index 0110dd7..11e270d 100644 --- a/radicle-git-ext/t/Cargo.toml +++ b/radicle-git-ext/t/Cargo.toml @@ -32,5 +32,5 @@ features = ["vendored-libgit2"] path = ".." features = ["serde", "minicbor"] -[dev-dependencies.test-helpers] +[dependencies.test-helpers] path = "../../test/test-helpers" diff --git a/radicle-git-ext/t/src/commit.rs b/radicle-git-ext/t/src/commit.rs index 6ea1427..4c33376 100644 --- a/radicle-git-ext/t/src/commit.rs +++ b/radicle-git-ext/t/src/commit.rs @@ -1,10 +1,13 @@ use std::{io, str::FromStr as _, string::ToString as _}; +use proptest::proptest; use radicle_git_ext::{ author::{self, Author}, commit::{headers::Headers, trailers::OwnedTrailer, Commit}, }; -use test_helpers::tempdir::WithTmpDir; +use test_helpers::tempdir::{self, WithTmpDir}; + +use crate::gen; const NO_TRAILER: &str = "\ tree 50d6ef440728217febf9e35716d8b0296608d7f8 @@ -152,27 +155,23 @@ fn test_conversion() { assert_eq!(Commit::from_str(UNSIGNED).unwrap().to_string(), UNSIGNED); } -#[test] -fn valid_commits() { - let radicle_git = format!( - "file://{}", - git2::Repository::discover(".").unwrap().path().display() - ); - let repo = WithTmpDir::new(|path| { - let repo = git2::Repository::clone(&radicle_git, path) - .map_err(|err| io::Error::new(io::ErrorKind::Other, err))?; - Ok::<_, io::Error>(repo) - }) - .unwrap(); +proptest! { + #[test] + fn valid_commits(commits in proptest::collection::vec(gen::commit::commit(), 5..20)) { + let repo = tempdir::WithTmpDir::new(|path| { + git2::Repository::init(path).map_err(|e| io::Error::new(io::ErrorKind::Other, e)) + }).unwrap(); + let commits = gen::commit::write_commits(&repo, commits).unwrap(); + repo.reference("refs/heads/master", *commits.last().unwrap(), true, "").unwrap(); - let mut walk = repo.revwalk().unwrap(); - walk.push_head().unwrap(); + let mut walk = repo.revwalk().unwrap(); + walk.push_head().unwrap(); - // take the first 20 commits and make sure we can parse them - for oid in walk.take(20) { - let oid = oid.unwrap(); - let commit = Commit::read(&repo, oid); - assert!(commit.is_ok(), "Oid: {oid}, Error: {commit:?}") + for oid in walk.take(20) { + let oid = oid.unwrap(); + let commit = Commit::read(&repo, oid); + assert!(commit.is_ok(), "Oid: {oid}, Error: {commit:?}"); + } } } @@ -182,6 +181,7 @@ fn write_valid_commit() { git2::Repository::init(path).map_err(|err| io::Error::new(io::ErrorKind::Other, err)) }) .unwrap(); + let author = Author { name: "Terry".to_owned(), email: "terry.pratchett@proton.mail".to_owned(), diff --git a/radicle-git-ext/t/src/gen.rs b/radicle-git-ext/t/src/gen.rs index a47a6ef..ff91486 100644 --- a/radicle-git-ext/t/src/gen.rs +++ b/radicle-git-ext/t/src/gen.rs @@ -5,4 +5,15 @@ //! Provides proptest generators +use proptest::strategy::Strategy; + +pub mod commit; pub mod urn; + +pub fn alphanumeric() -> impl Strategy { + "[a-zA-Z0-9_]+" +} + +pub fn alpha() -> impl Strategy { + "[a-zA-Z]+" +} diff --git a/radicle-git-ext/t/src/gen/commit.rs b/radicle-git-ext/t/src/gen/commit.rs new file mode 100644 index 0000000..22d0922 --- /dev/null +++ b/radicle-git-ext/t/src/gen/commit.rs @@ -0,0 +1,94 @@ +use std::convert::Infallible; + +use proptest::strategy::Strategy; +use radicle_git_ext::commit::{self, CommitData}; + +mod author; +mod headers; +mod trailers; + +pub use author::author; +pub use headers::headers; +pub use trailers::{trailer, trailers}; + +use super::alphanumeric; + +pub fn commit() -> impl Strategy> { + ( + TreeData::gen(), + author(), + author(), + headers(), + alphanumeric(), + trailers(3), + ) + .prop_map(|(tree, author, committer, headers, message, trailers)| { + CommitData::new(tree, vec![], author, committer, headers, message, trailers) + }) +} + +pub fn write_commits( + repo: &git2::Repository, + linear: Vec>, +) -> Result, commit::error::Write> { + let mut parent = None; + let mut commits = Vec::new(); + for commit in linear { + let commit = commit.map_tree(|tree| tree.write(repo))?; + let commit = match parent { + Some(parent) => commit + .map_parents::(|_| Ok(parent)) + .unwrap(), + None => commit + .map_parents::(|_| unreachable!("no parents")) + .unwrap(), + }; + let oid = commit.write(repo)?; + commits.push(oid); + parent = Some(oid); + } + Ok(commits) +} + +#[derive(Clone, Debug)] +pub enum TreeData { + Blob { name: String, data: String }, + Tree { name: String, inner: Vec }, +} + +impl TreeData { + fn gen() -> impl Strategy { + let leaf = + (alphanumeric(), alphanumeric()).prop_map(|(name, data)| Self::Blob { name, data }); + leaf.prop_recursive(8, 16, 5, |inner| { + (proptest::collection::vec(inner, 1..5), alphanumeric()) + .prop_map(|(inner, name)| Self::Tree { name, inner }) + }) + } + + fn write(&self, repo: &git2::Repository) -> Result { + let mut builder = repo.treebuilder(None)?; + self.write_(repo, &mut builder)?; + builder.write() + } + + fn write_( + &self, + repo: &git2::Repository, + builder: &mut git2::TreeBuilder, + ) -> Result { + match self { + Self::Blob { name, data } => { + let oid = repo.blob(data.as_bytes())?; + builder.insert(name, oid, git2::FileMode::Blob.into())?; + } + Self::Tree { name, inner } => { + for data in inner { + let oid = data.write_(repo, builder)?; + builder.insert(name, oid, git2::FileMode::Tree.into())?; + } + } + } + builder.write() + } +} diff --git a/radicle-git-ext/t/src/gen/commit/author.rs b/radicle-git-ext/t/src/gen/commit/author.rs new file mode 100644 index 0000000..58dbede --- /dev/null +++ b/radicle-git-ext/t/src/gen/commit/author.rs @@ -0,0 +1,19 @@ +use proptest::strategy::{Just, Strategy}; +use radicle_git_ext::author::{Author, Time}; + +use crate::gen; + +pub fn author() -> impl Strategy { + gen::alphanumeric().prop_flat_map(move |name| { + (Just(name), gen::alphanumeric()).prop_flat_map(|(name, domain)| { + (Just(name), Just(domain), (0..1000i64)).prop_map(move |(name, domain, time)| { + let email = format!("{name}@{domain}"); + Author { + name, + email, + time: Time::new(time, 0), + } + }) + }) + }) +} diff --git a/radicle-git-ext/t/src/gen/commit/headers.rs b/radicle-git-ext/t/src/gen/commit/headers.rs new file mode 100644 index 0000000..f4a0f25 --- /dev/null +++ b/radicle-git-ext/t/src/gen/commit/headers.rs @@ -0,0 +1,30 @@ +use proptest::{collection, prop_oneof, strategy::Strategy}; +use radicle_git_ext::commit::headers::Headers; + +use crate::gen; + +pub fn headers() -> impl Strategy { + collection::vec(prop_oneof![header(), signature()], 0..5).prop_map(|hs| { + let mut headers = Headers::new(); + for (k, v) in hs { + headers.push(&k, &v); + } + headers + }) +} + +fn header() -> impl Strategy { + (prop_oneof!["test", "foo", "foobar"], gen::alphanumeric()) +} + +pub fn signature() -> impl Strategy { + ("gpgsig", prop_oneof![pgp(), ssh()]) +} + +pub fn pgp() -> impl Strategy { + "-----BEGIN PGP SIGNATURE-----\r?\n([A-Za-z0-9+/=\r\n]+)\r?\n-----END PGP SIGNATURE-----" +} + +pub fn ssh() -> impl Strategy { + "-----BEGIN SSH SIGNATURE-----\r?\n([A-Za-z0-9+/=\r\n]+)\r?\n-----END SSH SIGNATURE-----" +} diff --git a/radicle-git-ext/t/src/gen/commit/trailers.rs b/radicle-git-ext/t/src/gen/commit/trailers.rs new file mode 100644 index 0000000..81bb7ce --- /dev/null +++ b/radicle-git-ext/t/src/gen/commit/trailers.rs @@ -0,0 +1,18 @@ +use proptest::{collection, strategy::Strategy}; +use radicle_git_ext::commit::trailers::{OwnedTrailer, Token, Trailer}; + +use crate::gen; + +pub fn trailers(n: usize) -> impl Strategy> { + collection::vec(trailer(), 0..n) +} + +pub fn trailer() -> impl Strategy { + (gen::alpha(), gen::alphanumeric()).prop_map(|(token, value)| { + Trailer { + token: Token::try_from(format!("X-{}", token).as_str()).unwrap(), + value: value.into(), + } + .to_owned() + }) +} diff --git a/radicle-git-ext/t/src/lib.rs b/radicle-git-ext/t/src/lib.rs index 6ca842c..62e2c90 100644 --- a/radicle-git-ext/t/src/lib.rs +++ b/radicle-git-ext/t/src/lib.rs @@ -12,3 +12,6 @@ mod commit; #[cfg(any(test, feature = "test"))] pub mod git_ref_format; + +#[cfg(any(test, feature = "test"))] +pub mod repository; diff --git a/radicle-git-ext/t/src/repository.rs b/radicle-git-ext/t/src/repository.rs new file mode 100644 index 0000000..56f580a --- /dev/null +++ b/radicle-git-ext/t/src/repository.rs @@ -0,0 +1,91 @@ +use std::{convert::Infallible, io, path::Path}; + +use git2::Oid; +use radicle_git_ext::{commit::CommitData, ref_format::RefString}; +use test_helpers::tempdir::{self, WithTmpDir}; + +use crate::gen::commit::{self, TreeData}; + +pub struct Fixture { + pub inner: WithTmpDir, + pub head: Option, +} + +/// Initialise a [`git2::Repository`] in a temporary directory. +/// +/// The provided `commits` will be added to the repository, and the +/// head commit will be returned. +pub fn fixture( + refname: &RefString, + commits: Vec>, +) -> io::Result { + let repo = tempdir::WithTmpDir::new(|path| git2::Repository::init(path).map_err(io_other))?; + let commits = commit::write_commits(&repo, commits).map_err(io_other)?; + let head = commits.last().copied(); + + if let Some(head) = head { + repo.reference(refname.as_str(), head, false, "Initialise repository") + .map_err(io_other)?; + } + + Ok(Fixture { inner: repo, head }) +} + +pub fn bare_fixture( + refname: &RefString, + commits: Vec>, +) -> io::Result { + let repo = + tempdir::WithTmpDir::new(|path| git2::Repository::init_bare(path).map_err(io_other))?; + let commits = commit::write_commits(&repo, commits).map_err(io_other)?; + let head = commits.last().copied(); + + if let Some(head) = head { + repo.reference(refname.as_str(), head, false, "Initialise repository") + .map_err(io_other)?; + } + + Ok(Fixture { inner: repo, head }) +} + +pub fn submodule<'a>( + parent: &'a git2::Repository, + child: &'a git2::Repository, + refname: &RefString, + head: Oid, + author: &git2::Signature, +) -> io::Result> { + let url = format!("file://{}", child.path().canonicalize()?.display()); + let mut sub = parent + .submodule(url.as_str(), Path::new("submodule"), true) + .map_err(io_other)?; + sub.open().map_err(io_other)?; + sub.clone(Some(&mut git2::SubmoduleUpdateOptions::default())) + .map_err(io_other)?; + sub.add_to_index(true).map_err(io_other)?; + sub.add_finalize().map_err(io_other)?; + { + let mut ix = parent.index().map_err(io_other)?; + let tree = ix.write_tree_to(parent).map_err(io_other)?; + let tree = parent.find_tree(tree).map_err(io_other)?; + let head = parent.find_commit(head).map_err(io_other)?; + parent + .commit( + Some(refname.as_str()), + author, + author, + "Commit submodule", + &tree, + &[&head], + ) + .map_err(io_other)?; + } + Ok(sub) +} + +fn io_other(e: E) -> io::Error +where + E: std::error::Error + Send + Sync + 'static, +{ + io::Error::new(io::ErrorKind::Other, e) +} diff --git a/radicle-surf/Cargo.toml b/radicle-surf/Cargo.toml index 859a839..c16aec4 100644 --- a/radicle-surf/Cargo.toml +++ b/radicle-surf/Cargo.toml @@ -33,6 +33,7 @@ base64 = "0.13" log = "0.4" nonempty = "0.5" thiserror = "1.0" +url = "2.5" [dependencies.git2] version = "0.18.1" @@ -53,9 +54,6 @@ version = "1" features = ["serde_derive"] optional = true -[dependencies.url] -version = "2.5" - [build-dependencies] anyhow = "1.0" flate2 = "1" diff --git a/radicle-surf/t/Cargo.toml b/radicle-surf/t/Cargo.toml index ef70620..81424da 100644 --- a/radicle-surf/t/Cargo.toml +++ b/radicle-surf/t/Cargo.toml @@ -17,6 +17,7 @@ nonempty = "0.5" pretty_assertions = "1.3.0" proptest = "1" serde_json = "1" +url = "2.5" [dev-dependencies.git2] version = "0.18.1" diff --git a/radicle-surf/t/src/submodule.rs b/radicle-surf/t/src/submodule.rs index a19d7fc..62129b8 100644 --- a/radicle-surf/t/src/submodule.rs +++ b/radicle-surf/t/src/submodule.rs @@ -1,21 +1,86 @@ -use std::path::Path; +use std::{convert::Infallible, path::Path}; +use proptest::{collection, proptest}; +use radicle_git_ext::commit::CommitData; +use radicle_git_ext::ref_format::refname; +use radicle_git_ext_test::gen; use radicle_surf::tree::EntryKind; +use radicle_surf::{fs, Branch, Repository}; -#[test] -fn test_submodule() { - use radicle_git_ext::ref_format::refname; - use radicle_surf::{fs, Branch, Repository}; +proptest! { + #[test] + fn test_submodule( + initial in gen::commit::commit(), + commits in collection::vec(gen::commit::commit(), 1..5) + ) { + prop::test_submodule(initial, commits) + } - let repo = Repository::discover(".").unwrap(); - let branch = Branch::local(refname!("surf/submodule-support")); - let dir = repo.root_dir(&branch).unwrap(); - let platinum = dir - .find_entry(&Path::new("radicle-surf/data/git-platinum"), &repo) - .unwrap(); - assert!(matches!(&platinum, fs::Entry::Submodule(module) if module.url().is_some())); + #[ignore = "segfault"] + #[test] + fn test_submodule_bare( + initial in gen::commit::commit(), + commits in collection::vec(gen::commit::commit(), 1..5) + ) { + prop::test_submodule_bare(initial, commits) + } - let surf = repo.tree(&branch, &Path::new("radicle-surf/data")).unwrap(); - let kind = EntryKind::from(platinum); - assert!(surf.entries().iter().any(|e| e.entry() == &kind)); +} + +mod prop { + use radicle_git_ext_test::{gen::commit, repository}; + + use super::*; + + pub fn test_submodule( + initial: CommitData, + commits: Vec>, + ) { + let refname = refname!("refs/heads/master"); + let author = git2::Signature::try_from(initial.author()).unwrap(); + + let submodule = repository::fixture(&refname, commits).unwrap(); + let repo = repository::fixture(&refname, vec![initial]).unwrap(); + + let head = repo.head.expect("missing initial commit"); + let sub = + repository::submodule(&repo.inner, &submodule.inner, &refname, head, &author).unwrap(); + + let repo = Repository::open(repo.inner.path()).unwrap(); + let branch = Branch::local(refname); + let dir = repo.root_dir(&branch).unwrap(); + + let platinum = dir.find_entry(&sub.path(), &repo).unwrap(); + assert!(matches!(&platinum, fs::Entry::Submodule(module) if module.url().is_some())); + + let root = repo.tree(&branch, &Path::new("")).unwrap(); + let kind = EntryKind::from(platinum); + assert!(root.entries().iter().any(|e| e.entry() == &kind)); + } + + pub fn test_submodule_bare( + initial: CommitData, + commits: Vec>, + ) { + let refname = refname!("refs/heads/master"); + let author = git2::Signature::try_from(initial.author()).unwrap(); + + let submodule = repository::fixture(&refname, commits).unwrap(); + let repo = repository::bare_fixture(&refname, vec![initial]).unwrap(); + + let head = repo.head.expect("missing initial commit"); + let sub = + repository::submodule(&repo.inner, &submodule.inner, &refname, head, &author).unwrap(); + + let repo = Repository::open(repo.inner.path()).unwrap(); + let branch = Branch::local(refname); + let dir = repo.root_dir(&branch).unwrap(); + + let platinum = dir.find_entry(&sub.path(), &repo).unwrap(); + assert!(matches!(&platinum, fs::Entry::Submodule(module) if module.url().is_some())); + + let root = repo.tree(&branch, &Path::new("")).unwrap(); + let kind = EntryKind::from(platinum); + assert!(root.entries().iter().any(|e| e.entry() == &kind)); + } } diff --git a/test/test-helpers/src/tempdir.rs b/test/test-helpers/src/tempdir.rs index 342ceb0..24a8b5c 100644 --- a/test/test-helpers/src/tempdir.rs +++ b/test/test-helpers/src/tempdir.rs @@ -11,6 +11,7 @@ use std::{ use tempfile::{tempdir, TempDir}; +#[derive(Debug)] pub struct WithTmpDir { _tmp: TempDir, inner: A,