7fe6993 Merge remote-tracking branch 'han/new-design-3' — radicle-git

modified radicle-surf/docs/refactor-design.md

@@ -68,6 +68,13 @@ use `DirectoryContents` for its items and not to use `Tree` or `Forest` at all.

 We also found the `list_directory()` method duplicates with `iter()` method.
 Hence `list_directory()` is removed, together with `SystemType` type.
 ## Remove `Vcs` trait
 The `Vcs` trait was introduced to support different version control backends,
 for example both Git and Pijul, and potentially others. However, since this
 port is part of `radicle-git` repo, we are only supporting Git going forward.
 We no longer need another layer of indirection defined by `Vcs` trait.
 ## The new API
 With the changes proposed in the previous section, we describe what the new API

modified radicle-surf/src/commit.rs

@@ -30,10 +30,7 @@ use crate::{

     file_system,
     person::Person,
     revision::Revision,
     vcs::{
         git::{self, BranchName, RepositoryRef, Rev},
         Vcs,
     },
     vcs::git::{self, BranchName, RepositoryRef, Rev},
 };
 use radicle_git_ext::Oid;

@@ -244,7 +241,7 @@ where

     };
     let stats = repo.get_stats(&rev)?;
     let headers = repo.get_history(rev)?.iter().map(Header::from).collect();
     let headers = repo.history(rev)?.iter().map(Header::from).collect();
     Ok(Commits { headers, stats })
 }

modified radicle-surf/src/object/tree.rs

@@ -32,10 +32,7 @@ use crate::{

     git::RepositoryRef,
     object::{Error, Info, ObjectType},
     revision::Revision,
     vcs::{
         git::{Branch, Rev},
         Vcs,
     },
     vcs::git::{Branch, Rev},
 };
 /// Result of a directory listing, carries other trees and blobs.

@@ -158,7 +155,7 @@ where

     entries.sort_by(|a, b| a.info.object_type.cmp(&b.info.object_type));
     let last_commit = if path.is_root() {
         Some(commit::Header::from(repo.get_history(rev).unwrap().first()))
         Some(commit::Header::from(repo.history(rev).unwrap().first()))
     } else {
         None
     };

modified radicle-surf/src/vcs.rs

@@ -15,10 +15,8 @@

 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 //! A model of a general VCS. The components consist of a [`History`]
 //! and a [`Vcs`] trait.
 //! A model of a general VCS History.
 // use crate::file_system::directory::Directory;
 use nonempty::NonEmpty;
 pub mod git;

@@ -134,34 +132,3 @@ impl<A> History<A> {

             .collect()
     }
 }
 pub(crate) trait GetVcs<Error>
 where
     Self: Sized,
 {
     /// The way to identify a Repository.
     type RepoId;
     /// Find a Repository
     fn get_repo(identifier: Self::RepoId) -> Result<Self, Error>;
 }
 /// The `VCS` trait encapsulates the minimal amount of information for
 /// interacting with some notion of `History` from a given
 /// Version-Control-System.
 pub trait Vcs<A, Error> {
     /// The way to identify a History.
     type HistoryId;
     /// The way to identify an artefact.
     type ArtefactId;
     /// Find a History in a Repo given a way to identify it
     fn get_history(&self, identifier: Self::HistoryId) -> Result<History<A>, Error>;
     /// Find all histories in a Repo
     fn get_histories(&self) -> Result<Vec<History<A>>, Error>;
     /// Identify artefacts of a Repository
     fn get_identifier(artefact: &A) -> Self::ArtefactId;
 }

modified radicle-surf/src/vcs/git/repo.rs

