From 70a9bc2d404d50587a92f3bdff297ba2da3fee36 Mon Sep 17 00:00:00 2001
From: "oppili.bsky.social" <nerdy@peppe.rs>
Date: Mon, 7 Apr 2025 10:02:51 +0000
Subject: [PATCH] knotserver: use common ancestor to calculate diffs

the existing diff calculation approach directly compares branches, this forces linear history for merge. this patch updates the diff logic to use the common ancestor commit to calculate diffs.

```
       P   Q
       o---o topic
      /
     /
o---o---o---o master
    A   X   Y
```

when diffing topic and master directly with difftree (Q and Y), the commits introduced in master since the creation of the branch, namely, X and Y, are considered "removed" from the generated patch. however, when diffing with the common ancestor A, only changes introduced by P and Q are included in the produced patch.

this calculation can be retired in favor of format-patch soon enough (i think).
---
 knotserver/git/diff.go | 36 ++++++++++++++++++++++++------------
 knotserver/routes.go   | 23 ++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/knotserver/git/diff.go b/knotserver/git/diff.go
index 59db755..3ee87b9 100644
--- a/knotserver/git/diff.go
+++ b/knotserver/git/diff.go
@@ -86,17 +86,7 @@ func (g *GitRepo) Diff() (*types.NiceDiff, error) {
 	return &nd, nil
 }
 
-func (g *GitRepo) DiffTree(rev1, rev2 string) (*types.DiffTree, error) {
-	commit1, err := g.resolveRevision(rev1)
-	if err != nil {
-		return nil, fmt.Errorf("Invalid revision: %s", rev1)
-	}
-
-	commit2, err := g.resolveRevision(rev2)
-	if err != nil {
-		return nil, fmt.Errorf("Invalid revision: %s", rev2)
-	}
-
+func (g *GitRepo) DiffTree(commit1, commit2 *object.Commit) (*types.DiffTree, error) {
 	tree1, err := commit1.Tree()
 	if err != nil {
 		return nil, err
@@ -130,7 +120,29 @@ func (g *GitRepo) DiffTree(rev1, rev2 string) (*types.DiffTree, error) {
 	}, nil
 }
 
-func (g *GitRepo) resolveRevision(revStr string) (*object.Commit, error) {
+func (g *GitRepo) MergeBase(commit1, commit2 *object.Commit) (*object.Commit, error) {
+	isAncestor, err := commit1.IsAncestor(commit2)
+	if err != nil {
+		return nil, err
+	}
+
+	if isAncestor {
+		return commit1, nil
+	}
+
+	mergeBase, err := commit1.MergeBase(commit2)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(mergeBase) == 0 {
+		return nil, fmt.Errorf("failed to find a merge-base")
+	}
+
+	return mergeBase[0], nil
+}
+
+func (g *GitRepo) ResolveRevision(revStr string) (*object.Commit, error) {
 	rev, err := g.r.ResolveRevision(plumbing.Revision(revStr))
 	if err != nil {
 		return nil, fmt.Errorf("resolving revision %s: %w", revStr, err)
diff --git a/knotserver/routes.go b/knotserver/routes.go
index 4d1b732..c56d9c3 100644
--- a/knotserver/routes.go
+++ b/knotserver/routes.go
@@ -775,7 +775,28 @@ func (h *Handle) Compare(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	difftree, err := gr.DiffTree(rev1, rev2)
+	commit1, err := gr.ResolveRevision(rev1)
+	if err != nil {
+		l.Error("error resolving revision 1", "msg", err.Error())
+		writeError(w, fmt.Sprintf("error resolving revision %s", rev1), http.StatusBadRequest)
+		return
+	}
+
+	commit2, err := gr.ResolveRevision(rev2)
+	if err != nil {
+		l.Error("error resolving revision 2", "msg", err.Error())
+		writeError(w, fmt.Sprintf("error resolving revision %s", rev2), http.StatusBadRequest)
+		return
+	}
+
+	mergeBase, err := gr.MergeBase(commit1, commit2)
+	if err != nil {
+		l.Error("failed to find merge-base", "msg", err.Error())
+		writeError(w, "failed to calculate diff", http.StatusBadRequest)
+		return
+	}
+
+	difftree, err := gr.DiffTree(mergeBase, commit2)
 	if err != nil {
 		l.Error("error comparing revisions", "msg", err.Error())
 		writeError(w, "error comparing revisions", http.StatusBadRequest)
-- 
2.51.2