Manual-merge check will close and mark merged PRs that are reset to master

[guillep2k]

Gitea Version: 1.11.0+dev-497-gc0f879546 c0f8795

If a PR's branch is reset to the content of the base repo's master, the PR will be closed and marked as merged by the manual-merge detection routine. This at least seems to be what's happened to https://gitea.com/gitea/go-sdk/pulls/203 (which incidentally was created from a master branch).

[6543]

code ref: "manuallyMerged"

code ref: "getMergeCommit"

[zeripath]

I guess if we just change these two lines to be Closed instead:

gitea/services/pull/check.go

Lines 156 to 158 in b983ff4

	notification.NotifyMergePullRequest(pr, merger, baseGitRepo)
	log.Info("manuallyMerged[%d]: Marked as manually merged into %s/%s by commit id: %s", pr.ID, pr.BaseRepo.Name, pr.BaseBranch, commit.ID.String())

[zeripath]

gitea/services/pull/check.go

Lines 142 to 146 in b983ff4

	pr.Merger = merger
	pr.MergerID = merger.ID
	if err = pr.SetMerged(); err != nil {
	log.Error("PullRequest[%d].setMerged : %v", pr.ID, err)

and these too.

[guillep2k]

But is it correct to close the PR? It can't be merged anymore, but the PR itself could receive further commits that change that.

[lunny]

It should be marked as merged but not closed if it's correct. But that's not the issue.

In fact the detection if it has been merged is not right I think.

[zeripath]

It's actually quite difficult - by the time ManualMerge is called we've lost the previous SHA.

In Pushtobaserepo we have both and can do the check but there's no state we can choose which says don't mark as merged

[lunny]

So maybe we should disable that currently to avoid making confuse.

[zeripath]

Fixed by #12543