From 2b3f12f6fd12afebb3b8397dc612621df6c730e2 Mon Sep 17 00:00:00 2001 From: Kyle D Date: Sun, 19 Feb 2023 20:56:07 -0700 Subject: [PATCH] Use beforeCommit instead of baseCommit (#22949) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- models/db/iterate_test.go | 2 +- models/fixtures/repo_unit.yml | 13 +++ routers/web/repo/compare.go | 11 ++- templates/repo/diff/box.tmpl | 2 +- .../repo20.git/hooks/post-receive.d/gitea | 2 +- .../repo20.git/hooks/pre-receive.d/gitea | 2 +- .../user2/repo20.git/hooks/update.d/gitea | 2 +- .../07/0b2e783a6b3e521a23fdead377a3e41a04410d | Bin 0 -> 128 bytes .../79/adb592126eddce5f656f56db797910db025af0 | Bin 0 -> 165 bytes .../8b/abce967f21b9dfa6987f943b91093dac58a4f0 | 1 + .../a4/202876cd8bbc3f38b7d99594edbe1bb7f97a6f | Bin 0 -> 191 bytes .../b0/246d5964a3630491bd06c756be46513e3d7035 | Bin 0 -> 21 bytes .../b6/7e43a07d48243a5f670ace063acd5e13f719df | Bin 0 -> 173 bytes .../c8/e31bc7688741a5287fcde4fbb8fc129ca07027 | 2 + .../cf/e3b3c1fd36fba04f9183287b106497e1afe986 | 3 + .../ea/f5f7510320b6a327fb308379de2f94d8859a54 | Bin 0 -> 30 bytes .../user2/repo20.git/refs/heads/add-csv | 1 + .../repo20.git/refs/heads/remove-files-a | 1 + .../repo20.git/refs/heads/remove-files-b | 1 + tests/integration/compare_test.go | 78 ++++++++++++++++++ 20 files changed, 110 insertions(+), 11 deletions(-) create mode 100644 tests/gitea-repositories-meta/user2/repo20.git/objects/07/0b2e783a6b3e521a23fdead377a3e41a04410d create mode 100644 tests/gitea-repositories-meta/user2/repo20.git/objects/79/adb592126eddce5f656f56db797910db025af0 create mode 100644 tests/gitea-repositories-meta/user2/repo20.git/objects/8b/abce967f21b9dfa6987f943b91093dac58a4f0 create mode 100644 tests/gitea-repositories-meta/user2/repo20.git/objects/a4/202876cd8bbc3f38b7d99594edbe1bb7f97a6f create mode 100644 tests/gitea-repositories-meta/user2/repo20.git/objects/b0/246d5964a3630491bd06c756be46513e3d7035 create mode 100644 tests/gitea-repositories-meta/user2/repo20.git/objects/b6/7e43a07d48243a5f670ace063acd5e13f719df create mode 100644 tests/gitea-repositories-meta/user2/repo20.git/objects/c8/e31bc7688741a5287fcde4fbb8fc129ca07027 create mode 100644 tests/gitea-repositories-meta/user2/repo20.git/objects/cf/e3b3c1fd36fba04f9183287b106497e1afe986 create mode 100644 tests/gitea-repositories-meta/user2/repo20.git/objects/ea/f5f7510320b6a327fb308379de2f94d8859a54 create mode 100644 tests/gitea-repositories-meta/user2/repo20.git/refs/heads/add-csv create mode 100644 tests/gitea-repositories-meta/user2/repo20.git/refs/heads/remove-files-a create mode 100644 tests/gitea-repositories-meta/user2/repo20.git/refs/heads/remove-files-b diff --git a/models/db/iterate_test.go b/models/db/iterate_test.go index 63487afa4..a713fe0d8 100644 --- a/models/db/iterate_test.go +++ b/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} diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml index 8706717ad..503b8c9dd 100644 --- a/models/fixtures/repo_unit.yml +++ b/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 diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 4e40867de..f21611c63 100644 --- a/routers/web/repo/compare.go +++ b/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) { - ctx.Data["BaseCommit"] = base +func setCompareContext(ctx *context.Context, before, head *git.Commit, headOwner, headName string) { + 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 } diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 62cc06955..1e7982435 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -71,7 +71,7 @@
{{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)}} diff --git a/tests/gitea-repositories-meta/user2/repo20.git/hooks/post-receive.d/gitea b/tests/gitea-repositories-meta/user2/repo20.git/hooks/post-receive.d/gitea index ee2ab2f2d..43a948da3 100755 --- a/tests/gitea-repositories-meta/user2/repo20.git/hooks/post-receive.d/gitea +++ b/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 diff --git a/tests/gitea-repositories-meta/user2/repo20.git/hooks/pre-receive.d/gitea b/tests/gitea-repositories-meta/user2/repo20.git/hooks/pre-receive.d/gitea index cdbc3c7c1..49d094063 100755 --- a/tests/gitea-repositories-meta/user2/repo20.git/hooks/pre-receive.d/gitea +++ b/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 diff --git a/tests/gitea-repositories-meta/user2/repo20.git/hooks/update.d/gitea b/tests/gitea-repositories-meta/user2/repo20.git/hooks/update.d/gitea index 7447b2fe0..38101c242 100755 --- a/tests/gitea-repositories-meta/user2/repo20.git/hooks/update.d/gitea +++ b/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 diff --git a/tests/gitea-repositories-meta/user2/repo20.git/objects/07/0b2e783a6b3e521a23fdead377a3e41a04410d b/tests/gitea-repositories-meta/user2/repo20.git/objects/07/0b2e783a6b3e521a23fdead377a3e41a04410d new file mode 100644 index 0000000000000000000000000000000000000000..7ec6df17fbb8e3b25c5c31f866f2dd4ec3ccafe0 GIT binary patch literal 128 zcmV-`0Du2@0V^p=O;s>7HfAs}00M@3WKAUhDKy*_wtRA`}fY#mbBaP(exd% i3XnkzOrobB+Nw=mblA;n>(lw}pKI*DUIhR*r8f`;wL;SX literal 0 HcmV?d00001 diff --git a/tests/gitea-repositories-meta/user2/repo20.git/objects/79/adb592126eddce5f656f56db797910db025af0 b/tests/gitea-repositories-meta/user2/repo20.git/objects/79/adb592126eddce5f656f56db797910db025af0 new file mode 100644 index 0000000000000000000000000000000000000000..0071ac75b4bfacbc195a4ed815c400b8704bb366 GIT binary patch literal 165 zcmV;W09yZe0V^p=O;s>7vt%$a00M@3WKAUhDKy*_wtRA`}fY#mbBaP(exd% z3XnkzOrobB+Nw=mblA;n>(lw}pKI*DUNtl@Ff%bxC`m0Y(JQGaVR-fRdmyvIw#Dkd T4Vo+O=})=QIx7SK^4La`N~u-D literal 0 HcmV?d00001 diff --git a/tests/gitea-repositories-meta/user2/repo20.git/objects/8b/abce967f21b9dfa6987f943b91093dac58a4f0 b/tests/gitea-repositories-meta/user2/repo20.git/objects/8b/abce967f21b9dfa6987f943b91093dac58a4f0 new file mode 100644 index 000000000..06bf6dc97 --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo20.git/objects/8b/abce967f21b9dfa6987f943b91093dac58a4f0 @@ -0,0 +1 @@ +xAn0 s+l [_ATkIJC>z[,f1Z!K̀SL5[,D҉':aRнeDl :^CglHd>iqrms1Km=?U3okߝ{@wʼE] \ No newline at end of file diff --git a/tests/gitea-repositories-meta/user2/repo20.git/objects/a4/202876cd8bbc3f38b7d99594edbe1bb7f97a6f b/tests/gitea-repositories-meta/user2/repo20.git/objects/a4/202876cd8bbc3f38b7d99594edbe1bb7f97a6f new file mode 100644 index 0000000000000000000000000000000000000000..5096e42a239f82cf71251c3b96bd97c339bf1dde GIT binary patch literal 191 zcmV;w06_nE0V^p=O;s>5Fkvt;00M@3WKAUhDKy*_wtRA`}fY#mbBaP(exd% zii}JK&FE!o!v9{$Pg*Ke^o1{B+r4GeHz6wl*~P#lditTQ+SEmd-MqFwo$vm+#{TP7 tLjwad6BC7!)Z!Ao0euMOo(*zQCZwY3m>%#Dd)%0y&8b6-|m-cci&JxdPk&^qziN zN|_YU<3U#uuw)|&12$v%n3`ue_I~YTd7o!qa?AOSym>S_jEvfFIjiYuei0seJ31 bo6MpDDw`hD_6J-u%u{{cpOg6jb7x6@F5XqV literal 0 HcmV?d00001 diff --git a/tests/gitea-repositories-meta/user2/repo20.git/objects/c8/e31bc7688741a5287fcde4fbb8fc129ca07027 b/tests/gitea-repositories-meta/user2/repo20.git/objects/c8/e31bc7688741a5287fcde4fbb8fc129ca07027 new file mode 100644 index 000000000..48bb1a43d --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo20.git/objects/c8/e31bc7688741a5287fcde4fbb8fc129ca07027 @@ -0,0 +1,2 @@ +xA +0@Q9\@N&w"L4Ҧ+Z؀vm`!&u֎mFKla8t]l;H}g9'2}{*lQ}&+i+unv0NX ,!{D&:uI *|i}a=gw<;p4EQH#1Q;~y$k@v-VC+? literal 0 HcmV?d00001 diff --git a/tests/gitea-repositories-meta/user2/repo20.git/refs/heads/add-csv b/tests/gitea-repositories-meta/user2/repo20.git/refs/heads/add-csv new file mode 100644 index 000000000..c95a51766 --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo20.git/refs/heads/add-csv @@ -0,0 +1 @@ +c8e31bc7688741a5287fcde4fbb8fc129ca07027 diff --git a/tests/gitea-repositories-meta/user2/repo20.git/refs/heads/remove-files-a b/tests/gitea-repositories-meta/user2/repo20.git/refs/heads/remove-files-a new file mode 100644 index 000000000..138f2f43d --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo20.git/refs/heads/remove-files-a @@ -0,0 +1 @@ +cfe3b3c1fd36fba04f9183287b106497e1afe986 diff --git a/tests/gitea-repositories-meta/user2/repo20.git/refs/heads/remove-files-b b/tests/gitea-repositories-meta/user2/repo20.git/refs/heads/remove-files-b new file mode 100644 index 000000000..04270e29b --- /dev/null +++ b/tests/gitea-repositories-meta/user2/repo20.git/refs/heads/remove-files-b @@ -0,0 +1 @@ +8babce967f21b9dfa6987f943b91093dac58a4f0 diff --git a/tests/integration/compare_test.go b/tests/integration/compare_test.go index dc2bff89f..509524ca5 100644 --- a/tests/integration/compare_test.go +++ b/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) +}