Use beforeCommit instead of baseCommit (#22949)

Replaces: https://github.com/go-gitea/gitea/pull/22947
Fixes https://github.com/go-gitea/gitea/issues/22946
Probably related to https://github.com/go-gitea/gitea/issues/19530

Basically, many of the diffs were broken because they were comparing to
the base commit, where a 3-dot diff should be comparing to the [last
common
ancestor](https://matthew-brett.github.io/pydagogue/git_diff_dots.html).

This should have an integration test so that we don’t run into this
issue again.

---------

Co-authored-by: Jonathan Tran <jonnytran@gmail.com>

models/db/iterate_test.go

@@ -25,7 +25,7 @@ func TestIterate(t *testing.T) {
 		return nil
 	})
 	assert.NoError(t, err)
-	assert.EqualValues(t, 81, repoCnt)
+	assert.EqualValues(t, 83, repoCnt)
 
 	err = db.Iterate(db.DefaultContext, nil, func(ctx context.Context, repoUnit *repo_model.RepoUnit) error {
 		reopUnit2 := repo_model.RepoUnit{ID: repoUnit.ID}

models/fixtures/repo_unit.yml

@@ -556,3 +556,16 @@
   repo_id: 54
   type: 1
   created_unix: 946684810
+
+-
+  id: 82
+  repo_id: 31
+  type: 1
+  created_unix: 946684810
+
+-
+  id: 83
+  repo_id: 31
+  type: 3
+  config: "{\"IgnoreWhitespaceConflicts\":false,\"AllowMerge\":true,\"AllowRebase\":true,\"AllowRebaseMerge\":true,\"AllowSquash\":true}"
+  created_unix: 946684810

routers/web/repo/compare.go

@@ -43,8 +43,8 @@ const (
 )
 
 // setCompareContext sets context data.
-func setCompareContext(ctx *context.Context, base, head *git.Commit, headOwner, headName string) {
+func setCompareContext(ctx *context.Context, before, head *git.Commit, headOwner, headName string) {
-	ctx.Data["BaseCommit"] = base
+	ctx.Data["BeforeCommit"] = before
 	ctx.Data["HeadCommit"] = head
 
 	ctx.Data["GetBlobByPathForCommit"] = func(commit *git.Commit, path string) *git.Blob {
@@ -59,7 +59,7 @@ func setCompareContext(ctx *context.Context, base, head *git.Commit, headOwner,
 		return blob
 	}
 
-	setPathsCompareContext(ctx, base, head, headOwner, headName)
+	setPathsCompareContext(ctx, before, head, headOwner, headName)
 	setImageCompareContext(ctx)
 	setCsvCompareContext(ctx)
 }
@@ -629,9 +629,8 @@ func PrepareCompareDiff(
 	}
 
 	baseGitRepo := ctx.Repo.GitRepo
-	baseCommitID := ci.CompareInfo.BaseCommitID
 
-	baseCommit, err := baseGitRepo.GetCommit(baseCommitID)
+	beforeCommit, err := baseGitRepo.GetCommit(beforeCommitID)
 	if err != nil {
 		ctx.ServerError("GetCommit", err)
 		return false
@@ -668,7 +667,7 @@ func PrepareCompareDiff(
 	ctx.Data["Username"] = ci.HeadUser.Name
 	ctx.Data["Reponame"] = ci.HeadRepo.Name
 
-	setCompareContext(ctx, baseCommit, headCommit, ci.HeadUser.Name, repo.Name)
+	setCompareContext(ctx, beforeCommit, headCommit, ci.HeadUser.Name, repo.Name)
 
 	return false
 }

templates/repo/diff/box.tmpl

@@ -71,7 +71,7 @@
 				<div id="diff-file-boxes" class="sixteen wide column">
 					{{range $i, $file := .Diff.Files}}
 						{{/*notice: the index of Diff.Files should not be used for element ID, because the index will be restarted from 0 when doing load-more for PRs with a lot of files*/}}
-						{{$blobBase := call $.GetBlobByPathForCommit $.BaseCommit $file.OldName}}
+						{{$blobBase := call $.GetBlobByPathForCommit $.BeforeCommit $file.OldName}}
 						{{$blobHead := call $.GetBlobByPathForCommit $.HeadCommit $file.Name}}
 						{{$isImage := or (call $.IsBlobAnImage $blobBase) (call $.IsBlobAnImage $blobHead)}}
 						{{$isCsv := (call $.IsCsvFile $file)}}

tests/gitea-repositories-meta/user2/repo20.git/hooks/post-receive.d/gitea

@@ -1,2 +1,2 @@
 #!/usr/bin/env bash
-"/home/tris/Projects/go/src/code.gitea.io/gitea/gitea" hook --config='/home/tris/Projects/go/src/code.gitea.io/gitea/custom/conf/app.ini' post-receive
+"$GITEA_ROOT/gitea" hook --config="$GITEA_ROOT/$GITEA_CONF" post-receive

tests/gitea-repositories-meta/user2/repo20.git/hooks/pre-receive.d/gitea

@@ -1,2 +1,2 @@
 #!/usr/bin/env bash
-"/home/tris/Projects/go/src/code.gitea.io/gitea/gitea" hook --config='/home/tris/Projects/go/src/code.gitea.io/gitea/custom/conf/app.ini' pre-receive
+"$GITEA_ROOT/gitea" hook --config="$GITEA_ROOT/$GITEA_CONF" pre-receive

tests/gitea-repositories-meta/user2/repo20.git/hooks/update.d/gitea

@@ -1,2 +1,2 @@
 #!/usr/bin/env bash
-"/home/tris/Projects/go/src/code.gitea.io/gitea/gitea" hook --config='/home/tris/Projects/go/src/code.gitea.io/gitea/custom/conf/app.ini' update $1 $2 $3
+"$GITEA_ROOT/gitea" hook --config="$GITEA_ROOT/$GITEA_CONF" update $1 $2 $3

tests/gitea-repositories-meta/user2/repo20.git/objects/8b/abce967f21b9dfa6987f943b91093dac58a4f0

@@ -0,0 +1 @@
+x
<01><>An<41>0s<>+<2B><><16>l<EFBFBD>	<09>[_A<5F>TkIJC><3E><><EFBFBD>z[,f1<66><31>Z<EFBFBD>!<21>K<EFBFBD>̀<EFBFBD><CD80>S<><53>L5[,<14>D҉'<27>:a<>Rнe<D0BD><65><EFBFBD>Dl<44> <20>:^C<0F>g<EFBFBD>lH<6C>d<EFBFBD><64><EFBFBD><EFBFBD>>iqr<71><72>m<EFBFBD><6D>s1<73><31><EFBFBD>K<EFBFBD><4B><EFBFBD>m=?U<><55>3o<33><0E><><EFBFBD><EFBFBD><07><><EFBFBD><11><><EFBFBD><EFBFBD>k<EFBFBD>ߝ{<7B><>@w<>ʼ<><CABC><03>E]

tests/gitea-repositories-meta/user2/repo20.git/objects/c8/e31bc7688741a5287fcde4fbb8fc129ca07027

@@ -0,0 +1,2 @@
+x
<01><>A
+<EFBFBD>0@Q<>9<EFBFBD>\@<40>N<EFBFBD>&
w<>"<22>L<EFBFBD><4C>4Ҧ<34><D2A6><EFBFBD>+<2B><><EFBFBD><EFBFBD><EFBFBD>Z<EFBFBD>؀<EFBFBD>vm<11>`<08><>!&<26>u֎m<D68E><6D><EFBFBD>FK<>l<EFBFBD><6C>a<EFBFBD><61><16>8t<38>]<5D>l;<3B><>H<EFBFBD>}g<><67>9'2}<7D>{<7B>*l<>Q<17>}&<26>+<1C>i+un<75>v<EFBFBD><76>0N<30>X<EFBFBD>	<09><><0E>,!<21>{D<><44><EFBFBD><EFBFBD>&<26>:uI	<09><>

tests/gitea-repositories-meta/user2/repo20.git/objects/cf/e3b3c1fd36fba04f9183287b106497e1afe986

@@ -0,0 +1,3 @@
+x
<01><>An<41>0EY<45>s
<01>=v2FBv=D5<44>';U<>T<EFBFBD><54><EFBFBD>
+<EFBFBD><EFBFBD><EFBFBD><EFBFBD><EFBFBD><EFBFBD>Z묀<0F><><EFBFBD>%<25>P(z<1E><>
<1D><><EFBFBD>p<EFBFBD>D<EFBFBD>%8<>!8[<5B>/o<>r<14>R<EFBFBD>1F<31>p<EFBFBD>HS.<12><><EFBFBD><EFBFBD><1D>3<EFBFBD>$<24><>]<1F><06><>E<EFBFBD><06>g<EFBFBD><67>ڴ<EFBFBD><DAB4>{<7B>y9<79><39>~<7E>{	<09>
<01>v<EFBFBD><76><EFBFBD><EFBFBD><EFBFBD>S<EFBFBD><53>gn<67><6E>
+<EFBFBD><EFBFBD><EFBFBD><EFBFBD>_<EFBFBD><EFBFBD><EFBFBD>2<EFBFBD><EFBFBD><EFBFBD>c6tuI<75>

tests/gitea-repositories-meta/user2/repo20.git/refs/heads/add-csv

@@ -0,0 +1 @@
+c8e31bc7688741a5287fcde4fbb8fc129ca07027

tests/gitea-repositories-meta/user2/repo20.git/refs/heads/remove-files-a

@@ -0,0 +1 @@
+cfe3b3c1fd36fba04f9183287b106497e1afe986

tests/gitea-repositories-meta/user2/repo20.git/refs/heads/remove-files-b

@@ -0,0 +1 @@
+8babce967f21b9dfa6987f943b91093dac58a4f0

tests/integration/compare_test.go

@@ -4,6 +4,7 @@
 package integration
 
 import (
+	"fmt"
 	"net/http"
 	"strings"
 	"testing"
@@ -40,3 +41,80 @@ func TestCompareDefault(t *testing.T) {
 	selection := htmlDoc.doc.Find(".choose.branch .filter.dropdown")
 	assert.Lenf(t, selection.Nodes, 2, "The template has changed")
 }
+
+// Ensure the comparison matches what we expect
+func inspectCompare(t *testing.T, htmlDoc *HTMLDoc, diffCount int, diffChanges []string) {
+	selection := htmlDoc.doc.Find("#diff-file-boxes").Children()
+
+	assert.Lenf(t, selection.Nodes, diffCount, "Expected %v diffed files, found: %v", diffCount, len(selection.Nodes))
+
+	for _, diffChange := range diffChanges {
+		selection = htmlDoc.doc.Find(fmt.Sprintf("[data-new-filename=\"%s\"]", diffChange))
+		assert.Lenf(t, selection.Nodes, 1, "Expected 1 match for [data-new-filename=\"%s\"], found: %v", diffChange, len(selection.Nodes))
+	}
+}
+
+// Git commit graph for repo20
+// * 8babce9 (origin/remove-files-b) Add a dummy file
+// * b67e43a Delete test.csv and link_hi
+// | * cfe3b3c (origin/remove-files-a) Delete test.csv and link_hi
+// |/
+// * c8e31bc (origin/add-csv) Add test csv file
+// * 808038d (HEAD -> master, origin/master, origin/HEAD) Added test links
+
+func TestCompareBranches(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	session := loginUser(t, "user2")
+
+	// Inderect compare remove-files-b (head) with add-csv (base) branch
+	//
+	//	'link_hi' and 'test.csv' are deleted, 'test.txt' is added
+	req := NewRequest(t, "GET", "/user2/repo20/compare/add-csv...remove-files-b")
+	resp := session.MakeRequest(t, req, http.StatusOK)
+	htmlDoc := NewHTMLParser(t, resp.Body)
+
+	diffCount := 3
+	diffChanges := []string{"link_hi", "test.csv", "test.txt"}
+
+	inspectCompare(t, htmlDoc, diffCount, diffChanges)
+
+	// Inderect compare remove-files-b (head) with remove-files-a (base) branch
+	//
+	//	'link_hi' and 'test.csv' are deleted, 'test.txt' is added
+
+	req = NewRequest(t, "GET", "/user2/repo20/compare/remove-files-a...remove-files-b")
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	htmlDoc = NewHTMLParser(t, resp.Body)
+
+	diffCount = 3
+	diffChanges = []string{"link_hi", "test.csv", "test.txt"}
+
+	inspectCompare(t, htmlDoc, diffCount, diffChanges)
+
+	// Inderect compare remove-files-a (head) with remove-files-b (base) branch
+	//
+	//	'link_hi' and 'test.csv' are deleted
+
+	req = NewRequest(t, "GET", "/user2/repo20/compare/remove-files-b...remove-files-a")
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	htmlDoc = NewHTMLParser(t, resp.Body)
+
+	diffCount = 2
+	diffChanges = []string{"link_hi", "test.csv"}
+
+	inspectCompare(t, htmlDoc, diffCount, diffChanges)
+
+	// Direct compare remove-files-b (head) with remove-files-a (base) branch
+	//
+	//	'test.txt' is deleted
+
+	req = NewRequest(t, "GET", "/user2/repo20/compare/remove-files-b..remove-files-a")
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	htmlDoc = NewHTMLParser(t, resp.Body)
+
+	diffCount = 1
+	diffChanges = []string{"test.txt"}
+
+	inspectCompare(t, htmlDoc, diffCount, diffChanges)
+}