diff --git a/git-storage/src/backend/read.rs b/git-storage/src/backend/read.rs index 9132981..45f611b 100644 --- a/git-storage/src/backend/read.rs +++ b/git-storage/src/backend/read.rs @@ -3,7 +3,7 @@ // This file is part of radicle-link, distributed under the GPLv3 with Radicle // Linking Exception. For full terms see the included LICENSE file. -use std::path::Path; +use std::{fmt, path::Path}; use git_ext::{error::is_not_found_err, Oid}; use std_ext::result::ResultExt as _; @@ -46,6 +46,14 @@ impl Read { } } +impl fmt::Debug for Read { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Read") + .field("raw", &self.raw.path()) + .finish() + } +} + pub mod error { use thiserror::Error; diff --git a/git-storage/src/backend/write.rs b/git-storage/src/backend/write.rs index c05d4e3..a15ae6d 100644 --- a/git-storage/src/backend/write.rs +++ b/git-storage/src/backend/write.rs @@ -36,6 +36,7 @@ pub mod error; /// For write access to the refdb see [`refdb::Write`]. /// /// To construct the `Write` storage use [`Read::open`]. +#[derive(Debug)] pub struct Write { inner: Read, info: UserInfo, @@ -568,7 +569,19 @@ impl odb::Write for Write { .map(Oid::from) } - fn write_tree(&self, builder: git2::TreeBuilder) -> Result { - builder.write().map(Oid::from) + fn write_tree(&self, builder: odb::TreeBuilder) -> Result { + let repo = self.as_raw(); + let mut tree = repo.treebuilder(None)?; + for entry in builder.iter() { + match entry { + odb::TreeEntry::Insert { + name, + oid, + filemode, + } => tree.insert(name, (*oid).into(), *filemode).map(|_| ())?, + odb::TreeEntry::Remove { name } => tree.remove(name)?, + } + } + tree.write().map(Oid::from) } } diff --git a/git-storage/src/odb.rs b/git-storage/src/odb.rs index 5b6f59a..c0124c5 100644 --- a/git-storage/src/odb.rs +++ b/git-storage/src/odb.rs @@ -20,7 +20,10 @@ // TODO: this doesn't abstract over the git2 types very well, but it's too much // hassle to massage that right now. -use std::error::Error; +use std::{ + error::Error, + path::{Path, PathBuf}, +}; pub use git2::{Blob, Commit, Object, Tag, Tree}; @@ -32,6 +35,71 @@ pub use read::Read; pub mod write; pub use write::Write; +/// A `TreeBuilder` represents a series of operations to build a git tree, see +/// *Tree Objects* [here][git-objects]. +/// +/// The [`TreeEntry`] variants represent a limited set of the actions one can +/// perform [git-objects]: +#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct TreeBuilder { + entries: Vec, +} + +impl TreeBuilder { + pub fn new() -> Self { + Self { + entries: Vec::new(), + } + } + + /// [`TreeBuilder::insert`] creates a [`TreeEntry::Insert`] entry in the + /// `TreeBuilder`. + pub fn insert(mut self, name: P, oid: Oid, mode: M) -> Self + where + P: AsRef, + M: Into, + { + let name = name.as_ref().to_path_buf(); + let filemode = mode.into(); + self.entries.push(TreeEntry::Insert { + name, + oid, + filemode, + }); + self + } + + /// [`TreeBuilder::remove`] creates a [`TreeEntry::Remove`] entry in the + /// `TreeBuilder`. + pub fn remove

