Add setting to disable the git apply step in test patch (#22130)

For a long time Gitea has tested PR patches using a git apply --check
method, and in fact prior to the introduction of a read-tree assisted
three-way merge in #18004, this was the only way of checking patches.

Since #18004, the git apply --check method has been a fallback method,
only used when the read-tree three-way merge method has detected a
conflict. The read-tree assisted three-way merge method is much faster
and less resource intensive method of detecting conflicts. #18004 kept
the git apply method around because it was thought possible that this
fallback might be able to rectify conflicts that the read-tree three-way
merge detected. I am not certain if this could ever be the case.

Given the uncertainty here and the now relative stability of the
read-tree method - this PR makes using this fallback optional and
disables it by default. The hope is that users will not notice any
significant difference in conflict detection and we will be able to
remove the git apply fallback in future, and/or improve the read-tree
three-way merge method to catch any conflicts that git apply method
might have been able to fix.

An additional benefit is that patch checking should be significantly
less resource intensive and much quicker.

(See
https://github.com/go-gitea/gitea/issues/22083\#issuecomment-1347961737)

Ref #22083

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>

1 files changed, 17 insertions, 4 deletions

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