From 75ea0d5dba5dbf2f84cef2d12460fdd566d43e62 Mon Sep 17 00:00:00 2001 From: oliverpool <3864879+oliverpool@users.noreply.github.com> Date: Thu, 4 May 2023 07:08:41 +0200 Subject: [PATCH] Faster git.GetDivergingCommits (#24482) Using `git rev-list --left-right` is almost 2x faster than calling `git rev-list` twice. Co-authored-by: silverwind --- modules/git/repo.go | 39 ++++++++++++++++----------------------- modules/git/repo_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/modules/git/repo.go b/modules/git/repo.go index 3637aa47c4..61930ab31d 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -244,35 +244,28 @@ type DivergeObject struct { Behind int } -func checkDivergence(ctx context.Context, repoPath, baseBranch, targetBranch string) (int, error) { - branches := fmt.Sprintf("%s..%s", baseBranch, targetBranch) - cmd := NewCommand(ctx, "rev-list", "--count").AddDynamicArguments(branches) +// GetDivergingCommits returns the number of commits a targetBranch is ahead or behind a baseBranch +func GetDivergingCommits(ctx context.Context, repoPath, baseBranch, targetBranch string) (do DivergeObject, err error) { + cmd := NewCommand(ctx, "rev-list", "--count", "--left-right"). + AddDynamicArguments(baseBranch + "..." + targetBranch) stdout, _, err := cmd.RunStdString(&RunOpts{Dir: repoPath}) if err != nil { - return -1, err + return do, err } - outInteger, errInteger := strconv.Atoi(strings.Trim(stdout, "\n")) - if errInteger != nil { - return -1, errInteger - } - return outInteger, nil -} - -// GetDivergingCommits returns the number of commits a targetBranch is ahead or behind a baseBranch -func GetDivergingCommits(ctx context.Context, repoPath, baseBranch, targetBranch string) (DivergeObject, error) { - // $(git rev-list --count master..feature) commits ahead of master - ahead, errorAhead := checkDivergence(ctx, repoPath, baseBranch, targetBranch) - if errorAhead != nil { - return DivergeObject{}, errorAhead + left, right, found := strings.Cut(strings.Trim(stdout, "\n"), "\t") + if !found { + return do, fmt.Errorf("git rev-list output is missing a tab: %q", stdout) } - // $(git rev-list --count feature..master) commits behind master - behind, errorBehind := checkDivergence(ctx, repoPath, targetBranch, baseBranch) - if errorBehind != nil { - return DivergeObject{}, errorBehind + do.Behind, err = strconv.Atoi(left) + if err != nil { + return do, err } - - return DivergeObject{ahead, behind}, nil + do.Ahead, err = strconv.Atoi(right) + if err != nil { + return do, err + } + return do, nil } // CreateBundle create bundle content to the target path diff --git a/modules/git/repo_test.go b/modules/git/repo_test.go index 044b9d4065..9db78153a1 100644 --- a/modules/git/repo_test.go +++ b/modules/git/repo_test.go @@ -4,6 +4,7 @@ package git import ( + "context" "path/filepath" "testing" @@ -29,3 +30,27 @@ func TestRepoIsEmpty(t *testing.T) { assert.NoError(t, err) assert.True(t, isEmpty) } + +func TestRepoGetDivergingCommits(t *testing.T) { + bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") + do, err := GetDivergingCommits(context.Background(), bareRepo1Path, "master", "branch2") + assert.NoError(t, err) + assert.Equal(t, DivergeObject{ + Ahead: 1, + Behind: 5, + }, do) + + do, err = GetDivergingCommits(context.Background(), bareRepo1Path, "master", "master") + assert.NoError(t, err) + assert.Equal(t, DivergeObject{ + Ahead: 0, + Behind: 0, + }, do) + + do, err = GetDivergingCommits(context.Background(), bareRepo1Path, "master", "test") + assert.NoError(t, err) + assert.Equal(t, DivergeObject{ + Ahead: 0, + Behind: 2, + }, do) +}