Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
REVIEW: Refactor MergeBase
Lorenz Leutgeb committed 9 months ago
commit d9e62b75d478d222984bc8560f35907f383a9f0b
parent d73d0895777097be18d131ee0166ad04f9338699
3 files changed +65 -48
modified crates/radicle/src/git/canonical.rs
@@ -348,13 +348,40 @@ pub struct MergeBase {

impl MergeBase {
    /// The merge base of the same commit is the commit itself.
-
    pub fn refl(oid: Oid) -> Self {
+
    #[cfg(test)]
+
    pub fn trivial(oid: Oid) -> Self {
        Self {
            a: oid,
            b: oid,
            base: oid,
        }
    }
+

+
    /// The result of a merge base computation is trivial.
+
    pub fn is_trivial(&self) -> bool {
+
        if self.a == self.b {
+
            // By definition, the merge base of a commit and itself is itself.
+
            // These asserts will catch our fall in case we set an invalid
+
            // base in this case.
+
            debug_assert_eq!(self.a, self.base);
+
            debug_assert_eq!(self.b, self.base);
+
            true
+
        } else {
+
            false
+
        }
+
    }
+

+
    /// Collapses a non-trivial and linear result of a merge base computation
+
    /// into a single commit, if possible.
+
    pub fn linear(self) -> Option<Oid> {
+
        if self.is_trivial() {
+
            None
+
        } else if self.a != self.base && self.b != self.base {
+
            None
+
        } else {
+
            Some(self.base)
+
        }
+
    }
}

/// The supported type of Git object and its [`Oid`], for canonical computation.
@@ -672,9 +699,9 @@ mod tests {
                b: c0.id(),
                base: c0.id(),
            },
-
            MergeBase::refl(c2.id()),
-
            MergeBase::refl(c1.id()),
-
            MergeBase::refl(c0.id()),
+
            MergeBase::trivial(c2.id()),
+
            MergeBase::trivial(c1.id()),
+
            MergeBase::trivial(c0.id()),
        ];

        let mut cq = CommitQuorum::new([c1, c2].iter(), 1);
@@ -740,10 +767,10 @@ mod tests {
                b: c0.id(),
                base: c0.id(),
            },
-
            MergeBase::refl(b2.id()),
-
            MergeBase::refl(c2.id()),
-
            MergeBase::refl(c1.id()),
-
            MergeBase::refl(c0.id()),
+
            MergeBase::trivial(b2.id()),
+
            MergeBase::trivial(c2.id()),
+
            MergeBase::trivial(c1.id()),
+
            MergeBase::trivial(c0.id()),
        ];

        let mut cq = CommitQuorum::new([c1, c2, b2].iter(), 1);
@@ -829,7 +856,7 @@ mod tests {
                b: c2.id(),
                base: c1.id(),
            },
-
            MergeBase::refl(b2.id()),
+
            MergeBase::trivial(b2.id()),
        ]);
        assert_eq!(cq.find_quorum().unwrap(), c2.id());

@@ -840,7 +867,7 @@ mod tests {
                b: c2.id(),
                base: c1.id(),
            },
-
            MergeBase::refl(c2.id()),
+
            MergeBase::trivial(c2.id()),
        ]);
        assert_eq!(cq.find_quorum(), Err(CommitQuorumFailure::NoCandidates));

@@ -856,8 +883,8 @@ mod tests {
                b: b2.id(),
                base: c1.id(),
            },
-
            MergeBase::refl(b2.id()),
-
            MergeBase::refl(c2.id()),
+
            MergeBase::trivial(b2.id()),
+
            MergeBase::trivial(c2.id()),
        ]);
        assert_eq!(cq.find_quorum(), Err(CommitQuorumFailure::NoCandidates));

@@ -883,9 +910,9 @@ mod tests {
                b: b2.id(),
                base: c1.id(),
            },
-
            MergeBase::refl(b2.id()),
-
            MergeBase::refl(c2.id()),
-
            MergeBase::refl(c3.id()),
+
            MergeBase::trivial(b2.id()),
+
            MergeBase::trivial(c2.id()),
+
            MergeBase::trivial(c3.id()),
        ]);
        assert_eq!(cq.find_quorum(), Err(CommitQuorumFailure::NoCandidates));
    }
