From 92a952aab545ccf3140bce1559d8c45185fc6c79 Mon Sep 17 00:00:00 2001 From: Ashok Siyani Date: Fri, 11 Oct 2024 16:36:28 +0100 Subject: [PATCH] add support to list commits of branch and merge (#24) * add support to list commits of branch and merge * added new method to repo pool * use correct name --- pkg/mirror/repo_pool.go | 27 ++++++ pkg/mirror/repository.go | 73 ++++++++++++-- pkg/mirror/repository_test.go | 61 ++++++++++++ pkg/mirror/z_e2e_test.go | 178 +++++++++++++++++++++++++++++++++- 4 files changed, 327 insertions(+), 12 deletions(-) diff --git a/pkg/mirror/repo_pool.go b/pkg/mirror/repo_pool.go index 09f623b..cf38a17 100644 --- a/pkg/mirror/repo_pool.go +++ b/pkg/mirror/repo_pool.go @@ -202,3 +202,30 @@ func (rp *RepoPool) Clone(ctx context.Context, remote, dst, branch, pathspec str } return repo.Clone(ctx, dst, branch, pathspec, rmGitDir) } + +// MergeCommits is wrapper around repositories MergeCommits method +func (rp *RepoPool) MergeCommits(ctx context.Context, remote, mergeCommitHash string) ([]CommitInfo, error) { + repo, err := rp.Repository(remote) + if err != nil { + return nil, err + } + return repo.MergeCommits(ctx, mergeCommitHash) +} + +// BranchCommits is wrapper around repositories BranchCommits method +func (rp *RepoPool) BranchCommits(ctx context.Context, remote, branch string) ([]CommitInfo, error) { + repo, err := rp.Repository(remote) + if err != nil { + return nil, err + } + return repo.BranchCommits(ctx, branch) +} + +// ListCommitsWithChangedFiles is wrapper around repositories ListCommitsWithChangedFiles method +func (rp *RepoPool) ListCommitsWithChangedFiles(ctx context.Context, remote, ref1, ref2 string) ([]CommitInfo, error) { + repo, err := rp.Repository(remote) + if err != nil { + return nil, err + } + return repo.ListCommitsWithChangedFiles(ctx, ref1, ref2) +} diff --git a/pkg/mirror/repository.go b/pkg/mirror/repository.go index ca7e470..04bc761 100644 --- a/pkg/mirror/repository.go +++ b/pkg/mirror/repository.go @@ -188,14 +188,10 @@ func (r *Repository) LogMsg(ctx context.Context, ref, path string) (string, erro // Subject returns commit subject of given commit hash func (r *Repository) Subject(ctx context.Context, hash string) (string, error) { - if err := r.ObjectExists(ctx, hash); err != nil { - return "", err - } - r.lock.RLock() defer r.lock.RUnlock() - args := []string{"show", `--no-patch`, `--format='%s'`, hash} + args := []string{"show", `--no-patch`, `--format=%s`, hash} msg, err := runGitCommand(ctx, r.log, r.envs, r.dir, args...) if err != nil { return "", err @@ -205,19 +201,80 @@ func (r *Repository) Subject(ctx context.Context, hash string) (string, error) { // ChangedFiles returns path of the changed files for given commit hash func (r *Repository) ChangedFiles(ctx context.Context, hash string) ([]string, error) { - if err := r.ObjectExists(ctx, hash); err != nil { + r.lock.RLock() + defer r.lock.RUnlock() + + args := []string{"show", `--name-only`, `--pretty=format:`, hash} + msg, err := runGitCommand(ctx, r.log, r.envs, r.dir, args...) + if err != nil { return nil, err } + return strings.Split(msg, "\n"), nil +} + +type CommitInfo struct { + Hash string + ChangedFiles []string +} + +// MergeCommits lists commits from the mergeCommitHash but not from the first +// parent of mergeCommitHash (mergeCommitHash^) in chronological order. (latest to oldest) +func (r *Repository) MergeCommits(ctx context.Context, mergeCommitHash string) ([]CommitInfo, error) { + return r.ListCommitsWithChangedFiles(ctx, mergeCommitHash+"^", mergeCommitHash) +} + +// BranchCommits lists commits from the tip of the branch but not from the HEAD +// of the repository in chronological order. (latest to oldest) +func (r *Repository) BranchCommits(ctx context.Context, branch string) ([]CommitInfo, error) { + return r.ListCommitsWithChangedFiles(ctx, "HEAD", branch) +} +// ListCommitsWithChangedFiles returns path of the changed files for given commit hash +// list all the commits and files which are reachable from 'ref2', but not from 'ref1' +// The output is given in reverse chronological order. +func (r *Repository) ListCommitsWithChangedFiles(ctx context.Context, ref1, ref2 string) ([]CommitInfo, error) { r.lock.RLock() defer r.lock.RUnlock() - args := []string{"show", `--name-only`, `--pretty=format:`, hash} + args := []string{"log", `--name-only`, `--pretty=format:%H`, ref1 + ".." + ref2} msg, err := runGitCommand(ctx, r.log, r.envs, r.dir, args...) if err != nil { return nil, err } - return strings.Split(msg, "\n"), nil + return ParseCommitWithChangedFilesList(msg), nil +} + +// ParseCommitWithChangedFilesList will parse following output of 'show/log' +// command with `--name-only`, `--pretty=format:%H` flags +// +// 72ea9c9de6963e97ac472d9ea996e384c6923cca +// +// 80e11d114dd3aa135c18573402a8e688599c69e0 +// one/readme.yaml +// one/hello.tf +// two/readme.yaml +func ParseCommitWithChangedFilesList(output string) []CommitInfo { + commitCount := 0 + Commits := []CommitInfo{} + + for _, line := range strings.Split(output, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + if IsFullCommitHash(line) { + Commits = append(Commits, CommitInfo{Hash: line}) + commitCount += 1 + continue + } + // if line is not commit or empty then its assumed to be changed file name + // also this file change belongs to the last commit + if commitCount > 0 { + Commits[commitCount-1].ChangedFiles = append(Commits[commitCount-1].ChangedFiles, line) + } + } + + return Commits } // ObjectExists returns error is given object is not valid or if it doesn't exists diff --git a/pkg/mirror/repository_test.go b/pkg/mirror/repository_test.go index 0399863..29a0244 100644 --- a/pkg/mirror/repository_test.go +++ b/pkg/mirror/repository_test.go @@ -166,3 +166,64 @@ func TestRepo_AddWorktreeLink(t *testing.T) { t.Errorf("Repo.AddWorktreeLink() worktreelinks mismatch (-want +got):\n%s", diff) } } + +func TestParseCommitWithChangedFilesList(t *testing.T) { + tests := []struct { + name string + output string + want []CommitInfo + }{ + { + "empty", + ` + + `, + []CommitInfo{}, + }, + { + "only_commit", + `267fc66a734de9e4de57d9d20c83566a69cd703c + `, + []CommitInfo{{Hash: "267fc66a734de9e4de57d9d20c83566a69cd703c"}}, + }, + { + "no_changed_files", + ` +267fc66a734de9e4de57d9d20c83566a69cd703c + + + `, + []CommitInfo{{Hash: "267fc66a734de9e4de57d9d20c83566a69cd703c"}}, + }, + { + "multiple_commits", + `267fc66a734de9e4de57d9d20c83566a69cd703c +1f68b80bc259e067fdb3dc4bb82cdbd43645e392 +one/hello.tf + +72ea9c9de6963e97ac472d9ea996e384c6923cca +readme + +80e11d114dd3aa135c18573402a8e688599c69e0 +one/readme +one/hello.tf +two/readme + + `, + []CommitInfo{ + {Hash: "267fc66a734de9e4de57d9d20c83566a69cd703c"}, + {Hash: "1f68b80bc259e067fdb3dc4bb82cdbd43645e392", ChangedFiles: []string{"one/hello.tf"}}, + {Hash: "72ea9c9de6963e97ac472d9ea996e384c6923cca", ChangedFiles: []string{"readme"}}, + {Hash: "80e11d114dd3aa135c18573402a8e688599c69e0", ChangedFiles: []string{"one/readme", "one/hello.tf", "two/readme"}}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ParseCommitWithChangedFilesList(tt.output) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("ParseCommitWithChangedFilesList() output mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/pkg/mirror/z_e2e_test.go b/pkg/mirror/z_e2e_test.go index df2791d..859064e 100644 --- a/pkg/mirror/z_e2e_test.go +++ b/pkg/mirror/z_e2e_test.go @@ -11,6 +11,8 @@ import ( "strings" "testing" "time" + + "github.com/google/go-cmp/cmp" ) const ( @@ -656,6 +658,16 @@ func Test_commit_hash_msg(t *testing.T) { assertCommitLog(t, repo, otherBranch, "dir1", dir1SHA3, t.Name()+"-dir1-main-3", []string{filepath.Join("dir1", "file")}) assertCommitLog(t, repo, otherBranch, "dir2", dir2SHA3, t.Name()+"-dir2-other-3", []string{filepath.Join("dir2", "file")}) + wantDiffList := []CommitInfo{ + {Hash: fileOtherSHA3, ChangedFiles: []string{"file"}}, + {Hash: dir2SHA3, ChangedFiles: []string{filepath.Join("dir2", "file")}}, + } + if got, err := repo.BranchCommits(txtCtx, otherBranch); err != nil { + t.Fatalf("unexpected error: %v", err) + } else if diff := cmp.Diff(wantDiffList, got); diff != "" { + t.Errorf("BranchCommits() mismatch (-want +got):\n%s", diff) + } + t.Log("TEST-4: forward HEAD and other-branch") dir1SHA4 := mustCommit(t, upstream, filepath.Join("dir1", "file"), t.Name()+"-dir1-main-4") @@ -683,6 +695,20 @@ func Test_commit_hash_msg(t *testing.T) { assertCommitLog(t, repo, otherBranch, "dir1", dir1SHA3, t.Name()+"-dir1-main-3", []string{filepath.Join("dir1", "file")}) assertCommitLog(t, repo, otherBranch, "dir2", dir2SHA4, t.Name()+"-dir2-other-4", []string{filepath.Join("dir2", "file")}) + wantDiffList = []CommitInfo{ + // new commits + {Hash: fileOtherSHA4, ChangedFiles: []string{"file"}}, + {Hash: dir2SHA4, ChangedFiles: []string{filepath.Join("dir2", "file")}}, + // old commits + {Hash: fileOtherSHA3, ChangedFiles: []string{"file"}}, + {Hash: dir2SHA3, ChangedFiles: []string{filepath.Join("dir2", "file")}}, + } + if got, err := repo.BranchCommits(txtCtx, otherBranch); err != nil { + t.Fatalf("unexpected error: %v", err) + } else if diff := cmp.Diff(wantDiffList, got); diff != "" { + t.Errorf("BranchCommits() mismatch (-want +got):\n%s", diff) + } + t.Log("TEST-4: move HEAD and other-branch backward to test3") mustExec(t, upstream, "git", "checkout", "-q", otherBranch) @@ -707,6 +733,17 @@ func Test_commit_hash_msg(t *testing.T) { assertCommitLog(t, repo, otherBranch, "dir1", dir1SHA3, t.Name()+"-dir1-main-3", []string{filepath.Join("dir1", "file")}) assertCommitLog(t, repo, otherBranch, "dir2", dir2SHA3, t.Name()+"-dir2-other-3", []string{filepath.Join("dir2", "file")}) + wantDiffList = []CommitInfo{ + // old commits + {Hash: fileOtherSHA3, ChangedFiles: []string{"file"}}, + {Hash: dir2SHA3, ChangedFiles: []string{filepath.Join("dir2", "file")}}, + } + if got, err := repo.BranchCommits(txtCtx, otherBranch); err != nil { + t.Fatalf("unexpected error: %v", err) + } else if diff := cmp.Diff(wantDiffList, got); diff != "" { + t.Errorf("BranchCommits() mismatch (-want +got):\n%s", diff) + } + t.Log("TEST-5: move HEAD backward to test1 and delete other-branch") mustExec(t, upstream, "git", "branch", "-D", otherBranch) @@ -740,6 +777,137 @@ func Test_commit_hash_msg(t *testing.T) { } } +func Test_commit_hash_files_merge(t *testing.T) { + testTmpDir := mustTmpDir(t) + defer os.RemoveAll(testTmpDir) + + upstream := filepath.Join(testTmpDir, testUpstreamRepo) + root := filepath.Join(testTmpDir, testRoot) + otherBranch := "other-branch" + + t.Log("TEST-1: init upstream and verify 1st commit after mirror") + + mustInitRepo(t, upstream, "file", t.Name()+"-main-1") + + repo := mustCreateRepoAndMirror(t, upstream, root, "", "") + + t.Log("TEST-2: forward HEAD and create dir1 on HEAD") + + dir1SHA2 := mustCommit(t, upstream, filepath.Join("dir1", "file"), t.Name()+"-dir1-main-2") + fileSHA2 := mustCommit(t, upstream, "file", t.Name()+"-main-2") + + // mirror new commits + if err := repo.Mirror(txtCtx); err != nil { + t.Fatalf("unable to mirror error: %v", err) + } + + assertCommitLog(t, repo, "HEAD", "", fileSHA2, t.Name()+"-main-2", []string{"file"}) + assertCommitLog(t, repo, "HEAD", "dir1", dir1SHA2, t.Name()+"-dir1-main-2", []string{filepath.Join("dir1", "file")}) + + t.Log("TEST-3: forward HEAD and create other-branch") + + dir1SHA3 := mustCommit(t, upstream, filepath.Join("dir1", "file"), t.Name()+"-dir1-main-3") + // create branch and push commits + mustExec(t, upstream, "git", "checkout", "-q", "-b", otherBranch) + dir2SHA3 := mustCommit(t, upstream, filepath.Join("dir2", "file"), t.Name()+"-dir2-other-3") + fileOtherSHA3 := mustCommit(t, upstream, "file", t.Name()+"-other-3") + // check out master and push more commit + mustExec(t, upstream, "git", "checkout", "-q", testMainBranch) + mustCommit(t, upstream, "file2", t.Name()+"-main-3") + // merge other branch to master and get merge commit + mustExec(t, upstream, "git", "merge", "--no-ff", otherBranch, "-m", "Merging otherBranch with no-ff v1") + mergeCommit1 := mustExec(t, upstream, "git", "rev-list", "-n1", "HEAD") + + // mirror new commits + if err := repo.Mirror(txtCtx); err != nil { + t.Fatalf("unable to mirror error: %v", err) + } + + assertCommitLog(t, repo, "HEAD", "", mergeCommit1, "Merging otherBranch with no-ff v1", []string{}) + assertCommitLog(t, repo, "HEAD", "dir1", dir1SHA3, t.Name()+"-dir1-main-3", []string{filepath.Join("dir1", "file")}) + assertCommitLog(t, repo, testMainBranch, "dir2", dir2SHA3, t.Name()+"-dir2-other-3", []string{filepath.Join("dir2", "file")}) + + wantDiffList := []CommitInfo{ + {Hash: mergeCommit1}, + {Hash: fileOtherSHA3, ChangedFiles: []string{"file"}}, + {Hash: dir2SHA3, ChangedFiles: []string{filepath.Join("dir2", "file")}}, + } + if got, err := repo.MergeCommits(txtCtx, mergeCommit1); err != nil { + t.Fatalf("unexpected error: %v", err) + } else if diff := cmp.Diff(wantDiffList, got); diff != "" { + t.Errorf("CommitsOfMergeCommit() mismatch (-want +got):\n%s", diff) + } + + t.Log("TEST-4: add more commits to same other-branch and merge") + + dir1SHA4 := mustCommit(t, upstream, filepath.Join("dir1", "file"), t.Name()+"-dir1-main-4") + // switch to other branch and push commits + mustExec(t, upstream, "git", "checkout", "-q", otherBranch) + dir2SHA4 := mustCommit(t, upstream, filepath.Join("dir2", "file"), t.Name()+"-dir2-other-4") + fileOtherSHA4 := mustCommit(t, upstream, "file", t.Name()+"-other-4") + // check out master and push more commit + mustExec(t, upstream, "git", "checkout", "-q", testMainBranch) + mustCommit(t, upstream, "file2", t.Name()+"-main-4") + // merge other branch to master and get merge commit + mustExec(t, upstream, "git", "merge", "--no-ff", otherBranch, "-m", "Merging otherBranch with no-ff v2") + mergeCommit2 := mustExec(t, upstream, "git", "rev-list", "-n1", "HEAD") + + // mirror new commits + if err := repo.Mirror(txtCtx); err != nil { + t.Fatalf("unable to mirror error: %v", err) + } + + assertCommitLog(t, repo, "HEAD", "", mergeCommit2, "Merging otherBranch with no-ff v2", []string{}) + assertCommitLog(t, repo, testMainBranch, "dir1", dir1SHA4, t.Name()+"-dir1-main-4", []string{filepath.Join("dir1", "file")}) + assertCommitLog(t, repo, testMainBranch, "dir2", dir2SHA4, t.Name()+"-dir2-other-4", []string{filepath.Join("dir2", "file")}) + + wantDiffList = []CommitInfo{ + {Hash: mergeCommit2}, + // new commits on same branch + {Hash: fileOtherSHA4, ChangedFiles: []string{"file"}}, + {Hash: dir2SHA4, ChangedFiles: []string{filepath.Join("dir2", "file")}}, + } + if got, err := repo.MergeCommits(txtCtx, mergeCommit2); err != nil { + t.Fatalf("unexpected error: %v", err) + } else if diff := cmp.Diff(wantDiffList, got); diff != "" { + t.Errorf("CommitsOfMergeCommit() mismatch (-want +got):\n%s", diff) + } + + t.Log("TEST-5: create new branch and then do squash merge") + otherBranch = otherBranch + "v2" + + dir1SHA5 := mustCommit(t, upstream, filepath.Join("dir1", "file"), t.Name()+"-dir1-main-5") + // create branch and push commits + mustExec(t, upstream, "git", "checkout", "-q", "-b", otherBranch) + mustCommit(t, upstream, filepath.Join("dir2", "file"), t.Name()+"-dir2-other-5") + mustCommit(t, upstream, "file", t.Name()+"-other-5") + // check out master and push more commit + mustExec(t, upstream, "git", "checkout", "-q", testMainBranch) + mustCommit(t, upstream, "file2", t.Name()+"-main-5") + // merge other branch to master and get merge commit + mustExec(t, upstream, "git", "merge", "--squash", otherBranch) + mustExec(t, upstream, "git", "commit", "-m", "Merging otherBranch with squash v1") + mergeCommit3 := mustExec(t, upstream, "git", "rev-list", "-n1", "HEAD") + + // mirror new commits + if err := repo.Mirror(txtCtx); err != nil { + t.Fatalf("unable to mirror error: %v", err) + } + + assertCommitLog(t, repo, "HEAD", "", mergeCommit3, "Merging otherBranch with squash v1", []string{}) + assertCommitLog(t, repo, "HEAD", "dir1", dir1SHA5, t.Name()+"-dir1-main-5", []string{filepath.Join("dir1", "file")}) + assertCommitLog(t, repo, "HEAD", "dir2", mergeCommit3, "Merging otherBranch with squash v1", []string{filepath.Join("dir2", "file"), "file"}) + + wantDiffList = []CommitInfo{ + {Hash: mergeCommit3, ChangedFiles: []string{filepath.Join("dir2", "file"), "file"}}, + } + if got, err := repo.MergeCommits(txtCtx, mergeCommit3); err != nil { + t.Fatalf("unexpected error: %v", err) + } else if diff := cmp.Diff(wantDiffList, got); diff != "" { + t.Errorf("CommitsOfMergeCommit() mismatch (-want +got):\n%s", diff) + } +} + func Test_clone_branch(t *testing.T) { testTmpDir := mustTmpDir(t) defer os.RemoveAll(testTmpDir) @@ -1578,10 +1746,12 @@ func assertCommitLog(t *testing.T, repo *Repository, t.Errorf("subject mismatch sha:%s got:%s want:%s", gotHash, got, wantSub) } - if got, err := repo.ChangedFiles(txtCtx, gotHash); err != nil { - t.Fatalf("unexpected error: %v", err) - } else if slices.Compare(got, wantChangedFiles) != 0 { - t.Errorf("changed file mismatch sha:%s got:%s want:%s", gotHash, got, wantChangedFiles) + if len(wantChangedFiles) > 0 { + if got, err := repo.ChangedFiles(txtCtx, gotHash); err != nil { + t.Fatalf("unexpected error: %v", err) + } else if slices.Compare(got, wantChangedFiles) != 0 { + t.Errorf("changed file mismatch sha:%s got:%s want:%s", gotHash, got, wantChangedFiles) + } } }