(mut self, name: P) -> Self + where + P: AsRef, + { + let name = name.as_ref().to_path_buf(); + self.entries.push(TreeEntry::Remove { name }); + self + } + + /// Iterate over the [`TreeEntry`]'s found in the `TreeBuilder`. + pub fn iter(&self) -> impl Iterator { + self.entries.iter() + } +} + +/// A `TreeEntry` represents a single operation within a [`TreeBuilder`]. +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum TreeEntry { + /// Insert the `oid` under the `name` path in the resulting tree. + /// The `filemode` determines the file mode for the file found at `name`. + Insert { + name: PathBuf, + oid: Oid, + filemode: i32, + }, + /// Remove the `name` path from the resulting tree. + Remove { name: PathBuf }, +} + /// Find the [`Object`] corresponding to the given `oid`. /// /// Will fail if the object does not exist. diff --git a/git-storage/src/odb/write.rs b/git-storage/src/odb/write.rs index 6122d96..67d9d01 100644 --- a/git-storage/src/odb/write.rs +++ b/git-storage/src/odb/write.rs @@ -6,7 +6,7 @@ use std::error::Error; use git_ext::Oid; use git_ref_format::RefStr; -use super::{Commit, Object, Read, Tree}; +use super::{Commit, Object, Read, Tree, TreeBuilder}; /// Read-write access to a git odb. /// @@ -58,9 +58,5 @@ pub trait Write: Read { R: AsRef; /// Write a [`super::Tree`] using the provided `builder`. - // XXX: It's annoying that this exposes git2 but the effort of abstracting the - // tree builder is too high - // TODO: instead of passing the builder, we could use an enum of operations to - // modify a builder - fn write_tree(&self, builder: git2::TreeBuilder) -> Result; + fn write_tree(&self, builder: TreeBuilder) -> Result; } diff --git a/git-storage/t/Cargo.toml b/git-storage/t/Cargo.toml new file mode 100644 index 0000000..3f4c777 --- /dev/null +++ b/git-storage/t/Cargo.toml @@ -0,0 +1,47 @@ +[package] +name = "git-storage-test" +version = "0.1.0" +edition = "2021" +license = "GPL-3.0-or-later" + +publish = false + +[lib] +doctest = false +test = true +doc = false + +[features] +test = [] + +[dependencies] +proptest = "1" + +[dependencies.git-storage] +path = ".." + +[dependencies.git2] +version = "0.15.0" +default-features = false +features = ["vendored-libgit2"] + +[dependencies.radicle-git-ext] +path = "../../radicle-git-ext/" + +[dependencies.test-helpers] +path = "../../test/test-helpers" + +[dev-dependencies.uuid] +version = "1" +features = ["v4"] + +[dev-dependencies.git-ext-test] +path = "../../radicle-git-ext/t" +features = ["test"] + +[dev-dependencies.git-ref-format] +path = "../../git-ref-format" + +[dev-dependencies.git-ref-format-test] +path = "../../git-ref-format/t" +features = ["test"] diff --git a/git-storage/t/src/gen.rs b/git-storage/t/src/gen.rs new file mode 100644 index 0000000..3ff4dc4 --- /dev/null +++ b/git-storage/t/src/gen.rs @@ -0,0 +1,108 @@ +// Copyright © 2022 The Radicle Link Contributors +// SPDX-License-Identifier: GPL-3.0-or-later + +use std::path::PathBuf; + +use git_storage::{ + odb::{self, write::Write as _}, + signature::UserInfo, + Write, +}; +use proptest::prelude::*; +use radicle_git_ext::Oid; + +use git2::FileMode; + +/// Represents a file in the git tree but without linking it to the repo yet +#[derive(Clone, Debug)] +pub struct File { + pub path: PathBuf, + pub inner: Vec, + pub mode: git2::FileMode, + pub oid: git2::Oid, +} + +/// Represents a Tree to be written with the ODB writer. +/// +/// This Tree does not have an explicit link to a repository, linking is +/// performed by writing it to the repo using the ODB writer. +/// +/// Used as a replacement of git2::TreeBuilder, which is more complicated to +/// build since it requires a repository from the beginning. +#[derive(Clone, Debug)] +pub struct Tree { + builder: odb::TreeBuilder, + files: Vec, +} + +impl Tree { + /// Write the files to the filesystem and the repository `storage` + pub fn write(self, storage: &Write) -> Result { + self.write_files(storage)?; + storage.write_tree(self.builder) + } + + /// Write the files to the filesystem + pub fn write_files(&self, storage: &Write) -> Result<(), git2::Error> { + for file in &self.files { + storage.write_blob(&file.inner)?; + } + Ok(()) + } +} + +/// Any valid filename +pub fn trivial() -> impl Strategy { + "[a-zA-Z0-9]+" +} + +pub fn gen_signature() -> impl Strategy { + trivial().prop_map(move |name| { + UserInfo { + name: name.clone(), + // TODO: is it worth to make this more realistic? + email: format!("{}@{}.com", &name, &name), + } + }) +} + +pub fn gen_bytes() -> impl Strategy> { + any::>() +} + +pub fn gen_mode() -> impl Strategy { + prop_oneof![Just(FileMode::Blob), Just(FileMode::BlobExecutable)] +} + +prop_compose! { + pub fn gen_file() + (path in trivial(), + blob in gen_bytes(), + mode in gen_mode()) + -> File { + let oid = git2::Oid::hash_object(git2::ObjectType::Blob, &blob).unwrap(); + File { + path: PathBuf::from(path), + inner: blob, + mode, + oid, + } + } +} + +pub fn gen_file_set(max_size: u8) -> impl Strategy> { + prop::collection::vec(gen_file(), 0..(max_size as usize)) +} + +/// Generates a [`odb::TreeBuilder`] and a set of [`File`]s. +/// +/// To write the resulting `Tree`, you MUST write the [`File::oid`] to the +/// repository first. +pub fn gen_tree(max_size: u8) -> impl Strategy { + gen_file_set(max_size).prop_map(move |files| { + let builder = files.iter().fold(odb::TreeBuilder::new(), |tree, file| { + tree.insert(file.path.clone(), file.oid.into(), file.mode) + }); + Tree { builder, files } + }) +} diff --git a/git-storage/t/src/lib.rs b/git-storage/t/src/lib.rs new file mode 100644 index 0000000..31295a2 --- /dev/null +++ b/git-storage/t/src/lib.rs @@ -0,0 +1,9 @@ +// Copyright © 2022 The Radicle Link Contributors +// SPDX-License-Identifier: GPL-3.0-or-later + +#[cfg(any(test, feature = "test"))] +pub mod gen; +#[cfg(test)] +mod properties; +#[cfg(any(test, feature = "test"))] +pub mod tmp; diff --git a/git-storage/t/src/properties.rs b/git-storage/t/src/properties.rs new file mode 100644 index 0000000..5ea704e --- /dev/null +++ b/git-storage/t/src/properties.rs @@ -0,0 +1,4 @@ +// Copyright © 2022 The Radicle Link Contributors +// SPDX-License-Identifier: GPL-3.0-or-later + +mod odb; diff --git a/git-storage/t/src/properties/odb.rs b/git-storage/t/src/properties/odb.rs new file mode 100644 index 0000000..f33e5c4 --- /dev/null +++ b/git-storage/t/src/properties/odb.rs @@ -0,0 +1,108 @@ +// Copyright © 2022 The Radicle Link Contributors +// SPDX-License-Identifier: GPL-3.0-or-later + +use git2::ObjectType; + +use git_ref_format::RefString; +use git_ref_format_test::gen::valid; +use git_storage::{ + odb::{Read as _, Write as _}, + signature::UserInfo, +}; + +use proptest::prelude::*; + +use crate::{gen, tmp}; + +// NOTE: It's enough to check the `Oid`s. If the contents were different the +// hash would be different. +pub mod prop { + use super::*; + + pub fn roundtrip_blob(user: UserInfo, bytes: &[u8]) { + let writer = tmp::writer(user); + + let oid = writer.write_blob(bytes).unwrap(); + + let readback = writer.find_blob(oid).unwrap().unwrap(); + assert_eq!(oid, readback.id().into()); + + let readback = writer.find_object(oid).unwrap().unwrap(); + assert_eq!(readback.kind(), Some(ObjectType::Blob)); + } + + pub fn roundtrip_tree(user: UserInfo, tree: gen::Tree) { + let writer = tmp::writer(user); + let oid = tree.write(&writer).unwrap(); + let readback = writer.find_tree(oid).unwrap().unwrap(); + assert_eq!(oid, readback.id().into()); + + let readback = writer.find_object(oid).unwrap().unwrap(); + assert_eq!(readback.kind(), Some(ObjectType::Tree)); + } + + pub fn roundtrip_commit(user: UserInfo, tree: gen::Tree, message: &str) { + let writer = tmp::writer(user); + let tree_oid = tree.write(&writer).unwrap(); + let tree = writer.find_tree(tree_oid).unwrap().unwrap(); + let commit_oid = writer.write_commit(&tree, &[], message).unwrap(); + + let readback = writer.find_commit(commit_oid).unwrap().unwrap(); + assert_eq!(readback.id(), commit_oid.into()); + + let readback = writer.find_object(commit_oid).unwrap().unwrap(); + assert_eq!(readback.kind(), Some(ObjectType::Commit)); + } + + pub fn roundtrip_tag(user: UserInfo, tree: gen::Tree, tag_name: RefString, message: &str) { + let writer = tmp::writer(user); + let tree_oid = tree.write(&writer).unwrap(); + let tree = writer.find_tree(tree_oid).unwrap().unwrap(); + let commit_oid = writer.write_commit(&tree, &[], message).unwrap(); + let commit_object = writer.find_object(commit_oid).unwrap().unwrap(); + + let tag_oid = writer.write_tag(tag_name, &commit_object, message).unwrap(); + + let readback = writer.find_tag(tag_oid).unwrap().unwrap(); + assert_eq!(tag_oid, readback.id().into()); + + let readback = writer.find_object(tag_oid).unwrap().unwrap(); + assert_eq!(readback.kind(), Some(ObjectType::Tag)); + } +} + +proptest! { + #[test] + fn roundtrip_blob(user in gen::gen_signature(), bytes in gen::gen_bytes()) { + prop::roundtrip_blob(user, &bytes) + } + + + #[test] + fn roundtrip_tree( + user in gen::gen_signature(), + tree in gen::gen_tree(10), + ) { + prop::roundtrip_tree(user, tree) + } + + #[test] + fn roundtrip_commit( + user in gen::gen_signature(), + tree in gen::gen_tree(10), + message in gen::trivial() + ) { + prop::roundtrip_commit(user, tree, &message) + } + + #[test] + fn roundtrip_tag( + user in gen::gen_signature(), + tree in gen::gen_tree(10), + message in gen::trivial(), + tag_name in valid() + ) { + let tag_name = RefString::try_from(tag_name).unwrap(); + prop::roundtrip_tag(user, tree, tag_name, &message); + } +} diff --git a/git-storage/t/src/tmp.rs b/git-storage/t/src/tmp.rs new file mode 100644 index 0000000..cd16102 --- /dev/null +++ b/git-storage/t/src/tmp.rs @@ -0,0 +1,14 @@ +use git2::Repository; +use git_storage::{signature::UserInfo, Write}; +use test_helpers::tempdir::WithTmpDir; + +pub type TmpWriter = WithTmpDir; +pub type TmpRepo = WithTmpDir; + +pub fn writer(user: UserInfo) -> TmpWriter { + WithTmpDir::new(|path| { + Write::open(path, user) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, format!("{}", e))) + }) + .unwrap() +} diff --git a/test/Cargo.toml b/test/Cargo.toml index 0241032..1ff5277 100644 --- a/test/Cargo.toml +++ b/test/Cargo.toml @@ -33,3 +33,8 @@ features = ["test"] [dev-dependencies.radicle-surf-test] path = "../radicle-surf/t" features = ["test"] + +[dev-dependencies.git-storage-test] +path = "../git-storage/t" +features = ["test"] +