Use merge tree to detect conflicts when possible (#36400)

In Git 2.38, the `merge-tree` command introduced the `--write-tree`
option, which works directly on bare repositories. In Git 2.40, a new parameter `--merge-base` introduced so we require Git 2.40 to use the merge tree feature.

This option produces the merged tree object ID, allowing us to perform
diffs between commits without creating a temporary repository. By
avoiding the overhead of setting up and tearing down temporary repos,
this approach delivers a notable performance improvement.

It also fixes a possible situation that conflict files might be empty
but it's a conflict status according to
https://git-scm.com/docs/git-merge-tree#_mistakes_to_avoid

Replace #35542

---------

Signed-off-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Lunny Xiao
2026-01-27 11:57:20 -08:00
committed by GitHub
parent 125257eacf
commit 1463426a27
29 changed files with 607 additions and 126 deletions
+3 -3
View File
@@ -246,7 +246,7 @@ func isSignedIfRequired(ctx context.Context, pr *issues_model.PullRequest, doer
// markPullRequestAsMergeable checks if pull request is possible to leaving checking status,
// and set to be either conflict or mergeable.
func markPullRequestAsMergeable(ctx context.Context, pr *issues_model.PullRequest) {
// If the status has not been changed to conflict by testPullRequestTmpRepoBranchMergeable then we are mergeable
// If the status has not been changed to conflict by the conflict checking functions then we are mergeable
if pr.Status == issues_model.PullRequestStatusChecking {
pr.Status = issues_model.PullRequestStatusMergeable
}
@@ -442,8 +442,8 @@ func checkPullRequestMergeable(id int64) {
return
}
if err := testPullRequestBranchMergeable(pr); err != nil {
log.Error("testPullRequestTmpRepoBranchMergeable[%-v]: %v", pr, err)
if err := checkPullRequestBranchMergeable(ctx, pr); err != nil {
log.Error("checkPullRequestBranchMergeable[%-v]: %v", pr, err)
pr.Status = issues_model.PullRequestStatusError
if err := pr.UpdateCols(ctx, "status"); err != nil {
log.Error("update pr [%-v] status to PullRequestStatusError failed: %v", pr, err)
+4 -4
View File
@@ -366,11 +366,11 @@ func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *use
if err != nil {
return "", fmt.Errorf("Failed to get full commit id for HEAD: %w", err)
}
mergeBaseSHA, err := git.GetFullCommitID(ctx, mergeCtx.tmpBasePath, "original_"+baseBranch)
mergeBaseSHA, err := git.GetFullCommitID(ctx, mergeCtx.tmpBasePath, "original_"+tmpRepoBaseBranch)
if err != nil {
return "", fmt.Errorf("Failed to get full commit id for origin/%s: %w", pr.BaseBranch, err)
}
mergeCommitID, err := git.GetFullCommitID(ctx, mergeCtx.tmpBasePath, baseBranch)
mergeCommitID, err := git.GetFullCommitID(ctx, mergeCtx.tmpBasePath, tmpRepoBaseBranch)
if err != nil {
return "", fmt.Errorf("Failed to get full commit id for the new merge: %w", err)
}
@@ -407,7 +407,7 @@ func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *use
)
mergeCtx.env = append(mergeCtx.env, repo_module.EnvPushTrigger+"="+string(pushTrigger))
pushCmd := gitcmd.NewCommand("push", "origin").AddDynamicArguments(baseBranch + ":" + git.BranchPrefix + pr.BaseBranch)
pushCmd := gitcmd.NewCommand("push", "origin").AddDynamicArguments(tmpRepoBaseBranch + ":" + git.BranchPrefix + pr.BaseBranch)
// Push back to upstream.
// This cause an api call to "/api/internal/hook/post-receive/...",
@@ -717,7 +717,7 @@ func SetMerged(ctx context.Context, pr *issues_model.PullRequest, mergedCommitID
return false, fmt.Errorf("ChangeIssueStatus: %w", err)
}
// We need to save all of the data used to compute this merge as it may have already been changed by testPullRequestBranchMergeable. FIXME: need to set some state to prevent testPullRequestBranchMergeable from running whilst we are merging.
// We need to save all of the data used to compute this merge as it may have already been changed by checkPullRequestBranchMergeable. FIXME: need to set some state to prevent checkPullRequestBranchMergeable from running whilst we are merging.
if cnt, err := db.GetEngine(ctx).Where("id = ?", pr.ID).
And("has_merged = ?", false).
Cols("has_merged, status, merge_base, merged_commit_id, merger_id, merged_unix, conflicted_files").
+1 -1
View File
@@ -11,7 +11,7 @@ import (
// doMergeStyleFastForwardOnly merges the tracking into the current HEAD - which is assumed to be staging branch (equal to the pr.BaseBranch)
func doMergeStyleFastForwardOnly(ctx *mergeContext) error {
cmd := gitcmd.NewCommand("merge", "--ff-only").AddDynamicArguments(trackingBranch)
cmd := gitcmd.NewCommand("merge", "--ff-only").AddDynamicArguments(tmpRepoTrackingBranch)
if err := runMergeCommand(ctx, repo_model.MergeStyleFastForwardOnly, cmd); err != nil {
log.Error("%-v Unable to merge tracking into base: %v", ctx.pr, err)
return err
+1 -1
View File
@@ -11,7 +11,7 @@ import (
// doMergeStyleMerge merges the tracking branch into the current HEAD - which is assumed to be the staging branch (equal to the pr.BaseBranch)
func doMergeStyleMerge(ctx *mergeContext, message string) error {
cmd := gitcmd.NewCommand("merge", "--no-ff", "--no-commit").AddDynamicArguments(trackingBranch)
cmd := gitcmd.NewCommand("merge", "--no-ff", "--no-commit").AddDynamicArguments(tmpRepoTrackingBranch)
if err := runMergeCommand(ctx, repo_model.MergeStyleMerge, cmd); err != nil {
log.Error("%-v Unable to merge tracking into base: %v", ctx.pr, err)
return err
+7 -20
View File
@@ -5,7 +5,6 @@ package pull
import (
"bufio"
"bytes"
"context"
"fmt"
"io"
@@ -21,6 +20,7 @@ import (
"code.gitea.io/gitea/modules/git/gitcmd"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/util"
asymkey_service "code.gitea.io/gitea/services/asymkey"
)
@@ -76,7 +76,7 @@ func createTemporaryRepoForMerge(ctx context.Context, pr *issues_model.PullReque
if expectedHeadCommitID != "" {
trackingCommitID, _, err := gitcmd.NewCommand("show-ref", "--hash").
AddDynamicArguments(git.BranchPrefix + trackingBranch).
AddDynamicArguments(git.BranchPrefix + tmpRepoTrackingBranch).
WithEnv(mergeCtx.env).
WithDir(mergeCtx.tmpBasePath).
RunStdString(ctx)
@@ -152,8 +152,8 @@ func prepareTemporaryRepoForMerge(ctx *mergeContext) error {
}
defer sparseCheckoutListFile.Close() // we will close it earlier but we need to ensure it is closed if there is an error
if err := getDiffTree(ctx, ctx.tmpBasePath, baseBranch, trackingBranch, sparseCheckoutListFile); err != nil {
log.Error("%-v getDiffTree(%s, %s, %s): %v", ctx.pr, ctx.tmpBasePath, baseBranch, trackingBranch, err)
if err := getDiffTree(ctx, ctx.tmpBasePath, tmpRepoBaseBranch, tmpRepoTrackingBranch, sparseCheckoutListFile); err != nil {
log.Error("%-v getDiffTree(%s, %s, %s): %v", ctx.pr, ctx.tmpBasePath, tmpRepoBaseBranch, tmpRepoTrackingBranch, err)
return fmt.Errorf("getDiffTree: %w", err)
}
@@ -206,19 +206,6 @@ func prepareTemporaryRepoForMerge(ctx *mergeContext) error {
// getDiffTree returns a string containing all the files that were changed between headBranch and baseBranch
// the filenames are escaped so as to fit the format required for .git/info/sparse-checkout
func getDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string, out io.Writer) error {
scanNullTerminatedStrings := func(data []byte, atEOF bool) (advance int, token []byte, err error) {
if atEOF && len(data) == 0 {
return 0, nil, nil
}
if i := bytes.IndexByte(data, '\x00'); i >= 0 {
return i + 1, data[0:i], nil
}
if atEOF {
return len(data), data, nil
}
return 0, nil, nil
}
cmd := gitcmd.NewCommand("diff-tree", "--no-commit-id", "--name-only", "-r", "-r", "-z", "--root")
diffOutReader, diffOutReaderClose := cmd.MakeStdoutPipe()
defer diffOutReaderClose()
@@ -227,7 +214,7 @@ func getDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string, o
WithPipelineFunc(func(ctx gitcmd.Context) error {
// Now scan the output from the command
scanner := bufio.NewScanner(diffOutReader)
scanner.Split(scanNullTerminatedStrings)
scanner.Split(util.BufioScannerSplit(0))
for scanner.Scan() {
treePath := scanner.Text()
// escape '*', '?', '[', spaces and '!' prefix
@@ -266,14 +253,14 @@ func (err ErrRebaseConflicts) Error() string {
// if there is a conflict it will return an ErrRebaseConflicts
func rebaseTrackingOnToBase(ctx *mergeContext, mergeStyle repo_model.MergeStyle) error {
// Checkout head branch
if err := ctx.PrepareGitCmd(gitcmd.NewCommand("checkout", "-b").AddDynamicArguments(stagingBranch, trackingBranch)).
if err := ctx.PrepareGitCmd(gitcmd.NewCommand("checkout", "-b").AddDynamicArguments(tmpRepoStagingBranch, tmpRepoTrackingBranch)).
RunWithStderr(ctx); err != nil {
return fmt.Errorf("unable to git checkout tracking as staging in temp repo for %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), err.Stderr())
}
ctx.outbuf.Reset()
// Rebase before merging
if err := ctx.PrepareGitCmd(gitcmd.NewCommand("rebase").AddDynamicArguments(baseBranch)).
if err := ctx.PrepareGitCmd(gitcmd.NewCommand("rebase").AddDynamicArguments(tmpRepoBaseBranch)).
RunWithStderr(ctx); err != nil {
// Rebase will leave a REBASE_HEAD file in .git if there is a conflict
if _, statErr := os.Stat(filepath.Join(ctx.tmpBasePath, ".git", "REBASE_HEAD")); statErr == nil {
+3 -3
View File
@@ -43,7 +43,7 @@ func doMergeRebaseFastForward(ctx *mergeContext) error {
return fmt.Errorf("Failed to get full commit id for HEAD: %w", err)
}
cmd := gitcmd.NewCommand("merge", "--ff-only").AddDynamicArguments(stagingBranch)
cmd := gitcmd.NewCommand("merge", "--ff-only").AddDynamicArguments(tmpRepoStagingBranch)
if err := runMergeCommand(ctx, repo_model.MergeStyleRebase, cmd); err != nil {
log.Error("Unable to merge staging into base: %v", err)
return err
@@ -88,7 +88,7 @@ func doMergeRebaseFastForward(ctx *mergeContext) error {
// Perform rebase merge with merge commit.
func doMergeRebaseMergeCommit(ctx *mergeContext, message string) error {
cmd := gitcmd.NewCommand("merge").AddArguments("--no-ff", "--no-commit").AddDynamicArguments(stagingBranch)
cmd := gitcmd.NewCommand("merge").AddArguments("--no-ff", "--no-commit").AddDynamicArguments(tmpRepoStagingBranch)
if err := runMergeCommand(ctx, repo_model.MergeStyleRebaseMerge, cmd); err != nil {
log.Error("Unable to merge staging into base: %v", err)
@@ -109,7 +109,7 @@ func doMergeStyleRebase(ctx *mergeContext, mergeStyle repo_model.MergeStyle, mes
}
// Checkout base branch again
if err := ctx.PrepareGitCmd(gitcmd.NewCommand("checkout").AddDynamicArguments(baseBranch)).
if err := ctx.PrepareGitCmd(gitcmd.NewCommand("checkout").AddDynamicArguments(tmpRepoBaseBranch)).
RunWithStderr(ctx); err != nil {
log.Error("git checkout base prior to merge post staging rebase %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), err.Stderr())
return fmt.Errorf("git checkout base prior to merge post staging rebase %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), err.Stderr())
+3 -3
View File
@@ -32,9 +32,9 @@ func getAuthorSignatureSquash(ctx *mergeContext) (*git.Signature, error) {
}
defer gitRepo.Close()
commits, err := gitRepo.CommitsBetweenIDs(trackingBranch, "HEAD")
commits, err := gitRepo.CommitsBetweenIDs(tmpRepoTrackingBranch, "HEAD")
if err != nil {
log.Error("%-v Unable to get commits between: %s %s: %v", ctx.pr, "HEAD", trackingBranch, err)
log.Error("%-v Unable to get commits between: %s %s: %v", ctx.pr, "HEAD", tmpRepoTrackingBranch, err)
return nil, err
}
@@ -58,7 +58,7 @@ func doMergeStyleSquash(ctx *mergeContext, message string) error {
return fmt.Errorf("getAuthorSignatureSquash: %w", err)
}
cmdMerge := gitcmd.NewCommand("merge", "--squash").AddDynamicArguments(trackingBranch)
cmdMerge := gitcmd.NewCommand("merge", "--squash").AddDynamicArguments(tmpRepoTrackingBranch)
if err := runMergeCommand(ctx, repo_model.MergeStyleSquash, cmdMerge); err != nil {
log.Error("%-v Unable to merge --squash tracking into base: %v", ctx.pr, err)
return err
+144
View File
@@ -0,0 +1,144 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package pull
import (
"context"
"errors"
"fmt"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/git/gitcmd"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/log"
)
// checkConflictsMergeTree uses git merge-tree to check for conflicts and if none are found checks if the patch is empty
// return true if there are conflicts otherwise return false
// pr.Status and pr.ConflictedFiles will be updated as necessary
func checkConflictsMergeTree(ctx context.Context, pr *issues_model.PullRequest, baseCommitID string) (bool, error) {
treeHash, conflict, conflictFiles, err := gitrepo.MergeTree(ctx, pr.BaseRepo, baseCommitID, pr.HeadCommitID, pr.MergeBase)
if err != nil {
return false, fmt.Errorf("MergeTree: %w", err)
}
if conflict {
pr.Status = issues_model.PullRequestStatusConflict
// sometimes git merge-tree will detect conflicts but not list any conflicted files
// so that pr.ConflictedFiles will be empty
pr.ConflictedFiles = conflictFiles
log.Trace("Found %d files conflicted: %v", len(pr.ConflictedFiles), pr.ConflictedFiles)
return true, nil
}
// Detect whether the pull request introduces changes by comparing the merged tree (treeHash)
// against the current base commit (baseCommitID) using `git diff-tree`. The command returns exit code 0
// if there is no diff between these trees (empty patch) and exit code 1 if there is a diff.
gitErr := gitrepo.RunCmd(ctx, pr.BaseRepo, gitcmd.NewCommand("diff-tree", "-r", "--quiet").
AddDynamicArguments(treeHash, baseCommitID))
switch {
case gitErr == nil:
log.Debug("PullRequest[%d]: Patch is empty - ignoring", pr.ID)
pr.Status = issues_model.PullRequestStatusEmpty
case gitcmd.IsErrorExitCode(gitErr, 1):
pr.Status = issues_model.PullRequestStatusMergeable
default:
return false, fmt.Errorf("run diff-tree exit abnormally: %w", gitErr)
}
return false, nil
}
func checkPullRequestMergeableByMergeTree(ctx context.Context, pr *issues_model.PullRequest) error {
// 1. Get head commit
if err := pr.LoadHeadRepo(ctx); err != nil {
return err
}
headGitRepo, err := gitrepo.OpenRepository(ctx, pr.HeadRepo)
if err != nil {
return fmt.Errorf("OpenRepository: %w", err)
}
defer headGitRepo.Close()
// 2. Get/open base repository
var baseGitRepo *git.Repository
if pr.IsSameRepo() {
baseGitRepo = headGitRepo
} else {
baseGitRepo, err = gitrepo.OpenRepository(ctx, pr.BaseRepo)
if err != nil {
return fmt.Errorf("OpenRepository: %w", err)
}
defer baseGitRepo.Close()
}
// 3. Get head commit id
if pr.Flow == issues_model.PullRequestFlowGithub {
pr.HeadCommitID, err = headGitRepo.GetRefCommitID(git.BranchPrefix + pr.HeadBranch)
if err != nil {
return fmt.Errorf("GetBranchCommitID: can't find commit ID for head: %w", err)
}
} else {
if pr.ID > 0 {
pr.HeadCommitID, err = baseGitRepo.GetRefCommitID(pr.GetGitHeadRefName())
if err != nil {
return fmt.Errorf("GetRefCommitID: can't find commit ID for head: %w", err)
}
} else if pr.HeadCommitID == "" { // for new pull request with agit, the head commit id must be provided
return errors.New("head commit ID is empty for pull request Agit flow")
}
}
// 4. fetch head commit id into the current repository
// it will be checked in 2 weeks by default from git if the pull request created failure.
if !pr.IsSameRepo() {
if !baseGitRepo.IsReferenceExist(pr.HeadCommitID) {
if err := gitrepo.FetchRemoteCommit(ctx, pr.BaseRepo, pr.HeadRepo, pr.HeadCommitID); err != nil {
return fmt.Errorf("FetchRemoteCommit: %w", err)
}
}
}
// 5. update merge base
baseCommitID, err := baseGitRepo.GetRefCommitID(git.BranchPrefix + pr.BaseBranch)
if err != nil {
return fmt.Errorf("GetBranchCommitID: can't find commit ID for base: %w", err)
}
pr.MergeBase, err = gitrepo.MergeBase(ctx, pr.BaseRepo, baseCommitID, pr.HeadCommitID)
if err != nil {
// if there is no merge base, then it's empty, still need to allow the pull request to be created
// not quite right (e.g.: why not reset the fields like below), but no interest to do more investigation at the moment
log.Error("MergeBase: unable to find merge base between %s and %s: %v", baseCommitID, pr.HeadCommitID, err)
pr.Status = issues_model.PullRequestStatusEmpty
return nil
}
// reset conflicted files and changed protected files
pr.ConflictedFiles = nil
pr.ChangedProtectedFiles = nil
// 6. if base == head, then it's an ancestor
if pr.HeadCommitID == pr.MergeBase {
pr.Status = issues_model.PullRequestStatusAncestor
return nil
}
// 7. Check for conflicts
conflicted, err := checkConflictsMergeTree(ctx, pr, baseCommitID)
if err != nil {
log.Error("checkConflictsMergeTree: %v", err)
pr.Status = issues_model.PullRequestStatusError
return fmt.Errorf("checkConflictsMergeTree: %w", err)
}
if conflicted || pr.Status == issues_model.PullRequestStatusEmpty {
return nil
}
// 8. Check for protected files changes
if err = checkPullFilesProtection(ctx, pr, baseGitRepo, pr.HeadCommitID); err != nil {
return fmt.Errorf("checkPullFilesProtection: %w", err)
}
return nil
}
+154
View File
@@ -0,0 +1,154 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package pull
import (
"context"
"fmt"
"testing"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/git/gitcmd"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func testPullRequestMergeCheck(t *testing.T,
targetFunc func(ctx context.Context, pr *issues_model.PullRequest) error,
pr *issues_model.PullRequest,
expectedStatus issues_model.PullRequestStatus,
expectedConflictedFiles []string,
expectedChangedProtectedFiles []string,
) {
assert.NoError(t, pr.LoadIssue(t.Context()))
assert.NoError(t, pr.LoadBaseRepo(t.Context()))
assert.NoError(t, pr.LoadHeadRepo(t.Context()))
pr.Status = issues_model.PullRequestStatusChecking
pr.ConflictedFiles = []string{"unrelated-conflicted-file"}
pr.ChangedProtectedFiles = []string{"unrelated-protected-file"}
pr.MergeBase = ""
pr.HeadCommitID = ""
err := targetFunc(t.Context(), pr)
require.NoError(t, err)
assert.Equal(t, expectedStatus, pr.Status)
assert.Equal(t, expectedConflictedFiles, pr.ConflictedFiles)
assert.Equal(t, expectedChangedProtectedFiles, pr.ChangedProtectedFiles)
assert.NotEmpty(t, pr.MergeBase)
assert.NotEmpty(t, pr.HeadCommitID)
}
func TestPullRequestMergeable(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
t.Run("NoConflict-MergeTree", func(t *testing.T) {
testPullRequestMergeCheck(t, checkPullRequestMergeableByMergeTree, pr, issues_model.PullRequestStatusMergeable, nil, nil)
})
t.Run("NoConflict-TmpRepo", func(t *testing.T) {
testPullRequestMergeCheck(t, checkPullRequestMergeableByTmpRepo, pr, issues_model.PullRequestStatusMergeable, nil, nil)
})
pr.BaseBranch, pr.HeadBranch = "test-merge-tree-conflict-base", "test-merge-tree-conflict-head"
conflictFiles := createConflictBranches(t, pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.HeadBranch)
t.Run("Conflict-MergeTree", func(t *testing.T) {
testPullRequestMergeCheck(t, checkPullRequestMergeableByMergeTree, pr, issues_model.PullRequestStatusConflict, conflictFiles, nil)
})
t.Run("Conflict-TmpRepo", func(t *testing.T) {
testPullRequestMergeCheck(t, checkPullRequestMergeableByTmpRepo, pr, issues_model.PullRequestStatusConflict, conflictFiles, nil)
})
pr.BaseBranch, pr.HeadBranch = "test-merge-tree-empty-base", "test-merge-tree-empty-head"
createEmptyBranches(t, pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.HeadBranch)
t.Run("Empty-MergeTree", func(t *testing.T) {
testPullRequestMergeCheck(t, checkPullRequestMergeableByMergeTree, pr, issues_model.PullRequestStatusEmpty, nil, nil)
})
t.Run("Empty-TmpRepo", func(t *testing.T) {
testPullRequestMergeCheck(t, checkPullRequestMergeableByTmpRepo, pr, issues_model.PullRequestStatusEmpty, nil, nil)
})
}
func createConflictBranches(t *testing.T, repoPath, baseBranch, headBranch string) []string {
conflictFile := "conflict.txt"
stdin := fmt.Sprintf(
`reset refs/heads/%[1]s
from refs/heads/master
commit refs/heads/%[1]s
mark :1
committer Test <test@example.com> 0 +0000
data 17
add conflict file
M 100644 inline %[3]s
data 4
base
commit refs/heads/%[1]s
mark :2
committer Test <test@example.com> 0 +0000
data 11
base change
from :1
M 100644 inline %[3]s
data 11
base change
reset refs/heads/%[2]s
from :1
commit refs/heads/%[2]s
mark :3
committer Test <test@example.com> 0 +0000
data 11
head change
from :1
M 100644 inline %[3]s
data 11
head change
`, baseBranch, headBranch, conflictFile)
err := gitcmd.NewCommand("fast-import").WithDir(repoPath).WithStdinBytes([]byte(stdin)).RunWithStderr(t.Context())
require.NoError(t, err)
return []string{conflictFile}
}
func createEmptyBranches(t *testing.T, repoPath, baseBranch, headBranch string) {
emptyFile := "empty.txt"
stdin := fmt.Sprintf(`reset refs/heads/%[1]s
from refs/heads/master
commit refs/heads/%[1]s
mark :1
committer Test <test@example.com> 0 +0000
data 14
add empty file
M 100644 inline %[3]s
data 4
base
reset refs/heads/%[2]s
from :1
commit refs/heads/%[2]s
mark :2
committer Test <test@example.com> 0 +0000
data 17
change empty file
from :1
M 100644 inline %[3]s
data 6
change
commit refs/heads/%[2]s
mark :3
committer Test <test@example.com> 0 +0000
data 17
revert empty file
from :2
M 100644 inline %[3]s
data 4
base
`, baseBranch, headBranch, emptyFile)
err := gitcmd.NewCommand("fast-import").WithDir(repoPath).WithStdinBytes([]byte(stdin)).RunWithStderr(t.Context())
require.NoError(t, err)
}
+35 -27
View File
@@ -21,7 +21,6 @@ import (
"code.gitea.io/gitea/modules/git/gitcmd"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/glob"
"code.gitea.io/gitea/modules/graceful"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/process"
"code.gitea.io/gitea/modules/setting"
@@ -67,10 +66,18 @@ var patchErrorSuffices = []string{
": does not exist in index",
}
func testPullRequestBranchMergeable(pr *issues_model.PullRequest) error {
ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("testPullRequestBranchMergeable: %s", pr))
func checkPullRequestBranchMergeable(ctx context.Context, pr *issues_model.PullRequest) error {
ctx, _, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("checkPullRequestBranchMergeable: %s", pr))
defer finished()
if git.DefaultFeatures().SupportGitMergeTree {
return checkPullRequestMergeableByMergeTree(ctx, pr)
}
return checkPullRequestMergeableByTmpRepo(ctx, pr)
}
func checkPullRequestMergeableByTmpRepo(ctx context.Context, pr *issues_model.PullRequest) error {
prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr)
if err != nil {
if !git_model.IsErrBranchNotExist(err) {
@@ -80,10 +87,6 @@ func testPullRequestBranchMergeable(pr *issues_model.PullRequest) error {
}
defer cancel()
return testPullRequestTmpRepoBranchMergeable(ctx, prCtx, pr)
}
func testPullRequestTmpRepoBranchMergeable(ctx context.Context, prCtx *prTmpRepoContext, pr *issues_model.PullRequest) error {
gitRepo, err := git.OpenRepository(ctx, prCtx.tmpBasePath)
if err != nil {
return fmt.Errorf("OpenRepository: %w", err)
@@ -91,16 +94,16 @@ func testPullRequestTmpRepoBranchMergeable(ctx context.Context, prCtx *prTmpRepo
defer gitRepo.Close()
// 1. update merge base
pr.MergeBase, _, err = gitcmd.NewCommand("merge-base", "--", "base", "tracking").WithDir(prCtx.tmpBasePath).RunStdString(ctx)
pr.MergeBase, _, err = gitcmd.NewCommand("merge-base", "--", tmpRepoBaseBranch, tmpRepoTrackingBranch).WithDir(prCtx.tmpBasePath).RunStdString(ctx)
if err != nil {
var err2 error
pr.MergeBase, err2 = gitRepo.GetRefCommitID(git.BranchPrefix + "base")
pr.MergeBase, err2 = gitRepo.GetRefCommitID(git.BranchPrefix + tmpRepoBaseBranch)
if err2 != nil {
return fmt.Errorf("GetMergeBase: %v and can't find commit ID for base: %w", err, err2)
}
}
pr.MergeBase = strings.TrimSpace(pr.MergeBase)
if pr.HeadCommitID, err = gitRepo.GetRefCommitID(git.BranchPrefix + "tracking"); err != nil {
if pr.HeadCommitID, err = gitRepo.GetRefCommitID(git.BranchPrefix + tmpRepoTrackingBranch); err != nil {
return fmt.Errorf("GetBranchCommitID: can't find commit ID for head: %w", err)
}
@@ -110,17 +113,19 @@ func testPullRequestTmpRepoBranchMergeable(ctx context.Context, prCtx *prTmpRepo
}
// 2. Check for conflicts
if conflicts, err := checkConflicts(ctx, pr, gitRepo, prCtx.tmpBasePath); err != nil || conflicts || pr.Status == issues_model.PullRequestStatusEmpty {
conflicts, err := checkConflictsByTmpRepo(ctx, pr, gitRepo, prCtx.tmpBasePath)
if err != nil {
return err
}
// 3. Check for protected files changes
if err = checkPullFilesProtection(ctx, pr, gitRepo); err != nil {
return fmt.Errorf("pr.CheckPullFilesProtection(): %v", err)
pr.ChangedProtectedFiles = nil
if conflicts || pr.Status == issues_model.PullRequestStatusEmpty {
return nil
}
if len(pr.ChangedProtectedFiles) > 0 {
log.Trace("Found %d protected files changed", len(pr.ChangedProtectedFiles))
// 3. Check for protected files changes
if err = checkPullFilesProtection(ctx, pr, gitRepo, tmpRepoTrackingBranch); err != nil {
return fmt.Errorf("pr.CheckPullFilesProtection(): %w", err)
}
pr.Status = issues_model.PullRequestStatusMergeable
@@ -307,14 +312,14 @@ func AttemptThreeWayMerge(ctx context.Context, gitPath string, gitRepo *git.Repo
return conflict, conflictedFiles, nil
}
func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo *git.Repository, tmpBasePath string) (bool, error) {
// 1. checkConflicts resets the conflict status - therefore - reset the conflict status
func checkConflictsByTmpRepo(ctx context.Context, pr *issues_model.PullRequest, gitRepo *git.Repository, tmpBasePath string) (bool, error) {
// 1. checkConflictsByTmpRepo resets the conflict status - therefore - reset the conflict status
pr.ConflictedFiles = nil
// 2. AttemptThreeWayMerge first - this is much quicker than plain patch to base
description := fmt.Sprintf("PR[%d] %s/%s#%d", pr.ID, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, pr.Index)
conflict, conflictFiles, err := AttemptThreeWayMerge(ctx,
tmpBasePath, gitRepo, pr.MergeBase, "base", "tracking", description)
tmpBasePath, gitRepo, pr.MergeBase, tmpRepoBaseBranch, tmpRepoTrackingBranch, description)
if err != nil {
return false, err
}
@@ -329,7 +334,7 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo *
return false, fmt.Errorf("unable to write unconflicted tree: %w\n`git ls-files -u`:\n%s", err, lsfiles)
}
treeHash = strings.TrimSpace(treeHash)
baseTree, err := gitRepo.GetTree("base")
baseTree, err := gitRepo.GetTree(tmpRepoBaseBranch)
if err != nil {
return false, err
}
@@ -379,10 +384,10 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo *
return false, nil
}
log.Trace("PullRequest[%d].testPullRequestTmpRepoBranchMergeable (patchPath): %s", pr.ID, patchPath)
log.Trace("PullRequest[%d].checkPullRequestMergeableByTmpRepo (patchPath): %s", pr.ID, patchPath)
// 4. Read the base branch in to the index of the temporary repository
_, _, err = gitcmd.NewCommand("read-tree", "base").WithDir(tmpBasePath).RunStdString(ctx)
_, _, err = gitcmd.NewCommand("read-tree", tmpRepoBaseBranch).WithDir(tmpBasePath).RunStdString(ctx)
if err != nil {
return false, fmt.Errorf("git read-tree %s: %w", pr.BaseBranch, err)
}
@@ -434,7 +439,7 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo *
scanner := bufio.NewScanner(stderrReader)
for scanner.Scan() {
line := scanner.Text()
log.Trace("PullRequest[%d].testPullRequestTmpRepoBranchMergeable: stderr: %s", pr.ID, line)
log.Trace("PullRequest[%d].checkPullRequestMergeableByTmpRepo: stderr: %s", pr.ID, line)
if strings.HasPrefix(line, prefix) {
conflict = true
filepath := strings.TrimSpace(strings.Split(line[len(prefix):], ":")[0])
@@ -459,8 +464,8 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo *
conflicts.Add(filepath)
}
}
// only list 10 conflicted files
if len(conflicts) >= 10 {
// only list part of conflicted files
if len(conflicts) >= gitrepo.MaxConflictedDetectFiles {
break
}
}
@@ -570,7 +575,7 @@ func CheckUnprotectedFiles(repo *git.Repository, branchName, oldCommitID, newCom
}
// checkPullFilesProtection check if pr changed protected files and save results
func checkPullFilesProtection(ctx context.Context, pr *issues_model.PullRequest, gitRepo *git.Repository) error {
func checkPullFilesProtection(ctx context.Context, pr *issues_model.PullRequest, gitRepo *git.Repository, headRef string) error {
if pr.Status == issues_model.PullRequestStatusEmpty {
pr.ChangedProtectedFiles = nil
return nil
@@ -586,9 +591,12 @@ func checkPullFilesProtection(ctx context.Context, pr *issues_model.PullRequest,
return nil
}
pr.ChangedProtectedFiles, err = CheckFileProtection(gitRepo, pr.HeadBranch, pr.MergeBase, "tracking", pb.GetProtectedFilePatterns(), 10, os.Environ())
pr.ChangedProtectedFiles, err = CheckFileProtection(gitRepo, pr.HeadBranch, pr.MergeBase, headRef, pb.GetProtectedFilePatterns(), 10, os.Environ())
if err != nil && !IsErrFilePathProtected(err) {
return err
}
if len(pr.ChangedProtectedFiles) > 0 {
log.Trace("Found %d protected files changed in PR %s#%d", len(pr.ChangedProtectedFiles), pr.BaseRepo.FullName(), pr.Index)
}
return nil
}
+1 -1
View File
@@ -63,7 +63,7 @@ func readUnmergedLsFileLines(ctx context.Context, tmpBasePath string, outputChan
lsFilesReader, lsFilesReaderClose := cmd.MakeStdoutPipe()
defer lsFilesReaderClose()
err := cmd.WithDir(tmpBasePath).
WithPipelineFunc(func(_ gitcmd.Context) error {
WithPipelineFunc(func(gitcmd.Context) error {
bufferedReader := bufio.NewReader(lsFilesReader)
for {
+6 -11
View File
@@ -90,16 +90,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
}
}
prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr)
if err != nil {
if !git_model.IsErrBranchNotExist(err) {
log.Error("CreateTemporaryRepoForPR %-v: %v", pr, err)
}
return err
}
defer cancel()
if err := testPullRequestTmpRepoBranchMergeable(ctx, prCtx, pr); err != nil {
if err := checkPullRequestBranchMergeable(ctx, pr); err != nil {
return err
}
@@ -128,6 +119,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
pr.Issue = issue
issue.PullRequest = pr
var err error
if pr.Flow == issues_model.PullRequestFlowGithub {
err = PushToBaseRepo(ctx, pr)
} else {
@@ -168,6 +160,9 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
// Request reviews, these should be requested before other notifications because they will add request reviews record
// on database
permDoer, err := access_model.GetUserRepoPermission(ctx, repo, issue.Poster)
if err != nil {
return err
}
for _, reviewer := range opts.Reviewers {
if _, err = issue_service.ReviewRequest(ctx, pr.Issue, issue.Poster, &permDoer, reviewer, true); err != nil {
return err
@@ -301,7 +296,7 @@ func ChangeTargetBranch(ctx context.Context, pr *issues_model.PullRequest, doer
pr.BaseBranch = targetBranch
// Refresh patch
if err := testPullRequestBranchMergeable(pr); err != nil {
if err := checkPullRequestBranchMergeable(ctx, pr); err != nil {
return err
}
+6 -8
View File
@@ -23,9 +23,9 @@ import (
// Temporary repos created here use standard branch names to help simplify
// merging code
const (
baseBranch = "base" // equivalent to pr.BaseBranch
trackingBranch = "tracking" // equivalent to pr.HeadBranch
stagingBranch = "staging" // this is used for a working branch
tmpRepoBaseBranch = "base" // equivalent to pr.BaseBranch
tmpRepoTrackingBranch = "tracking" // equivalent to pr.HeadBranch
tmpRepoStagingBranch = "staging" // this is used for a working branch
)
type prTmpRepoContext struct {
@@ -95,7 +95,6 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest)
}
remoteRepoName := "head_repo"
baseBranch := "base"
fetchArgs := gitcmd.TrustedCmdArgs{"--no-tags"}
if git.DefaultFeatures().CheckVersionAtLeast("2.25.0") {
@@ -135,14 +134,14 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest)
}
if err := prCtx.PrepareGitCmd(gitcmd.NewCommand("fetch", "origin").AddArguments(fetchArgs...).
AddDashesAndList(git.BranchPrefix+pr.BaseBranch+":"+git.BranchPrefix+baseBranch, git.BranchPrefix+pr.BaseBranch+":"+git.BranchPrefix+"original_"+baseBranch)).
AddDashesAndList(git.BranchPrefix+pr.BaseBranch+":"+git.BranchPrefix+tmpRepoBaseBranch, git.BranchPrefix+pr.BaseBranch+":"+git.BranchPrefix+"original_"+tmpRepoBaseBranch)).
RunWithStderr(ctx); err != nil {
log.Error("%-v Unable to fetch origin base branch [%s:%s -> base, original_base in %s]: %v:\n%s\n%s", pr, pr.BaseRepo.FullName(), pr.BaseBranch, tmpBasePath, err, prCtx.outbuf.String(), err.Stderr())
cancel()
return nil, nil, fmt.Errorf("Unable to fetch origin base branch [%s:%s -> base, original_base in tmpBasePath]: %w\n%s\n%s", pr.BaseRepo.FullName(), pr.BaseBranch, err, prCtx.outbuf.String(), err.Stderr())
}
if err := prCtx.PrepareGitCmd(gitcmd.NewCommand("symbolic-ref").AddDynamicArguments("HEAD", git.BranchPrefix+baseBranch)).
if err := prCtx.PrepareGitCmd(gitcmd.NewCommand("symbolic-ref").AddDynamicArguments("HEAD", git.BranchPrefix+tmpRepoBaseBranch)).
RunWithStderr(ctx); err != nil {
log.Error("%-v Unable to set HEAD as base branch in [%s]: %v\n%s\n%s", pr, tmpBasePath, err, prCtx.outbuf.String(), err.Stderr())
cancel()
@@ -162,7 +161,6 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest)
return nil, nil, fmt.Errorf("Unable to add head repository as head_repo [%s -> tmpBasePath]: %w\n%s\n%s", pr.HeadRepo.FullName(), err, prCtx.outbuf.String(), err.Stderr())
}
trackingBranch := "tracking"
objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName)
// Fetch head branch
var headBranch string
@@ -173,7 +171,7 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest)
} else {
headBranch = pr.GetGitHeadRefName()
}
if err := prCtx.PrepareGitCmd(gitcmd.NewCommand("fetch").AddArguments(fetchArgs...).AddDynamicArguments(remoteRepoName, headBranch+":"+trackingBranch)).
if err := prCtx.PrepareGitCmd(gitcmd.NewCommand("fetch").AddArguments(fetchArgs...).AddDynamicArguments(remoteRepoName, headBranch+":"+tmpRepoTrackingBranch)).
RunWithStderr(ctx); err != nil {
cancel()
if exist, _ := git_model.IsBranchExist(ctx, pr.HeadRepo.ID, pr.HeadBranch); !exist {
+4 -4
View File
@@ -28,7 +28,7 @@ func updateHeadByRebaseOnToBase(ctx context.Context, pr *issues_model.PullReques
defer cancel()
// Determine the old merge-base before the rebase - we use this for LFS push later on
oldMergeBase, _, _ := gitcmd.NewCommand("merge-base").AddDashesAndList(baseBranch, trackingBranch).
oldMergeBase, _, _ := gitcmd.NewCommand("merge-base").AddDashesAndList(tmpRepoBaseBranch, tmpRepoTrackingBranch).
WithDir(mergeCtx.tmpBasePath).RunStdString(ctx)
oldMergeBase = strings.TrimSpace(oldMergeBase)
@@ -42,11 +42,11 @@ func updateHeadByRebaseOnToBase(ctx context.Context, pr *issues_model.PullReques
// It's questionable about where this should go - either after or before the push
// I think in the interests of data safety - failures to push to the lfs should prevent
// the push as you can always re-rebase.
if err := LFSPush(ctx, mergeCtx.tmpBasePath, baseBranch, oldMergeBase, &issues_model.PullRequest{
if err := LFSPush(ctx, mergeCtx.tmpBasePath, tmpRepoBaseBranch, oldMergeBase, &issues_model.PullRequest{
HeadRepoID: pr.BaseRepoID,
BaseRepoID: pr.HeadRepoID,
}); err != nil {
log.Error("Unable to push lfs objects between %s and %s up to head branch in %-v: %v", baseBranch, oldMergeBase, pr, err)
log.Error("Unable to push lfs objects between %s and %s up to head branch in %-v: %v", tmpRepoBaseBranch, oldMergeBase, pr, err)
return err
}
}
@@ -65,7 +65,7 @@ func updateHeadByRebaseOnToBase(ctx context.Context, pr *issues_model.PullReques
}
pushCmd := gitcmd.NewCommand("push", "-f", "head_repo").
AddDynamicArguments(stagingBranch + ":" + git.BranchPrefix + pr.HeadBranch)
AddDynamicArguments(tmpRepoStagingBranch + ":" + git.BranchPrefix + pr.HeadBranch)
// Push back to the head repository.
// TODO: this cause an api call to "/api/internal/hook/post-receive/...",
+2 -2
View File
@@ -81,7 +81,7 @@ func GetCommitGraph(r *git.Repository, page, maxAllowedColors int, hidePRRefs bo
line := scanner.Bytes()
if bytes.IndexByte(line, '*') >= 0 {
if err := parser.AddLineToGraph(graph, row, line); err != nil {
return ctx.CancelWithCause(err)
return ctx.CancelPipeline(err)
}
break
}
@@ -92,7 +92,7 @@ func GetCommitGraph(r *git.Repository, page, maxAllowedColors int, hidePRRefs bo
row++
line := scanner.Bytes()
if err := parser.AddLineToGraph(graph, row, line); err != nil {
return ctx.CancelWithCause(err)
return ctx.CancelPipeline(err)
}
}
return scanner.Err()