From b5406ec3e0b009efb8599dcef6a695cd0cbc3b89 Mon Sep 17 00:00:00 2001 From: Adam Majer Date: Mon, 4 Dec 2023 17:31:54 +0100 Subject: [PATCH] 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: #28318 --- modules/git/repo_branch_gogit.go | 41 ++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/modules/git/repo_branch_gogit.go b/modules/git/repo_branch_gogit.go index f9896a7a09047..1c0d9a18aa9d9 100644 --- a/modules/git/repo_branch_gogit.go +++ b/modules/git/repo_branch_gogit.go @@ -8,6 +8,7 @@ package git import ( "context" + "sort" "strings" "github.com/go-git/go-git/v5/plumbing" @@ -52,32 +53,46 @@ 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. +// Branches are returned with sort of `-commiterdate` as the nogogit +// implementation. This requires full fetch, sort and then the +// skip/limit applies later as gogit returns in undefined order. func (repo *Repository) GetBranchNames(skip, limit int) ([]string, int, error) { - var branchNames []string + type BranchData struct { + name string + committerDate int64 + } + var branchData []BranchData - branches, err := repo.gogitRepo.Branches() + branchIter, err := repo.gogitRepo.Branches() if err != nil { return nil, 0, err } - i := 0 - count := 0 - _ = branches.ForEach(func(branch *plumbing.Reference) error { - count++ - if i < skip { - i++ - return nil - } else if limit != 0 && count > skip+limit { + _ = branchIter.ForEach(func(branch *plumbing.Reference) error { + obj, err := repo.gogitRepo.CommitObject(branch.Hash()) + if err != nil { + // skip branch if can't find commit return nil } - branchNames = append(branchNames, strings.TrimPrefix(branch.Name().String(), BranchPrefix)) + branchData = append(branchData, BranchData{strings.TrimPrefix(branch.Name().String(), BranchPrefix), obj.Committer.When.Unix()}) return nil }) - // TODO: Sort? + sort.Slice(branchData, func(i, j int) bool { + return !(branchData[i].committerDate < branchData[j].committerDate) + }) + + var branchNames []string + maxPos := len(branchData) + if limit > 0 { + maxPos = min(skip+limit, maxPos) + } + for i := skip; i < maxPos; i++ { + branchNames = append(branchNames, branchData[i].name) + } - return branchNames, count, nil + return branchNames, len(branchData), nil } // WalkReferences walks all the references from the repository