Skip to content

Commit 743bbaa

Browse files
SuperSandro2000wxiaoguang
SuperSandro2000
and
wxiaoguang
authored
fix: refactor git error handling and make archive streaming handle non-existing commit id (#38007)
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
1 parent e88650c commit 743bbaa

10 files changed

Lines changed: 57 additions & 50 deletions

File tree

‎modules/git/gitcmd/error.go‎

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"fmt"
1010
"os/exec"
1111
"strings"
12+
13+
"gitea.dev/modules/util"
1214
)
1315

1416
type RunStdError interface {
@@ -41,32 +43,14 @@ func (r *runStdError) Stderr() string {
4143
}
4244

4345
func ErrorAsStderr(err error) (string, bool) {
44-
var runErr RunStdError
45-
if errors.As(err, &runErr) {
46+
if runErr, ok := errors.AsType[RunStdError](err); ok {
4647
return runErr.Stderr(), true
4748
}
4849
return "", false
4950
}
5051

51-
func StderrHasPrefix(err error, prefix string) bool {
52-
stderr, ok := ErrorAsStderr(err)
53-
if !ok {
54-
return false
55-
}
56-
return strings.HasPrefix(stderr, prefix)
57-
}
58-
59-
func StderrContains(err error, sub string) bool {
60-
stderr, ok := ErrorAsStderr(err)
61-
if !ok {
62-
return false
63-
}
64-
return strings.Contains(stderr, sub)
65-
}
66-
6752
func IsErrorExitCode(err error, code int) bool {
68-
var exitError *exec.ExitError
69-
if errors.As(err, &exitError) {
53+
if exitError, ok := errors.AsType[*exec.ExitError](err); ok {
7054
return exitError.ExitCode() == code
7155
}
7256
return false
@@ -85,11 +69,41 @@ func IsErrorCanceledOrKilled(err error) bool {
8569
return errors.Is(err, context.Canceled) || IsErrorSignalKilled(err)
8670
}
8771

88-
func IsStdErrorNotValidObjectName(err error) bool {
72+
type StderrPrefix string
73+
74+
type StderrSubStr string
75+
76+
const (
77+
StderrNotValidObjectName StderrPrefix = "fatal: not a valid object name"
78+
StderrNotTreeObject StderrPrefix = "fatal: not a tree object"
79+
StderrPathSpec StderrPrefix = "fatal: pathspec"
80+
StderrBadRevision StderrPrefix = "fatal: bad revision"
81+
82+
StderrNoSuchRemote1 StderrPrefix = "fatal: no such remote" // git < 2.30, exit status 128
83+
StderrNoSuchRemote2 StderrPrefix = "error: no such remote" // git >= 2.30. exit status 2
84+
85+
// fatal: ambiguous argument 'origin': unknown revision or path not in the working tree.
86+
StderrUnknownRevisionOrPath StderrSubStr = "unknown revision or path not in the working tree"
87+
)
88+
89+
func IsStderr[T StderrPrefix | StderrSubStr](err error, check T) bool {
8990
stderr, ok := ErrorAsStderr(err)
90-
// Git is lowercasing the "fatal: Not a valid object name" error message
91-
// ref: https://lore.kernel.org/git/pull.2052.git.1771836302101.gitgitgadget@gmail.com
92-
return ok && strings.Contains(strings.ToLower(stderr), "fatal: not a valid object name")
91+
if !ok {
92+
return false
93+
}
94+
checkLen := len(check)
95+
if len(stderr) < checkLen {
96+
return false
97+
}
98+
switch any(check).(type) {
99+
case StderrPrefix:
100+
// Git is lowercasing the "fatal: Not a valid object name" error message
101+
// ref: https://lore.kernel.org/git/pull.2052.git.1771836302101.gitgitgadget@gmail.com
102+
return util.AsciiEqualFold(stderr[:checkLen], string(check))
103+
case StderrSubStr:
104+
return strings.Contains(stderr, string(check))
105+
}
106+
return false
93107
}
94108

95109
type pipelineError struct {

‎modules/git/remote.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,7 @@ func (err *ErrInvalidCloneAddr) Unwrap() error {
6666

6767
// IsRemoteNotExistError checks the prefix of the error message to see whether a remote does not exist.
6868
func IsRemoteNotExistError(err error) bool {
69-
// see: https://github.com/go-gitea/gitea/issues/32889#issuecomment-2571848216
70-
// Should not add space in the end, sometimes git will add a `:`
71-
prefix1 := "fatal: No such remote" // git < 2.30, exit status 128
72-
prefix2 := "error: No such remote" // git >= 2.30. exit status 2
73-
return gitcmd.StderrHasPrefix(err, prefix1) || gitcmd.StderrHasPrefix(err, prefix2)
69+
return gitcmd.IsStderr(err, gitcmd.StderrNoSuchRemote1) || gitcmd.IsStderr(err, gitcmd.StderrNoSuchRemote2)
7470
}
7571

7672
// ParseRemoteAddr checks if given remote address is valid,

‎modules/git/repo_commit.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ func (repo *Repository) GetTagCommitID(name string) (string, error) {
2424
return repo.GetRefCommitID(TagPrefix + name)
2525
}
2626

27-
// GetCommit returns commit object of by ID string.
28-
func (repo *Repository) GetCommit(commitID string) (*Commit, error) {
29-
id, err := repo.ConvertToGitID(commitID)
27+
// GetCommit returns a commit object of by the git ref.
28+
func (repo *Repository) GetCommit(ref string) (*Commit, error) {
29+
id, err := repo.ConvertToGitID(ref)
3030
if err != nil {
3131
return nil, err
3232
}

‎modules/git/repo_commit_nogogit.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -109,16 +109,16 @@ func (repo *Repository) getCommitWithBatch(batch CatFileBatch, id ObjectID) (*Co
109109
}
110110
}
111111

112-
// ConvertToGitID returns a GitHash object from a potential ID string
113-
func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) {
112+
// ConvertToGitID returns a git object ID from the git ref, it doesn't guarantee the returned ID really exists
113+
func (repo *Repository) ConvertToGitID(ref string) (ObjectID, error) {
114114
objectFormat, err := repo.GetObjectFormat()
115115
if err != nil {
116116
return nil, err
117117
}
118-
if len(commitID) == objectFormat.FullLength() && objectFormat.IsValid(commitID) {
119-
ID, err := NewIDFromString(commitID)
118+
if len(ref) == objectFormat.FullLength() && objectFormat.IsValid(ref) {
119+
id, err := NewIDFromString(ref)
120120
if err == nil {
121-
return ID, nil
121+
return id, nil
122122
}
123123
}
124124

@@ -127,10 +127,10 @@ func (repo *Repository) ConvertToGitID(commitID string) (ObjectID, error) {
127127
return nil, err
128128
}
129129
defer cancel()
130-
info, err := batch.QueryInfo(commitID)
130+
info, err := batch.QueryInfo(ref)
131131
if err != nil {
132132
if IsErrNotExist(err) {
133-
return nil, ErrNotExist{commitID, ""}
133+
return nil, ErrNotExist{ref, ""}
134134
}
135135
return nil, err
136136
}

‎modules/git/tree_nogogit.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package git
77

88
import (
99
"io"
10-
"strings"
1110

1211
"gitea.dev/modules/git/gitcmd"
1312
)
@@ -65,7 +64,7 @@ func (t *Tree) ListEntries() (Entries, error) {
6564

6665
stdout, _, runErr := gitcmd.NewCommand("ls-tree", "-l").AddDynamicArguments(t.ID.String()).WithDir(t.repo.Path).RunStdBytes(t.repo.Ctx)
6766
if runErr != nil {
68-
if gitcmd.IsStdErrorNotValidObjectName(runErr) || strings.Contains(runErr.Error(), "fatal: not a tree object") {
67+
if gitcmd.IsStderr(runErr, gitcmd.StderrNotValidObjectName) || gitcmd.IsStderr(runErr, gitcmd.StderrNotTreeObject) {
6968
return nil, ErrNotExist{
7069
ID: t.ID.String(),
7170
}

‎routers/api/v1/repo/pull.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1435,7 +1435,7 @@ func GetPullRequestCommits(ctx *context.APIContext) {
14351435
compareInfo, err = git_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, git.RefNameFromBranch(pr.BaseBranch), git.RefName(pr.GetGitHeadRefName()), false, false)
14361436
}
14371437

1438-
if gitcmd.StderrHasPrefix(err, "fatal: bad revision") {
1438+
if gitcmd.IsStderr(err, gitcmd.StderrBadRevision) {
14391439
ctx.APIError(http.StatusNotFound, "invalid base branch or revision")
14401440
return
14411441
} else if err != nil {

‎routers/web/repo/pull.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -376,9 +376,7 @@ func (prInfo *pullRequestViewInfo) prepareViewFillCompareInfo(ctx *context.Conte
376376
pull := prInfo.issue.PullRequest
377377
prInfo.CompareInfo, err = git_service.GetCompareInfo(ctx, ctx.Repo.Repository, ctx.Repo.Repository, ctx.Repo.GitRepo, baseRef, git.RefName(pull.GetGitHeadRefName()), false, false)
378378
if err != nil {
379-
isKnownErrorForBroken := gitcmd.IsStdErrorNotValidObjectName(err) ||
380-
// fatal: ambiguous argument 'origin': unknown revision or path not in the working tree.
381-
gitcmd.StderrContains(err, "unknown revision or path not in the working tree")
379+
isKnownErrorForBroken := gitcmd.IsStderr(err, gitcmd.StderrNotValidObjectName) || gitcmd.IsStderr(err, gitcmd.StderrUnknownRevisionOrPath)
382380
if !isKnownErrorForBroken {
383381
log.Error("GetCompareInfo: %v", err)
384382
}

‎services/repository/archiver/archiver.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,13 @@ func NewRequest(repo *repo_model.Repository, gitRepo *git.Repository, archiveRef
5656
}
5757

5858
// Get corresponding commit.
59-
commitID, err := gitRepo.ConvertToGitID(archiveRefShortName)
59+
commit, err := gitRepo.GetCommit(archiveRefShortName)
6060
if err != nil {
6161
return nil, util.NewNotExistErrorf("unrecognized repository reference: %s", archiveRefShortName)
6262
}
6363

6464
r := &ArchiveRequest{Repo: repo, archiveRefShortName: archiveRefShortName, Type: archiveType, Paths: paths}
65-
r.CommitID = commitID.String()
65+
r.CommitID = commit.ID.String()
6666
return r, nil
6767
}
6868

@@ -330,7 +330,7 @@ func ServeRepoArchive(ctx *gitea_context.Base, archiveReq *ArchiveRequest) error
330330
// because errors may happen in git command and such cases aren't in our control.
331331
httplib.ServeSetHeaders(ctx.Resp, httplib.ServeHeaderOptions{Filename: downloadName})
332332
if err := archiveReq.Stream(ctx, ctx.Resp); err != nil && !ctx.Written() {
333-
if gitcmd.StderrHasPrefix(err, "fatal: pathspec") {
333+
if gitcmd.IsStderr(err, gitcmd.StderrPathSpec) || gitcmd.IsStderr(err, gitcmd.StderrNotTreeObject) {
334334
return util.NewInvalidArgumentErrorf("path doesn't exist or is invalid")
335335
}
336336
return fmt.Errorf("archive repo %s: failed to stream: %w", archiveReq.Repo.FullName(), err)

‎services/wiki/wiki.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func prepareGitPath(gitRepo *git.Repository, defaultWikiBranch string, wikiPath
5959
// Look for both files
6060
filesInIndex, err := gitRepo.LsTree(defaultWikiBranch, unescaped, gitPath)
6161
if err != nil {
62-
if gitcmd.IsStdErrorNotValidObjectName(err) {
62+
if gitcmd.IsStderr(err, gitcmd.StderrNotValidObjectName) {
6363
return false, gitPath, nil // branch doesn't exist
6464
}
6565
log.Error("Wiki LsTree failed, err: %v", err)

‎tests/integration/wiki_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ EOF
7272

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

0 commit comments

Comments
 (0)