From 5ba2216a8cde2bde26aacb57e35c42b94ef48b21 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 14 Jul 2025 13:23:21 -0700 Subject: [PATCH] improvements --- modules/git/repo_compare.go | 28 ---------------------- modules/git/repo_compare_test.go | 30 ----------------------- routers/web/repo/pull.go | 41 +++++--------------------------- services/pull/merge_merge.go | 27 +++++++++++++++++++++ 4 files changed, 33 insertions(+), 93 deletions(-) diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index ff44506e13c..d32e5c9e1e0 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -5,14 +5,10 @@ package git import ( - "bufio" "bytes" "context" - "errors" "fmt" "io" - "os" - "path/filepath" "regexp" "strconv" "strings" @@ -193,8 +189,6 @@ func GetDiffShortStatByCmdArgs(ctx context.Context, repoPath string, trustedArgs var shortStatFormat = regexp.MustCompile( `\s*(\d+) files? changed(?:, (\d+) insertions?\(\+\))?(?:, (\d+) deletions?\(-\))?`) -var patchCommits = regexp.MustCompile(`^From\s(\w+)\s`) - func parseDiffStat(stdout string) (numFiles, totalAdditions, totalDeletions int, err error) { if len(stdout) == 0 || stdout == "\n" { return 0, 0, 0, nil @@ -282,25 +276,3 @@ func (repo *Repository) GetFilesChangedBetween(base, head string) ([]string, err return split, err } - -// ReadPatchCommit will check if a diff patch exists and return stats -func (repo *Repository) ReadPatchCommit(prID int64) (commitSHA string, err error) { - // Migrated repositories download patches to "pulls" location - patchFile := fmt.Sprintf("pulls/%d.patch", prID) - loadPatch, err := os.Open(filepath.Join(repo.Path, patchFile)) - if err != nil { - return "", err - } - defer loadPatch.Close() - // Read only the first line of the patch - usually it contains the first commit made in patch - scanner := bufio.NewScanner(loadPatch) - scanner.Scan() - // Parse the Patch stats, sometimes Migration returns a 404 for the patch file - commitSHAGroups := patchCommits.FindStringSubmatch(scanner.Text()) - if len(commitSHAGroups) != 0 { - commitSHA = commitSHAGroups[1] - } else { - return "", errors.New("patch file doesn't contain valid commit ID") - } - return commitSHA, nil -} diff --git a/modules/git/repo_compare_test.go b/modules/git/repo_compare_test.go index 7bd6eab85d3..c1ebdd6a692 100644 --- a/modules/git/repo_compare_test.go +++ b/modules/git/repo_compare_test.go @@ -45,36 +45,6 @@ func TestGetFormatPatch(t *testing.T) { assert.Contains(t, patch, "Subject: [PATCH] Add file2.txt") } -func TestReadPatch(t *testing.T) { - // Ensure we can read the patch files - bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") - repo, err := openRepositoryWithDefaultContext(bareRepo1Path) - if err != nil { - assert.NoError(t, err) - return - } - defer repo.Close() - // This patch doesn't exist - noFile, err := repo.ReadPatchCommit(0) - assert.Error(t, err) - - // This patch is an empty one (sometimes it's a 404) - noCommit, err := repo.ReadPatchCommit(1) - assert.Error(t, err) - - // This patch is legit and should return a commit - oldCommit, err := repo.ReadPatchCommit(2) - if err != nil { - assert.NoError(t, err) - return - } - - assert.Empty(t, noFile) - assert.Empty(t, noCommit) - assert.Len(t, oldCommit, 40) - assert.Equal(t, "6e8e2a6f9efd71dbe6917816343ed8415ad696c3", oldCommit) -} - func TestReadWritePullHead(t *testing.T) { // Ensure we can write SHA1 head corresponding to PR and open them bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 97c0c15302d..581d97bd4f6 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -210,46 +210,17 @@ func GetPullDiffStats(ctx *context.Context) { ctx.Data["DiffShortStat"] = diffShortStat } -func calcMergeBase(ctx *context.Context, pull *issues_model.PullRequest) string { - var commitSHA, parentCommit string - // If there is a head or a patch file, and it is readable, grab info - commitSHA, err := ctx.Repo.GitRepo.GetRefCommitID(pull.GetGitHeadRefName()) - if err != nil { - // Head File does not exist, try the patch - // FIXME: it seems this patch file is not used in the code, but it is still read - commitSHA, err = ctx.Repo.GitRepo.ReadPatchCommit(pull.Index) - if err == nil { - // Recreate pull head in files for next time - if err := git.UpdateRef(ctx, ctx.Repo.GitRepo.Path, pull.GetGitHeadRefName(), commitSHA); err != nil { - log.Error("Could not write head file", err) - } - } else { - // There is no history available - log.Trace("No history file available for PR %d", pull.Index) - } - } - if commitSHA != "" { - // Get immediate parent of the first commit in the patch, grab history back - parentCommit, _, err = git.NewCommand("rev-list", "-1", "--skip=1").AddDynamicArguments(commitSHA).RunStdString(ctx, &git.RunOpts{Dir: ctx.Repo.GitRepo.Path}) - if err == nil { - parentCommit = strings.TrimSpace(parentCommit) - } - // Special case on Git < 2.25 that doesn't fail on immediate empty history - if err != nil || parentCommit == "" { - log.Info("No known parent commit for PR %d, error: %v", pull.Index, err) - // bring at least partial history if it can work - parentCommit = commitSHA - } - } - return parentCommit -} - func GetMergedBaseCommitID(ctx *context.Context, pull *issues_model.PullRequest) string { if pull.MergeBase != "" { return pull.MergeBase } - return calcMergeBase(ctx, pull) + var err error + pull.MergeBase, err = pull_service.CalcMergeBase(ctx, pull) + if err != nil { + log.Error("CalcMergeBase: %v", err) + } + return pull.MergeBase } func preparePullViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.CompareInfo { diff --git a/services/pull/merge_merge.go b/services/pull/merge_merge.go index 118d21c7cd9..d7f059d6acd 100644 --- a/services/pull/merge_merge.go +++ b/services/pull/merge_merge.go @@ -4,6 +4,10 @@ package pull import ( + "context" + "strings" + + issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" @@ -23,3 +27,26 @@ func doMergeStyleMerge(ctx *mergeContext, message string) error { } return nil } + +// CalcMergeBase calculates the merge base for a pull request. +func CalcMergeBase(ctx context.Context, pr *issues_model.PullRequest) (string, error) { + repoPath := pr.BaseRepo.RepoPath() + if pr.HasMerged { + mergeBase, _, err := git.NewCommand("merge-base").AddDashesAndList(pr.MergedCommitID+"^", pr.GetGitHeadRefName()). + RunStdString(ctx, &git.RunOpts{Dir: repoPath}) + return strings.TrimSpace(mergeBase), err + } + + mergeBase, _, err := git.NewCommand("merge-base").AddDashesAndList(pr.BaseBranch, pr.GetGitHeadRefName()). + RunStdString(ctx, &git.RunOpts{Dir: repoPath}) + if err != nil { + var err2 error + mergeBase, _, err2 = git.NewCommand("rev-parse").AddDynamicArguments(git.BranchPrefix+pr.BaseBranch). + RunStdString(ctx, &git.RunOpts{Dir: repoPath}) + if err2 != nil { + log.Error("Unable to get merge base for PR ID %d, Index %d in %s/%s. Error: %v & %v", pr.ID, pr.Index, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err, err2) + return "", err2 + } + } + return strings.TrimSpace(mergeBase), nil +}