Skip to content

Commit b5406ec

Browse files
committed
Make gogit Repository.GetBranchNames consistent
nogogit GetBranchNames() lists branches sorted in reverse commit date order. On the other hand the gogit implementation doesn't apply any ordering resulting in unpredictable behaviour. In my case, the unit tests requiring particular order fail repo_branch_test.go:24: Error Trace: ./gitea/modules/git/repo_branch_test.go:24 Error: elements differ extra elements in list A: ([]interface {}) (len=1) { (string) (len=6) "master" } extra elements in list B: ([]interface {}) (len=1) { (string) (len=7) "branch1" } listA: ([]string) (len=2) { (string) (len=6) "master", (string) (len=7) "branch2" } listB: ([]string) (len=2) { (string) (len=7) "branch1", (string) (len=7) "branch2" } Test: TestRepository_GetBranches To fix this, we sort branches based on their commit date in gogit implementation. Fixes: go-gitea#28318
1 parent ec1feed commit b5406ec

File tree

1 file changed

+28
-13
lines changed

1 file changed

+28
-13
lines changed

modules/git/repo_branch_gogit.go

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package git
88

99
import (
1010
"context"
11+
"sort"
1112
"strings"
1213

1314
"github.com/go-git/go-git/v5/plumbing"
@@ -52,32 +53,46 @@ func (repo *Repository) IsBranchExist(name string) bool {
5253

5354
// GetBranches returns branches from the repository, skipping "skip" initial branches and
5455
// returning at most "limit" branches, or all branches if "limit" is 0.
56+
// Branches are returned with sort of `-commiterdate` as the nogogit
57+
// implementation. This requires full fetch, sort and then the
58+
// skip/limit applies later as gogit returns in undefined order.
5559
func (repo *Repository) GetBranchNames(skip, limit int) ([]string, int, error) {
56-
var branchNames []string
60+
type BranchData struct {
61+
name string
62+
committerDate int64
63+
}
64+
var branchData []BranchData
5765

58-
branches, err := repo.gogitRepo.Branches()
66+
branchIter, err := repo.gogitRepo.Branches()
5967
if err != nil {
6068
return nil, 0, err
6169
}
6270

63-
i := 0
64-
count := 0
65-
_ = branches.ForEach(func(branch *plumbing.Reference) error {
66-
count++
67-
if i < skip {
68-
i++
69-
return nil
70-
} else if limit != 0 && count > skip+limit {
71+
_ = branchIter.ForEach(func(branch *plumbing.Reference) error {
72+
obj, err := repo.gogitRepo.CommitObject(branch.Hash())
73+
if err != nil {
74+
// skip branch if can't find commit
7175
return nil
7276
}
7377

74-
branchNames = append(branchNames, strings.TrimPrefix(branch.Name().String(), BranchPrefix))
78+
branchData = append(branchData, BranchData{strings.TrimPrefix(branch.Name().String(), BranchPrefix), obj.Committer.When.Unix()})
7579
return nil
7680
})
7781

78-
// TODO: Sort?
82+
sort.Slice(branchData, func(i, j int) bool {
83+
return !(branchData[i].committerDate < branchData[j].committerDate)
84+
})
85+
86+
var branchNames []string
87+
maxPos := len(branchData)
88+
if limit > 0 {
89+
maxPos = min(skip+limit, maxPos)
90+
}
91+
for i := skip; i < maxPos; i++ {
92+
branchNames = append(branchNames, branchData[i].name)
93+
}
7994

80-
return branchNames, count, nil
95+
return branchNames, len(branchData), nil
8196
}
8297

8398
// WalkReferences walks all the references from the repository

0 commit comments

Comments
 (0)