From d8ad9228caf88cbae5ddb339d8e6583dcf82452a Mon Sep 17 00:00:00 2001 From: Giteabot Date: Wed, 4 Dec 2024 15:59:48 +0800 Subject: [PATCH] Fix gogit `GetRefCommitID` (#32705) (#32712) Backport #32705 by @Zettat123 Fix #32335 When we call `GetRefCommitID` and the reference is already a commit ID, the `GetRefCommitID` with go-git will return a `NotExist` error. This PR improves the `GetRefCommitID` for go-git. If the input is already a commit ID, it will be returned directly. Co-authored-by: Zettat123 --- modules/git/repo_commit_gogit.go | 11 +++++++++-- modules/git/repo_commit_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/modules/git/repo_commit_gogit.go b/modules/git/repo_commit_gogit.go index 84580be9a5..993013eef7 100644 --- a/modules/git/repo_commit_gogit.go +++ b/modules/git/repo_commit_gogit.go @@ -14,9 +14,16 @@ import ( "github.com/go-git/go-git/v5/plumbing/object" ) -// GetRefCommitID returns the last commit ID string of given reference (branch or tag). +// GetRefCommitID returns the last commit ID string of given reference. func (repo *Repository) GetRefCommitID(name string) (string, error) { - ref, err := repo.gogitRepo.Reference(plumbing.ReferenceName(name), true) + if plumbing.IsHash(name) { + return name, nil + } + refName := plumbing.ReferenceName(name) + if err := refName.Validate(); err != nil { + return "", err + } + ref, err := repo.gogitRepo.Reference(refName, true) if err != nil { if err == plumbing.ErrReferenceNotFound { return "", ErrNotExist{ diff --git a/modules/git/repo_commit_test.go b/modules/git/repo_commit_test.go index 19983b47b1..4c26fa2a48 100644 --- a/modules/git/repo_commit_test.go +++ b/modules/git/repo_commit_test.go @@ -101,3 +101,28 @@ func TestRepository_CommitsBetweenIDs(t *testing.T) { assert.Len(t, commits, c.ExpectedCommits, "case %d", i) } } + +func TestGetRefCommitID(t *testing.T) { + bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") + bareRepo1, err := openRepositoryWithDefaultContext(bareRepo1Path) + assert.NoError(t, err) + defer bareRepo1.Close() + + // these test case are specific to the repo1_bare test repo + testCases := []struct { + Ref string + ExpectedCommitID string + }{ + {RefNameFromBranch("master").String(), "ce064814f4a0d337b333e646ece456cd39fab612"}, + {RefNameFromBranch("branch1").String(), "2839944139e0de9737a044f78b0e4b40d989a9e3"}, + {RefNameFromTag("test").String(), "3ad28a9149a2864384548f3d17ed7f38014c9e8a"}, + {"ce064814f4a0d337b333e646ece456cd39fab612", "ce064814f4a0d337b333e646ece456cd39fab612"}, + } + + for _, testCase := range testCases { + commitID, err := bareRepo1.GetRefCommitID(testCase.Ref) + if assert.NoError(t, err) { + assert.Equal(t, testCase.ExpectedCommitID, commitID) + } + } +}