diff --git a/services/gitdiff/git_diff_tree.go b/services/gitdiff/git_diff_tree.go index fb66555bc9e45..31f5e6df1f9af 100644 --- a/services/gitdiff/git_diff_tree.go +++ b/services/gitdiff/git_diff_tree.go @@ -7,6 +7,7 @@ import ( "bufio" "context" "fmt" + "io" "strings" "code.gitea.io/gitea/modules/git" @@ -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 } @@ -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() @@ -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: @@ -112,13 +120,9 @@ func parseGitDiffTree(output string) ([]*DiffTreeRecord, error) { See: 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() diff --git a/services/gitdiff/git_diff_tree_test.go b/services/gitdiff/git_diff_tree_test.go index ac703d5a4d35c..95a9749054113 100644 --- a/services/gitdiff/git_diff_tree_test.go +++ b/services/gitdiff/git_diff_tree_test.go @@ -4,6 +4,7 @@ package gitdiff import ( + "strings" "testing" "code.gitea.io/gitea/models/db" @@ -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", @@ -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", @@ -112,6 +133,74 @@ 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 { @@ -119,7 +208,7 @@ func TestGitDiffTree(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) @@ -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) }) @@ -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) }) diff --git a/services/gitdiff/testdata/academic-module/config b/services/gitdiff/testdata/academic-module/config index 1bc26be5146e8..22a81f4e0cb40 100644 --- a/services/gitdiff/testdata/academic-module/config +++ b/services/gitdiff/testdata/academic-module/config @@ -1,7 +1,7 @@ [core] repositoryformatversion = 0 filemode = true - bare = false + bare = true logallrefupdates = true ignorecase = true precomposeunicode = true diff --git a/services/gitdiff/testdata/academic-module/logs/HEAD b/services/gitdiff/testdata/academic-module/logs/HEAD index 16b2e1c0f6025..1af6768ef30b5 100644 --- a/services/gitdiff/testdata/academic-module/logs/HEAD +++ b/services/gitdiff/testdata/academic-module/logs/HEAD @@ -1 +1,2 @@ 0000000000000000000000000000000000000000 bd7063cc7c04689c4d082183d32a604ed27a24f9 Lunny Xiao 1574829684 +0800 clone: from https://try.gitea.io/shemgp-aiias/academic-module +0000000000000000000000000000000000000000 819dc756646ffd0730a163b5a8da65b84a6d504e Alexander McRae 1737998543 -0800 push diff --git a/services/gitdiff/testdata/academic-module/logs/refs/heads/master b/services/gitdiff/testdata/academic-module/logs/refs/heads/master index 16b2e1c0f6025..2ac9c08a0d64f 100644 --- a/services/gitdiff/testdata/academic-module/logs/refs/heads/master +++ b/services/gitdiff/testdata/academic-module/logs/refs/heads/master @@ -1 +1,2 @@ 0000000000000000000000000000000000000000 bd7063cc7c04689c4d082183d32a604ed27a24f9 Lunny Xiao 1574829684 +0800 clone: from https://try.gitea.io/shemgp-aiias/academic-module +bd7063cc7c04689c4d082183d32a604ed27a24f9 819dc756646ffd0730a163b5a8da65b84a6d504e Alexander McRae 1737998543 -0800 push diff --git a/services/gitdiff/testdata/academic-module/logs/refs/heads/update-readme b/services/gitdiff/testdata/academic-module/logs/refs/heads/update-readme new file mode 100644 index 0000000000000..96707bf56fcf5 --- /dev/null +++ b/services/gitdiff/testdata/academic-module/logs/refs/heads/update-readme @@ -0,0 +1 @@ +0000000000000000000000000000000000000000 528846b39d8f7e68e8081d586ea3e94be23afa7f Alexander McRae 1737998161 -0800 push diff --git a/services/gitdiff/testdata/academic-module/objects/34/4e0ca8aa791cc4164fb0ea645f334fd40d00f0 b/services/gitdiff/testdata/academic-module/objects/34/4e0ca8aa791cc4164fb0ea645f334fd40d00f0 new file mode 100644 index 0000000000000..66dcc4d52e94d --- /dev/null +++ b/services/gitdiff/testdata/academic-module/objects/34/4e0ca8aa791cc4164fb0ea645f334fd40d00f0 @@ -0,0 +1,2 @@ +xJ!)NDEWgqYusC{* AgfZw76E*;GnWg @KmZ5Hu)rԓRϪoXc'y-'S