Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 3 commits into from
Dec 19, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0feb4dc
Add setting to disable the git apply step in test patch
zeripath Dec 14, 2022
f5867ba
Merge branch 'main' into disable-git-apply-patch-in-test-patch
lunny Dec 19, 2022
1c18a3a
Merge branch 'main' into disable-git-apply-patch-in-test-patch
KN4CK3R Dec 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions custom/conf/app.example.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Expand Down
1 change: 1 addition & 0 deletions docs/content/doc/advanced/config-cheat-sheet.en-us.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`)

Expand Down
2 changes: 2 additions & 0 deletions modules/setting/repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ var (
DefaultMergeMessageOfficialApproversOnly bool
PopulateSquashCommentWithCommitMessages bool
AddCoCommitterTrailers bool
TestConflictingPatchesWithGitApply bool
} `ini:"repository.pull-request"`

// Issue Setting
Expand Down Expand Up @@ -204,6 +205,7 @@ var (
DefaultMergeMessageOfficialApproversOnly bool
PopulateSquashCommentWithCommitMessages bool
AddCoCommitterTrailers bool
TestConflictingPatchesWithGitApply bool
}{
WorkInProgressPrefixes: []string{"WIP:", "[WIP]"},
// Same as GitHub. See
Expand Down
21 changes: 17 additions & 4 deletions services/pull/patch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand Down