3184059 REVIEW(WIP): refactor changes — heartwood

modified crates/radicle-cli/examples/git/git-push-converge.md

@@ -91,7 +91,7 @@ commit:

 ``` ~alice (stderr)
 $ git push rad -f
 warn: could not determine commit for canonical reference 'refs/heads/master', no commit with at least 3 vote(s) found (threshold not met)
 warn: could not determine target for canonical reference 'refs/heads/master', no object with at least 3 vote(s) found (threshold not met)
 warn: it is recommended to find a commit to agree upon
 ✓ Synced with 2 seed(s)
 To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi

modified crates/radicle-cli/examples/rad-id-threshold.md

@@ -64,7 +64,7 @@ $ git commit -v -m "Define power requirements"

 ``` ~alice (stderr) RAD_SOCKET=/dev/null
 $ git push rad master
 ✓ Canonical reference refs/heads/master updated to target commit f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354
 ✓ Canonical reference refs/heads/master updated to target commit 3e674d1a1df90807e934f9ae5da2591dd6848a33
 To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi
    f2de534..3e674d1  master -> master
 ```

modified crates/radicle-remote-helper/src/push.rs

@@ -120,6 +120,10 @@ pub enum Error {

     PushAction(#[from] error::PushAction),
     #[error(transparent)]
     Canonical(#[from] error::CanonicalUnrecoverable),
     #[error(transparent)]
     CanonicalInit(#[from] radicle::git::canonical::error::CanonicalError),
     #[error("could not determine object type for {oid}")]
     UnknownObjectType { oid: git::Oid },
 }
 /// Push command.

@@ -349,9 +353,11 @@ pub fn run(

                         // Note that we *do* allow rolling back to a previous commit on the
                         // canonical branch.
                         if let Some(canonical) = rules.canonical(dst.clone(), stored)? {
                             let kind = working.find_object(**src, None)?.kind();
                             let canonical =
                                 canonical::Canonical::new(me, *src, kind.unwrap(), canonical);
                             let kind = working
                                 .find_object(**src, None)?
                                 .kind()
                                 .ok_or(Error::UnknownObjectType { oid: *src })?;
                             let canonical = canonical::Canonical::new(me, *src, kind, canonical);
                             match canonical.quorum(&working) {
                                 Ok(quorum) => set_canonical_refs.push(quorum),
                                 Err(e) => canonical::io::handle_error(e, &dst, hints)?,

modified crates/radicle/src/git/canonical.rs

@@ -1,15 +1,20 @@

 pub mod error;
 use error::*;
 pub mod rules;
 use nonempty::NonEmpty;
 pub use rules::{MatchedRule, RawRule, Rules, ValidRule};
 use std::cell::Cell;
 use std::collections::BTreeMap;
 use std::fmt;
 use raw::ObjectType;
 use raw::Repository;
 use crate::prelude::Did;
 use crate::storage::git;
 use super::raw;
 use super::{Oid, Qualified};

@@ -29,7 +34,33 @@ use super::{Oid, Qualified};

 pub struct Canonical<'a, 'b> {
     refname: Qualified<'a>,
     rule: &'b ValidRule,
     tips: BTreeMap<Did, (Oid, git2::ObjectType)>,
     tips: BTreeMap<Did, (Oid, CanonicalObject)>,
 }
 /// Support Git object types for canonical computation
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 enum CanonicalObject {
     Commit,
     Tag,
 }
 impl fmt::Display for CanonicalObject {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             CanonicalObject::Commit => f.write_str("commit"),
             CanonicalObject::Tag => f.write_str("tag"),
         }
     }
 }
 impl CanonicalObject {
     fn new(kind: git::raw::ObjectType) -> Option<Self> {
         match kind {
             ObjectType::Commit => Some(Self::Commit),
             ObjectType::Tag => Some(Self::Tag),
             _ => None,
         }
     }
 }
 impl<'a, 'b> Canonical<'a, 'b> {

@@ -39,31 +70,47 @@ impl<'a, 'b> Canonical<'a, 'b> {

         repo: &Repository,
         refname: Qualified<'a>,
         rule: &'b ValidRule,
     ) -> Result<Self, raw::Error> {
     ) -> Result<Self, CanonicalError> {
         let mut tips = BTreeMap::new();
         for delegate in rule.allowed().iter() {
             let name = &refname.with_namespace(delegate.as_key().into());
             let reference = match repo.find_reference(&name) {
             let reference = match repo.find_reference(name) {
                 Ok(reference) => reference,
                 Err(e) if super::ext::is_not_found_err(&e) => {
                     log::warn!(
                         target: "radicle",
                         "Missing `refs/namespaces/{}/{refname}` while calculating the canonical reference",
                         delegate.as_key()
                         "Missing `{name}` while calculating the canonical reference",
                     );
                     continue;
                 }
                 Err(e) => return Err(e),
                 Err(e) => return Err(CanonicalError::find_reference(name, e)),
             };
             let Some(oid) = reference.target() else {
                 log::warn!(target: "radicle", "Missing target for reference `{name}`");
                 continue;
             };
             let Some(kind) = repo.find_object(oid, None)?.kind() else {
                 continue;
             };
             let kind = match repo.find_object(oid, None) {
                 Ok(object) => object.kind().and_then(CanonicalObject::new).ok_or_else(|| {
                     CanonicalError::invalid_object_type(
                         repo.path().to_path_buf(),
                         *delegate,
                         oid.into(),
                         object.kind(),
                     )
                 }),
                 Err(err) if super::ext::is_not_found_err(&err) => {
                     Err(CanonicalError::missing_object(
                         repo.path().to_path_buf(),
                         *delegate,
                         oid.into(),
                         err,
                     ))
                 }
                 Err(err) => Err(CanonicalError::find_object(oid.into(), err)),
             }?;
             tips.insert(*delegate, (oid.into(), kind));
         }

@@ -89,8 +136,10 @@ impl<'a, 'b> Canonical<'a, 'b> {

     /// In some cases, we allow the vote to be modified. For example, when the
     /// `did` is pushing a new commit, we may want to see if the new commit will
     /// reach a quorum.
     pub fn modify_vote(&mut self, did: Did, new: (Oid, git2::ObjectType)) {
         self.tips.insert(did, new);
     pub fn modify_vote(&mut self, did: Did, (oid, kind): (Oid, git2::ObjectType)) {
         if let Some(kind) = CanonicalObject::new(kind) {
             self.tips.insert(did, (oid, kind));
         }
     }
     /// Check that the provided `did` is part of the set of allowed

@@ -115,7 +164,39 @@ impl<'a, 'b> Canonical<'a, 'b> {

         repo: &Repository,
         (candidate, commit): (&Did, &Oid),
     ) -> Result<bool, ConvergesError> {
         let mut common_kind = ObjectType::Any;
         /// Ensures [`Oid`]s are of the same object type
         enum Objects {
             Commits(NonEmpty<Oid>),
             Tags(NonEmpty<Oid>),
         }
         impl Objects {
             fn new(oid: Oid, kind: CanonicalObject) -> Self {
                 match kind {
                     CanonicalObject::Commit => Self::Commits(NonEmpty::new(oid)),
                     CanonicalObject::Tag => Self::Tags(NonEmpty::new(oid)),
                 }
             }
             fn insert(mut self, oid: Oid, kind: CanonicalObject) -> Result<Self, CanonicalObject> {
                 match self {
                     Objects::Commits(ref mut commits) => match kind {
                         CanonicalObject::Commit => {
                             commits.push(oid);
                             Ok(self)
                         }
                         CanonicalObject::Tag => Err(CanonicalObject::Tag),
                     },
                     Objects::Tags(ref mut tags) => match kind {
                         CanonicalObject::Commit => {
                             tags.push(oid);
                             Ok(self)
                         }
                         CanonicalObject::Tag => Err(CanonicalObject::Commit),
                     },
                 }
             }
         }
         let heads = {
             let heads = self

@@ -123,75 +204,77 @@ impl<'a, 'b> Canonical<'a, 'b> {

                 .iter()
                 .filter_map(|(did, tip)| (did != candidate).then_some((did, tip)));
             let mut result = Vec::with_capacity(heads.size_hint().0);
             for (did, (oid, kind)) in heads {
                 if common_kind == ObjectType::Any {
                     common_kind = *kind;
                 } else if common_kind != *kind {
                     return Err(ConvergesError::invalid_object_kind(
                         repo.path().to_path_buf(),
                         *did,
                         *oid,
                         Some(*kind),
                     ));
             let mut objects = None;
             for (_, (oid, kind)) in heads {
                 let oid = *oid;
                 let kind = *kind;
                 match objects {
                     None => objects = Some(Objects::new(oid, kind)),
                     Some(objs) => {
                         objects = Some(objs.insert(oid, kind).map_err(|expected| {
                             ConvergesError::mismatched_object(
                                 repo.path().to_path_buf(),
                                 oid,
                                 kind,
                                 expected,
                             )
                         })?)
                     }
                 }
                 result.push(Self::ensure_commit_or_tag(*did, *oid, repo)?);
             }
             result
             objects
         };
         if common_kind == ObjectType::Commit {
             for (head, _) in heads {
                 let (ahead, behind) = repo
                     .graph_ahead_behind(**commit, *head)
                     .map_err(|err| ConvergesError::graph_descendant(*commit, head, err))?;
                 if ahead * behind == 0 {
                     return Ok(true);
         match heads {
             None => Ok(true),
             Some(Objects::Tags(_)) => Ok(true),
             Some(Objects::Commits(heads)) => {
                 for head in heads {
                     let (ahead, behind) = repo
                         .graph_ahead_behind(**commit, *head)
                         .map_err(|err| ConvergesError::graph_descendant(*commit, head, err))?;
                     if ahead * behind == 0 {
                         return Ok(true);
                     }
                 }
                 Ok(false)
             }
         } else {
             return Ok(true);
         }
         Ok(false)
     }
     fn filter_candidates(&self, kind: raw::ObjectType) -> BTreeMap<Oid, Cell<usize>> {
     fn votes_by_kind(&self, kind: CanonicalObject) -> Votes {
         let filtered = self
             .tips
             .values()
             .filter_map(|(tip_oid, tip_kind)| (kind == *tip_kind).then_some(tip_oid));
             .filter_map(|(tip_oid, tip_kind)| (kind == *tip_kind).then_some(*tip_oid));
         let mut candidates = BTreeMap::<_, Cell<usize>>::new();
         candidates = filtered.fold(candidates, |mut candidates, oid| {
             candidates.entry(*oid).or_default().update(|x| x + 1);
             candidates
         });
         candidates
         filtered.collect()
     }
     fn quorum_tag(&self) -> Result<Oid, QuorumError> {
         let mut candidates = self.filter_candidates(raw::ObjectType::Tag);
         let mut votes = self.votes_by_kind(CanonicalObject::Tag);
         // Keep tags which pass the threshold.
         candidates.retain(|_, votes| votes.get() >= self.threshold());
         votes.votes_past_threshold(self.threshold());
         if candidates.len() > 1 {
         if votes.number_of_candidates() > 1 {
             return Err(QuorumError::DivergingTags {
                 refname: self.refname.to_string(),
                 threshold: self.threshold(),
                 candidates: candidates.keys().cloned().collect(),
                 candidates: votes.candidates().cloned().collect(),
             });
         }
         let (longest, _) = candidates.pop_first().ok_or(QuorumError::NoCandidates {
             refname: self.refname.to_string(),
             threshold: self.threshold(),
         })?;
         let tag = votes
             .pop_first_candidate()
             .ok_or(QuorumError::NoCandidates {
                 refname: self.refname.to_string(),
                 threshold: self.threshold(),
             })?;
         Ok((*longest).into())
         Ok((*tag).into())
     }
     /// Computes the quorum or "canonical" tip based on the tips, of `Canonical`,

@@ -202,14 +285,14 @@ impl<'a, 'b> Canonical<'a, 'b> {

     /// Also returns an error if `heads` is empty or `threshold` cannot be
     /// satisified with the number of heads given.
     fn quorum_commit(&self, repo: &raw::Repository) -> Result<Oid, QuorumError> {
         let mut candidates = self.filter_candidates(raw::ObjectType::Commit);
         let mut candidates = self.votes_by_kind(CanonicalObject::Commit);
         // Build a list of candidate commits and count how many "votes" each of them has.
         // Commits get a point for each direct vote, as well as for being part of the ancestry
         // of a commit given to this function. Only commits given to the function are considered.
         for (i, head) in candidates.keys().enumerate() {
         for (i, head) in candidates.candidates().enumerate() {
             // Compare this head to all other heads ahead of it in the list.
             for other in candidates.keys().skip(i + 1) {
             for other in candidates.candidates().skip(i + 1) {
                 // N.b. if heads are equal then skip it, otherwise it will end up as
                 // a double vote.
                 debug_assert!(*head != *other);

@@ -217,22 +300,24 @@ impl<'a, 'b> Canonical<'a, 'b> {

                 let base = Oid::from(repo.merge_base(**head, **other)?);
                 if base == *other || base == *head {
                     candidates.get(&base).unwrap().update(|votes| votes + 1);
                     candidates.vote(&base);
                 }
             }
         }
         // Keep commits which pass the threshold.
         candidates.retain(|_, votes| votes.get() >= self.threshold());
         candidates.votes_past_threshold(self.threshold());
         let (mut longest, _) = candidates.pop_first().ok_or(QuorumError::NoCandidates {
             refname: self.refname.to_string(),
             threshold: self.threshold(),
         })?;
         let mut longest = candidates
             .pop_first_candidate()
             .ok_or(QuorumError::NoCandidates {
                 refname: self.refname.to_string(),
                 threshold: self.threshold(),
             })?;
         // Now that all scores are calculated, figure out what is the longest branch
         // that passes the threshold. In case of divergence, return an error.
         for head in candidates.keys() {
         for head in candidates.candidates() {
             let base = repo.merge_base(**head, *longest)?;
             if base == *longest {

@@ -303,40 +388,69 @@ impl<'a, 'b> Canonical<'a, 'b> {

     fn threshold(&self) -> usize {
         (*self.rule.threshold()).into()
     }
 }
     fn ensure_commit_or_tag(
         from: Did,
         commit_or_tag: Oid,
         working: &Repository,
     ) -> Result<(Oid, ObjectType), ConvergesError> {
         match working.find_object(*commit_or_tag, None) {
             Ok(object) => match object.kind() {
                 Some(kind @ ObjectType::Commit) | Some(kind @ ObjectType::Tag) => {
                     Ok((object.id().into(), kind))
                 }
                 kind => Err(ConvergesError::invalid_object_kind(
                     working.path().to_path_buf(),
                     from,
                     commit_or_tag,
                     kind,
                 )),
             },
             Err(err) if err.code() == raw::ErrorCode::NotFound => {
                 Err(ConvergesError::missing_object(
                     working.path().to_path_buf(),
                     from,
                     commit_or_tag,
                     err,
                 ))
             }
             Err(err) => Err(ConvergesError::invalid_object(
                 working.path().to_path_buf(),
                 from,
                 commit_or_tag,
                 err,
             )),
 #[derive(Debug, Default, PartialEq, Eq)]
 struct Votes {
     inner: BTreeMap<Oid, Cell<u8>>,
 }
 impl FromIterator<Oid> for Votes {
     fn from_iter<T: IntoIterator<Item = Oid>>(iter: T) -> Self {
         iter.into_iter().fold(Self::default(), |mut votes, oid| {
             *votes.inner.entry(oid).or_default().get_mut() += 1;
             votes
         })
     }
 }
 impl FromIterator<(Oid, u8)> for Votes {
     fn from_iter<T: IntoIterator<Item = (Oid, u8)>>(iter: T) -> Self {
         iter.into_iter()
             .fold(Self::default(), |mut votes, (oid, n)| {
                 *votes.inner.entry(oid).or_default().get_mut() += n;
                 votes
             })
     }
 }
 impl Votes {
     /// Increase the vote count for `oid`.
     ///
     /// If `oid` does not exist in the set of [`Votes`] yet, then no vote will
     /// be added.
     #[inline]
     fn vote(&self, oid: &Oid) {
         if let Some(votes) = self.inner.get(oid) {
             votes.set(votes.get() + 1)
         }
     }
     /// Filter the candidates by the ones that have a number of votes that pass
     /// the `threshold`.
     #[inline]
     fn votes_past_threshold(&mut self, threshold: usize) {
         self.inner
             .retain(|_, votes| votes.get() as usize >= threshold);
     }
     /// Get the number of candidates this set of votes has.
     #[inline]
     fn number_of_candidates(&self) -> usize {
         self.inner.len()
     }
     /// Get the set candidates.
     #[inline]
     fn candidates(&self) -> impl Iterator<Item = &Oid> {
         self.inner.keys()
     }
     /// Pop off the first candidate from the set of votes.
     #[inline]
     fn pop_first_candidate(&mut self) -> Option<Oid> {
         self.inner.pop_first().map(|(oid, _)| oid)
     }
 }
 #[cfg(test)]

@@ -355,13 +469,18 @@ mod tests {

         threshold: usize,
         repo: &git::raw::Repository,
     ) -> Result<Oid, QuorumError> {
         let tips: BTreeMap<Did, (Oid, git2::ObjectType)> = heads
         let tips: BTreeMap<Did, (Oid, CanonicalObject)> = heads
             .iter()
             .enumerate()
             .map(|(i, head)| {
                 let signer = Device::mock_from_seed([(i + 1) as u8; 32]);
                 let did = Did::from(signer.public_key());
                 let kind = repo.find_object(*head, None).unwrap().kind().unwrap();
                 let kind = repo
                     .find_object(*head, None)
                     .unwrap()
                     .kind()
                     .and_then(CanonicalObject::new)
                     .unwrap();
                 (did, ((*head).into(), kind))
             })
             .collect();

@@ -725,4 +844,23 @@ mod tests {

             Err(QuorumError::NoCandidates { .. })
         );
     }
     #[test]
     fn test_votes() {
         let tmp = tempfile::tempdir().unwrap();
         let (repo, c0) = fixtures::repository(tmp.path());
         let c0: git::Oid = c0.into();
         let c1 = fixtures::commit("C1", &[*c0], &repo);
         let c2 = fixtures::commit("C2", &[*c1], &repo);
         let c3 = fixtures::commit("C3", &[*c1], &repo);
         let b2 = fixtures::commit("B2", &[*c1], &repo);
         let votes = Votes::from_iter([c0, c1, c2, c3]);
         votes.vote(&c1);
         votes.vote(&c2);
         votes.vote(&b2);
         assert_eq!(
             votes,
             [(c0, 1), (c1, 2), (c2, 2), (c3, 1)].into_iter().collect()
         );
     }
 }

modified crates/radicle/src/git/canonical/error.rs

@@ -4,6 +4,8 @@ use thiserror::Error;

 use crate::{git::raw, git::Oid, prelude::Did};
 use super::CanonicalObject;
 /// Error that can occur when calculation the [`Canonical::quorum`].
 #[derive(Debug, Error)]
 pub enum QuorumError {

@@ -68,15 +70,70 @@ pub struct InvalidObjectType {

 }
 #[derive(Debug, Error)]
 pub enum ConvergesError {
 #[error("the object {oid} in the repository {repo:?} is of unexpected type {found} and was expected to be {expected}")]
 pub struct MismatchedObject {
     repo: PathBuf,
     oid: Oid,
     found: CanonicalObject,
     expected: CanonicalObject,
 }
 #[derive(Debug, Error)]
 pub enum CanonicalError {
     #[error(transparent)]
     GraphDescendant(#[from] GraphDescendant),
     InvalidObjectType(#[from] InvalidObjectType),
     #[error(transparent)]
     MissingObject(#[from] MissingObject),
     #[error("failed to find object {oid} due to: {source}")]
     FindObject { oid: Oid, source: git2::Error },
     #[error("failed to find reference {name} due to: {source}")]
     FindReference { name: String, source: git2::Error },
 }
 impl CanonicalError {
     pub(super) fn invalid_object_type(
         repo: PathBuf,
         did: Did,
         oid: Oid,
         kind: Option<git2::ObjectType>,
     ) -> Self {
         InvalidObjectType {
             repo,
             did,
             oid,
             kind,
         }
         .into()
     }
     pub(super) fn missing_object(repo: PathBuf, did: Did, oid: Oid, err: git2::Error) -> Self {
         MissingObject {
             repo,
             did,
             commit: oid,
             source: err,
         }
         .into()
     }
     pub(super) fn find_object(oid: Oid, err: git2::Error) -> Self {
         Self::FindObject { oid, source: err }
     }
     pub(crate) fn find_reference(name: &str, e: git2::Error) -> CanonicalError {
         Self::FindReference {
             name: name.to_string(),
             source: e,
         }
     }
 }
 #[derive(Debug, Error)]
 pub enum ConvergesError {
     #[error(transparent)]
     InvalidObject(#[from] InvalidObject),
     GraphDescendant(#[from] GraphDescendant),
     #[error(transparent)]
     InvalidObjectType(#[from] InvalidObjectType),
     MismatchedObject(#[from] MismatchedObject),
 }
 impl ConvergesError {

@@ -88,35 +145,17 @@ impl ConvergesError {

         })
     }
     pub(super) fn missing_object(repo: PathBuf, did: Did, commit: Oid, err: raw::Error) -> Self {
         Self::MissingObject(MissingObject {
             repo,
             did,
             commit,
             source: err,
         })
     }
     pub(super) fn invalid_object(repo: PathBuf, did: Did, commit: Oid, err: raw::Error) -> Self {
         Self::InvalidObject(InvalidObject {
             repo,
             did,
             commit,
             source: err,
         })
     }
     pub(super) fn invalid_object_kind(
     pub(super) fn mismatched_object(
         repo: PathBuf,
         did: Did,
         oid: Oid,
         kind: Option<git2::ObjectType>,
         found: CanonicalObject,
         expected: CanonicalObject,
     ) -> Self {
         Self::InvalidObjectType(InvalidObjectType {
         Self::MismatchedObject(MismatchedObject {
             repo,
             did,
             oid,
             kind,
             found,
             expected,
         })
     }
 }

modified crates/radicle/src/git/canonical/rules.rs

@@ -20,6 +20,7 @@ use serde_json as json;

 use thiserror::Error;
 use crate::git;
 use crate::git::canonical;
 use crate::git::canonical::Canonical;
 use crate::git::fmt::{refname, RefString};
 use crate::git::refspec::QualifiedPattern;

@@ -634,7 +635,7 @@ impl Rules {

         &'a self,
         refname: Qualified<'b>,
         repo: &Repository,
     ) -> Result<Option<Canonical<'b, 'a>>, git::raw::Error> {
     ) -> Result<Option<Canonical<'b, 'a>>, canonical::error::CanonicalError> {
         if let Some((_, rule)) = self.matches(&refname).next() {
             Ok(Some(Canonical::new(&repo.backend, refname, rule)?))
         } else {

modified crates/radicle/src/storage.rs

@@ -126,6 +126,8 @@ pub enum RepositoryError {

     DefaultBranchRule(#[from] doc::DefaultBranchRuleError),
     #[error("failed to get canonical reference rules: {0}")]
     CanonicalRefs(#[from] doc::CanonicalRefsError),
     #[error(transparent)]
     Canonical(#[from] canonical::error::CanonicalError),
 }
 impl RepositoryError {