False Merge Conflict On Pull Request - A blocking issue for using Gitea #4432

New Issue

GiteaMirror · 2025-11-02T05:50:30-06:00

GiteaMirror commented

2025-11-02 05:50:30 -06:00

Originally created by @clintwood on GitHub (Dec 2, 2019).

First off thank you for this OS project - amazing work!!

Gitea version (or commit ref): 1.11.0+dev-352-g66028d58f (gitea.io)
Also on private hosted v1.10.0, and latest dev build in Docker
Can you reproduce the bug at https://try.gitea.io:
- [x ] Yes (https://try.gitea.io/clintwood/pr-conflict-issue/pulls/2)

Description

Gitea shows a false merge/PR conflict when there is none.

This issue is also present in many other scenarios that involve pushing changes to a branch being merged into which do not cause a merge conflict. I have simplified this issue by using the most trivial example to clearly illustrate the issue.

This issue is also on my locally hosted servers of which I have both v1.10.0 and the latest dev build in Docker - they all (including on try.gitea.io) exhibit the same issue.

NOTE: This is a blocking issue for using Gitea even in a trivial scenario.

Steps to reproduce:

Create repo on Server (https://try.gitea.io/)

Clone (empty) repo from Server
Add a file (README.md) with some text
Commit initial commit
Push to branch master on Server

Create branch A off of master
Add a line to file (non-conflicting - i.e. at the end)
Commit A
Push to branch A on Server

Create branch B off of A
Add line to file
Commit B
Push to branch B on Server

On Server:
Create PR for A into master
Create PR for B into master
Do PR A merge on Server
PR B now shows merge conflict as shown in screenshot

On my local instance of Gitea (v1.10.0) which has the same issue, I was able to do a git diff of master with B (on the bare repo) and it correctly showed only the changes that were made on branch B (since PR A was now merged int master) which is not a conflict if B was merged into master (i.e. for PR B).

To prove that last statement, do the following additional steps:

Locally:
Checkout master
Pull master from Server (https://try.gitea.io/) - it updates no problem (i.e. master + A - merged from PR A)
Merge B into master - works 100% - no conflicts

Screenshots

What you see in PR #2 (PR B) in the example.

Originally created by @clintwood on GitHub (Dec 2, 2019). First off thank you for this OS project - amazing work!! - Gitea version (or commit ref): 1.11.0+dev-352-g66028d58f (gitea.io) - Also on private hosted v1.10.0, and latest dev build in Docker - Can you reproduce the bug at https://try.gitea.io: - [x ] Yes (https://try.gitea.io/clintwood/pr-conflict-issue/pulls/2) ## Description Gitea shows a _false_ merge/PR conflict when there is none. This issue is also present in many _other scenarios_ that involve pushing changes to a branch being merged into which do _not_ cause a merge conflict. I have simplified this issue by using the most trivial example to clearly illustrate the issue. This issue is also on my locally hosted servers of which I have both v1.10.0 and the latest dev build in Docker - they all (including on try.gitea.io) exhibit the same issue. **NOTE:** This is a blocking issue for using Gitea even in a trivial scenario. Steps to reproduce: ``` Create repo on Server (https://try.gitea.io/) Clone (empty) repo from Server Add a file (README.md) with some text Commit initial commit Push to branch master on Server Create branch A off of master Add a line to file (non-conflicting - i.e. at the end) Commit A Push to branch A on Server Create branch B off of A Add line to file Commit B Push to branch B on Server On Server: Create PR for A into master Create PR for B into master Do PR A merge on Server PR B now shows merge conflict as shown in screenshot ``` On my local instance of Gitea (v1.10.0) which has the same issue, I was able to do a `git diff` of `master` with `B` (on the bare repo) and it correctly showed only the changes that were made on branch `B` (since PR A was now merged int `master`) which is not a conflict if `B` was merged into `master` (i.e. for PR B). To prove that last statement, do the following additional steps: ``` Locally: Checkout master Pull master from Server (https://try.gitea.io/) - it updates no problem (i.e. master + A - merged from PR A) Merge B into master - works 100% - no conflicts ``` ## Screenshots What you see in PR [#2](https://try.gitea.io/clintwood/pr-conflict-issue/pulls/2) (PR B) in the example. ![image](https://user-images.githubusercontent.com/2149388/69960423-125e8380-1512-11ea-80cc-94880d7709d3.png)

GiteaMirror added the issue/confirmed type/bug labels 2025-11-02 05:50:30 -06:00

GiteaMirror closed this issue

2025-11-02 05:50:30 -06:00

GiteaMirror commented

2025-11-02 05:50:31 -06:00

@clintwood commented on GitHub (Dec 2, 2019):

This issue is likely related to #8630 but for clarity this trivial example is not complicated by changes to the PR's target branch (if that makes sense).

@clintwood commented on GitHub (Dec 2, 2019): This issue is likely related to #8630 but for clarity this trivial example is not complicated by changes to the PR's target branch (if that makes sense).

GiteaMirror commented

2025-11-02 05:50:31 -06:00

@clintwood commented on GitHub (Dec 2, 2019):

Further observation: If you close PR B and reopen it then you are able to proceed with the merging the pull request (PR B).

Speculation: It seems like the internal PR merge patch is not being updated to reflect changes from the previously merged pull request (PR A).

@clintwood commented on GitHub (Dec 2, 2019): Further observation: If you close PR B and reopen it then you are able to proceed with the merging the pull request (PR B). Speculation: It seems like the internal _PR merge patch_ is not being updated to reflect changes from the previously merged pull request (PR A).

GiteaMirror commented

2025-11-02 05:50:31 -06:00

@lunny commented on GitHub (Dec 2, 2019):

Any error logs could you find on your system about merging.

@lunny commented on GitHub (Dec 2, 2019): Any error logs could you find on your system about merging.

GiteaMirror commented

2025-11-02 05:50:32 -06:00

@clintwood commented on GitHub (Dec 2, 2019):

@lunny I'm not seeing any errors in the logs that jump out at me - is there anything specific I should be looking for!?

Edit: The example on gitea.io is currently in the errored state - i.e. showing a merge conflict when there is none.

@clintwood commented on GitHub (Dec 2, 2019): @lunny I'm not seeing any errors in the logs that jump out at me - is there anything specific I should be looking for!? Edit: The [example on gitea.io](https://try.gitea.io/clintwood/pr-conflict-issue/pulls/2) is currently in the errored state - i.e. showing a merge conflict when there is none.

GiteaMirror commented

2025-11-02 05:50:32 -06:00

@markusamshove commented on GitHub (Dec 3, 2019):

Related: #6417

@markusamshove commented on GitHub (Dec 3, 2019): Related: #6417

GiteaMirror commented

2025-11-02 05:50:32 -06:00

@zeripath commented on GitHub (Dec 8, 2019):

So the issue is that when we determine conflict we don't actually try to merge - we create a patch and try to apply that.

The code testing the patch is:
95a57394af/models/pull.go (L498-L595)

With the patch being created here:
95a57394af/models/pull.go (L506)

Which looks a bit like an odd construction, and maps to:

95a57394af/models/repo.go (L895)

With the file appearing to be generated in functions below this using repo.SavePatch(int64, []byte) or the lowercase variant.

There's multiple issues with this, not least the fact that patches can be arbitrarily large and this function implies we're storing these in memory at some point!!

It looks like savepatch is called in two places:

82e0383d21/models/pull.go (L640)

82e0383d21/models/pull.go (L808)

And I would guess that either they're generated plainly from head to merge base or they're not run necessarily at the right time.

This code needs completely redoing. We absolutely cannot store arbitrary patches in memory - diff and blame are other places where this happens. It appears that the only reason for this to go in to memory is to check its length (!) I'm not certain of the utility of creating a patch file to be stored on the server in any case - we don't use it for anything else afaics - and as shown above it is probably better to just generate on the fly at testing when we know what we're patching against.

(This reading of patch files in to memory is likely the cause of several other issues complaining about high memory use.)

@zeripath commented on GitHub (Dec 8, 2019): So the issue is that when we determine conflict we don't actually try to merge - we create a patch and try to apply that. The code testing the patch is: https://github.com/go-gitea/gitea/blob/95a57394af4a1eec7105e5d58fa90c946c6d4089/models/pull.go#L498-L595 With the patch being created here: https://github.com/go-gitea/gitea/blob/95a57394af4a1eec7105e5d58fa90c946c6d4089/models/pull.go#L506 Which looks a bit like an odd construction, and maps to: https://github.com/go-gitea/gitea/blob/95a57394af4a1eec7105e5d58fa90c946c6d4089/models/repo.go#L895 With the file appearing to be generated in functions below this using `repo.SavePatch(int64, []byte)` or the lowercase variant. There's multiple issues with this, not least the fact that patches can be arbitrarily large and this function implies we're storing these in memory at some point!! It looks like savepatch is called in two places: https://github.com/go-gitea/gitea/blob/82e0383d2104f454af5b3e0e768f0497113f3b13/models/pull.go#L640 https://github.com/go-gitea/gitea/blob/82e0383d2104f454af5b3e0e768f0497113f3b13/models/pull.go#L808 And I would guess that either they're generated plainly from head to merge base or they're not run necessarily at the right time. This code needs completely redoing. We absolutely cannot store arbitrary patches in memory - diff and blame are other places where this happens. It appears that the only reason for this to go in to memory is to check its length (!) I'm not certain of the utility of creating a patch file to be stored on the server in any case - we don't use it for anything else afaics - and as shown above it is probably better to just generate on the fly at testing when we know what we're patching against. (This reading of patch files in to memory is likely the cause of several other issues complaining about high memory use.)

GiteaMirror commented

2025-11-02 05:50:32 -06:00

@zeripath commented on GitHub (Dec 22, 2019):

I should note that the above was fixed by #9302

@zeripath commented on GitHub (Dec 22, 2019): I should note that the above was fixed by #9302

GiteaMirror commented

2025-11-02 05:50:33 -06:00

@zeripath commented on GitHub (Dec 22, 2019):

Would anyone be able to test on master to see if #9302 has solved the issue?

@zeripath commented on GitHub (Dec 22, 2019): Would anyone be able to test on master to see if #9302 has solved the issue?

GiteaMirror commented

2025-11-02 05:50:33 -06:00