diff --git a/models/git/commit_status.go b/models/git/commit_status.go index fa98cba7baf..81783bbf858 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -23,6 +23,7 @@ import ( "gitea.dev/modules/setting" "gitea.dev/modules/timeutil" "gitea.dev/modules/translation" + "gitea.dev/modules/util" "xorm.io/builder" ) @@ -119,7 +120,7 @@ WHEN NOT MATCHED func GetNextCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) { _, err := git.NewIDFromString(sha) if err != nil { - return 0, git.ErrInvalidSHA{SHA: sha} + return 0, util.NewInvalidArgumentErrorf("invalid sha: %v", err) } switch { diff --git a/models/issues/pull.go b/models/issues/pull.go index 2b93b926b01..6de5338a536 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -414,7 +414,7 @@ func (pr *PullRequest) getReviewedByLines(ctx context.Context, writer io.Writer) } // GetGitHeadRefName returns git ref for hidden pull request branch -func (pr *PullRequest) GetGitHeadRefName() string { +func (pr *PullRequest) GetGitHeadRefName() string { // TODO: make it return RefName but not string return fmt.Sprintf("%s%d/head", git.PullPrefix, pr.Index) } diff --git a/modules/git/commit.go b/modules/git/commit.go index 21288ad8459..9600ec1b6ae 100644 --- a/modules/git/commit.go +++ b/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 } diff --git a/modules/git/commit_test.go b/modules/git/commit_test.go index 5e3d2fba71b..53c4b46e8ea 100644 --- a/modules/git/commit_test.go +++ b/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)) +} diff --git a/modules/git/gitcmd/error.go b/modules/git/gitcmd/error.go index 8d5ec5f5e69..21c0aa45255 100644 --- a/modules/git/gitcmd/error.go +++ b/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 diff --git a/modules/git/gitcmd/error_test.go b/modules/git/gitcmd/error_test.go new file mode 100644 index 00000000000..7111d216dce --- /dev/null +++ b/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) + } +} diff --git a/modules/git/object_id.go b/modules/git/object_id.go index 25dfef3ec51..53ae5ec1a1a 100644 --- a/modules/git/object_id.go +++ b/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 -} diff --git a/modules/git/ref.go b/modules/git/ref.go index 6ce49687384..0c9aeae8c0a 100644 --- a/modules/git/ref.go +++ b/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) } diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 20827062df3..7e6db9abee5 100644 --- a/modules/git/repo_commit.go +++ b/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) diff --git a/modules/git/repo_commit_test.go b/modules/git/repo_commit_test.go index fc905d24257..c57dec95030 100644 --- a/modules/git/repo_commit_test.go +++ b/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) } diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 2fc18de58b9..1754a19b4bd 100644 --- a/modules/git/repo_compare.go +++ b/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 [...] -- [...]' 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 } diff --git a/modules/gitrepo/compare.go b/modules/gitrepo/compare.go index 16280312305..2a552986b34 100644 --- a/modules/gitrepo/compare.go +++ b/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 diff --git a/modules/repository/branch.go b/modules/repository/branch.go index e91eec7167c..48c5a65da80 100644 --- a/modules/repository/branch.go +++ b/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: "", }) } diff --git a/modules/repository/repo.go b/modules/repository/repo.go index aa2a65c1549..35dd74cd6d1 100644 --- a/modules/repository/repo.go +++ b/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(), }) } // diff --git a/services/asymkey/sign.go b/services/asymkey/sign.go index 18deeee135b..8ff62f60c87 100644 --- a/services/asymkey/sign.go +++ b/services/asymkey/sign.go @@ -365,7 +365,7 @@ func AllHeadCommitsVerified(ctx context.Context, pr *issues_model.PullRequest, g if err != nil { return false, err } - commitList, err := headCommit.CommitsBeforeUntil(mergeBaseCommit) + commitList, err := headCommit.CommitsBeforeUntil(git.RefNameFromCommit(mergeBaseCommit)) if err != nil { return false, err } diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index db569c85501..278f1be40ed 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -289,7 +289,7 @@ func SyncPullMirror(ctx context.Context, repoID int64) bool { log.Error("SyncMirrors [repo_id: %v]: unable to GetMirrorByRepoID: %v", repoID, err) return false } - repo := m.GetRepository(ctx) // force load repository of mirror + m.GetRepository(ctx) // force load repository of mirror ctx, _, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("Syncing Mirror %s/%s", m.Repo.OwnerName, m.Repo.Name)) defer finished() @@ -355,41 +355,27 @@ func SyncPullMirror(ctx context.Context, repoID int64) bool { continue } - // Push commits - oldCommitID, err := gitrepo.GetFullCommitID(ctx, repo, result.OldCommitID) + oldCommitID, newCommitID := result.OldCommitID, result.NewCommitID + commits, err := gitRepo.CommitsBetween(newCommitID, oldCommitID, setting.UI.FeedMaxCommitNum) if err != nil { - log.Error("SyncMirrors [repo: %-v]: unable to get GetFullCommitID[%s]: %v", m.Repo, result.OldCommitID, err) + log.Error("SyncMirrors [repo: %-v]: unable to get CommitsBetween [new_commit_id: %s, old_commit_id: %s]: %v", m.Repo, newCommitID, oldCommitID, err) continue } - newCommitID, err := gitrepo.GetFullCommitID(ctx, repo, result.NewCommitID) - if err != nil { - log.Error("SyncMirrors [repo: %-v]: unable to get GetFullCommitID [%s]: %v", m.Repo, result.NewCommitID, err) - continue - } - commits, err := gitRepo.CommitsBetweenIDs(newCommitID, oldCommitID) - if err != nil { - log.Error("SyncMirrors [repo: %-v]: unable to get CommitsBetweenIDs [new_commit_id: %s, old_commit_id: %s]: %v", m.Repo, newCommitID, oldCommitID, err) - continue - } - theCommits := repo_module.GitToPushCommits(commits) - if len(theCommits.Commits) > setting.UI.FeedMaxCommitNum { - theCommits.Commits = theCommits.Commits[:setting.UI.FeedMaxCommitNum] - } - newCommit, err := gitRepo.GetCommit(newCommitID) + newCommit, err := gitRepo.GetCommit(newCommitID.String()) if err != nil { log.Error("SyncMirrors [repo: %-v]: unable to get commit %s: %v", m.Repo, newCommitID, err) continue } theCommits.HeadCommit = repo_module.CommitToPushCommit(newCommit) - theCommits.CompareURL = m.Repo.ComposeCompareURL(oldCommitID, newCommitID) + theCommits.CompareURL = m.Repo.ComposeCompareURL(oldCommitID.String(), newCommitID.String()) notify_service.SyncPushCommits(ctx, m.Repo.MustOwner(ctx), m.Repo, &repo_module.PushUpdateOptions{ RefFullName: result.RefName, - OldCommitID: oldCommitID, - NewCommitID: newCommitID, + OldCommitID: oldCommitID.String(), + NewCommitID: newCommitID.String(), }, theCommits) } log.Trace("SyncMirrors [repo: %-v]: done notifying updated branches/tags - now updating last commit time", m.Repo) diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go index 229730257f2..54b91594fb0 100644 --- a/services/pull/merge_squash.go +++ b/services/pull/merge_squash.go @@ -22,7 +22,7 @@ func getAuthorSignatureSquash(ctx *mergeContext) (*git.Signature, error) { return nil, err } - // Try to get an signature from the same user in one of the commits, as the + // Try to get a signature from the same user in one of the commits, as the // poster email might be private or commits might have a different signature // than the primary email address of the poster. gitRepo, err := git.OpenRepository(ctx, ctx.tmpBasePath) @@ -32,9 +32,9 @@ func getAuthorSignatureSquash(ctx *mergeContext) (*git.Signature, error) { } defer gitRepo.Close() - commits, err := gitRepo.CommitsBetweenIDs(tmpRepoTrackingBranch, "HEAD") + commits, err := gitRepo.CommitsBetween(git.RefNameFromBranch(tmpRepoTrackingBranch), git.RefNameHead, -1) if err != nil { - log.Error("%-v Unable to get commits between: %s %s: %v", ctx.pr, "HEAD", tmpRepoTrackingBranch, err) + log.Error("%-v Unable to get commits between: head and tracking branch: %v", ctx.pr, err) return nil, err } diff --git a/services/pull/pull.go b/services/pull/pull.go index a12adab99db..4491406b33d 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -797,31 +797,23 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ } defer closer.Close() - var headCommit *git.Commit + var headCommitRef git.RefName if pr.Flow == issues_model.PullRequestFlowGithub { - headCommit, err = gitRepo.GetBranchCommit(pr.HeadBranch) + headCommitRef = git.RefNameFromBranch(pr.HeadBranch) } else { pr.HeadCommitID, err = gitRepo.GetRefCommitID(pr.GetGitHeadRefName()) if err != nil { log.Error("Unable to get head commit: %s Error: %v", pr.GetGitHeadRefName(), err) return "" } - headCommit, err = gitRepo.GetCommit(pr.HeadCommitID) - } - if err != nil { - log.Error("Unable to get head commit: %s Error: %v", pr.HeadBranch, err) - return "" + headCommitRef = git.RefNameFromCommit(pr.HeadCommitID) } - mergeBase, err := gitRepo.GetCommit(pr.MergeBase) - if err != nil { - log.Error("Unable to get merge base commit: %s Error: %v", pr.MergeBase, err) - return "" - } + mergeBaseRef := git.RefNameFromCommit(pr.MergeBase) limit := setting.Repository.PullRequest.DefaultMergeMessageCommitsLimit - commits, err := gitRepo.CommitsBetweenLimit(headCommit, mergeBase, limit, 0) + limitedCommits, err := gitRepo.CommitsBetween(headCommitRef, mergeBaseRef, limit) if err != nil { log.Error("Unable to get commits between: %s %s Error: %v", pr.HeadBranch, pr.MergeBase, err) return "" @@ -830,7 +822,7 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ posterSig := pr.Issue.Poster.NewGitSig().String() uniqueAuthors := make(container.Set[string]) - authors := make([]string, 0, len(commits)) + authors := make([]string, 0, len(limitedCommits)) stringBuilder := strings.Builder{} if !setting.Repository.PullRequest.PopulateSquashCommentWithCommitMessages { @@ -848,7 +840,7 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ // use PR's commit messages as squash commit message // commits list is in reverse chronological order maxMsgSize := setting.Repository.PullRequest.DefaultMergeMessageSize - for _, commit := range slices.Backward(commits) { + for _, commit := range slices.Backward(limitedCommits) { msg := strings.TrimSpace(commit.MessageUTF8()) if msg == "" { continue @@ -875,7 +867,7 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ } // collect co-authors - for _, commit := range commits { + for _, commit := range limitedCommits { authorString := commit.Author.String() if uniqueAuthors.Add(authorString) && authorString != posterSig { // Compare use account as well to avoid adding the same author multiple times @@ -892,7 +884,7 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ skip := limit limit = 30 for { - commits, err = gitRepo.CommitsBetweenLimit(headCommit, mergeBase, limit, skip) + commits, err := gitRepo.CommitsBetween(headCommitRef, mergeBaseRef, limit, skip) if err != nil { log.Error("Unable to get commits between: %s %s Error: %v", pr.HeadBranch, pr.MergeBase, err) return "" diff --git a/services/release/notes.go b/services/release/notes.go index e62335c98b5..bf41d3426f6 100644 --- a/services/release/notes.go +++ b/services/release/notes.go @@ -26,32 +26,32 @@ type GenerateReleaseNotesOptions struct { PreviousTag string } -// GenerateReleaseNotes builds the markdown snippet for release notes. +// GenerateReleaseNotes builds the Markdown snippet for release notes. func GenerateReleaseNotes(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, opts GenerateReleaseNotesOptions) (string, error) { headCommit, err := resolveHeadCommit(gitRepo, opts.TagName, opts.TagTarget) if err != nil { return "", err } - isFirstRelease, err := isFirstRelease(ctx, repo.ID) + isFirstRelease, err := repoReleaseIsEmpty(ctx, repo.ID) if err != nil { - return "", fmt.Errorf("isFirstRelease: %w", err) + return "", fmt.Errorf("repoReleaseIsEmpty: %w", err) } - baseCommitID := "" + var baseCommitID git.RefName if opts.PreviousTag != "" { baseCommit, err := gitRepo.GetCommit(opts.PreviousTag) if err != nil { - return "", util.ErrorWrapTranslatable(util.ErrNotExist, "repo.release.generate_notes_tag_not_found", opts.TagName) + return "", util.ErrorWrapTranslatable(util.ErrNotExist, "repo.release.generate_notes_tag_not_found", opts.PreviousTag) } - baseCommitID = baseCommit.ID.String() + baseCommitID = baseCommit.ID.RefName() } else if !isFirstRelease { return "", util.ErrorWrapTranslatable(util.ErrNotExist, "repo.release.generate_notes_tag_not_found", opts.TagName) } - commits, err := gitRepo.CommitsBetweenIDs(headCommit.ID.String(), baseCommitID) + commits, err := gitRepo.CommitsBetween(headCommit.ID.RefName(), baseCommitID, -1) if err != nil { - return "", fmt.Errorf("CommitsBetweenIDs: %w", err) + return "", fmt.Errorf("CommitsBetween: %w", err) } prs, err := collectPullRequestsFromCommits(ctx, repo.ID, commits) @@ -74,7 +74,7 @@ func GenerateReleaseNotes(ctx context.Context, repo *repo_model.Repository, gitR return content, nil } -func isFirstRelease(ctx context.Context, repoID int64) (bool, error) { +func repoReleaseIsEmpty(ctx context.Context, repoID int64) (bool, error) { count, err := db.Count[repo_model.Release](ctx, repo_model.FindReleasesOptions{ RepoID: repoID, IncludeDrafts: false, diff --git a/services/repository/push.go b/services/repository/push.go index c0f4405d3ef..895e3de0200 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -297,7 +297,7 @@ func pushNewBranch(ctx context.Context, repo *repo_model.Repository, pusher *use } func pushUpdateBranch(_ context.Context, repo *repo_model.Repository, pusher *user_model.User, opts *repo_module.PushUpdateOptions, newCommit *git.Commit) ([]*git.Commit, error) { - l, err := newCommit.CommitsBeforeUntil(opts.OldCommitID) + l, err := newCommit.CommitsBeforeUntil(git.RefNameFromCommit(opts.OldCommitID)) if err != nil { return nil, fmt.Errorf("newCommit.CommitsBeforeUntil: %w", err) }