Skip to content

Commit

Permalink
Update diff-tree wrapper to allow user to use --merge-base
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
McRaeAlex committed Jan 27, 2025
1 parent da24f58 commit d6e1065
Show file tree
Hide file tree
Showing 14 changed files with 131 additions and 32 deletions.
48 changes: 26 additions & 22 deletions services/gitdiff/git_diff_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"bufio"
"context"
"fmt"
"io"
"strings"

"code.gitea.io/gitea/modules/git"
Expand All @@ -29,9 +30,11 @@ type DiffTreeRecord struct {
BaseBlobID string
}

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

func runGitDiffTree(ctx context.Context, gitRepo *git.Repository, baseSha, headSha string) ([]*DiffTreeRecord, error) {
baseCommitID, headCommitID, err := validateGitDiffTreeArguments(gitRepo, baseSha, headSha)
func runGitDiffTree(ctx context.Context, gitRepo *git.Repository, useMergeBase bool, baseSha, headSha string) ([]*DiffTreeRecord, error) {
useMergeBase, baseCommitID, headCommitID, err := validateGitDiffTreeArguments(gitRepo, useMergeBase, baseSha, headSha)
if err != nil {
return nil, err
}

cmd := git.NewCommand(ctx, "diff-tree", "--raw", "-r", "--find-renames").AddDynamicArguments(baseCommitID, headCommitID)
cmd := git.NewCommand(ctx, "diff-tree", "--raw", "-r", "--find-renames", "--root")
if useMergeBase {
cmd.AddArguments("--merge-base")
}
cmd.AddDynamicArguments(baseCommitID, headCommitID)
stdout, _, runErr := cmd.RunStdString(&git.RunOpts{Dir: gitRepo.Path})
if runErr != nil {
log.Warn("git diff-tree: %v", runErr)
return nil, runErr
}

return parseGitDiffTree(stdout)
return parseGitDiffTree(strings.NewReader(stdout))
}

func validateGitDiffTreeArguments(gitRepo *git.Repository, baseSha, headSha string) (string, string, error) {
func validateGitDiffTreeArguments(gitRepo *git.Repository, useMergeBase bool, baseSha, headSha string) (bool, string, string, error) {
// if the head is empty its an error
if headSha == "" {
return "", "", fmt.Errorf("headSha is empty")
return false, "", "", fmt.Errorf("headSha is empty")
}

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

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

return objectFormat.EmptyTree().String(), headCommitID, nil
// We set use merge base to false because we have no base commit
return false, objectFormat.EmptyTree().String(), headCommitID, nil
}

baseCommit, err := headCommit.Parent(0)
if err != nil {
return "", "", fmt.Errorf("baseSha is '', attempted to use parent of commit %s, got error: %v", headCommit.ID.String(), err)
return false, "", "", fmt.Errorf("baseSha is '', attempted to use parent of commit %s, got error: %v", headCommit.ID.String(), err)
}
return baseCommit.ID.String(), headCommitID, nil
return useMergeBase, baseCommit.ID.String(), headCommitID, nil
}

// try and get the base commit
baseCommit, err := gitRepo.GetCommit(baseSha)
// propagate the error if we couldn't get the base commit
if err != nil {
return "", "", fmt.Errorf("failed to get base commit %s: %v", baseSha, err)
return useMergeBase, "", "", fmt.Errorf("failed to get base commit %s: %v", baseSha, err)
}

return baseCommit.ID.String(), headCommit.ID.String(), nil
return useMergeBase, baseCommit.ID.String(), headCommit.ID.String(), nil
}

