diff --git a/crates/radicle-cli/examples/git/git-push-diverge.md b/crates/radicle-cli/examples/git/git-push-diverge.md index 5d0ff736..39543d7d 100644 --- a/crates/radicle-cli/examples/git/git-push-diverge.md +++ b/crates/radicle-cli/examples/git/git-push-diverge.md @@ -45,7 +45,7 @@ integrate Bob's changes before pushing ours: $ git push rad hint: you are attempting to push a commit that would cause your upstream to diverge from the canonical reference refs/heads/master hint: to integrate the remote changes, run `git pull --rebase` and try again -error: refusing to update branch to commit that is not a descendant of canonical head +error: refusing to update canonical reference to commit that is not a descendant of current canonical head error: failed to push some refs to 'rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi' ``` diff --git a/crates/radicle-remote-helper/src/push.rs b/crates/radicle-remote-helper/src/push.rs index 044dbf32..dab29ac2 100644 --- a/crates/radicle-remote-helper/src/push.rs +++ b/crates/radicle-remote-helper/src/push.rs @@ -1,5 +1,6 @@ #![allow(clippy::too_many_arguments)] +mod canonical; mod error; use std::collections::HashMap; @@ -19,7 +20,6 @@ use radicle::cob::patch; use radicle::cob::patch::cache::Patches as _; use radicle::crypto; use radicle::explorer::ExplorerResource; -use radicle::git::canonical; use radicle::identity::{CanonicalRefs, Did}; use radicle::node; use radicle::node::{Handle, NodeId}; @@ -31,7 +31,7 @@ use radicle::{git, rad}; use radicle_cli as cli; use radicle_cli::terminal as term; -use crate::{hint, read_line, warn, Options}; +use crate::{hint, read_line, Options}; #[derive(Debug, Error)] pub enum Error { @@ -45,9 +45,6 @@ pub enum Error { /// No public key is given #[error("no public key given as a remote namespace, perhaps you are attempting to push to restricted refs")] NoKey, - /// Head being pushed diverges from canonical head. - #[error("refusing to update branch to commit that is not a descendant of canonical head")] - HeadsDiverge(git::Oid, git::Oid), /// User tried to delete the canonical branch. #[error("refusing to delete default branch ref '{0}'")] DeleteForbidden(git::RefString), @@ -121,6 +118,8 @@ pub enum Error { CanonicalRefs(#[from] radicle::identity::doc::CanonicalRefsError), #[error(transparent)] PushAction(#[from] error::PushAction), + #[error(transparent)] + Canonical(#[from] error::CanonicalUnrecoverable), } /// Push command. @@ -224,7 +223,7 @@ impl PushAction { let patch = git::Oid::from_str(oid) .map_err(|err| error::PushAction::InvalidPatchId { suffix: oid.to_string(), - err, + source: err, }) .map(patch::PatchId::from)?; Ok(Self::UpdatePatch { dst, patch }) @@ -348,46 +347,11 @@ pub fn run( // // Note that we *do* allow rolling back to a previous commit on the // canonical branch. - if let Some(mut canonical) = rules.canonical(dst.clone(), stored)? { - if canonical.is_allowed(&me) { - let head = *src; - let converges = canonical.converges(&working, (&me, &head))?; - - // If `canonical` is empty then we're creating a new reference. - // If we're the only delegate then we need to modify our vote. - if converges || canonical.has_no_tips() || canonical.is_only(&me) { - canonical.modify_vote(me, head); - } - - match canonical.quorum(&working) { - Ok((dst, canonical_oid)) => { - // Canonical head is an ancestor of head. - let is_ff = head == canonical_oid - || working - .graph_descendant_of(*head, *canonical_oid)?; - - if !is_ff && !converges { - if hints { - hint( - format!("you are attempting to push a commit that would cause \ - your upstream to diverge from the canonical reference {dst}"), - ); - hint( - "to integrate the remote changes, run `git pull --rebase` \ - and try again", - ); - } - return Err(Error::HeadsDiverge(head, canonical_oid)); - } - set_canonical_refs - .push((dst.clone().to_owned(), canonical_oid)); - } - Err(canonical::QuorumError::Git(e)) => return Err(e.into()), - Err(e) => { - warn(e.to_string()); - warn("it is recommended to find a commit to agree upon"); - } - } + if let Some(canonical) = rules.canonical(dst.clone(), stored)? { + let canonical = canonical::Canonical::new(me, *src, canonical); + match canonical.quorum(&working) { + Ok(quorum) => set_canonical_refs.push(quorum), + Err(e) => canonical::io::handle_error(e, &dst, hints)?, } } push(src, &dst, *force, &nid, &working, stored, patches, &signer) diff --git a/crates/radicle-remote-helper/src/push/canonical.rs b/crates/radicle-remote-helper/src/push/canonical.rs new file mode 100644 index 00000000..0a509011 --- /dev/null +++ b/crates/radicle-remote-helper/src/push/canonical.rs @@ -0,0 +1,129 @@ +use radicle::git; +use radicle::git::raw::Repository; +use radicle::prelude::Did; + +use super::error; + +pub(crate) struct Vote { + did: Did, + commit: git::Oid, +} + +/// Validates a vote to update a canonical reference during push. +pub(crate) struct Canonical<'a, 'b> { + vote: Vote, + canonical: git::canonical::Canonical<'a, 'b>, +} + +impl<'a, 'b> Canonical<'a, 'b> { + pub fn new(me: Did, head: git::Oid, canonical: git::canonical::Canonical<'a, 'b>) -> Self { + Self { + vote: Vote { + did: me, + commit: head, + }, + canonical, + } + } + + /// Calculates the quorum of the [`git::canonical::Canonical`] provided. + /// + /// In some cases, it ensures that the [`head`] is attempting to converge + /// with the set of commits of the other [`Did`]s. + /// + /// If a quorum is found, then it is also ensured that the new [`head`] is a + /// descendant of the current canonical commit, otherwise the commits are + /// considered diverging. + /// + /// # Errors + /// + /// Ensures that the commits of the other [`Did`]s are in the working + /// copy, and that checks that any two commits are related in the graph. + /// + /// Ensures that the new head and the canonical commit do not diverge. + /// + /// [`head`]: crate::push::canonical::Canonical::head + pub fn quorum( + mut self, + working: &Repository, + ) -> Result<(git::Qualified<'a>, git::Oid), error::Canonical> { + let converges = self + .canonical + .converges(working, (&self.vote.did, &self.vote.commit))?; + if converges || self.canonical.has_no_tips() || self.canonical.is_only(&self.vote.did) { + self.canonical.modify_vote(self.vote.did, self.vote.commit); + } + + match self.canonical.quorum(working) { + Ok((cref, quorum_head)) => { + // Canonical head is an ancestor of head. + let is_ff = self.vote.commit == quorum_head + || working + .graph_descendant_of(*self.vote.commit, *quorum_head) + .map_err(|err| { + error::Canonical::graph_descendant(self.vote.commit, quorum_head, err) + })?; + + if !is_ff && !converges { + Err(error::Canonical::heads_diverge( + self.vote.commit, + quorum_head, + )) + } else { + Ok((cref, quorum_head)) + } + } + Err(err) => Err(err.into()), + } + } +} + +pub(crate) mod io { + use radicle::git; + use radicle::git::canonical; + + use crate::push::error; + use crate::{hint, warn}; + + /// Handle recoverable errors, printing relevant information to the + /// terminal. Otherwise, convert the error into an unrecoverable error + /// [`error::CanonicalUnrecoverable`]. + pub fn handle_error( + e: error::Canonical, + canonical: &git::Qualified, + hints: bool, + ) -> Result<(), error::CanonicalUnrecoverable> { + match e { + error::Canonical::GraphDescendant(e) => Err(e.into()), + error::Canonical::Converges(e) => Err(e.into()), + error::Canonical::HeadsDiverge(e) => { + if hints { + hint( + format!("you are attempting to push a commit that would cause \ + your upstream to diverge from the canonical reference {canonical}"), + ); + hint( + "to integrate the remote changes, run `git pull --rebase` \ + and try again", + ); + } + Err(e.into()) + } + error::Canonical::Quorum(e) => match e { + e @ canonical::QuorumError::Diverging { .. } => { + warn(e.to_string()); + warn("it is recommended to find a commit to agree upon"); + Ok(()) + } + e @ canonical::QuorumError::NoCandidates { .. } => { + warn(e.to_string()); + warn("it is recommended to find a commit to agree upon"); + Ok(()) + } + canonical::QuorumError::Git(err) => { + Err(error::CanonicalUnrecoverable::Git { source: err }) + } + }, + } + } +} diff --git a/crates/radicle-remote-helper/src/push/error.rs b/crates/radicle-remote-helper/src/push/error.rs index 924c226b..add16353 100644 --- a/crates/radicle-remote-helper/src/push/error.rs +++ b/crates/radicle-remote-helper/src/push/error.rs @@ -1,6 +1,61 @@ -use radicle::storage::git; +use radicle::git; +use radicle::git::canonical; use thiserror::Error; +#[derive(Debug, Error)] +pub enum CanonicalUnrecoverable { + #[error(transparent)] + GraphDescendant(#[from] GraphDescendant), + #[error(transparent)] + Converges(#[from] canonical::ConvergesError), + #[error(transparent)] + HeadsDiverge(#[from] HeadsDiverge), + #[error("failure while computing canonical reference: {source}")] + Git { source: git::raw::Error }, +} + +#[derive(Debug, Error)] +pub enum Canonical { + #[error(transparent)] + GraphDescendant(GraphDescendant), + #[error(transparent)] + Converges(#[from] canonical::ConvergesError), + #[error(transparent)] + HeadsDiverge(HeadsDiverge), + #[error(transparent)] + Quorum(#[from] canonical::QuorumError), +} + +impl Canonical { + pub fn graph_descendant(head: git::Oid, canonical: git::Oid, source: git::raw::Error) -> Self { + Self::GraphDescendant(GraphDescendant { + head, + canonical, + source, + }) + } + + pub fn heads_diverge(head: git::Oid, canonical: git::Oid) -> Self { + Self::HeadsDiverge(HeadsDiverge { head, canonical }) + } +} + +#[derive(Debug, Error)] +#[error("failed to check if {head} is an ancestor of {canonical} due to: {source}")] +pub struct GraphDescendant { + head: git::Oid, + canonical: git::Oid, + source: git::raw::Error, +} + +#[derive(Debug, Error)] +/// Head being pushed diverges from canonical head. +#[error("refusing to update canonical reference to commit that is not a descendant of current canonical head")] +pub struct HeadsDiverge { + head: git::Oid, + canonical: git::Oid, +} + #[derive(Debug, Error)] pub enum PushAction { #[error("invalid reference {refname}, expected qualified reference starting with `refs/`")] @@ -8,7 +63,6 @@ pub enum PushAction { #[error("found refs/heads/patches/{suffix} where {suffix} was an invalid Patch ID")] InvalidPatchId { suffix: String, - #[source] - err: git::raw::Error, + source: git::raw::Error, }, } diff --git a/crates/radicle/src/git/canonical.rs b/crates/radicle/src/git/canonical.rs index eefcfd37..11a4ae3b 100644 --- a/crates/radicle/src/git/canonical.rs +++ b/crates/radicle/src/git/canonical.rs @@ -2,6 +2,7 @@ pub mod rules; pub use rules::{MatchedRule, RawRule, Rules, ValidRule}; use std::collections::BTreeMap; +use std::path::PathBuf; use raw::Repository; use thiserror::Error; @@ -50,6 +51,70 @@ pub enum QuorumError { Git(#[from] git2::Error), } +#[derive(Debug, Error)] +#[error("failed to check if {head} is an ancestor of {canonical} due to: {source}")] +pub struct GraphDescendant { + head: Oid, + canonical: Oid, + source: raw::Error, +} + +#[derive(Debug, Error)] +#[error("the commit {commit} for {did} is missing from the repository {repo:?}")] +pub struct MissingCommit { + repo: PathBuf, + did: Did, + commit: Oid, + source: raw::Error, +} + +#[derive(Debug, Error)] +#[error("could not determine whether the commit {commit} for {did} is part of the repository {repo:?} due to: {source}")] +pub struct InvalidCommit { + repo: PathBuf, + did: Did, + commit: Oid, + source: raw::Error, +} + +#[derive(Debug, Error)] +pub enum ConvergesError { + #[error(transparent)] + GraphDescendant(#[from] GraphDescendant), + #[error(transparent)] + MissingCommit(#[from] MissingCommit), + #[error(transparent)] + InvalidCommit(#[from] InvalidCommit), +} + +impl ConvergesError { + pub fn graph_descendant(head: Oid, canonical: Oid, source: raw::Error) -> Self { + Self::GraphDescendant(GraphDescendant { + head, + canonical, + source, + }) + } + + pub fn missing_commit(repo: PathBuf, did: Did, commit: Oid, err: raw::Error) -> Self { + Self::MissingCommit(MissingCommit { + repo, + did, + commit, + source: err, + }) + } + + pub fn invalid_commit(repo: PathBuf, did: Did, commit: Oid, err: raw::Error) -> Self { + Self::InvalidCommit(InvalidCommit { + repo, + did, + commit, + source: err, + }) + } +} + impl<'a, 'b> Canonical<'a, 'b> { /// Construct the set of canonical tips given for the given `rule` and /// the reference `refname`. @@ -124,14 +189,25 @@ impl<'a, 'b> Canonical<'a, 'b> { pub fn converges( &self, repo: &Repository, - candidate: (&Did, &Oid), - ) -> Result { - for tip in self - .tips - .iter() - .filter_map(|(did, tip)| (did != candidate.0).then_some(tip)) - { - let (ahead, behind) = repo.graph_ahead_behind(**candidate.1, **tip)?; + (candidate, commit): (&Did, &Oid), + ) -> Result { + let heads = { + let mut heads = self + .tips + .iter() + .filter_map(|(did, tip)| (did != candidate).then_some((did, tip))); + heads.try_fold( + Vec::with_capacity(heads.size_hint().0), + |mut heads, (did, head)| { + heads.push(Self::ensure_commit(*did, *head, repo)?); + Ok::<_, ConvergesError>(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); } @@ -225,6 +301,21 @@ impl<'a, 'b> Canonical<'a, 'b> { fn threshold(&self) -> usize { (*self.rule.threshold()).into() } + + fn ensure_commit(from: Did, commit: Oid, working: &Repository) -> Result { + match working.find_commit(*commit).map(|_| commit) { + Ok(oid) => Ok(oid), + Err(err) if err.code() == raw::ErrorCode::NotFound => Err( + ConvergesError::missing_commit(working.path().to_path_buf(), from, commit, err), + ), + Err(err) => Err(ConvergesError::invalid_commit( + working.path().to_path_buf(), + from, + commit, + err, + )), + } + } } #[cfg(test)]