Skip to content

Commit f57eba0

Browse files
zeripathzeripath
committed
Improve checkBranchName
1 parent 6c9c38a commit f57eba0

File tree

3 files changed

+75
-42
lines changed

3 files changed

+75
-42
lines changed

‎modules/git/repo_branch_gogit.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
package git
1010

1111
import (
12+
"context"
1213
"strings"
1314

1415
"github.com/go-git/go-git/v5/plumbing"
@@ -79,3 +80,26 @@ func (repo *Repository) GetBranches(skip, limit int) ([]string, int, error) {
7980

8081
return branchNames, count, nil
8182
}
83+
84+
// WalkReferences walks all the references from the repository
85+
func WalkReferences(ctx context.Context, repoPath string, walkfn func(string) error) (int, error) {
86+
repo, err := OpenRepositoryCtx(ctx, repoPath)
87+
if err != nil {
88+
return 0, err
89+
}
90+
defer repo.Close()
91+
92+
i := 0
93+
iter, err := repo.gogitRepo.References()
94+
if err != nil {
95+
return i, err
96+
}
97+
defer iter.Close()
98+
99+
err = iter.ForEach(func(ref *plumbing.Reference) error {
100+
err := walkfn(string(ref.Name()))
101+
i++
102+
return err
103+
})
104+
return i, err
105+
}

‎modules/git/repo_branch_nogogit.go

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,26 @@ func (repo *Repository) GetBranches(skip, limit int) ([]string, int, error) {
6767
return callShowRef(repo.Ctx, repo.Path, BranchPrefix, "--heads", skip, limit)
6868
}
6969

70+
// WalkReferences walks all the references from the repository
71+
func WalkReferences(ctx context.Context, repoPath string, walkfn func(string) error) (int, error) {
72+
return walkShowRef(ctx, repoPath, "", 0, 0, walkfn)
73+
}
74+
7075
// callShowRef return refs, if limit = 0 it will not limit
7176
func callShowRef(ctx context.Context, repoPath, prefix, arg string, skip, limit int) (branchNames []string, countAll int, err error) {
77+
countAll, err = walkShowRef(ctx, repoPath, arg, limit, skip, func(branchName string) error {
78+
branchName = strings.TrimPrefix(branchName, prefix)
79+
if len(branchName) > 0 {
80+
branchName = branchName[:len(branchName)-1]
81+
}
82+
branchNames = append(branchNames, branchName)
83+
84+
return nil
85+
})
86+
return
87+
}
88+
89+
func walkShowRef(ctx context.Context, repoPath, arg string, skip, limit int, walkfn func(string) error) (countAll int, err error) {
7290
stdoutReader, stdoutWriter := io.Pipe()
7391
defer func() {
7492
_ = stdoutReader.Close()
@@ -77,7 +95,11 @@ func callShowRef(ctx context.Context, repoPath, prefix, arg string, skip, limit
7795

7896
go func() {
7997
stderrBuilder := &strings.Builder{}
80-
err := NewCommandContext(ctx, "show-ref", arg).RunInDirPipeline(repoPath, stdoutWriter, stderrBuilder)
98+
args := []string{"show-ref"}
99+
if arg != "" {
100+
args = append(args, arg)
101+
}
102+
err := NewCommandContext(ctx, args...).RunInDirPipeline(repoPath, stdoutWriter, stderrBuilder)
81103
if err != nil {
82104
if stderrBuilder.Len() == 0 {
83105
_ = stdoutWriter.Close()
@@ -94,10 +116,10 @@ func callShowRef(ctx context.Context, repoPath, prefix, arg string, skip, limit
94116
for i < skip {
95117
_, isPrefix, err := bufReader.ReadLine()
96118
if err == io.EOF {
97-
return branchNames, i, nil
119+
return i, nil
98120
}
99121
if err != nil {
100-
return nil, 0, err
122+
return 0, err
101123
}
102124
if !isPrefix {
103125
i++
@@ -112,39 +134,39 @@ func callShowRef(ctx context.Context, repoPath, prefix, arg string, skip, limit
112134
_, err = bufReader.ReadSlice(' ')
113135
}
114136
if err == io.EOF {
115-
return branchNames, i, nil
137+
return i, nil
116138
}
117139
if err != nil {
118-
return nil, 0, err
140+
return 0, err
119141
}
120142

121143
branchName, err := bufReader.ReadString('\n')
122144
if err == io.EOF {
123145
// This shouldn't happen... but we'll tolerate it for the sake of peace
124-
return branchNames, i, nil
146+
return i, nil
125147
}
126148
if err != nil {
127-
return nil, i, err
149+
return i, err
128150
}
129-
branchName = strings.TrimPrefix(branchName, prefix)
130-
if len(branchName) > 0 {
131-
branchName = branchName[:len(branchName)-1]
151+
152+
err = walkfn(branchName)
153+
if err != nil {
154+
return i, err
132155
}
133-
branchNames = append(branchNames, branchName)
134156
i++
135157
}
136158
// count all refs
137159
for limit != 0 {
138160
_, isPrefix, err := bufReader.ReadLine()
139161
if err == io.EOF {
140-
return branchNames, i, nil
162+
return i, nil
141163
}
142164
if err != nil {
143-
return nil, 0, err
165+
return 0, err
144166
}
145167
if !isPrefix {
146168
i++
147169
}
148170
}
149-
return branchNames, i, nil
171+
return i, nil
150172
}

‎services/repository/branch.go

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"context"
99
"errors"
1010
"fmt"
11+
"strings"
1112

1213
"code.gitea.io/gitea/models"
1314
user_model "code.gitea.io/gitea/models/user"
@@ -66,40 +67,26 @@ func GetBranches(ctx context.Context, repo *models.Repository, skip, limit int)
6667
}
6768

6869
// checkBranchName validates branch name with existing repository branches
69-
// FIXME: There really has to be faster mechanism than this. git cat-file --batch-check will check a branchname
70-
// and or tag name... (git.DefaultContext - as a marker to come back to this)
7170
func checkBranchName(ctx context.Context, repo *models.Repository, name string) error {
72-
gitRepo, err := git.OpenRepositoryCtx(ctx, repo.RepoPath())
73-
if err != nil {
74-
return err
75-
}
76-
defer gitRepo.Close()
77-
78-
branches, _, err := gitRepo.GetBranches(0, 0)
79-
if err != nil {
80-
return err
81-
}
82-
83-
for _, branch := range branches {
84-
if branch == name {
71+
_, err := git.WalkReferences(ctx, repo.RepoPath(), func(refName string) error {
72+
switch {
73+
case refName == git.BranchPrefix+name:
8574
return models.ErrBranchAlreadyExists{
86-
BranchName: branch,
75+
BranchName: name,
8776
}
88-
} else if (len(branch) < len(name) && branch+"/" == name[0:len(branch)+1]) ||
89-
(len(branch) > len(name) && name+"/" == branch[0:len(name)+1]) {
77+
case strings.HasPrefix(refName, git.BranchPrefix+name+"/"):
9078
return models.ErrBranchNameConflict{
91-
BranchName: branch,
79+
BranchName: name,
80+
}
81+
case refName == git.TagPrefix+name:
82+
return models.ErrTagAlreadyExists{
83+
TagName: name,
9284
}
9385
}
94-
}
95-
96-
if _, err := gitRepo.GetTag(name); err == nil {
97-
return models.ErrTagAlreadyExists{
98-
TagName: name,
99-
}
100-
}
86+
return nil
87+
})
10188

102-
return nil
89+
return err
10390
}
10491

10592
// CreateNewBranchFromCommit creates a new repository branch
@@ -109,7 +96,7 @@ func CreateNewBranchFromCommit(ctx context.Context, doer *user_model.User, repo
10996
return err
11097
}
11198

112-
if err := git.Push(git.DefaultContext, repo.RepoPath(), git.PushOptions{
99+
if err := git.Push(ctx, repo.RepoPath(), git.PushOptions{
113100
Remote: repo.RepoPath(),
114101
Branch: fmt.Sprintf("%s:%s%s", commit, git.BranchPrefix, branchName),
115102
Env: models.PushingEnvironment(doer, repo),

0 commit comments

Comments
 (0)