aboutsummaryrefslogtreecommitdiffstats
path: root/modules/git
diff options
context:
space:
mode:
authorzeripath <art27@cantab.net>2021-12-08 19:08:16 +0000
committerGitHub <noreply@github.com>2021-12-08 19:08:16 +0000
commit9e6e1dc950f06bbd000d5b6438f39113e8902082 (patch)
tree204f359885c2bda09603de7de90b93f61c8e6922 /modules/git
parentb59875aa123f2cc3a5026d30ac557e99c05603a6 (diff)
downloadgitea-9e6e1dc950f06bbd000d5b6438f39113e8902082.tar.gz
gitea-9e6e1dc950f06bbd000d5b6438f39113e8902082.zip
Improve checkBranchName (#17901)
The current implementation of checkBranchName is highly inefficient involving opening the repository, the listing all of the branch names checking them individually before then using using opened repo to get the tags. This PR avoids this by simply walking the references from show-ref instead of opening the repository (in the nogogit case). Signed-off-by: Andrew Thornton <art27@cantab.net>
Diffstat (limited to 'modules/git')
-rw-r--r--modules/git/repo_branch.go11
-rw-r--r--modules/git/repo_branch_gogit.go26
-rw-r--r--modules/git/repo_branch_nogogit.go50
-rw-r--r--modules/git/repo_branch_test.go8
4 files changed, 73 insertions, 22 deletions
diff --git a/modules/git/repo_branch.go b/modules/git/repo_branch.go
index 98b1bc8ae7..01933d7ade 100644
--- a/modules/git/repo_branch.go
+++ b/modules/git/repo_branch.go
@@ -95,7 +95,12 @@ func GetBranchesByPath(path string, skip, limit int) ([]*Branch, int, error) {
}
defer gitRepo.Close()
- brs, countAll, err := gitRepo.GetBranches(skip, limit)
+ return gitRepo.GetBranches(skip, limit)
+}
+
+// GetBranches returns a slice of *git.Branch
+func (repo *Repository) GetBranches(skip, limit int) ([]*Branch, int, error) {
+ brs, countAll, err := repo.GetBranchNames(skip, limit)
if err != nil {
return nil, 0, err
}
@@ -103,9 +108,9 @@ func GetBranchesByPath(path string, skip, limit int) ([]*Branch, int, error) {
branches := make([]*Branch, len(brs))
for i := range brs {
branches[i] = &Branch{
- Path: path,
+ Path: repo.Path,
Name: brs[i],
- gitRepo: gitRepo,
+ gitRepo: repo,
}
}
diff --git a/modules/git/repo_branch_gogit.go b/modules/git/repo_branch_gogit.go
index 6bf14b3999..d159aafd6f 100644
--- a/modules/git/repo_branch_gogit.go
+++ b/modules/git/repo_branch_gogit.go
@@ -9,6 +9,7 @@
package git
import (
+ "context"
"strings"
"github.com/go-git/go-git/v5/plumbing"
@@ -52,7 +53,7 @@ func (repo *Repository) IsBranchExist(name string) bool {
// GetBranches returns branches from the repository, skipping skip initial branches and
// returning at most limit branches, or all branches if limit is 0.
-func (repo *Repository) GetBranches(skip, limit int) ([]string, int, error) {
+func (repo *Repository) GetBranchNames(skip, limit int) ([]string, int, error) {
var branchNames []string
branches, err := repo.gogitRepo.Branches()
@@ -79,3 +80,26 @@ func (repo *Repository) GetBranches(skip, limit int) ([]string, int, error) {
return branchNames, count, nil
}
+
+// WalkReferences walks all the references from the repository
+func WalkReferences(ctx context.Context, repoPath string, walkfn func(string) error) (int, error) {
+ repo, err := OpenRepositoryCtx(ctx, repoPath)
+ if err != nil {
+ return 0, err
+ }
+ defer repo.Close()
+
+ i := 0
+ iter, err := repo.gogitRepo.References()
+ if err != nil {
+ return i, err
+ }
+ defer iter.Close()
+
+ err = iter.ForEach(func(ref *plumbing.Reference) error {
+ err := walkfn(string(ref.Name()))
+ i++
+ return err
+ })
+ return i, err
+}
diff --git a/modules/git/repo_branch_nogogit.go b/modules/git/repo_branch_nogogit.go
index 1928c7515b..55952acda4 100644
--- a/modules/git/repo_branch_nogogit.go
+++ b/modules/git/repo_branch_nogogit.go
@@ -61,14 +61,29 @@ func (repo *Repository) IsBranchExist(name string) bool {
return repo.IsReferenceExist(BranchPrefix + name)
}
-// GetBranches returns branches from the repository, skipping skip initial branches and
+// GetBranchNames returns branches from the repository, skipping skip initial branches and
// returning at most limit branches, or all branches if limit is 0.
-func (repo *Repository) GetBranches(skip, limit int) ([]string, int, error) {
+func (repo *Repository) GetBranchNames(skip, limit int) ([]string, int, error) {
return callShowRef(repo.Ctx, repo.Path, BranchPrefix, "--heads", skip, limit)
}
+// WalkReferences walks all the references from the repository
+func WalkReferences(ctx context.Context, repoPath string, walkfn func(string) error) (int, error) {
+ return walkShowRef(ctx, repoPath, "", 0, 0, walkfn)
+}
+
// callShowRef return refs, if limit = 0 it will not limit
func callShowRef(ctx context.Context, repoPath, prefix, arg string, skip, limit int) (branchNames []string, countAll int, err error) {
+ countAll, err = walkShowRef(ctx, repoPath, arg, skip, limit, func(branchName string) error {
+ branchName = strings.TrimPrefix(branchName, prefix)
+ branchNames = append(branchNames, branchName)
+
+ return nil
+ })
+ return
+}
+
+func walkShowRef(ctx context.Context, repoPath, arg string, skip, limit int, walkfn func(string) error) (countAll int, err error) {
stdoutReader, stdoutWriter := io.Pipe()
defer func() {
_ = stdoutReader.Close()
@@ -77,7 +92,11 @@ func callShowRef(ctx context.Context, repoPath, prefix, arg string, skip, limit
go func() {
stderrBuilder := &strings.Builder{}
- err := NewCommandContext(ctx, "show-ref", arg).RunInDirPipeline(repoPath, stdoutWriter, stderrBuilder)
+ args := []string{"show-ref"}
+ if arg != "" {
+ args = append(args, arg)
+ }
+ err := NewCommandContext(ctx, args...).RunInDirPipeline(repoPath, stdoutWriter, stderrBuilder)
if err != nil {
if stderrBuilder.Len() == 0 {
_ = stdoutWriter.Close()
@@ -94,10 +113,10 @@ func callShowRef(ctx context.Context, repoPath, prefix, arg string, skip, limit
for i < skip {
_, isPrefix, err := bufReader.ReadLine()
if err == io.EOF {
- return branchNames, i, nil
+ return i, nil
}
if err != nil {
- return nil, 0, err
+ return 0, err
}
if !isPrefix {
i++
@@ -112,39 +131,42 @@ func callShowRef(ctx context.Context, repoPath, prefix, arg string, skip, limit
_, err = bufReader.ReadSlice(' ')
}
if err == io.EOF {
- return branchNames, i, nil
+ return i, nil
}
if err != nil {
- return nil, 0, err
+ return 0, err
}
branchName, err := bufReader.ReadString('\n')
if err == io.EOF {
// This shouldn't happen... but we'll tolerate it for the sake of peace
- return branchNames, i, nil
+ return i, nil
}
if err != nil {
- return nil, i, err
+ return i, err
}
- branchName = strings.TrimPrefix(branchName, prefix)
+
if len(branchName) > 0 {
branchName = branchName[:len(branchName)-1]
}
- branchNames = append(branchNames, branchName)
+ err = walkfn(branchName)
+ if err != nil {
+ return i, err
+ }
i++
}
// count all refs
for limit != 0 {
_, isPrefix, err := bufReader.ReadLine()
if err == io.EOF {
- return branchNames, i, nil
+ return i, nil
}
if err != nil {
- return nil, 0, err
+ return 0, err
}
if !isPrefix {
i++
}
}
- return branchNames, i, nil
+ return i, nil
}
diff --git a/modules/git/repo_branch_test.go b/modules/git/repo_branch_test.go
index 05d5237e6a..ac5f5deea9 100644
--- a/modules/git/repo_branch_test.go
+++ b/modules/git/repo_branch_test.go
@@ -17,21 +17,21 @@ func TestRepository_GetBranches(t *testing.T) {
assert.NoError(t, err)
defer bareRepo1.Close()
- branches, countAll, err := bareRepo1.GetBranches(0, 2)
+ branches, countAll, err := bareRepo1.GetBranchNames(0, 2)
assert.NoError(t, err)
assert.Len(t, branches, 2)
assert.EqualValues(t, 3, countAll)
assert.ElementsMatch(t, []string{"branch1", "branch2"}, branches)
- branches, countAll, err = bareRepo1.GetBranches(0, 0)
+ branches, countAll, err = bareRepo1.GetBranchNames(0, 0)
assert.NoError(t, err)
assert.Len(t, branches, 3)
assert.EqualValues(t, 3, countAll)
assert.ElementsMatch(t, []string{"branch1", "branch2", "master"}, branches)
- branches, countAll, err = bareRepo1.GetBranches(5, 1)
+ branches, countAll, err = bareRepo1.GetBranchNames(5, 1)
assert.NoError(t, err)
assert.Len(t, branches, 0)
@@ -48,7 +48,7 @@ func BenchmarkRepository_GetBranches(b *testing.B) {
defer bareRepo1.Close()
for i := 0; i < b.N; i++ {
- _, _, err := bareRepo1.GetBranches(0, 0)
+ _, _, err := bareRepo1.GetBranchNames(0, 0)
if err != nil {
b.Fatal(err)
}