From d6e1065aabf3097d91f46a97ee664cf4b5cd0ca8 Mon Sep 17 00:00:00 2001 From: Alexander McRae Date: Mon, 27 Jan 2025 10:02:44 -0800 Subject: [PATCH] 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 --- services/gitdiff/git_diff_tree.go | 48 ++++---- services/gitdiff/git_diff_tree_test.go | 105 ++++++++++++++++-- .../gitdiff/testdata/academic-module/config | 2 +- .../testdata/academic-module/logs/HEAD | 1 + .../academic-module/logs/refs/heads/master | 1 + .../logs/refs/heads/update-readme | 1 + .../34/4e0ca8aa791cc4164fb0ea645f334fd40d00f0 | 2 + .../52/8846b39d8f7e68e8081d586ea3e94be23afa7f | Bin 0 -> 166 bytes .../64/ba8d2d41f195318e69793debe5abca244a26a3 | Bin 0 -> 422 bytes .../81/9dc756646ffd0730a163b5a8da65b84a6d504e | Bin 0 -> 173 bytes .../ca/07ea08ae0fa243d6e0a4129843c5a25a02f499 | Bin 0 -> 34 bytes .../cb/054bf7bad450a8602dec25e6fc9925a6d84332 | Bin 0 -> 422 bytes .../academic-module/refs/heads/master | 2 +- .../academic-module/refs/heads/update-readme | 1 + 14 files changed, 131 insertions(+), 32 deletions(-) create mode 100644 services/gitdiff/testdata/academic-module/logs/refs/heads/update-readme create mode 100644 services/gitdiff/testdata/academic-module/objects/34/4e0ca8aa791cc4164fb0ea645f334fd40d00f0 create mode 100644 services/gitdiff/testdata/academic-module/objects/52/8846b39d8f7e68e8081d586ea3e94be23afa7f create mode 100644 services/gitdiff/testdata/academic-module/objects/64/ba8d2d41f195318e69793debe5abca244a26a3 create mode 100644 services/gitdiff/testdata/academic-module/objects/81/9dc756646ffd0730a163b5a8da65b84a6d504e create mode 100644 services/gitdiff/testdata/academic-module/objects/ca/07ea08ae0fa243d6e0a4129843c5a25a02f499 create mode 100644 services/gitdiff/testdata/academic-module/objects/cb/054bf7bad450a8602dec25e6fc9925a6d84332 create mode 100644 services/gitdiff/testdata/academic-module/refs/heads/update-readme 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-'S0(lhjI)la}B;xt*KcJB*q!d66s_QmGOY3k}7q zCr%hlDKmwsI)3!q*5I<(lkaK`uJXeMCw_-BFU#LPi+#=8Hw@Y5TFVd#j#v<#d>;GT U{71XrUAY@HV_nU?00V8Fkvt;00IT){JgZxbcTa@XOEe*@4Pqn!lR7Ng;8oIfv>gU z3X1b{QW>tMT-alKQ+!g!q@!`>Y-a>>qs8JNDqIpv5|a{(QyG}IGS8OWG|%qdj=vQb z9rh*MXJxB`C~?gz$t=lCEyiPtM@dNm!;;%VO1tOsmkTX9WfA{py$Jh0_PG!f0*dm> zGE-8EiWzLBPQ~vF6n(~fPFAV;?wt;ewjcWq4GhdoOca7#9bJ4~^>R}fp3QwUd(*{h z&feXc>&l~U9D33dJP~40P-=00X;CuJErt!8`4WPs(ys)5`qX90sqK73H2|t6zqACX z;IY_lUV$GIo_P4R z>y#f`t65}y`HG_kE$;qMg}M1Dr8%h(WpfmNwAELO9#sxxJFXslQz3l6+c~JRg2d$P z#B`{-^t)xfRkPYY&63^=wi;v=m=Y#v{v+ctc&xH btxn9aCf40L!duFObYIZWc31TUw*O8#-_%z_ literal 0 HcmV?d00001 diff --git a/services/gitdiff/testdata/academic-module/objects/ca/07ea08ae0fa243d6e0a4129843c5a25a02f499 b/services/gitdiff/testdata/academic-module/objects/ca/07ea08ae0fa243d6e0a4129843c5a25a02f499 new file mode 100644 index 0000000000000000000000000000000000000000..c84b02fc4994ebff8cfc11b2f95c3706e99dc254 GIT binary patch literal 34 qcmb^&YtzxJnQ+Ci6MJAM*si-GY*dc literal 0 HcmV?d00001 diff --git a/services/gitdiff/testdata/academic-module/objects/cb/054bf7bad450a8602dec25e6fc9925a6d84332 b/services/gitdiff/testdata/academic-module/objects/cb/054bf7bad450a8602dec25e6fc9925a6d84332 new file mode 100644 index 0000000000000000000000000000000000000000..60c4acc0be412c35e63bd16afaaee527968fc524 GIT binary patch literal 422 zcmV;X0a^Zd0V^p=O;s>8Fkvt;00IT){JgZxbcTa@XOEe*@4Pqn!lR7Ng;8oIfv>gU z3X1b{QW>tMT-alKQ+!g!q@!`>Y-a>>qs8JNDqIpv5|a{(QyG}IGS8OWG|%qdj=vQb z9rh*MXJxB`C~?gz$t=lCEyiPtM@dNm!;;%VO1tOsmkTX9WfA{py$Jh0_PG!f0*dm> zGE-8EiWzLBPQ~vF6n(~fPFAV;?wt;ewjcWq4GhdoOca7#9bJ4~^>R}fPO-n@SjWG} z`PzdeLNlC?E{bCMG81A@P-=00X;CuJErt!8`4WPs(ys)5`qX90sqK73H2|t6zqACX z;IY_lUV$GIo_P4R z>y#f`t65}y`HG_kE$;qMg}M1Dr8%h(WpfmNwAELO9#sxxJFXslQz3l6+c~JRg2d$P z#B`{-^t)xfRkPYY