func parseGitDiffTree(output string) ([]*DiffTreeRecord, error) {
func parseGitDiffTree(gitOutput io.Reader) ([]*DiffTreeRecord, error) {
/*
The output of `git diff-tree --raw -r --find-renames` is of the form:
Expand All @@ -112,13 +120,9 @@ func parseGitDiffTree(output string) ([]*DiffTreeRecord, error) {
See: <https://git-scm.com/docs/git-diff-tree#_raw_output_format> for more details
*/
if output == "" {
return []*DiffTreeRecord{}, nil
}

results := make([]*DiffTreeRecord, 0)

lines := bufio.NewScanner(strings.NewReader(output))
lines := bufio.NewScanner(gitOutput)
for lines.Scan() {
line := lines.Text()

Expand Down
105 changes: 97 additions & 8 deletions services/gitdiff/git_diff_tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package gitdiff

import (
"strings"
"testing"

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

func TestGitDiffTree(t *testing.T) {
test := []struct {
Name string
RepoPath string
BaseSha string
HeadSha string
Expected *DiffTree
Name string
RepoPath string
BaseSha string
HeadSha string
useMergeBase bool
Expected *DiffTree
}{
{
Name: "happy path",
Expand Down Expand Up @@ -85,6 +87,25 @@ func TestGitDiffTree(t *testing.T) {
},
},
},
{
Name: "first commit (no parent), merge base = true",
RepoPath: "./testdata/academic-module",
HeadSha: "07901f79ee86272fa8935f2fe546273adaf02c89",
useMergeBase: true,
Expected: &DiffTree{
Files: []*DiffTreeRecord{
{
Status: "added",
HeadPath: "README.md",
BasePath: "README.md",
HeadMode: git.EntryModeBlob,
BaseMode: git.EntryModeNoEntry,
HeadBlobID: "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
BaseBlobID: "0000000000000000000000000000000000000000",
},
},
},
},
{
Name: "base and head same",
RepoPath: "./testdata/academic-module",
Expand Down Expand Up @@ -112,14 +133,82 @@ func TestGitDiffTree(t *testing.T) {
},
},
},
{
Name: "useMergeBase false",
RepoPath: "./testdata/academic-module",
BaseSha: "819dc756646ffd0730a163b5a8da65b84a6d504e",
HeadSha: "528846b39d8f7e68e8081d586ea3e94be23afa7f", // this commit can be found on the update-readme branch
useMergeBase: false,
Expected: &DiffTree{
Files: []*DiffTreeRecord{
{
Status: "modified",
HeadPath: "README.md",
BasePath: "README.md",
HeadMode: git.EntryModeBlob,
BaseMode: git.EntryModeBlob,
HeadBlobID: "ca07ea08ae0fa243d6e0a4129843c5a25a02f499",
BaseBlobID: "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
},
{
Status: "modified",
HeadPath: "webpack.mix.js",
BasePath: "webpack.mix.js",
HeadMode: git.EntryModeBlob,
BaseMode: git.EntryModeBlob,
HeadBlobID: "26825d9cb822e237b600153a26dd1e0e68457196",
BaseBlobID: "344e0ca8aa791cc4164fb0ea645f334fd40d00f0",
},
},
},
},
{
Name: "useMergeBase true",
RepoPath: "./testdata/academic-module",
BaseSha: "819dc756646ffd0730a163b5a8da65b84a6d504e",
HeadSha: "528846b39d8f7e68e8081d586ea3e94be23afa7f", // this commit can be found on the update-readme branch
useMergeBase: true,
Expected: &DiffTree{
Files: []*DiffTreeRecord{
{
Status: "modified",
HeadPath: "README.md",
BasePath: "README.md",
HeadMode: git.EntryModeBlob,
BaseMode: git.EntryModeBlob,
HeadBlobID: "ca07ea08ae0fa243d6e0a4129843c5a25a02f499",
BaseBlobID: "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
},
},
},
},
{
Name: "no base set",
RepoPath: "./testdata/academic-module",
HeadSha: "528846b39d8f7e68e8081d586ea3e94be23afa7f", // this commit can be found on the update-readme branch
useMergeBase: false,
Expected: &DiffTree{
Files: []*DiffTreeRecord{
{
Status: "modified",
HeadPath: "README.md",
BasePath: "README.md",
HeadMode: git.EntryModeBlob,
BaseMode: git.EntryModeBlob,
HeadBlobID: "ca07ea08ae0fa243d6e0a4129843c5a25a02f499",
BaseBlobID: "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391",
},
},
},
},
}

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

diffPaths, err := GetDiffTree(db.DefaultContext, gitRepo, tt.BaseSha, tt.HeadSha)
diffPaths, err := GetDiffTree(db.DefaultContext, gitRepo, tt.useMergeBase, tt.BaseSha, tt.HeadSha)
require.NoError(t, err)

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

diffPaths, err := GetDiffTree(db.DefaultContext, gitRepo, tt.BaseSha, tt.HeadSha)
diffPaths, err := GetDiffTree(db.DefaultContext, gitRepo, true, tt.BaseSha, tt.HeadSha)
assert.Error(t, err)
assert.Nil(t, diffPaths)
})
Expand Down Expand Up @@ -310,7 +399,7 @@ func TestParseGitDiffTree(t *testing.T) {

for _, tt := range test {
t.Run(tt.Name, func(t *testing.T) {
entries, err := parseGitDiffTree(tt.GitOutput)
entries, err := parseGitDiffTree(strings.NewReader(tt.GitOutput))
assert.NoError(t, err)
assert.Equal(t, tt.Expected, entries)
})
Expand Down
2 changes: 1 addition & 1 deletion services/gitdiff/testdata/academic-module/config
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[core]
repositoryformatversion = 0
filemode = true
bare = false
bare = true
logallrefupdates = true
ignorecase = true
precomposeunicode = true
Expand Down
1 change: 1 addition & 0 deletions services/gitdiff/testdata/academic-module/logs/HEAD
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
0000000000000000000000000000000000000000 bd7063cc7c04689c4d082183d32a604ed27a24f9 Lunny Xiao <[email protected]> 1574829684 +0800 clone: from https://try.gitea.io/shemgp-aiias/academic-module
0000000000000000000000000000000000000000 819dc756646ffd0730a163b5a8da65b84a6d504e Alexander McRae <[email protected]> 1737998543 -0800 push
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
0000000000000000000000000000000000000000 bd7063cc7c04689c4d082183d32a604ed27a24f9 Lunny Xiao <[email protected]> 1574829684 +0800 clone: from https://try.gitea.io/shemgp-aiias/academic-module
bd7063cc7c04689c4d082183d32a604ed27a24f9 819dc756646ffd0730a163b5a8da65b84a6d504e Alexander McRae <[email protected]> 1737998543 -0800 push
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0000000000000000000000000000000000000000 528846b39d8f7e68e8081d586ea3e94be23afa7f Alexander McRae <[email protected]> 1737998161 -0800 push
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
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�႙*���� ����#
)᣷�~�r�}e�z�
Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -1 +1 @@
bd7063cc7c04689c4d082183d32a604ed27a24f9
819dc756646ffd0730a163b5a8da65b84a6d504e
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
528846b39d8f7e68e8081d586ea3e94be23afa7f

0 comments on commit d6e1065

Please sign in to comment.