improvements

Author: lunny

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
-}

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")

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.GetGitRefName())
-	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.GetGitRefName(), 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 {

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.GetGitRefName()).
+			RunStdString(ctx, &git.RunOpts{Dir: repoPath})
+		return strings.TrimSpace(mergeBase), err
+	}
+
+	mergeBase, _, err := git.NewCommand("merge-base").AddDashesAndList(pr.BaseBranch, pr.GetGitRefName()).
+		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
+}