chore: fix git commit "rev-list" (#38069)

Fix the copied & pasted messy code, fix #38067

Now, "limit=-1" means "no limit"

modules/git/commit.go

@@ -130,13 +130,9 @@ func (c *Commit) CommitsBeforeLimit(num int) ([]*Commit, error) {
 	return c.repo.getCommitsBeforeLimit(c.ID, num)
 }
 
-// CommitsBeforeUntil returns the commits between commitID to current revision
-func (c *Commit) CommitsBeforeUntil(commitID string) ([]*Commit, error) {
-	endCommit, err := c.repo.GetCommit(commitID)
-	if err != nil {
-		return nil, err
-	}
-	return c.repo.CommitsBetween(c, endCommit)
+// CommitsBeforeUntil returns the commits in range "[cur, ref)"
+func (c *Commit) CommitsBeforeUntil(ref RefName) ([]*Commit, error) {
+	return c.repo.CommitsBetween(c.ID.RefName(), ref, -1)
 }
 
 // SearchCommitsOptions specify the parameters for SearchCommits
@@ -257,11 +253,15 @@ func IsStringLikelyCommitID(objFmt ObjectFormat, s string, minLength ...int) boo
 	if len(s) < minLen || len(s) > maxLen {
 		return false
 	}
+	return isStringLowerHex(s)
+}
+
+func isStringLowerHex(s string) bool {
 	for _, c := range s {
 		isHex := (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f')
 		if !isHex {
 			return false
 		}
 	}
-	return true
+	return len(s) > 0 // it accepts odd length because "shorten commit id" can be 7-chars
 }

modules/git/commit_test.go

@@ -199,3 +199,10 @@ func Test_GetCommitBranchStart(t *testing.T) {
 	assert.NotEmpty(t, startCommitID)
 	assert.Equal(t, "95bb4d39648ee7e325106df01a621c530863a653", startCommitID)
 }
+
+func TestIsStringLikelyCommitID(t *testing.T) {
+	assert.True(t, IsStringLikelyCommitID(nil, "abc", 3))
+	assert.False(t, IsStringLikelyCommitID(nil, "abc", 4))
+	assert.True(t, IsStringLikelyCommitID(nil, strings.Repeat("a", 64), 4))
+	assert.False(t, IsStringLikelyCommitID(nil, strings.Repeat("a", 65), 4))
+}

modules/git/gitcmd/error.go

@@ -10,6 +10,7 @@ import (
 	"os/exec"
 	"strings"
 
+	"gitea.dev/modules/setting"
 	"gitea.dev/modules/util"
 )
 
@@ -69,9 +70,10 @@ func IsErrorCanceledOrKilled(err error) bool {
 	return errors.Is(err, context.Canceled) || IsErrorSignalKilled(err)
 }
 
-type StderrPrefix string
-
-type StderrSubStr string
+type (
+	StderrPrefix   string
+	StderrWildcard string
+)
 
 const (
 	StderrNotValidObjectName StderrPrefix = "fatal: not a valid object name"
@@ -82,11 +84,11 @@ const (
 	StderrNoSuchRemote1 StderrPrefix = "fatal: no such remote" // git < 2.30, exit status 128
 	StderrNoSuchRemote2 StderrPrefix = "error: no such remote" // git >= 2.30. exit status 2
 
-	// fatal: ambiguous argument 'origin': unknown revision or path not in the working tree.
-	StderrUnknownRevisionOrPath StderrSubStr = "unknown revision or path not in the working tree"
+	StderrUnknownRevisionOrPath StderrWildcard = "fatal: *: unknown revision or path not in the working tree"
+	StderrNoMergeBase           StderrWildcard = "fatal: *: no merge base"
 )
 
-func IsStderr[T StderrPrefix | StderrSubStr](err error, check T) bool {
+func IsStderr[T StderrPrefix | StderrWildcard](err error, check T) bool {
 	stderr, ok := ErrorAsStderr(err)
 	if !ok {
 		return false
@@ -100,9 +102,11 @@ func IsStderr[T StderrPrefix | StderrSubStr](err error, check T) bool {
 		// Git is lowercasing the "fatal: Not a valid object name" error message
 		// ref: https://lore.kernel.org/git/pull.2052.git.1771836302101.gitgitgadget@gmail.com
 		return util.AsciiEqualFold(stderr[:checkLen], string(check))
-	case StderrSubStr:
-		return strings.Contains(stderr, string(check))
+	case StderrWildcard:
+		prefix, remaining, _ := strings.Cut(string(check), "*")
+		return strings.HasPrefix(stderr, prefix) && strings.Contains(stderr, remaining)
 	}
+	setting.PanicInDevOrTesting("invalid stderr type %T", check)
 	return false
 }
 
@@ -122,8 +126,7 @@ func wrapPipelineError(err error) error {
 }
 
 func UnwrapPipelineError(err error) (error, bool) { //nolint:revive // this is for error unwrapping
-	var pe pipelineError
-	if errors.As(err, &pe) {
+	if pe, ok := errors.AsType[pipelineError](err); ok {
 		return pe.error, true
 	}
 	return nil, false

modules/git/gitcmd/error_test.go

@@ -0,0 +1,23 @@
+// Copyright 2026 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package gitcmd
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsStderr(t *testing.T) {
+	cases := []struct {
+		check  StderrWildcard
+		stderr string
+	}{
+		{StderrUnknownRevisionOrPath, "fatal: ambiguous argument 'origin': unknown revision or path not in the working tree...."},
+		{StderrNoMergeBase, "fatal: origin/main..HEAD: no merge base...."},
+	}
+	for _, tc := range cases {
+		assert.True(t, IsStderr(&runStdError{stderr: tc.stderr}, tc.check), "stderr: %s", tc.stderr)
+	}
+}

modules/git/object_id.go

@@ -11,6 +11,7 @@ import (
 
 type ObjectID interface {
 	String() string
+	RefName() RefName
 	IsZero() bool
 	RawValue() []byte
 	Type() ObjectFormat
@@ -18,10 +19,16 @@ type ObjectID interface {
 
 type Sha1Hash [20]byte
 
+var _ ObjectID = (*Sha1Hash)(nil)
+
 func (h *Sha1Hash) String() string {
 	return hex.EncodeToString(h[:])
 }
 
+func (h *Sha1Hash) RefName() RefName {
+	return RefName(h.String())
+}
+
 func (h *Sha1Hash) IsZero() bool {
 	empty := Sha1Hash{}
 	return bytes.Equal(empty[:], h[:])
@@ -29,8 +36,6 @@ func (h *Sha1Hash) IsZero() bool {
 func (h *Sha1Hash) RawValue() []byte { return h[:] }
 func (*Sha1Hash) Type() ObjectFormat { return Sha1ObjectFormat }
 
-var _ ObjectID = &Sha1Hash{}
-
 func MustIDFromString(hexHash string) ObjectID {
 	id, err := NewIDFromString(hexHash)
 	if err != nil {
@@ -41,10 +46,16 @@ func MustIDFromString(hexHash string) ObjectID {
 
 type Sha256Hash [32]byte
 
+var _ ObjectID = (*Sha256Hash)(nil)
+
 func (h *Sha256Hash) String() string {
 	return hex.EncodeToString(h[:])
 }
 
+func (h *Sha256Hash) RefName() RefName {
+	return RefName(h.String())
+}
+
 func (h *Sha256Hash) IsZero() bool {
 	empty := Sha256Hash{}
 	return bytes.Equal(empty[:], h[:])
@@ -93,11 +104,3 @@ func IsEmptyCommitID(commitID string) bool {
 func ComputeBlobHash(hashType ObjectFormat, content []byte) ObjectID {
 	return hashType.ComputeHash(ObjectBlob, content)
 }
-
-type ErrInvalidSHA struct {
-	SHA string
-}
-
-func (err ErrInvalidSHA) Error() string {
-	return "invalid sha: " + err.SHA
-}

modules/git/ref.go

@@ -7,6 +7,7 @@ import (
 	"regexp"
 	"strings"
 
+	"gitea.dev/modules/setting"
 	"gitea.dev/modules/util"
 )
 
@@ -72,6 +73,8 @@ const ForPrefix = "refs/for/"
 // RefName represents a full git reference name
 type RefName string
 
+const RefNameHead = "HEAD"
+
 func RefNameFromBranch(shortName string) RefName {
 	return RefName(BranchPrefix + shortName)
 }
@@ -81,6 +84,10 @@ func RefNameFromTag(shortName string) RefName {
 }
 
 func RefNameFromCommit(shortName string) RefName {
+	if !isStringLowerHex(shortName) {
+		setting.PanicInDevOrTesting("BUG! invalid commit id %s", shortName)
+		return RefName("refs/invalid-commit/" + shortName)
+	}
 	return RefName(shortName)
 }
 

modules/git/repo_commit.go

@@ -287,48 +287,29 @@ func (repo *Repository) CommitsByFileAndRange(opts CommitsByFileAndRangeOptions)
 	return commits, hasMore, err
 }
 
-// FilesCountBetween return the number of files changed between two commits
-func (repo *Repository) FilesCountBetween(startCommitID, endCommitID string) (int, error) {
-	stdout, _, err := gitcmd.NewCommand("diff", "--name-only").
-		AddDynamicArguments(startCommitID + "..." + endCommitID).
-		WithDir(repo.Path).
-		RunStdString(repo.Ctx)
-	if err != nil && strings.Contains(err.Error(), "no merge base") {
-		// git >= 2.28 now returns an error if startCommitID and endCommitID have become unrelated.
-		// previously it would return the results of git diff --name-only startCommitID endCommitID so let's try that...
-		stdout, _, err = gitcmd.NewCommand("diff", "--name-only").
-			AddDynamicArguments(startCommitID, endCommitID).
-			WithDir(repo.Path).
-			RunStdString(repo.Ctx)
+// CommitsBetween returns a list that contains commits between [after, before). After is the first item in the slice.
+// If "before" and "after" are not related, it returns the all commits for the "after" commit.
+func (repo *Repository) CommitsBetween(afterRef, beforeRef RefName, limit int, optSkip ...int) ([]*Commit, error) {
+	gitCmd := func() *gitcmd.Command {
+		cmd := gitcmd.NewCommand("rev-list").WithDir(repo.Path)
+		if limit >= 0 {
+			cmd.AddOptionValues("--max-count", strconv.Itoa(limit))
+		}
+		if len(optSkip) > 0 {
+			cmd.AddOptionValues("--skip", strconv.Itoa(optSkip[0]))
+		}
+		return cmd
 	}
-	if err != nil {
-		return 0, err
-	}
-	return len(strings.Split(stdout, "\n")) - 1, nil
-}
-
-// CommitsBetween returns a list that contains commits between [before, last).
-// If before is detached (removed by reset + push) it is not included.
-func (repo *Repository) CommitsBetween(last, before *Commit) ([]*Commit, error) {
 	var stdout []byte
 	var err error
-	if before == nil {
-		stdout, _, err = gitcmd.NewCommand("rev-list").
-			AddDynamicArguments(last.ID.String()).
-			WithDir(repo.Path).
-			RunStdBytes(repo.Ctx)
+	if beforeRef == "" {
+		stdout, _, err = gitCmd().AddDynamicArguments(afterRef.String()).RunStdBytes(repo.Ctx)
 	} else {
-		stdout, _, err = gitcmd.NewCommand("rev-list").
-			AddDynamicArguments(before.ID.String() + ".." + last.ID.String()).
-			WithDir(repo.Path).
-			RunStdBytes(repo.Ctx)
-		if err != nil && strings.Contains(err.Error(), "no merge base") {
+		stdout, _, err = gitCmd().AddDynamicArguments(beforeRef.String() + ".." + afterRef.String()).RunStdBytes(repo.Ctx)
+		if gitcmd.IsStderr(err, gitcmd.StderrNoMergeBase) {
 			// future versions of git >= 2.28 are likely to return an error if before and last have become unrelated.
-			// previously it would return the results of git rev-list before last so let's try that...
-			stdout, _, err = gitcmd.NewCommand("rev-list").
-				AddDynamicArguments(before.ID.String(), last.ID.String()).
-				WithDir(repo.Path).
-				RunStdBytes(repo.Ctx)
+			// if the beforeRef and afterRef are not related (no merge base), just get all commits pushed by afterRef
+			stdout, _, err = gitCmd().AddDynamicArguments(afterRef.String()).RunStdBytes(repo.Ctx)
 		}
 	}
 	if err != nil {
@@ -337,57 +318,6 @@ func (repo *Repository) CommitsBetween(last, before *Commit) ([]*Commit, error)
 	return repo.parsePrettyFormatLogToList(bytes.TrimSpace(stdout))
 }
 
-// CommitsBetweenLimit returns a list that contains at most limit commits skipping the first skip commits between [before, last)
-func (repo *Repository) CommitsBetweenLimit(last, before *Commit, limit, skip int) ([]*Commit, error) {
-	var stdout []byte
-	var err error
-	if before == nil {
-		stdout, _, err = gitcmd.NewCommand("rev-list").
-			AddOptionValues("--max-count", strconv.Itoa(limit)).
-			AddOptionValues("--skip", strconv.Itoa(skip)).
-			AddDynamicArguments(last.ID.String()).
-			WithDir(repo.Path).
-			RunStdBytes(repo.Ctx)
-	} else {
-		stdout, _, err = gitcmd.NewCommand("rev-list").
-			AddOptionValues("--max-count", strconv.Itoa(limit)).
-			AddOptionValues("--skip", strconv.Itoa(skip)).
-			AddDynamicArguments(before.ID.String() + ".." + last.ID.String()).
-			WithDir(repo.Path).
-			RunStdBytes(repo.Ctx)
-		if err != nil && strings.Contains(err.Error(), "no merge base") {
-			// future versions of git >= 2.28 are likely to return an error if before and last have become unrelated.
-			// previously it would return the results of git rev-list --max-count n before last so let's try that...
-			stdout, _, err = gitcmd.NewCommand("rev-list").
-				AddOptionValues("--max-count", strconv.Itoa(limit)).
-				AddOptionValues("--skip", strconv.Itoa(skip)).
-				AddDynamicArguments(before.ID.String(), last.ID.String()).
-				WithDir(repo.Path).
-				RunStdBytes(repo.Ctx)
-		}
-	}
-	if err != nil {
-		return nil, err
-	}
-	return repo.parsePrettyFormatLogToList(bytes.TrimSpace(stdout))
-}
-
-// CommitsBetweenIDs return commits between twoe commits
-func (repo *Repository) CommitsBetweenIDs(last, before string) ([]*Commit, error) {
-	lastCommit, err := repo.GetCommit(last)
-	if err != nil {
-		return nil, err
-	}
-	if before == "" {
-		return repo.CommitsBetween(lastCommit, nil)
-	}
-	beforeCommit, err := repo.GetCommit(before)
-	if err != nil {
-		return nil, err
-	}
-	return repo.CommitsBetween(lastCommit, beforeCommit)
-}
-
 // commitsBefore the limit is depth, not total number of returned commits.
 func (repo *Repository) commitsBefore(id ObjectID, limit int) ([]*Commit, error) {
 	cmd := gitcmd.NewCommand("log", prettyLogFormat)

modules/git/repo_commit_test.go

@@ -85,15 +85,15 @@ func TestIsCommitInBranch(t *testing.T) {
 	assert.False(t, result)
 }
 
-func TestRepository_CommitsBetweenIDs(t *testing.T) {
+func TestRepository_CommitsBetween(t *testing.T) {
 	bareRepo1Path := filepath.Join(testReposDir, "repo4_commitsbetween")
 	bareRepo1, err := OpenRepository(t.Context(), bareRepo1Path)
 	assert.NoError(t, err)
 	defer bareRepo1.Close()
 
 	cases := []struct {
-		OldID           string
-		NewID           string
+		OldID           RefName
+		NewID           RefName
 		ExpectedCommits int
 	}{
 		{"fdc1b615bdcff0f0658b216df0c9209e5ecb7c78", "78a445db1eac62fe15e624e1137965969addf344", 1}, // com1 -> com2
@@ -101,7 +101,7 @@ func TestRepository_CommitsBetweenIDs(t *testing.T) {
 		{"78a445db1eac62fe15e624e1137965969addf344", "a78e5638b66ccfe7e1b4689d3d5684e42c97d7ca", 1}, // com2 -> com2_new
 	}
 	for i, c := range cases {
-		commits, err := bareRepo1.CommitsBetweenIDs(c.NewID, c.OldID)
+		commits, err := bareRepo1.CommitsBetween(c.NewID, c.OldID, -1)
 		assert.NoError(t, err)
 		assert.Len(t, commits, c.ExpectedCommits, "case %d", i)
 	}

modules/git/repo_compare.go

@@ -40,25 +40,16 @@ func (repo *Repository) GetDiffNumChangedFiles(base, head string, directComparis
 		separator = ".."
 	}
 
-	// avoid: ambiguous argument 'refs/a...refs/b': unknown revision or path not in the working tree. Use '--': 'git <command> [<revision>...] -- [<file>...]'
 	if err := gitcmd.NewCommand("diff", "-z", "--name-only").
 		AddDynamicArguments(base + separator + head).
 		AddArguments("--").
 		WithDir(repo.Path).
 		WithStdoutCopy(w).
 		RunWithStderr(repo.Ctx); err != nil {
-		if strings.Contains(err.Stderr(), "no merge base") {
+		if gitcmd.IsStderr(err, gitcmd.StderrNoMergeBase) {
 			// git >= 2.28 now returns an error if base and head have become unrelated.
-			// previously it would return the results of git diff -z --name-only base head so let's try that...
-			w = &lineCountWriter{}
-			if err = gitcmd.NewCommand("diff", "-z", "--name-only").
-				AddDynamicArguments(base, head).
-				AddArguments("--").
-				WithDir(repo.Path).
-				WithStdoutCopy(w).
-				RunWithStderr(repo.Ctx); err == nil {
-				return w.numLines, nil
-			}
+			// it doesn't make sense to count the changed files in this case because UI won't display such diff
+			return 0, nil
 		}
 		return 0, err
 	}

modules/gitrepo/compare.go

@@ -56,11 +56,10 @@ func GetCommitIDsBetweenReverse(ctx context.Context, repo Repository, startRef,
 		return cmd
 	}
 	stdout, _, err := RunCmdString(ctx, repo, genCmd(startRef+".."+endRef))
-	// example git error message: fatal: origin/main..HEAD: no merge base
-	if err != nil && strings.Contains(err.Stderr(), "no merge base") {
-		// future versions of git >= 2.28 are likely to return an error if before and last have become unrelated.
+	if gitcmd.IsStderr(err, gitcmd.StderrNoMergeBase) {
+		// if the start and end are not related (no merge base), just get all commits pushed by "end ref"
 		// previously it would return the results of git rev-list before last so let's try that...
-		stdout, _, err = RunCmdString(ctx, repo, genCmd(startRef, endRef))
+		stdout, _, err = RunCmdString(ctx, repo, genCmd(endRef))
 	}
 	if err != nil {
 		return nil, err

modules/repository/branch.go

@@ -20,8 +20,8 @@ import (
 // SyncResult describes a reference update detected during sync.
 type SyncResult struct {
 	RefName     git.RefName
-	OldCommitID string
-	NewCommitID string
+	OldCommitID git.RefName
+	NewCommitID git.RefName
 }
 
 // SyncRepoBranches synchronizes branch table with repository branches
@@ -104,7 +104,7 @@ func SyncRepoBranchesWithRepo(ctx context.Context, repo *repo_model.Repository,
 			syncResults = append(syncResults, &SyncResult{
 				RefName:     git.RefNameFromBranch(branch),
 				OldCommitID: "",
-				NewCommitID: commit.ID.String(),
+				NewCommitID: commit.ID.RefName(),
 			})
 		} else if commit.ID.String() != dbb.CommitID || dbb.IsDeleted {
 			toUpdate = append(toUpdate, &git_model.Branch{
@@ -118,8 +118,8 @@ func SyncRepoBranchesWithRepo(ctx context.Context, repo *repo_model.Repository,
 			})
 			syncResults = append(syncResults, &SyncResult{
 				RefName:     git.RefNameFromBranch(branch),
-				OldCommitID: dbb.CommitID,
-				NewCommitID: commit.ID.String(),
+				OldCommitID: git.RefNameFromCommit(dbb.CommitID),
+				NewCommitID: commit.ID.RefName(),
 			})
 		}
 	}
@@ -129,7 +129,7 @@ func SyncRepoBranchesWithRepo(ctx context.Context, repo *repo_model.Repository,
 			toRemove = append(toRemove, dbBranch.ID)
 			syncResults = append(syncResults, &SyncResult{
 				RefName:     git.RefNameFromBranch(dbBranch.Name),
-				OldCommitID: dbBranch.CommitID,
+				OldCommitID: git.RefNameFromCommit(dbBranch.CommitID),
 				NewCommitID: "",
 			})
 		}

modules/repository/repo.go

@@ -210,7 +210,7 @@ func SyncReleasesWithTags(ctx context.Context, repo *repo_model.Repository, gitR
 			syncResults = append(syncResults, &SyncResult{
 				RefName:     git.RefNameFromTag(tag.Name),
 				OldCommitID: "",
-				NewCommitID: tag.Object.String(),
+				NewCommitID: tag.Object.RefName(),
 			})
 		}
 		for _, deleteID := range deletes {
@@ -220,20 +220,20 @@ func SyncReleasesWithTags(ctx context.Context, repo *repo_model.Repository, gitR
 			}
 			syncResults = append(syncResults, &SyncResult{
 				RefName:     git.RefNameFromTag(release.TagName),
-				OldCommitID: release.Sha1,
+				OldCommitID: git.RefNameFromCommit(release.Sha1),
 				NewCommitID: "",
 			})
 		}
 		for _, tag := range updates {
 			release := dbReleasesByTag[tag.Name]
-			oldSha := ""
+			var oldCommitID git.RefName
 			if release != nil {
-				oldSha = release.Sha1
+				oldCommitID = git.RefNameFromCommit(release.Sha1)
 			}
 			syncResults = append(syncResults, &SyncResult{
 				RefName:     git.RefNameFromTag(tag.Name),
-				OldCommitID: oldSha,
-				NewCommitID: tag.Object.String(),
+				OldCommitID: oldCommitID,
+				NewCommitID: tag.Object.RefName(),
 			})
 		}
 		//