fix: refactor git error handling and make archive streaming handle non-existing commit id (#38007)

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

modules/git/gitcmd/error.go

@@ -9,6 +9,8 @@ import (
 	"fmt"
 	"os/exec"
 	"strings"
+
+	"gitea.dev/modules/util"
 )
 
 type RunStdError interface {
@@ -41,32 +43,14 @@ func (r *runStdError) Stderr() string {
 }
 
 func ErrorAsStderr(err error) (string, bool) {
-	var runErr RunStdError
-	if errors.As(err, &runErr) {
+	if runErr, ok := errors.AsType[RunStdError](err); ok {
 		return runErr.Stderr(), true
 	}
 	return "", false
 }
 
-func StderrHasPrefix(err error, prefix string) bool {
-	stderr, ok := ErrorAsStderr(err)
-	if !ok {
-		return false
-	}
-	return strings.HasPrefix(stderr, prefix)
-}
-
-func StderrContains(err error, sub string) bool {
-	stderr, ok := ErrorAsStderr(err)
-	if !ok {
-		return false
-	}
-	return strings.Contains(stderr, sub)
-}
-
 func IsErrorExitCode(err error, code int) bool {
-	var exitError *exec.ExitError
-	if errors.As(err, &exitError) {
+	if exitError, ok := errors.AsType[*exec.ExitError](err); ok {
 		return exitError.ExitCode() == code
 	}
 	return false
@@ -85,11 +69,41 @@ func IsErrorCanceledOrKilled(err error) bool {
 	return errors.Is(err, context.Canceled) || IsErrorSignalKilled(err)
 }
 
-func IsStdErrorNotValidObjectName(err error) bool {
+type StderrPrefix string
+
+type StderrSubStr string
+
+const (
+	StderrNotValidObjectName StderrPrefix = "fatal: not a valid object name"
+	StderrNotTreeObject      StderrPrefix = "fatal: not a tree object"
+	StderrPathSpec           StderrPrefix = "fatal: pathspec"
+	StderrBadRevision        StderrPrefix = "fatal: bad revision"
+
+	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"
+)
+
+func IsStderr[T StderrPrefix | StderrSubStr](err error, check T) bool {
 	stderr, ok := ErrorAsStderr(err)
-	// 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 ok && strings.Contains(strings.ToLower(stderr), "fatal: not a valid object name")
+	if !ok {
+		return false
+	}
+	checkLen := len(check)
+	if len(stderr) < checkLen {
+		return false
+	}
+	switch any(check).(type) {
+	case StderrPrefix:
+		// 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))
+	}
+	return false
 }
 
 type pipelineError struct {

modules/git/remote.go

@@ -66,11 +66,7 @@ func (err *ErrInvalidCloneAddr) Unwrap() error {
 
 // IsRemoteNotExistError checks the prefix of the error message to see whether a remote does not exist.
 func IsRemoteNotExistError(err error) bool {
-	// see: https://github.com/go-gitea/gitea/issues/32889#issuecomment-2571848216
-	// Should not add space in the end, sometimes git will add a `:`
-	prefix1 := "fatal: No such remote" // git < 2.30, exit status 128
-	prefix2 := "error: No such remote" // git >= 2.30. exit status 2
-	return gitcmd.StderrHasPrefix(err, prefix1) || gitcmd.StderrHasPrefix(err, prefix2)
+	return gitcmd.IsStderr(err, gitcmd.StderrNoSuchRemote1) || gitcmd.IsStderr(err, gitcmd.StderrNoSuchRemote2)
 }
 
 // ParseRemoteAddr checks if given remote address is valid,

modules/git/repo_commit.go

@@ -24,9 +24,9 @@ func (repo *Repository) GetTagCommitID(name string) (string, error) {
 	return repo.GetRefCommitID(TagPrefix + name)
 }
 
-// GetCommit returns commit object of by ID string.
-func (repo *Repository) GetCommit(commitID string) (*Commit, error) {
-	id, err := repo.ConvertToGitID(commitID)
+// GetCommit returns a commit object of by the git ref.
+func (repo *Repository) GetCommit(ref string) (*Commit, error) {
+	id, err := repo.ConvertToGitID(ref)
 	if err != nil {
 		return nil, err
 	}

modules/git/repo_commit_nogogit.go

@@ -109,16 +109,16 @@ func (repo *Repository) getCommitWithBatch(batch CatFileBatch, id ObjectID) (*Co
 	}
 }
 
-// ConvertToGitID returns a GitHash object from a potential ID string
-func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) {
+// ConvertToGitID returns a git object ID from the git ref, it doesn't guarantee the returned ID really exists
+func (repo *Repository) ConvertToGitID(ref string) (ObjectID, error) {
 	objectFormat, err := repo.GetObjectFormat()
 	if err != nil {
 		return nil, err
 	}
-	if len(commitID) == objectFormat.FullLength() && objectFormat.IsValid(commitID) {
-		ID, err := NewIDFromString(commitID)
+	if len(ref) == objectFormat.FullLength() && objectFormat.IsValid(ref) {
+		id, err := NewIDFromString(ref)
 		if err == nil {
-			return ID, nil
+			return id, nil
 		}
 	}
 
@@ -127,10 +127,10 @@ func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) {
 		return nil, err
 	}
 	defer cancel()
-	info, err := batch.QueryInfo(commitID)
+	info, err := batch.QueryInfo(ref)
 	if err != nil {
 		if IsErrNotExist(err) {
-			return nil, ErrNotExist{commitID, ""}
+			return nil, ErrNotExist{ref, ""}
 		}
 		return nil, err
 	}

modules/git/tree_nogogit.go

@@ -7,7 +7,6 @@ package git
 
 import (
 	"io"
-	"strings"
 
 	"gitea.dev/modules/git/gitcmd"
 )
@@ -65,7 +64,7 @@ func (t *Tree) ListEntries() (Entries, error) {
 
 	stdout, _, runErr := gitcmd.NewCommand("ls-tree", "-l").AddDynamicArguments(t.ID.String()).WithDir(t.repo.Path).RunStdBytes(t.repo.Ctx)
 	if runErr != nil {
-		if gitcmd.IsStdErrorNotValidObjectName(runErr) || strings.Contains(runErr.Error(), "fatal: not a tree object") {
+		if gitcmd.IsStderr(runErr, gitcmd.StderrNotValidObjectName) || gitcmd.IsStderr(runErr, gitcmd.StderrNotTreeObject) {
 			return nil, ErrNotExist{
 				ID: t.ID.String(),
 			}

routers/api/v1/repo/pull.go

@@ -1435,7 +1435,7 @@ func GetPullRequestCommits(ctx *context.APIContext) {
 		compareInfo, err = git_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, git.RefNameFromBranch(pr.BaseBranch), git.RefName(pr.GetGitHeadRefName()), false, false)
 	}
 
-	if gitcmd.StderrHasPrefix(err, "fatal: bad revision") {
+	if gitcmd.IsStderr(err, gitcmd.StderrBadRevision) {
 		ctx.APIError(http.StatusNotFound, "invalid base branch or revision")
 		return
 	} else if err != nil {

routers/web/repo/pull.go

@@ -376,9 +376,7 @@ func (prInfo *pullRequestViewInfo) prepareViewFillCompareInfo(ctx *context.Conte
 	pull := prInfo.issue.PullRequest
 	prInfo.CompareInfo, err = git_service.GetCompareInfo(ctx, ctx.Repo.Repository, ctx.Repo.Repository, ctx.Repo.GitRepo, baseRef, git.RefName(pull.GetGitHeadRefName()), false, false)
 	if err != nil {
-		isKnownErrorForBroken := gitcmd.IsStdErrorNotValidObjectName(err) ||
-			// fatal: ambiguous argument 'origin': unknown revision or path not in the working tree.
-			gitcmd.StderrContains(err, "unknown revision or path not in the working tree")
+		isKnownErrorForBroken := gitcmd.IsStderr(err, gitcmd.StderrNotValidObjectName) || gitcmd.IsStderr(err, gitcmd.StderrUnknownRevisionOrPath)
 		if !isKnownErrorForBroken {
 			log.Error("GetCompareInfo: %v", err)
 		}

services/repository/archiver/archiver.go

@@ -56,13 +56,13 @@ func NewRequest(repo *repo_model.Repository, gitRepo *git.Repository, archiveRef
 	}
 
 	// Get corresponding commit.
-	commitID, err := gitRepo.ConvertToGitID(archiveRefShortName)
+	commit, err := gitRepo.GetCommit(archiveRefShortName)
 	if err != nil {
 		return nil, util.NewNotExistErrorf("unrecognized repository reference: %s", archiveRefShortName)
 	}
 
 	r := &ArchiveRequest{Repo: repo, archiveRefShortName: archiveRefShortName, Type: archiveType, Paths: paths}
-	r.CommitID = commitID.String()
+	r.CommitID = commit.ID.String()
 	return r, nil
 }
 
@@ -330,7 +330,7 @@ func ServeRepoArchive(ctx *gitea_context.Base, archiveReq *ArchiveRequest) error
 		// because errors may happen in git command and such cases aren't in our control.
 		httplib.ServeSetHeaders(ctx.Resp, httplib.ServeHeaderOptions{Filename: downloadName})
 		if err := archiveReq.Stream(ctx, ctx.Resp); err != nil && !ctx.Written() {
-			if gitcmd.StderrHasPrefix(err, "fatal: pathspec") {
+			if gitcmd.IsStderr(err, gitcmd.StderrPathSpec) || gitcmd.IsStderr(err, gitcmd.StderrNotTreeObject) {
 				return util.NewInvalidArgumentErrorf("path doesn't exist or is invalid")
 			}
 			return fmt.Errorf("archive repo %s: failed to stream: %w", archiveReq.Repo.FullName(), err)

services/wiki/wiki.go

@@ -59,7 +59,7 @@ func prepareGitPath(gitRepo *git.Repository, defaultWikiBranch string, wikiPath
 	// Look for both files
 	filesInIndex, err := gitRepo.LsTree(defaultWikiBranch, unescaped, gitPath)
 	if err != nil {
-		if gitcmd.IsStdErrorNotValidObjectName(err) {
+		if gitcmd.IsStderr(err, gitcmd.StderrNotValidObjectName) {
 			return false, gitPath, nil // branch doesn't exist
 		}
 		log.Error("Wiki LsTree failed, err: %v", err)

tests/integration/wiki_test.go

@@ -72,7 +72,7 @@ EOF
 
 	// reader can't push
 	_, _, runErr = gitcmd.NewCommand("push", "origin", "refs/heads/master").WithDir(dstLocalPath).RunStdString(t.Context())
-	assert.True(t, gitcmd.StderrContains(runErr, "remote: Repository not found\n"))
+	assert.Contains(t, runErr.Stderr(), "remote: Repository not found\n")
 	req := NewRequest(t, "GET", "/user2/repo1/wiki/raw/Home.md")
 	resp := MakeRequest(t, req, http.StatusOK)
 	assert.Contains(t, resp.Body.String(), "This is the home page!")