diff --git a/modules/git/compare.go b/modules/git/compare.go index 90e98986e74..696ba1fff48 100644 --- a/modules/git/compare.go +++ b/modules/git/compare.go @@ -46,21 +46,13 @@ func GetDivergingCommits(ctx context.Context, repo RepositoryFacade, baseBranch, // GetCommitIDsBetweenReverse returns the last commit IDs between two commits in reverse order (from old to new) with limit. // If the result exceeds the limit, the old commits IDs will be ignored func GetCommitIDsBetweenReverse(ctx context.Context, repo RepositoryFacade, startRef, endRef, notRef string, limit int) ([]string, error) { - genCmd := func(reversions ...string) *gitcmd.Command { - cmd := gitcmd.NewCommand("rev-list", "--reverse"). - AddArguments("-n").AddDynamicArguments(strconv.Itoa(limit)). - AddDynamicArguments(reversions...) - if notRef != "" { // --not should be kept as the last parameter of git command, otherwise the result will be wrong - cmd.AddOptionValues("--not", notRef) - } - return cmd - } - stdout, _, err := genCmd(startRef + ".." + endRef).WithRepo(repo).RunStdString(ctx) - 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 = genCmd(endRef).WithRepo(repo).RunStdString(ctx) + cmd := gitcmd.NewCommand("rev-list", "--reverse"). + AddArguments("-n").AddDynamicArguments(strconv.Itoa(limit)). + AddDynamicArguments(startRef + ".." + endRef) + if notRef != "" { // --not should be kept as the last parameter of git command, otherwise the result will be wrong + cmd.AddOptionValues("--not", notRef) } + stdout, _, err := cmd.WithRepo(repo).RunStdString(ctx) if err != nil { return nil, err } diff --git a/modules/git/gitcmd/error.go b/modules/git/gitcmd/error.go index 42c2e92f1c8..4c83ecca96c 100644 --- a/modules/git/gitcmd/error.go +++ b/modules/git/gitcmd/error.go @@ -95,14 +95,10 @@ const ( StderrNoSuchPath StderrPrefix = "fatal: no such path" StderrBadObject StderrPrefix = "fatal: bad object " - StderrNoSuchRemote StderrPrefix = "error: no such remote" - StderrAuthenticationFailed StderrPrefix = "fatal: Authentication failed for" StderrCouldNotReadUsername StderrPrefix = "fatal: could not read Username" - StderrUnknownRevisionOrPath StderrRegexp = "^fatal: .*: unknown revision or path not in the working tree" - StderrNoMergeBase StderrRegexp = "^fatal: .*: no merge base" - StderrFileNoEnoughLines StderrRegexp = `^fatal: file .* has only \d+ lines?` + StderrFileNoEnoughLines StderrRegexp = `^fatal: file .* has only \d+ lines?` ) func matchStderrCheck(stderr string, checkIntf StderrCheck) (match bool) { diff --git a/modules/git/gitcmd/error_test.go b/modules/git/gitcmd/error_test.go index 8f90f64ff5f..625537630d8 100644 --- a/modules/git/gitcmd/error_test.go +++ b/modules/git/gitcmd/error_test.go @@ -14,8 +14,6 @@ func TestIsStderr(t *testing.T) { check StderrCheck stderr string }{ - {StderrUnknownRevisionOrPath, "fatal: ambiguous argument 'origin': unknown revision or path not in the working tree...."}, - {StderrNoMergeBase, "fatal: origin/main..HEAD: no merge base...."}, {StderrFileNoEnoughLines, "fatal: file foo/bar has only 1 line"}, {StderrAuthenticationFailed, "Cloning into 'repo'...\r\nremote: Invalid username or token.\r\nfatal: Authentication failed for 'https://host/repo.git/'\r\n"}, } diff --git a/modules/git/remote.go b/modules/git/remote.go index d711a976739..601e5e7cc1c 100644 --- a/modules/git/remote.go +++ b/modules/git/remote.go @@ -64,9 +64,9 @@ func (err *ErrInvalidCloneAddr) Unwrap() error { return util.ErrInvalidArgument } -// IsRemoteNotExistError checks the prefix of the error message to see whether a remote does not exist. +// IsRemoteNotExistError checks whether a "git remote" subcommand failed because the remote does not exist func IsRemoteNotExistError(err error) bool { - return gitcmd.IsStderr(err, gitcmd.StderrNoSuchRemote) + return gitcmd.IsErrorExitCode(err, 2) } // ParseRemoteAddr checks if given remote address is valid, diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 71baafa2fdb..1a48ec962e7 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -317,28 +317,19 @@ func (repo *Repository) CommitsByFileAndRange(ctx context.Context, opts CommitsB // 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(ctx context.Context, afterRef, beforeRef RefName, limit int, optSkip ...int) ([]*Commit, error) { - gitCmd := func() *gitcmd.Command { - cmd := gitcmd.NewCommand("rev-list").WithRepo(repo) - if limit >= 0 { - cmd.AddOptionValues("--max-count", strconv.Itoa(limit)) - } - if len(optSkip) > 0 { - cmd.AddOptionValues("--skip", strconv.Itoa(optSkip[0])) - } - return cmd + cmd := gitcmd.NewCommand("rev-list").WithRepo(repo) + if limit >= 0 { + cmd.AddOptionValues("--max-count", strconv.Itoa(limit)) + } + if len(optSkip) > 0 { + cmd.AddOptionValues("--skip", strconv.Itoa(optSkip[0])) } - var stdout []byte - var err error if beforeRef == "" { - stdout, _, err = gitCmd().AddDynamicArguments(afterRef.String()).RunStdBytes(ctx) + cmd.AddDynamicArguments(afterRef.String()) } else { - stdout, _, err = gitCmd().AddDynamicArguments(beforeRef.String() + ".." + afterRef.String()).RunStdBytes(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. - // 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(ctx) - } + cmd.AddDynamicArguments(beforeRef.String() + ".." + afterRef.String()) } + stdout, _, err := cmd.RunStdBytes(ctx) if err != nil { return nil, err } diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 7475df16f04..93e634571ec 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -46,11 +46,6 @@ func (repo *Repository) GetDiffNumChangedFiles(ctx context.Context, base, head s WithRepo(repo). WithStdoutCopy(w). RunWithStderr(ctx); err != nil { - if gitcmd.IsStderr(err, gitcmd.StderrNoMergeBase) { - // git >= 2.28 now returns an error if base and head have become unrelated. - // 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 } return w.numLines, nil diff --git a/modules/git/tree.go b/modules/git/tree.go index 37f9a17b9d8..74185f9641b 100644 --- a/modules/git/tree.go +++ b/modules/git/tree.go @@ -83,7 +83,7 @@ type Tree struct { } // ListEntries returns all entries of current tree. -func (t *Tree) ListEntries(ctx context.Context, gitRepo *Repository) (Entries, error) { +func (t *Tree) ListEntries(_ctx context.Context, gitRepo *Repository) (Entries, error) { if t.entriesParsed { return t.entries, nil } @@ -109,36 +109,22 @@ func (t *Tree) ListEntries(ctx context.Context, gitRepo *Repository) (Entries, e return nil, err } } - if info.Type == "tree" { - t.entries, err = catBatchParseTreeEntries(t.ID.Type(), t, rd, info.Size) + if info.Type != "tree" { // resolve annotated tags to their tree, a blob yields ErrNotExist + if err := DiscardFull(rd, info.Size+1); err != nil { + return nil, err + } + info, rd, err = batch.QueryContent(t.ID.String() + "^{tree}") if err != nil { return nil, err } - t.entriesParsed = true - return t.entries, nil } - // Not a tree just use ls-tree instead - if err := DiscardFull(rd, info.Size+1); err != nil { + t.entries, err = catBatchParseTreeEntries(t.ID.Type(), t, rd, info.Size) + if err != nil { return nil, err } - - stdout, _, runErr := gitcmd.NewCommand("ls-tree", "-l").AddDynamicArguments(t.ID.String()).WithRepo(gitRepo).RunStdBytes(ctx) - if runErr != nil { - if gitcmd.IsStderr(runErr, gitcmd.StderrNotValidObjectName) || gitcmd.IsStderr(runErr, gitcmd.StderrNotTreeObject) { - return nil, ErrNotExist{ - ID: t.ID.String(), - } - } - return nil, runErr - } - - t.entries, err = parseTreeEntries(stdout, t) - if err == nil { - t.entriesParsed = true - } - - return t.entries, err + t.entriesParsed = true + return t.entries, nil } // listEntriesRecursive returns all entries of current tree recursively including all subtrees diff --git a/modules/git/tree_test.go b/modules/git/tree_test.go index 822159acbb5..e2eb8f0adba 100644 --- a/modules/git/tree_test.go +++ b/modules/git/tree_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSubTree_Issue29101(t *testing.T) { @@ -26,6 +27,24 @@ func TestSubTree_Issue29101(t *testing.T) { } } +func TestTreeListEntriesResolvesTagAndRejectsBlob(t *testing.T) { + repo, err := OpenRepositoryLocal(t.Context(), filepath.Join(testReposDir, "repo1_bare")) + require.NoError(t, err) + defer repo.Close() + + entries, err := newTree(MustIDFromString("3ad28a9149a2864384548f3d17ed7f38014c9e8a")).ListEntries(t.Context(), repo) + require.NoError(t, err) + names := make([]string, 0, len(entries)) + for _, entry := range entries { + names = append(names, entry.Name()) + } + assert.Equal(t, []string{"file1.txt", "file2.txt", "foo"}, names) + assert.True(t, entries[2].IsDir()) + + _, err = newTree(MustIDFromString("e2129701f1a4d54dc44f03c93bca0a2aec7c5449")).ListEntries(t.Context(), repo) + assert.True(t, IsErrNotExist(err)) +} + func Test_GetTreePathLatestCommit(t *testing.T) { repo, err := OpenRepositoryLocal(t.Context(), filepath.Join(testReposDir, "repo6_blame")) assert.NoError(t, err) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 83517b9228a..f8cdd4613c4 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -370,8 +370,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 := errors.Is(err, util.ErrNotExist) || gitcmd.IsStderr(err, gitcmd.StderrNotValidObjectName) || gitcmd.IsStderr(err, gitcmd.StderrUnknownRevisionOrPath) - if !isKnownErrorForBroken { + if !errors.Is(err, util.ErrNotExist) && !gitcmd.IsStderr(err, gitcmd.StderrNotValidObjectName) { log.Error("GetCompareInfo: %v", err) } prInfo.IsPullRequestBroken = true diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index 6fb07a8ea33..caa6b64dc69 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -43,7 +43,7 @@ func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error } err = git.ManagedRemoteAdd(ctx, repo, remoteName, addr, git.RemoteOptionMirrorFetch) - if err != nil && !git.IsRemoteNotExistError(err) { + if err != nil { return err } @@ -56,7 +56,7 @@ func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error } err = git.ManagedRemoteAdd(ctx, repo.WikiStorageRepo(), remoteName, wikiRemotePath, git.RemoteOptionMirrorFetch) - if err != nil && !git.IsRemoteNotExistError(err) { + if err != nil { return err } } diff --git a/services/wiki/wiki.go b/services/wiki/wiki.go index 1eec2bf549f..df406490baf 100644 --- a/services/wiki/wiki.go +++ b/services/wiki/wiki.go @@ -15,7 +15,6 @@ import ( "gitea.dev/models/unit" user_model "gitea.dev/models/user" "gitea.dev/modules/git" - "gitea.dev/modules/git/gitcmd" "gitea.dev/modules/globallock" "gitea.dev/modules/graceful" "gitea.dev/modules/log" @@ -50,17 +49,18 @@ func InitWiki(ctx context.Context, repo *repo_model.Repository) error { } // prepareGitPath try to find a suitable file path with file name by the given raw wiki name. +// An empty defaultWikiBranch means the branch doesn't exist yet. // return: existence, prepared file path with name, error func prepareGitPath(ctx context.Context, gitRepo *git.Repository, defaultWikiBranch string, wikiPath WebPath) (bool, string, error) { unescaped := string(wikiPath) + ".md" gitPath := WebPathToGitPath(wikiPath) + if defaultWikiBranch == "" { + return false, gitPath, nil + } // Look for both files filesInIndex, err := gitRepo.LsTree(ctx, defaultWikiBranch, unescaped, gitPath) if err != nil { - if gitcmd.IsStderr(err, gitcmd.StderrNotValidObjectName) { - return false, gitPath, nil // branch doesn't exist - } log.Error("Wiki LsTree failed, err: %v", err) return false, gitPath, err } @@ -136,7 +136,7 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model } } - isWikiExist, newWikiPath, err := prepareGitPath(ctx, gitRepo, repo.DefaultWikiBranch, newWikiName) + isWikiExist, newWikiPath, err := prepareGitPath(ctx, gitRepo, cloneOpts.Branch, newWikiName) if err != nil { return err } @@ -152,7 +152,7 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model isOldWikiExist := true oldWikiPath := newWikiPath if oldWikiName != newWikiName { - isOldWikiExist, oldWikiPath, err = prepareGitPath(ctx, gitRepo, repo.DefaultWikiBranch, oldWikiName) + isOldWikiExist, oldWikiPath, err = prepareGitPath(ctx, gitRepo, cloneOpts.Branch, oldWikiName) if err != nil { return err } diff --git a/services/wiki/wiki_test.go b/services/wiki/wiki_test.go index 9040164f79e..43befa95714 100644 --- a/services/wiki/wiki_test.go +++ b/services/wiki/wiki_test.go @@ -300,18 +300,7 @@ func TestPrepareWikiFileName(t *testing.T) { } func TestPrepareWikiFileName_FirstPage(t *testing.T) { - // Now create a temporaryDirectory - tmpDir := t.TempDir() - - err := git.InitRepositoryLocal(t.Context(), tmpDir, true, git.Sha1ObjectFormat.Name(), "") - assert.NoError(t, err) - - gitRepo, err := git.OpenRepositoryLocal(t.Context(), tmpDir) - require.NoError(t, err) - - defer gitRepo.Close() - - existence, newWikiPath, err := prepareGitPath(t.Context(), gitRepo, "master", "Home") + existence, newWikiPath, err := prepareGitPath(t.Context(), nil, "", "Home") assert.False(t, existence) assert.NoError(t, err) assert.Equal(t, "Home.md", newWikiPath)