@@ -20,21 +20,18 @@ use crate::{

     file_system,
     file_system::{directory, DirectoryContents, Label},
     vcs,
     vcs::{
         git::{
             error::*,
             reference::{glob::RefGlob, Ref, Rev},
             Branch,
             BranchName,
             Commit,
             Namespace,
             RefScope,
             Signature,
             Stats,
             Tag,
             TagName,
         },
         Vcs,
     vcs::git::{
         error::*,
         reference::{glob::RefGlob, Ref, Rev},
         Branch,
         BranchName,
         Commit,
         Namespace,
         RefScope,
         Signature,
         Stats,
         Tag,
         TagName,
     },
 };
 use directory::Directory;

@@ -145,21 +142,15 @@ impl<'a> RepositoryRef<'a> {

         Ok(namespaces?.into_iter().collect())
     }
     pub(super) fn reference<R, P>(&self, reference: R, check: P) -> Result<History, Error>
     pub(super) fn ref_history<R>(&self, reference: R) -> Result<History, Error>
     where
         R: Into<Ref>,
         P: FnOnce(&git2::Reference) -> Option<Error>,
     {
         let reference = match self.which_namespace()? {
             None => reference.into(),
             Some(namespace) => reference.into().namespaced(namespace),
         }
         .find_ref(self)?;
         if let Some(err) = check(&reference) {
             return Err(err);
         }
         self.to_history(&reference)
     }

@@ -207,7 +198,7 @@ impl<'a> RepositoryRef<'a> {

     /// Gets stats of `rev`.
     pub fn get_stats(&self, rev: &Rev) -> Result<Stats, Error> {
         let branches = self.list_branches(RefScope::Local)?.len();
         let history = self.get_history(rev.clone())?;
         let history = self.history(rev.clone())?;
         let commits = history.len();
         let contributors = history

@@ -280,12 +271,6 @@ impl<'a> RepositoryRef<'a> {

         Ok(commit)
     }
     /// Build a [`History`] using the `head` reference.
     pub fn head_history(&self) -> Result<History, Error> {
         let head = self.repo_ref.head()?;
         self.to_history(&head)
     }
     /// Turn a [`git2::Reference`] into a [`History`] by completing
     /// a revwalk over the first commit in the reference.
     pub(super) fn to_history(&self, history: &git2::Reference<'a>) -> Result<History, Error> {

@@ -439,10 +424,15 @@ impl<'a> RepositoryRef<'a> {

             opts.skip_binary_check(true);
         }
         let diff =
         let mut diff =
             self.repo_ref
                 .diff_tree_to_tree(old_tree.as_ref(), Some(&new_tree), Some(&mut opts))?;
         // Detect renames by default.
         let mut find_opts = git2::DiffFindOptions::new();
         find_opts.renames(true);
         diff.find_similar(Some(&mut find_opts))?;
         Ok(diff)
     }

@@ -539,40 +529,17 @@ impl<'a> RepositoryRef<'a> {

             rev: commit.id().to_string(),
         })
     }
 }
 impl<'a> Vcs<Commit, Error> for RepositoryRef<'a> {
     type HistoryId = Rev;
     type ArtefactId = Oid;
     fn get_history(&self, history_id: Self::HistoryId) -> Result<History, Error> {
         match history_id {
             Rev::Ref(reference) => self.reference(reference, |_| None),
     /// Returns the history of `rev`.
     pub fn history(&self, rev: Rev) -> Result<History, Error> {
         match rev {
             Rev::Ref(reference) => self.ref_history(reference),
             Rev::Oid(oid) => {
                 let commit = self.get_git2_commit(oid)?;
                 self.commit_to_history(commit)
             },
         }
     }
     fn get_histories(&self) -> Result<Vec<History>, Error> {
         self.repo_ref
             .references()
             .map_err(Error::from)
             .and_then(|mut references| {
                 references.try_fold(vec![], |mut acc, reference| {
                     reference.map_err(Error::from).and_then(|r| {
                         let history = self.to_history(&r)?;
                         acc.push(history);
                         Ok(acc)
                     })
                 })
             })
     }
     fn get_identifier(artifact: &Commit) -> Self::ArtefactId {
         artifact.id
     }
 }
 impl<'a> std::fmt::Debug for RepositoryRef<'a> {

@@ -608,16 +575,6 @@ impl<'a> From<&'a Repository> for RepositoryRef<'a> {

     }
 }
 impl vcs::GetVcs<Error> for Repository {
     type RepoId = String;
     fn get_repo(repo_id: Self::RepoId) -> Result<Self, Error> {
         git2::Repository::open(&repo_id)
             .map(Repository)
             .map_err(Error::from)
     }
 }
 impl From<git2::Repository> for Repository {
     fn from(repo: git2::Repository) -> Self {
         Repository(repo)

modified radicle-surf/t/src/git.rs

@@ -27,15 +27,14 @@ fn test_submodule_failure() {

 mod namespace {
     use super::*;
     use pretty_assertions::{assert_eq, assert_ne};
     use radicle_surf::vcs::Vcs;
     #[test]
     fn switch_to_banana() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let repo = repo.as_ref();
         let history_master = repo.get_history(Branch::local("master").into())?;
         let history_master = repo.history(Branch::local("master").into())?;
         repo.switch_namespace("golden")?;
         let history_banana = repo.get_history(Branch::local("banana").into())?;
         let history_banana = repo.history(Branch::local("banana").into())?;
         assert_ne!(history_master, history_banana);

@@ -46,14 +45,14 @@ mod namespace {

     fn me_namespace() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let repo = repo.as_ref();
         let history = repo.get_history(Branch::local("master").into())?;
         let history = repo.history(Branch::local("master").into())?;
         assert_eq!(repo.which_namespace(), Ok(None));
         repo.switch_namespace("me")?;
         assert_eq!(repo.which_namespace(), Ok(Some(Namespace::try_from("me")?)));
         let history_feature = repo.get_history(Branch::local("feature/#1194").into())?;
         let history_feature = repo.history(Branch::local("feature/#1194").into())?;
         assert_eq!(history, history_feature);
         let expected_branches: Vec<Branch> = vec![Branch::local("feature/#1194")];

@@ -77,7 +76,7 @@ mod namespace {

     fn golden_namespace() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let repo = repo.as_ref();
         let history = repo.get_history(Branch::local("master").into())?;
         let history = repo.history(Branch::local("master").into())?;
         assert_eq!(repo.which_namespace(), Ok(None));

@@ -88,7 +87,7 @@ mod namespace {

             Ok(Some(Namespace::try_from("golden")?))
         );
         let golden_history = repo.get_history(Branch::local("master").into())?;
         let golden_history = repo.history(Branch::local("master").into())?;
         assert_eq!(history, golden_history);
         let expected_branches: Vec<Branch> = vec![Branch::local("banana"), Branch::local("master")];

@@ -116,7 +115,7 @@ mod namespace {

     fn silver_namespace() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let repo = repo.as_ref();
         let history = repo.get_history(Branch::local("master").into())?;
         let history = repo.history(Branch::local("master").into())?;
         assert_eq!(repo.which_namespace(), Ok(None));

@@ -125,7 +124,7 @@ mod namespace {

             repo.which_namespace(),
             Ok(Some(Namespace::try_from("golden/silver")?))
         );
         let silver_history = repo.get_history(Branch::local("master").into())?;
         let silver_history = repo.history(Branch::local("master").into())?;
         assert_ne!(history, silver_history);
         let expected_branches: Vec<Branch> = vec![Branch::local("master")];

@@ -140,8 +139,6 @@ mod namespace {

 #[cfg(test)]
 mod rev {
     use radicle_surf::vcs::Vcs;
     use super::*;
     use std::str::FromStr;

@@ -160,7 +157,7 @@ mod rev {

     fn _master() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let repo = repo.as_ref();
         let history = repo.get_history(Branch::remote("master", "origin").into())?;
         let history = repo.history(Branch::remote("master", "origin").into())?;
         let commit1 = Oid::from_str("3873745c8f6ffb45c990eb23b491d4b4b6182f95")?;
         assert!(

@@ -197,7 +194,7 @@ mod rev {

     fn commit() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let rev: Rev = Oid::from_str("3873745c8f6ffb45c990eb23b491d4b4b6182f95")?.into();
         let history = repo.as_ref().get_history(rev)?;
         let history = repo.as_ref().history(rev)?;
         let commit1 = Oid::from_str("3873745c8f6ffb45c990eb23b491d4b4b6182f95")?;
         assert!(history

@@ -215,7 +212,7 @@ mod rev {

     fn commit_parents() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let rev: Rev = Oid::from_str("3873745c8f6ffb45c990eb23b491d4b4b6182f95")?.into();
         let history = repo.as_ref().get_history(rev)?;
         let history = repo.as_ref().history(rev)?;
         let commit = history.first();
         assert_eq!(

@@ -230,7 +227,7 @@ mod rev {

     fn commit_short() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let rev: Rev = repo.as_ref().oid("3873745c8")?.into();
         let history = repo.as_ref().get_history(rev)?;
         let history = repo.as_ref().history(rev)?;
         let commit1 = Oid::from_str("3873745c8f6ffb45c990eb23b491d4b4b6182f95")?;
         assert!(history

@@ -248,7 +245,7 @@ mod rev {

     fn tag() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let rev: Rev = TagName::new("v0.2.0").into();
         let history = repo.as_ref().get_history(rev)?;
         let history = repo.as_ref().history(rev)?;
         let commit1 = Oid::from_str("2429f097664f9af0c5b7b389ab998b2199ffa977")?;
         assert_eq!(history.first().id, commit1);

@@ -259,8 +256,6 @@ mod rev {

 #[cfg(test)]
 mod last_commit {
     use radicle_surf::vcs::Vcs;
     use super::*;
     use std::str::FromStr;

@@ -378,7 +373,7 @@ mod last_commit {

         let expected_oid = repo
             .as_ref()
             .get_history(Branch::local("master").into())
             .history(Branch::local("master").into())
             .unwrap()
             .first()
             .id;

@@ -464,6 +459,42 @@ mod diff {

         Ok(())
     }
     #[test]
     fn test_branch_diff() -> Result<(), Error> {
         let repo = Repository::new(GIT_PLATINUM)?;
         let repo = repo.as_ref();
         let diff = repo.diff(
             &Branch::local("master").into(),
             &Branch::local("dev").into(),
         )?;
         println!("Diff two branches: master -> dev");
         println!(
             "result: created {} deleted {} moved {} modified {}",
             diff.created.len(),
             diff.deleted.len(),
             diff.moved.len(),
             diff.modified.len()
         );
         assert_eq!(diff.created.len(), 1);
         assert_eq!(diff.deleted.len(), 11);
         assert_eq!(diff.moved.len(), 1);
         assert_eq!(diff.modified.len(), 2);
         for c in diff.created.iter() {
             println!("created: {}", &c.path);
         }
         for d in diff.deleted.iter() {
             println!("deleted: {}", &d.path);
         }
         for m in diff.moved.iter() {
             println!("moved: {} -> {}", &m.old_path, &m.new_path);
         }
         for m in diff.modified.iter() {
             println!("modified: {}", &m.path);
         }
         Ok(())
     }
     #[cfg(feature = "serialize")]
     #[test]
     fn test_diff_serde() -> Result<(), Error> {