Skip to content

Commit d6e1065

Browse files
McRaeAlexMcRaeAlex
committed
Update diff-tree wrapper to allow user to use --merge-base
* Updates services/gitdiff/testdata/acedemic-module * Changes it to be a bare repo * Adds a branch which updates the readme * Adds a commit which updates the webpack config * Updates test cases
1 parent da24f58 commit d6e1065

14 files changed

+131
-32
lines changed

‎services/gitdiff/git_diff_tree.go

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"bufio"
88
"context"
99
"fmt"
10+
"io"
1011
"strings"
1112

1213
"code.gitea.io/gitea/modules/git"
@@ -29,9 +30,11 @@ type DiffTreeRecord struct {
2930
BaseBlobID string
3031
}
3132

32-
// GetDiffTree returns the list of path of the files that have changed between the two commits
33-
func GetDiffTree(ctx context.Context, gitRepo *git.Repository, baseSha, headSha string) (*DiffTree, error) {
34-
gitDiffTreeRecords, err := runGitDiffTree(ctx, gitRepo, baseSha, headSha)
33+
// GetDiffTree returns the list of path of the files that have changed between the two commits.
34+
// If useMergeBase is true, the diff will be calculated using the merge base of the two commits.
35+
// This is the same behavior as using a three-dot diff in git diff.
36+
func GetDiffTree(ctx context.Context, gitRepo *git.Repository, useMergeBase bool, baseSha, headSha string) (*DiffTree, error) {
37+
gitDiffTreeRecords, err := runGitDiffTree(ctx, gitRepo, useMergeBase, baseSha, headSha)
3538
if err != nil {
3639
return nil, err
3740
}
@@ -41,32 +44,36 @@ func GetDiffTree(ctx context.Context, gitRepo *git.Repository, baseSha, headSha
4144
}, nil
4245
}
4346

44-
func runGitDiffTree(ctx context.Context, gitRepo *git.Repository, baseSha, headSha string) ([]*DiffTreeRecord, error) {
45-
baseCommitID, headCommitID, err := validateGitDiffTreeArguments(gitRepo, baseSha, headSha)
47+
func runGitDiffTree(ctx context.Context, gitRepo *git.Repository, useMergeBase bool, baseSha, headSha string) ([]*DiffTreeRecord, error) {
48+
useMergeBase, baseCommitID, headCommitID, err := validateGitDiffTreeArguments(gitRepo, useMergeBase, baseSha, headSha)
4649
if err != nil {
4750
return nil, err
4851
}
4952

50-
cmd := git.NewCommand(ctx, "diff-tree", "--raw", "-r", "--find-renames").AddDynamicArguments(baseCommitID, headCommitID)
53+
cmd := git.NewCommand(ctx, "diff-tree", "--raw", "-r", "--find-renames", "--root")
54+
if useMergeBase {
55+
cmd.AddArguments("--merge-base")
56+
}
57+
cmd.AddDynamicArguments(baseCommitID, headCommitID)
5158
stdout, _, runErr := cmd.RunStdString(&git.RunOpts{Dir: gitRepo.Path})
5259
if runErr != nil {
5360
log.Warn("git diff-tree: %v", runErr)
5461
return nil, runErr
5562
}
5663

57-
return parseGitDiffTree(stdout)
64+
return parseGitDiffTree(strings.NewReader(stdout))
5865
}
5966

60-
func validateGitDiffTreeArguments(gitRepo *git.Repository, baseSha, headSha string) (string, string, error) {
67+
func validateGitDiffTreeArguments(gitRepo *git.Repository, useMergeBase bool, baseSha, headSha string) (bool, string, string, error) {
6168
// if the head is empty its an error
6269
if headSha == "" {
63-
return "", "", fmt.Errorf("headSha is empty")
70+
return false, "", "", fmt.Errorf("headSha is empty")
6471
}
6572

6673
// if the head commit doesn't exist its and error
6774
headCommit, err := gitRepo.GetCommit(headSha)
6875
if err != nil {
69-
return "", "", fmt.Errorf("failed to get commit headSha: %v", err)
76+
return false, "", "", fmt.Errorf("failed to get commit headSha: %v", err)
7077
}
7178
headCommitID := headCommit.ID.String()
7279

@@ -77,30 +84,31 @@ func validateGitDiffTreeArguments(gitRepo *git.Repository, baseSha, headSha stri
7784
if headCommit.ParentCount() == 0 {
7885
objectFormat, err := gitRepo.GetObjectFormat()
7986
if err != nil {
80-
return "", "", err
87+
return false, "", "", err
8188
}
8289

83-
return objectFormat.EmptyTree().String(), headCommitID, nil
90+
// We set use merge base to false because we have no base commit
91+
return false, objectFormat.EmptyTree().String(), headCommitID, nil
8492
}
8593

8694
baseCommit, err := headCommit.Parent(0)
8795
if err != nil {
88-
return "", "", fmt.Errorf("baseSha is '', attempted to use parent of commit %s, got error: %v", headCommit.ID.String(), err)
96+
return false, "", "", fmt.Errorf("baseSha is '', attempted to use parent of commit %s, got error: %v", headCommit.ID.String(), err)
8997
}
90-
return baseCommit.ID.String(), headCommitID, nil
98+
return useMergeBase, baseCommit.ID.String(), headCommitID, nil
9199
}
92100

93101
// try and get the base commit
94102
baseCommit, err := gitRepo.GetCommit(baseSha)
95103
// propagate the error if we couldn't get the base commit
96104
if err != nil {
97-
return "", "", fmt.Errorf("failed to get base commit %s: %v", baseSha, err)
105+
return useMergeBase, "", "", fmt.Errorf("failed to get base commit %s: %v", baseSha, err)
98106
}
99107

100-
return baseCommit.ID.String(), headCommit.ID.String(), nil
108+
return useMergeBase, baseCommit.ID.String(), headCommit.ID.String(), nil
101109
}
102110

103-
func parseGitDiffTree(output string) ([]*DiffTreeRecord, error) {
111+
func parseGitDiffTree(gitOutput io.Reader) ([]*DiffTreeRecord, error) {
104112
/*
105113
The output of `git diff-tree --raw -r --find-renames` is of the form:
106114
@@ -112,13 +120,9 @@ func parseGitDiffTree(output string) ([]*DiffTreeRecord, error) {
112120
113121
See: <https://git-scm.com/docs/git-diff-tree#_raw_output_format> for more details
114122
*/
115-
if output == "" {
116-
return []*DiffTreeRecord{}, nil
117-
}
118-
119123
results := make([]*DiffTreeRecord, 0)
120124

121-
lines := bufio.NewScanner(strings.NewReader(output))
125+
lines := bufio.NewScanner(gitOutput)
122126
for lines.Scan() {
123127
line := lines.Text()
124128

‎services/gitdiff/git_diff_tree_test.go

Lines changed: 97 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package gitdiff
55

66
import (
7+
"strings"
78
"testing"
89

910
"code.gitea.io/gitea/models/db"
@@ -15,11 +16,12 @@ import (
1516

1617
func TestGitDiffTree(t *testing.T) {
1718
test := []struct {
18-
Name string
19-
RepoPath string
20-
BaseSha string
21-
HeadSha string
22-
Expected *DiffTree
19+
Name string
20+
RepoPath string
21+
BaseSha string
22+
HeadSha string
23+
useMergeBase bool
24+
Expected *DiffTree
2325
}{
2426
{
2527
Name: "happy path",
@@ -85,6 +87,25 @@ func TestGitDiffTree(t *testing.T) {
8587
},
8688
},
8789
},
90+
{
91+
Name: "first commit (no parent), merge base = true",
92+
RepoPath: "./testdata/academic-module",
93+
HeadSha: "07901f79ee86272fa8935f2fe546273adaf02c89",
94+
useMergeBase: true,
95+
Expected: &DiffTree{
96+
Files: []*DiffTreeRecord{
97+
{
98+
Status: "added",
99+
HeadPath: "README.md",
100+
BasePath: "README.md",
101+
HeadMode: git.EntryModeBlob,
102+
BaseMode: git.EntryModeNoEntry,
103+
HeadBlobID: "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
104+
BaseBlobID: "0000000000000000000000000000000000000000",
105+
},
106+
},
107+
},
108+
},
88109
{
89110
Name: "base and head same",
90111
RepoPath: "./testdata/academic-module",
@@ -112,14 +133,82 @@ func TestGitDiffTree(t *testing.T) {
112133
},
113134
},
114135
},
136+
{
137+
Name: "useMergeBase false",
138+
RepoPath: "./testdata/academic-module",
139+
BaseSha: "819dc756646ffd0730a163b5a8da65b84a6d504e",
140+
HeadSha: "528846b39d8f7e68e8081d586ea3e94be23afa7f", // this commit can be found on the update-readme branch
141+
useMergeBase: false,
142+
Expected: &DiffTree{
143+
Files: []*DiffTreeRecord{
144+
{
145+
Status: "modified",
146+
HeadPath: "README.md",
147+
BasePath: "README.md",
148+
HeadMode: git.EntryModeBlob,
149+
BaseMode: git.EntryModeBlob,
150+
HeadBlobID: "ca07ea08ae0fa243d6e0a4129843c5a25a02f499",
151+
BaseBlobID: "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
152+
},
153+
{
154+
Status: "modified",
155+
HeadPath: "webpack.mix.js",
156+
BasePath: "webpack.mix.js",
157+
HeadMode: git.EntryModeBlob,
158+
BaseMode: git.EntryModeBlob,
159+
HeadBlobID: "26825d9cb822e237b600153a26dd1e0e68457196",
160+
BaseBlobID: "344e0ca8aa791cc4164fb0ea645f334fd40d00f0",
161+
},
162+
},
163+
},
164+
},
165+
{
166+
Name: "useMergeBase true",
167+
RepoPath: "./testdata/academic-module",
168+
BaseSha: "819dc756646ffd0730a163b5a8da65b84a6d504e",
169+
HeadSha: "528846b39d8f7e68e8081d586ea3e94be23afa7f", // this commit can be found on the update-readme branch
170+
useMergeBase: true,
171+
Expected: &DiffTree{
172+
Files: []*DiffTreeRecord{
173+
{
174+
Status: "modified",
175+
HeadPath: "README.md",
176+
BasePath: "README.md",
177+
HeadMode: git.EntryModeBlob,
178+
BaseMode: git.EntryModeBlob,
179+
HeadBlobID: "ca07ea08ae0fa243d6e0a4129843c5a25a02f499",
180+
BaseBlobID: "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
181+
},
182+
},
183+
},
184+
},
185+
{
186+
Name: "no base set",
187+
RepoPath: "./testdata/academic-module",
188+
HeadSha: "528846b39d8f7e68e8081d586ea3e94be23afa7f", // this commit can be found on the update-readme branch
189+
useMergeBase: false,
190+
Expected: &DiffTree{
191+
Files: []*DiffTreeRecord{
192+
{
193+
Status: "modified",
194+
HeadPath: "README.md",
195+
BasePath: "README.md",
196+
HeadMode: git.EntryModeBlob,
197+
BaseMode: git.EntryModeBlob,
198+
HeadBlobID: "ca07ea08ae0fa243d6e0a4129843c5a25a02f499",
199+
BaseBlobID: "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
200+
},
201+
},
202+
},
203+
},
115204
}
116205

117206
for _, tt := range test {
118207
t.Run(tt.Name, func(t *testing.T) {
119208
gitRepo, err := git.OpenRepository(git.DefaultContext, tt.RepoPath)
120209
assert.NoError(t, err)
121210

122-
diffPaths, err := GetDiffTree(db.DefaultContext, gitRepo, tt.BaseSha, tt.HeadSha)
211+
diffPaths, err := GetDiffTree(db.DefaultContext, gitRepo, tt.useMergeBase, tt.BaseSha, tt.HeadSha)
123212
require.NoError(t, err)
124213

125214
assert.Equal(t, tt.Expected, diffPaths)
@@ -158,7 +247,7 @@ func TestGitDiffTreeErrors(t *testing.T) {
158247
gitRepo, err := git.OpenRepository(git.DefaultContext, tt.RepoPath)
159248
assert.NoError(t, err)
160249

161-
diffPaths, err := GetDiffTree(db.DefaultContext, gitRepo, tt.BaseSha, tt.HeadSha)
250+
diffPaths, err := GetDiffTree(db.DefaultContext, gitRepo, true, tt.BaseSha, tt.HeadSha)
162251
assert.Error(t, err)
163252
assert.Nil(t, diffPaths)
164253
})
@@ -310,7 +399,7 @@ func TestParseGitDiffTree(t *testing.T) {
310399

311400
for _, tt := range test {
312401
t.Run(tt.Name, func(t *testing.T) {
313-
entries, err := parseGitDiffTree(tt.GitOutput)
402+
entries, err := parseGitDiffTree(strings.NewReader(tt.GitOutput))
314403
assert.NoError(t, err)
315404
assert.Equal(t, tt.Expected, entries)
316405
})

‎services/gitdiff/testdata/academic-module/config

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
[core]
22
repositoryformatversion = 0
33
filemode = true
4-
bare = false
4+
bare = true
55
logallrefupdates = true
66
ignorecase = true
77
precomposeunicode = true

‎services/gitdiff/testdata/academic-module/logs/HEAD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
0000000000000000000000000000000000000000 bd7063cc7c04689c4d082183d32a604ed27a24f9 Lunny Xiao <xiaolunwen@gmail.com> 1574829684 +0800 clone: from https://try.gitea.io/shemgp-aiias/academic-module
2+
0000000000000000000000000000000000000000 819dc756646ffd0730a163b5a8da65b84a6d504e Alexander McRae <alex@allspice.io> 1737998543 -0800 push

‎services/gitdiff/testdata/academic-module/logs/refs/heads/master

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
0000000000000000000000000000000000000000 bd7063cc7c04689c4d082183d32a604ed27a24f9 Lunny Xiao <xiaolunwen@gmail.com> 1574829684 +0800 clone: from https://try.gitea.io/shemgp-aiias/academic-module
2+
bd7063cc7c04689c4d082183d32a604ed27a24f9 819dc756646ffd0730a163b5a8da65b84a6d504e Alexander McRae <alex@allspice.io> 1737998543 -0800 push

‎services/gitdiff/testdata/academic-module/logs/refs/heads/update-readme

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
0000000000000000000000000000000000000000 528846b39d8f7e68e8081d586ea3e94be23afa7f Alexander McRae <alex@allspice.io> 1737998161 -0800 push

‎services/gitdiff/testdata/academic-module/objects/34/4e0ca8aa791cc4164fb0ea645f334fd40d00f0

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
x���J!���)�N���D�E��Wg�qYus�C�{*�� A���gfZ��w76E*�;<AƷ�g|5�\p*���Ň���C0��H�i�ꊰ���z;�r\)]�� ���\k�w�B�xt>Gn��W��g���@�K�m�Z����5���Hu)���r�ԓ�RϪ�oXc�'��y�-'���S<�q�႙*���� ����#
2+
)᣷�~�r�}e�z�

‎services/gitdiff/testdata/academic-module/objects/52/8846b39d8f7e68e8081d586ea3e94be23afa7f

166 Bytes
Binary file not shown.

‎services/gitdiff/testdata/academic-module/objects/64/ba8d2d41f195318e69793debe5abca244a26a3

422 Bytes
Binary file not shown.

‎services/gitdiff/testdata/academic-module/objects/81/9dc756646ffd0730a163b5a8da65b84a6d504e

173 Bytes
Binary file not shown.

‎services/gitdiff/testdata/academic-module/objects/ca/07ea08ae0fa243d6e0a4129843c5a25a02f499

34 Bytes
Binary file not shown.

‎services/gitdiff/testdata/academic-module/objects/cb/054bf7bad450a8602dec25e6fc9925a6d84332

422 Bytes
Binary file not shown.

‎services/gitdiff/testdata/academic-module/refs/heads/master

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
bd7063cc7c04689c4d082183d32a604ed27a24f9
1+
819dc756646ffd0730a163b5a8da65b84a6d504e

‎services/gitdiff/testdata/academic-module/refs/heads/update-readme

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
528846b39d8f7e68e8081d586ea3e94be23afa7f

0 commit comments

Comments
 (0)