@@ -1070,8 +1097,8 @@ mod tests {

        let mut cq = CommitQuorum::new([a1, a1, c2, c2, c1].iter(), 2);
        cq.found_merge_bases([
-
            MergeBase::refl(a1.id()),
-
            MergeBase::refl(c2.id()),
+
            MergeBase::trivial(a1.id()),
+
            MergeBase::trivial(c2.id()),
            MergeBase {
                a: a1.id(),
                b: c2.id(),
@@ -1095,8 +1122,8 @@ mod tests {

        let mut cq = CommitQuorum::new([a1, a1, c2, c2, c1].iter(), 1);
        cq.found_merge_bases([
-
            MergeBase::refl(a1.id()),
-
            MergeBase::refl(c2.id()),
+
            MergeBase::trivial(a1.id()),
+
            MergeBase::trivial(c2.id()),
            MergeBase {
                a: a1.id(),
                b: c2.id(),
@@ -1120,7 +1147,7 @@ mod tests {

        let mut cq = CommitQuorum::new([a1, a1, c2, c2, c1].iter(), 1);
        cq.found_merge_bases([
-
            MergeBase::refl(a1.id()),
+
            MergeBase::trivial(a1.id()),
            MergeBase {
                a: a1.id(),
                b: c2.id(),
@@ -1134,8 +1161,8 @@ mod tests {

        let mut cq = CommitQuorum::new([b2, b2, c2, c2].iter(), 1);
        cq.found_merge_bases([
-
            MergeBase::refl(b2.id()),
-
            MergeBase::refl(c2.id()),
+
            MergeBase::trivial(b2.id()),
+
            MergeBase::trivial(c2.id()),
            MergeBase {
                a: b2.id(),
                b: c2.id(),
@@ -1149,8 +1176,8 @@ mod tests {

        let mut cq = CommitQuorum::new([b2, b2, c2, c2, a1].iter(), 1);
        cq.found_merge_bases([
-
            MergeBase::refl(b2.id()),
-
            MergeBase::refl(c2.id()),
+
            MergeBase::trivial(b2.id()),
+
            MergeBase::trivial(c2.id()),
            MergeBase {
                a: b2.id(),
                b: c2.id(),
@@ -1302,7 +1329,7 @@ mod tests {

        let mut cq = CommitQuorum::new([m1, m1, b2].iter(), 2);
        cq.found_merge_bases([
-
            MergeBase::refl(m1.id()),
+
            MergeBase::trivial(m1.id()),
            MergeBase {
                a: m1.id(),
                b: b2.id(),
@@ -1313,7 +1340,7 @@ mod tests {

        let mut cq = CommitQuorum::new([m1, m1, c2].iter(), 2);
        cq.found_merge_bases([
-
            MergeBase::refl(m1.id()),
+
            MergeBase::trivial(m1.id()),
            MergeBase {
                a: m1.id(),
                b: c2.id(),
@@ -1324,7 +1351,7 @@ mod tests {

        let mut cq = CommitQuorum::new([m2, m2, b2].iter(), 2);
        cq.found_merge_bases([
-
            MergeBase::refl(m2.id()),
+
            MergeBase::trivial(m2.id()),
            MergeBase {
                a: m2.id(),
                b: b2.id(),
@@ -1335,7 +1362,7 @@ mod tests {

        let mut cq = CommitQuorum::new([m2, m2, a1].iter(), 2);
        cq.found_merge_bases([
-
            MergeBase::refl(m2.id()),
+
            MergeBase::trivial(m2.id()),
            MergeBase {
                a: m2.id(),
                b: a1.id(),
@@ -1346,8 +1373,8 @@ mod tests {

        let mut cq = CommitQuorum::new([m1, m1, b2, b2].iter(), 2);
        cq.found_merge_bases([
-
            MergeBase::refl(m1.id()),
-
            MergeBase::refl(b2.id()),
+
            MergeBase::trivial(m1.id()),
+
            MergeBase::trivial(b2.id()),
            MergeBase {
                a: m1.id(),
                b: b2.id(),
@@ -1358,8 +1385,8 @@ mod tests {

        let mut cq = CommitQuorum::new([m1, m1, c2, c2].iter(), 2);
        cq.found_merge_bases([
-
            MergeBase::refl(m1.id()),
-
            MergeBase::refl(c2.id()),
+
            MergeBase::trivial(m1.id()),
+
            MergeBase::trivial(c2.id()),
            MergeBase {
                a: m1.id(),
                b: c2.id(),
modified crates/radicle/src/git/canonical/quorum.rs
@@ -301,10 +301,10 @@ mod test {
                b: c0,
                base: c0,
            },
-
            MergeBase::refl(b2),
-
            MergeBase::refl(c2),
-
            MergeBase::refl(c1),
-
            MergeBase::refl(c0),
+
            MergeBase::trivial(b2),
+
            MergeBase::trivial(c2),
+
            MergeBase::trivial(c1),
+
            MergeBase::trivial(c0),
        ];
        let mut merge_bases = MergeBases::default();
        for i in input {
modified crates/radicle/src/git/canonical/voting.rs
@@ -74,19 +74,9 @@ impl CommitVoting {

    /// Record a merge base, and add to the vote if it counts towards the
    /// result.
-
    pub fn found_merge_base(
-
        &mut self,
-
        MergeBase {
-
            a: candidate,
-
            b: other,
-
            base,
-
        }: MergeBase,
-
    ) {
+
    pub fn found_merge_base(&mut self, merge_base: MergeBase) {
        // Avoid double counting the same commits
-
        let is_same = candidate == other;
-
        if !is_same && (base == candidate || base == other) {
-
            self.votes.vote(base);
-
        }
+
        merge_base.linear().map(|oid| self.votes.vote(oid));
    }

    /// Finish the voting process and get the [`Votes`] from the