diff --git a/models/issues/pull.go b/models/issues/pull.go index 1ffcd683d5..18977ed212 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -666,9 +666,10 @@ func HasWorkInProgressPrefix(title string) bool { return false } -// IsFilesConflicted determines if the Pull Request has changes conflicting with the target branch. +// IsFilesConflicted determines if the Pull Request has changes conflicting with the target branch. +// Sometimes a conflict may not list any files func (pr *PullRequest) IsFilesConflicted() bool { - return len(pr.ConflictedFiles) > 0 + return pr.Status == PullRequestStatusConflict } // GetWorkInProgressPrefix returns the prefix used to mark the pull request as a work in progress. diff --git a/modules/git/git.go b/modules/git/git.go index 25b1955002..37371ac59f 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -32,6 +32,7 @@ type Features struct { SupportedObjectFormats []ObjectFormat // sha1, sha256 SupportCheckAttrOnBare bool // >= 2.40 SupportCatFileBatchCommand bool // >= 2.36, support `git cat-file --batch-command` + SupportGitMergeTree bool // >= 2.40 // we also need "--merge-base" } var defaultFeatures *Features @@ -77,6 +78,7 @@ func loadGitVersionFeatures() (*Features, error) { } features.SupportCheckAttrOnBare = features.CheckVersionAtLeast("2.40") features.SupportCatFileBatchCommand = features.CheckVersionAtLeast("2.36") + features.SupportGitMergeTree = features.CheckVersionAtLeast("2.40") // we also need "--merge-base" return features, nil } diff --git a/modules/git/gitcmd/command.go b/modules/git/gitcmd/command.go index 413b53299a..f780cdf6c9 100644 --- a/modules/git/gitcmd/command.go +++ b/modules/git/gitcmd/command.go @@ -45,6 +45,7 @@ type Command struct { cmdStartTime time.Time parentPipeFiles []*os.File + parentPipeReaders []*os.File childrenPipeFiles []*os.File // only os.Pipe and in-memory buffers can work with Stdin safely, see https://github.com/golang/go/issues/77227 if the command would exit unexpectedly @@ -283,6 +284,7 @@ func (c *Command) makeStdoutStderr(w *io.Writer) (PipeReader, func()) { } c.childrenPipeFiles = append(c.childrenPipeFiles, pw) c.parentPipeFiles = append(c.parentPipeFiles, pr) + c.parentPipeReaders = append(c.parentPipeReaders, pr) *w /* stdout, stderr */ = pw return &pipeReader{f: pr}, func() { pr.Close() } } @@ -348,7 +350,13 @@ func (c *Command) WithStdoutCopy(w io.Writer) *Command { return c } -func (c *Command) WithPipelineFunc(f func(Context) error) *Command { +// WithPipelineFunc sets the pipeline function for the command. +// The pipeline function will be called in the Run / Wait function after the command is started successfully. +// The function can read/write from/to the command's stdio pipes (if any). +// The pipeline function can cancel (kill) the command by calling ctx.CancelPipeline before the command finishes. +// The returned error of Run / Wait can be joined errors from the pipeline function, context cause, and command exit error. +// Caller can get the pipeline function's error (if any) by UnwrapPipelineError. +func (c *Command) WithPipelineFunc(f func(ctx Context) error) *Command { c.opts.PipelineFunc = f return c } @@ -444,6 +452,12 @@ func (c *Command) closePipeFiles(files []*os.File) { } } +func (c *Command) discardPipeReaders(files []*os.File) { + for _, f := range files { + _, _ = io.Copy(io.Discard, f) + } +} + func (c *Command) Wait() error { defer func() { // The reader in another goroutine might be still reading the stdout, so we shouldn't close the pipes here @@ -454,15 +468,31 @@ func (c *Command) Wait() error { if c.opts.PipelineFunc != nil { errPipeline := c.opts.PipelineFunc(&cmdContext{Context: c.cmdCtx, cmd: c}) - // after the pipeline function returns, we can safely cancel the command context and close the pipes, the data in pipes should have been consumed - c.cmdCancel(errPipeline) + + if context.Cause(c.cmdCtx) == nil { + // if the context is not canceled explicitly, we need to discard the unread data, + // and wait for the command to exit normally, and then get its exit code + c.discardPipeReaders(c.parentPipeReaders) + } // else: canceled command will be killed, and the exit code is caused by kill + + // after the pipeline function returns, we can safely close the pipes, then wait for the command to exit c.closePipeFiles(c.parentPipeFiles) errWait := c.cmd.Wait() - errCause := context.Cause(c.cmdCtx) - // the pipeline function should be able to know whether it succeeds or fails - if errPipeline == nil && (errCause == nil || errors.Is(errCause, context.Canceled)) { - return nil + errCause := context.Cause(c.cmdCtx) // in case the cause is set during Wait(), get the final cancel cause + + if unwrapped, ok := UnwrapPipelineError(errCause); ok { + if unwrapped != errPipeline { + panic("unwrapped context pipeline error should be the same one returned by pipeline function") + } + if unwrapped == nil { + // the pipeline function declares that there is no error, and it cancels (kills) the command ahead, + // so we should ignore the errors from "wait" and "cause" + errWait, errCause = nil, nil + } } + + // some legacy code still need to access the error returned by pipeline function by "==" but not "errors.Is" + // so we need to make sure the original error is able to be unwrapped by UnwrapPipelineError return errors.Join(wrapPipelineError(errPipeline), errCause, errWait) } diff --git a/modules/git/gitcmd/context.go b/modules/git/gitcmd/context.go index dbcea0e626..a32f92ff3a 100644 --- a/modules/git/gitcmd/context.go +++ b/modules/git/gitcmd/context.go @@ -10,9 +10,9 @@ import ( type Context interface { context.Context - // CancelWithCause is a helper function to cancel the context with a specific error cause - // And it returns the same error for convenience, to break the PipelineFunc easily - CancelWithCause(err error) error + // CancelPipeline is a helper function to cancel the command context (kill the command) with a specific error cause, + // it returns the same error for convenience to break the PipelineFunc easily + CancelPipeline(err error) error // In the future, this interface will be extended to support stdio pipe readers/writers } @@ -22,7 +22,11 @@ type cmdContext struct { cmd *Command } -func (c *cmdContext) CancelWithCause(err error) error { - c.cmd.cmdCancel(err) +func (c *cmdContext) CancelPipeline(err error) error { + // pipelineError is used to distinguish between: + // * context canceled by pipeline caller with/without error (normal cancellation) + // * context canceled by parent context (still context.Canceled error) + // * other causes + c.cmd.cmdCancel(pipelineError{err}) return err } diff --git a/modules/git/gitcmd/error.go b/modules/git/gitcmd/error.go index 30d2164b2b..066b37f10d 100644 --- a/modules/git/gitcmd/error.go +++ b/modules/git/gitcmd/error.go @@ -92,10 +92,10 @@ func wrapPipelineError(err error) error { return pipelineError{err} } -func ErrorAsPipeline(err error) error { - var pipelineErr pipelineError - if errors.As(err, &pipelineErr) { - return pipelineErr.error +func UnwrapPipelineError(err error) (error, bool) { //nolint:revive // this is for error unwrapping + var pe pipelineError + if errors.As(err, &pe) { + return pe.error, true } - return nil + return nil, false } diff --git a/modules/git/grep.go b/modules/git/grep.go index aac7b6b51d..051a7a1d40 100644 --- a/modules/git/grep.go +++ b/modules/git/grep.go @@ -102,7 +102,7 @@ func GrepSearch(ctx context.Context, repo *Repository, search string, opts GrepO } if line == "" { if len(results) >= opts.MaxResultLimit { - return ctx.CancelWithCause(nil) + return ctx.CancelPipeline(nil) } isInBlock = false continue diff --git a/modules/git/repo_branch_nogogit.go b/modules/git/repo_branch_nogogit.go index dee6d9d3e8..f925aab3e4 100644 --- a/modules/git/repo_branch_nogogit.go +++ b/modules/git/repo_branch_nogogit.go @@ -101,7 +101,7 @@ func WalkShowRef(ctx context.Context, repoPath string, extraArgs gitcmd.TrustedC stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe() defer stdoutReaderClose() cmd.WithDir(repoPath). - WithPipelineFunc(func(c gitcmd.Context) error { + WithPipelineFunc(func(gitcmd.Context) error { bufReader := bufio.NewReader(stdoutReader) for i < skip { _, isPrefix, err := bufReader.ReadLine() @@ -165,7 +165,7 @@ func WalkShowRef(ctx context.Context, repoPath string, extraArgs gitcmd.TrustedC return nil }) err = cmd.RunWithStderr(ctx) - if errPipeline := gitcmd.ErrorAsPipeline(err); errPipeline != nil { + if errPipeline, ok := gitcmd.UnwrapPipelineError(err); ok { return i, errPipeline // keep the old behavior: return pipeline error directly } return i, err diff --git a/modules/gitrepo/main_test.go b/modules/gitrepo/main_test.go index 6e6636ce77..d41a1a6ad4 100644 --- a/modules/gitrepo/main_test.go +++ b/modules/gitrepo/main_test.go @@ -4,29 +4,21 @@ package gitrepo import ( - "os" "path/filepath" "testing" - "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/tempdir" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/test" ) func TestMain(m *testing.M) { - gitHomePath, cleanup, err := tempdir.OsTempDir("gitea-test").MkdirTempRandom("git-home") - if err != nil { - log.Fatal("Unable to create temp dir: %v", err) - } - defer cleanup() - // resolve repository path relative to the test directory testRootDir := test.SetupGiteaRoot() repoPath = func(repo Repository) string { - return filepath.Join(testRootDir, "/modules/git/tests/repos", repo.RelativePath()) + if filepath.IsAbs(repo.RelativePath()) { + return repo.RelativePath() // for testing purpose only + } + return filepath.Join(testRootDir, "modules/git/tests/repos", repo.RelativePath()) } - - setting.Git.HomePath = gitHomePath - os.Exit(m.Run()) + git.RunGitTests(m) } diff --git a/modules/gitrepo/merge_tree.go b/modules/gitrepo/merge_tree.go new file mode 100644 index 0000000000..6151b1179f --- /dev/null +++ b/modules/gitrepo/merge_tree.go @@ -0,0 +1,59 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package gitrepo + +import ( + "bufio" + "context" + "fmt" + + "code.gitea.io/gitea/modules/git/gitcmd" + "code.gitea.io/gitea/modules/util" +) + +const MaxConflictedDetectFiles = 10 + +// MergeTree performs a merge between two commits (baseRef and headRef) with an optional merge base. +// It returns the resulting tree hash, a list of conflicted files (if any), and an error if the operation fails. +// If there are no conflicts, the list of conflicted files will be nil. +func MergeTree(ctx context.Context, repo Repository, baseRef, headRef, mergeBase string) (treeID string, isErrHasConflicts bool, conflictFiles []string, _ error) { + cmd := gitcmd.NewCommand("merge-tree", "--write-tree", "-z", "--name-only", "--no-messages"). + AddOptionFormat("--merge-base=%s", mergeBase). + AddDynamicArguments(baseRef, headRef) + + stdout, stdoutClose := cmd.MakeStdoutPipe() + defer stdoutClose() + cmd.WithPipelineFunc(func(ctx gitcmd.Context) error { + // https://git-scm.com/docs/git-merge-tree/2.38.0#OUTPUT + // For a conflicted merge, the output is: + // NUL + // NUL + // NUL + // ... + scanner := bufio.NewScanner(stdout) + scanner.Split(util.BufioScannerSplit(0)) + for scanner.Scan() { + line := scanner.Text() + if treeID == "" { // first line is tree ID + treeID = line + continue + } + conflictFiles = append(conflictFiles, line) + if len(conflictFiles) >= MaxConflictedDetectFiles { + break + } + } + return scanner.Err() + }) + + err := RunCmdWithStderr(ctx, repo, cmd) + // For a successful, non-conflicted merge, the exit status is 0. When the merge has conflicts, the exit status is 1. + // A merge can have conflicts without having individual files conflict + // https://git-scm.com/docs/git-merge-tree/2.38.0#_mistakes_to_avoid + isErrHasConflicts = gitcmd.IsErrorExitCode(err, 1) + if err == nil || isErrHasConflicts { + return treeID, isErrHasConflicts, conflictFiles, nil + } + return "", false, nil, fmt.Errorf("run merge-tree failed: %w", err) +} diff --git a/modules/gitrepo/merge_tree_test.go b/modules/gitrepo/merge_tree_test.go new file mode 100644 index 0000000000..9327a0c3d8 --- /dev/null +++ b/modules/gitrepo/merge_tree_test.go @@ -0,0 +1,82 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package gitrepo + +import ( + "path/filepath" + "testing" + + "code.gitea.io/gitea/modules/git/gitcmd" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func prepareRepoDirRenameConflict(t *testing.T) string { + repoDir := filepath.Join(t.TempDir(), "repo-dir-rename-conflict.git") + require.NoError(t, gitcmd.NewCommand("init", "--bare").AddDynamicArguments(repoDir).Run(t.Context())) + stdin := `blob +mark :1 +data 2 +b + +blob +mark :2 +data 2 +c + +reset refs/heads/master +commit refs/heads/master +mark :3 +author test 1769202331 -0800 +committer test 1769202331 -0800 +data 2 +O +M 100644 :1 z/b +M 100644 :2 z/c + +commit refs/heads/split +mark :4 +author test 1769202336 -0800 +committer test 1769202336 -0800 +data 2 +A +from :3 +M 100644 :2 w/c +M 100644 :1 y/b +D z/b +D z/c + +blob +mark :5 +data 2 +d + +commit refs/heads/add +mark :6 +author test 1769202342 -0800 +committer test 1769202342 -0800 +data 2 +B +from :3 +M 100644 :5 z/d +` + require.NoError(t, gitcmd.NewCommand("fast-import").WithDir(repoDir).WithStdinBytes([]byte(stdin)).Run(t.Context())) + return repoDir +} + +func TestMergeTreeDirectoryRenameConflictWithoutFiles(t *testing.T) { + repoDir := prepareRepoDirRenameConflict(t) + require.DirExists(t, repoDir) + repo := &mockRepository{path: repoDir} + + mergeBase, err := MergeBase(t.Context(), repo, "add", "split") + require.NoError(t, err) + + treeID, conflicted, conflictedFiles, err := MergeTree(t.Context(), repo, "add", "split", mergeBase) + require.NoError(t, err) + assert.True(t, conflicted) + assert.Empty(t, conflictedFiles) + assert.Equal(t, "5e3dd4cfc5b11e278a35b2daa83b7274175e3ab1", treeID) +} diff --git a/modules/util/buffer.go b/modules/util/buffer.go new file mode 100644 index 0000000000..c5af750292 --- /dev/null +++ b/modules/util/buffer.go @@ -0,0 +1,22 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package util + +import "bytes" + +func BufioScannerSplit(b byte) func(data []byte, atEOF bool) (advance int, token []byte, err error) { + // reference: bufio.ScanLines + return 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, b); i >= 0 { + return i + 1, data[0:i], nil + } + if atEOF { + return len(data), data, nil + } + return 0, nil, nil + } +} diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index f43539f173..417698544f 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -1796,6 +1796,7 @@ "repo.pulls.remove_prefix": "Remove %s prefix", "repo.pulls.data_broken": "This pull request is broken due to missing fork information.", "repo.pulls.files_conflicted": "This pull request has changes conflicting with the target branch.", + "repo.pulls.files_conflicted_no_listed_files": "(No conflicting files listed)", "repo.pulls.is_checking": "Checking for merge conflicts…", "repo.pulls.is_ancestor": "This branch is already included in the target branch. There is nothing to merge.", "repo.pulls.is_empty": "The changes on this branch are already on the target branch. This will be an empty commit.", diff --git a/routers/private/hook_verification.go b/routers/private/hook_verification.go index 71ba9cc74c..fd26ba89e2 100644 --- a/routers/private/hook_verification.go +++ b/routers/private/hook_verification.go @@ -34,7 +34,7 @@ func verifyCommits(oldCommitID, newCommitID string, repo *git.Repository, env [] WithDir(repo.Path). WithPipelineFunc(func(ctx gitcmd.Context) error { err := readAndVerifyCommitsFromShaReader(stdoutReader, repo, env) - return ctx.CancelWithCause(err) + return ctx.CancelPipeline(err) }). Run(repo.Ctx) if err != nil && !isErrUnverifiedCommit(err) { @@ -70,7 +70,7 @@ func readAndVerifyCommit(sha string, repo *git.Repository, env []string) error { } verification := asymkey_service.ParseCommitWithSignature(ctx, commit) if !verification.Verified { - return ctx.CancelWithCause(&errUnverifiedCommit{commit.ID.String()}) + return ctx.CancelPipeline(&errUnverifiedCommit{commit.ID.String()}) } return nil }). diff --git a/services/pull/check.go b/services/pull/check.go index 44ec17f510..8826fca280 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -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) diff --git a/services/pull/merge.go b/services/pull/merge.go index 6dab23aa21..4925302797 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -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"). diff --git a/services/pull/merge_ff_only.go b/services/pull/merge_ff_only.go index 22a560e29c..2ed09a7161 100644 --- a/services/pull/merge_ff_only.go +++ b/services/pull/merge_ff_only.go @@ -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 diff --git a/services/pull/merge_merge.go b/services/pull/merge_merge.go index bc94400f21..2bc1fb9a34 100644 --- a/services/pull/merge_merge.go +++ b/services/pull/merge_merge.go @@ -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 diff --git a/services/pull/merge_prepare.go b/services/pull/merge_prepare.go index cee62275d3..1131a23d85 100644 --- a/services/pull/merge_prepare.go +++ b/services/pull/merge_prepare.go @@ -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 { diff --git a/services/pull/merge_rebase.go b/services/pull/merge_rebase.go index a3b01848fd..9dbe67a6c6 100644 --- a/services/pull/merge_rebase.go +++ b/services/pull/merge_rebase.go @@ -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()) diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go index d92660d60b..6c101c8e89 100644 --- a/services/pull/merge_squash.go +++ b/services/pull/merge_squash.go @@ -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 diff --git a/services/pull/merge_tree.go b/services/pull/merge_tree.go new file mode 100644 index 0000000000..18d59fabd1 --- /dev/null +++ b/services/pull/merge_tree.go @@ -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 +} diff --git a/services/pull/merge_tree_test.go b/services/pull/merge_tree_test.go new file mode 100644 index 0000000000..6fa2cf7022 --- /dev/null +++ b/services/pull/merge_tree_test.go @@ -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 0 +0000 +data 17 +add conflict file +M 100644 inline %[3]s +data 4 +base + +commit refs/heads/%[1]s +mark :2 +committer Test 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 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 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 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 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) +} diff --git a/services/pull/patch.go b/services/pull/patch.go index f0429e7bec..30f07f8931 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -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 } diff --git a/services/pull/patch_unmerged.go b/services/pull/patch_unmerged.go index b8416abab2..78a31a8704 100644 --- a/services/pull/patch_unmerged.go +++ b/services/pull/patch_unmerged.go @@ -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 { diff --git a/services/pull/pull.go b/services/pull/pull.go index 60d0b2776b..285e489078 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -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 } diff --git a/services/pull/temp_repo.go b/services/pull/temp_repo.go index ec546ac997..d0da870241 100644 --- a/services/pull/temp_repo.go +++ b/services/pull/temp_repo.go @@ -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 { diff --git a/services/pull/update_rebase.go b/services/pull/update_rebase.go index a107bdf376..6b90e5d776 100644 --- a/services/pull/update_rebase.go +++ b/services/pull/update_rebase.go @@ -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/...", diff --git a/services/repository/gitgraph/graph.go b/services/repository/gitgraph/graph.go index 6e114f901a..8d9bec47f8 100644 --- a/services/repository/gitgraph/graph.go +++ b/services/repository/gitgraph/graph.go @@ -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() diff --git a/templates/repo/issue/view_content/pull_merge_box.tmpl b/templates/repo/issue/view_content/pull_merge_box.tmpl index d47d426dfa..2e145c670e 100644 --- a/templates/repo/issue/view_content/pull_merge_box.tmpl +++ b/templates/repo/issue/view_content/pull_merge_box.tmpl @@ -82,6 +82,8 @@
    {{range .ConflictedFiles}}
  • {{.}}
    • + {{else}} +
  • {{ctx.Locale.Tr "repo.pulls.files_conflicted_no_listed_files"}}
    • {{end}}
{{else if .IsPullRequestBroken}}