diff options
| -rw-r--r-- | custom/conf/app.example.ini | 3 | ||||
| -rw-r--r-- | docs/content/doc/advanced/config-cheat-sheet.en-us.md | 1 | ||||
| -rw-r--r-- | modules/setting/repository.go | 2 | ||||
| -rw-r--r-- | services/pull/patch.go | 21 |
4 files changed, 23 insertions, 4 deletions
diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 8824d31a4a..754eab452f 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1036,6 +1036,9 @@ ROUTER = console ;; ;; Add co-authored-by and co-committed-by trailers if committer does not match author ;ADD_CO_COMMITTER_TRAILERS = true +;; +;; In addition to testing patches using the three-way merge method, re-test conflicting patches with git apply +;TEST_CONFLICTING_PATCHES_WITH_GIT_APPLY = false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index dcf91c3396..0268938187 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -134,6 +134,7 @@ In addition there is _`StaticRootPath`_ which can be set as a built-in at build - `DEFAULT_MERGE_MESSAGE_OFFICIAL_APPROVERS_ONLY`: **true**: In default merge messages only include approvers who are officially allowed to review. - `POPULATE_SQUASH_COMMENT_WITH_COMMIT_MESSAGES`: **false**: In default squash-merge messages include the commit message of all commits comprising the pull request. - `ADD_CO_COMMITTER_TRAILERS`: **true**: Add co-authored-by and co-committed-by trailers to merge commit messages if committer does not match author. +- `TEST_CONFLICTING_PATCHES_WITH_GIT_APPLY`: **false**: PR patches are tested using a three-way merge method to discover if there are conflicts. If this setting is set to **true**, conflicting patches will be retested using `git apply` - This was the previous behaviour in 1.18 (and earlier) but is somewhat inefficient. Please report if you find that this setting is required. ### Repository - Issue (`repository.issue`) diff --git a/modules/setting/repository.go b/modules/setting/repository.go index 19594369be..ea288d2ed2 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -82,6 +82,7 @@ var ( DefaultMergeMessageOfficialApproversOnly bool PopulateSquashCommentWithCommitMessages bool AddCoCommitterTrailers bool + TestConflictingPatchesWithGitApply bool } `ini:"repository.pull-request"` // Issue Setting @@ -204,6 +205,7 @@ var ( DefaultMergeMessageOfficialApproversOnly bool PopulateSquashCommentWithCommitMessages bool AddCoCommitterTrailers bool + TestConflictingPatchesWithGitApply bool }{ WorkInProgressPrefixes: []string{"WIP:", "[WIP]"}, // Same as GitHub. See diff --git a/services/pull/patch.go b/services/pull/patch.go index e0da410c4d..9ef8b86043 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -23,6 +23,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/process" repo_module "code.gitea.io/gitea/modules/repository" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "github.com/gobwas/glob" @@ -289,13 +290,15 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo * // 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, _, err := AttemptThreeWayMerge(ctx, + conflict, conflictFiles, err := AttemptThreeWayMerge(ctx, tmpBasePath, gitRepo, pr.MergeBase, "base", "tracking", description) if err != nil { return false, err } if !conflict { + // No conflicts detected so we need to check if the patch is empty... + // a. Write the newly merged tree and check the new tree-hash var treeHash string treeHash, _, err = git.NewCommand(ctx, "write-tree").RunStdString(&git.RunOpts{Dir: tmpBasePath}) if err != nil { @@ -307,6 +310,8 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo * if err != nil { return false, err } + + // b. compare the new tree-hash with the base tree hash if treeHash == baseTree.ID.String() { log.Debug("PullRequest[%d]: Patch is empty - ignoring", pr.ID) pr.Status = issues_model.PullRequestStatusEmpty @@ -315,9 +320,17 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo * return false, nil } - // 3. OK read-tree has failed so we need to try a different thing - this might actually succeed where the above fails due to whitespace handling. + // 3. OK the three-way merge method has detected conflicts + // 3a. Are still testing with GitApply? If not set the conflict status and move on + if !setting.Repository.PullRequest.TestConflictingPatchesWithGitApply { + pr.Status = issues_model.PullRequestStatusConflict + pr.ConflictedFiles = conflictFiles + + log.Trace("Found %d files conflicted: %v", len(pr.ConflictedFiles), pr.ConflictedFiles) + return true, nil + } - // 3a. Create a plain patch from head to base + // 3b. Create a plain patch from head to base tmpPatchFile, err := os.CreateTemp("", "patch") if err != nil { log.Error("Unable to create temporary patch file! Error: %v", err) @@ -340,7 +353,7 @@ func checkConflicts(ctx context.Context, pr *issues_model.PullRequest, gitRepo * patchPath := tmpPatchFile.Name() tmpPatchFile.Close() - // 3b. if the size of that patch is 0 - there can be no conflicts! + // 3c. if the size of that patch is 0 - there can be no conflicts! if stat.Size() == 0 { log.Debug("PullRequest[%d]: Patch is empty - ignoring", pr.ID) pr.Status = issues_model.PullRequestStatusEmpty |