Conflicting files shown on pull request but merging without conflict #13679

mschoettle · 2020-11-23T16:23:32Z

Gitea version (or commit ref): 1.12.5 and 1.14.0+dev-251-g78204a7a7
Git version: 2.24.1
Operating system:
Database (use [x]):
- PostgreSQL
- MySQL
- MSSQL
- SQLite
Can you reproduce the bug at https://try.gitea.io:
- Yes (provide example URL)
- No
Log gist:

Description

I created a pull request which showed me that one of the files was conflicting. I then wanted to resolve this conflict locally but the merge worked just fine.

I re-created this on try.gitea.io with a simplified example, see this test repo: https://try.gitea.io/test-gitea3/conflict-test

Steps taken:

created initial version on master
created branch test
modified same file on branch test
modified same file on branch master
created PR

Result:

This pull request has changes conflicting with the target branch.
settings.py

This is the output when merging locally:

$ git merge test
Auto-merging settings.py
Merge made by the 'recursive' strategy.
 settings.py | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

The text was updated successfully, but these errors were encountered:

6543 · 2020-11-23T16:29:06Z

the easyest workarounds would either be a rebase or a merge from base int head-branch localy and push result

lunny · 2020-11-23T23:43:06Z

I found sometimes github also have the problem. It seems it depends on different merge style.

mschoettle · 2020-11-24T03:27:22Z

Interesting. How are the conflicting files determined?

a1012112796 · 2020-11-24T10:06:47Z

check logic is here
note: tracking is the head branch

gitea/services/pull/patch.go

Lines 111 to 126 in 24330f7

    
           if err := gitRepo.GetDiff(pr.MergeBase, "tracking", tmpPatchFile); err != nil { 
        
           	tmpPatchFile.Close() 
        
           	log.Error("Unable to get patch file from %s to %s in %s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err) 
        
           	return false, fmt.Errorf("Unable to get patch file from %s to %s in %s Error: %v", pr.MergeBase, pr.HeadBranch, pr.BaseRepo.FullName(), err) 
        
           } 
        
           stat, err := tmpPatchFile.Stat() 
        
           if err != nil { 
        
           	tmpPatchFile.Close() 
        
           	return false, fmt.Errorf("Unable to stat patch file: %v", err) 
        
           } 
        
           patchPath := tmpPatchFile.Name() 
        
           tmpPatchFile.Close() 
        
           // 1a. if the size of that patch is 0 - there can be no conflicts! 
        
           if stat.Size() == 0 {

a1012112796 · 2020-11-24T10:31:26Z

maybe we can try this way

# try merge by ``--no-commit``
zzc:conflict-test (master)$ git merge --no-commit --no-ff test_2 
Auto-merging aa.c
CONFLICT (content): Merge conflict in aa.c
Automatic merge failed; fix conflicts and then commit the result.
# get conflict markers by ``--check``
zzc:conflict-test (master *+|MERGING)$ git diff --check 
aa.c:3: leftover conflict marker
aa.c:5: leftover conflict marker
aa.c:7: leftover conflict marker
zzc:conflict-test (master *+|MERGING)$

zeripath · 2020-11-24T10:38:19Z

$ git merge test
Auto-merging settings.py
Merge made by the 'recursive' strategy.
 settings.py | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

Please note the Merge made by 'recursive' strategy. That means that the merge required a checked out copy of the codebase to do the merge. We cannot do that.

@a1012112796 your proposal would also require a complete checked out copy of the codebase.

a1012112796 · 2020-11-24T11:09:27Z

$ git merge test
Auto-merging settings.py
Merge made by the 'recursive' strategy.
 settings.py | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)
Please note the merge made by 'recursive' strategy. That means that the merge required a checked out copy of the codebase to do the merge. We cannot do that.

@a1012112796 your proposal would also require a complete checked out copy of the codebase.

@zacheryph secound choice, should check diff beturn head branch and merge base if they are not same. only when the file has been changed is in git diff merge_base head_branch and git diff merge_base base_branch is maybe real conflict file, need check it again by git merge-file. how about this way?

MarkusAmshove · 2021-04-24T07:49:57Z

Related: #6417

stale · 2021-06-26T03:13:46Z

This issue has been automatically marked as stale because it has not had recent activity. I am here to help clear issues left open even if solved or waiting for more insight. This issue will be closed if no further activity occurs during the next 2 weeks. If the issue is still valid just add a comment to keep it alive. Thank you for your contributions.

zeripath · 2021-06-26T08:15:49Z

I think we need to be clear here that the automatic system in gitea can never be as good a merger as git on the command line.

Why?

Well git on the command line has a checked out copy and can do octopus merges and take account of history to resolve conflicts that the system cannot do.

We can only do basic diff patch applications.

lunny · 2021-06-26T11:00:39Z

In fact, github also has the problem.

MarkusAmshove · 2021-06-26T14:40:31Z

#6417 demonstrates a case where GitHub shows a PR as mergeable while Gitea doesn't, although I haven't tried that exact case in Gitea in a long time, maybe there have been improvements already

fergusonsa · 2021-07-19T15:49:47Z

I have a similar issue. Using git, I created a branch from a feature branch, deleted over 1000 of old files and directories. One of the directories contained 10 .docx Word files. I committed these changes, pushed them to my remote repository on the Gitea v1.14.1 server, and created a pull request to push the deletes to the feature branch. Gitea complained that "This pull request has changes conflicting with the target branch. ", listing the the 10 .docx Word files.

No option was provided to merge the feature branch to my branch.
I tried to checkout and pull the feature branch, but git said it was already up to date. I checked out my branch and tried merging the feature branch but git again said it was already up to date.

In Gitea, on the Files Changed tab, it does show the contents of the docx files as being deleted.

I even tried only including the deletion of the directory and the 10 docx files in a pull request, and Gitea still complained about it.

`
$ git --version
git version 2.13.2.windows.1
$ git checkout feature-branch
Checking out files: 100% (1470/1470), done.
Switched to branch 'feature-branch'
Your branch is up-to-date with 'origin/feature-branch'.
$ git pull
Already up-to-date.
$ git checkout -b my-branch
Switched to a new branch 'my-branch'
$ rm -rf documents
$ git add documents/
$ git commit -m "Deleting directory and docx files"
[my-branch e71b098b0] Deleting directory and docx files 11 files changed, 50 deletions(-)
delete mode 100644 documents/design_doc_01.docx
delete mode 100644 documents/design_doc_02.docx
delete mode 100644 documents/design_doc_03.docx
delete mode 100644 documents/design_doc_04.docx
delete mode 100644 documents/design_doc_05.docx
delete mode 100644 documents/design_doc_06.docx
delete mode 100644 documents/design_doc_07.docx
delete mode 100644 documents/design_doc_08.docx
delete mode 100644 documents/design_doc_09.docx
delete mode 100644 documents/design_doc_10.docx
delete mode 100644 documents/readme.htm
$ git push --set-upstream origin my-branch
Counting objects: 2, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (2/2), 248 bytes | 0 bytes/s, done.
Total 2 (delta 1), reused 0 (delta 0)
remote:
remote: Create a new pull request for 'my-branch':
remote: https://giteaServer/domain/app1/compare/feature-branch...my-branch
remote:
remote: . Processing 1 references
remote: Processed 1 references in total
To https://giteaServer/domain/app1.git

[new branch] my-branch -> my-branch
Branch my-branch set up to track remote branch my-branch from origin.
`

zeripath · 2021-07-27T21:12:21Z

At the risk of repeating myself - we cannot be as good at conflict resolution as git with a worktree.

Having a worktree allows git to run git merge-index, git merge-one-file and git merge-file allowing it to resolve intra file conflicts.

Without a worktree present - which we cannot have - we are limited to a much stronger set of conditions.

If you genuinely believe that the is a better algorithm for merge collision detection please provide it - however, it can only be run in a bare repository - not one with a worktree. (You are allow to create an index file though.) The above demonstration is a false comparison because there is a worktree present.

a1012112796 · 2021-07-28T00:14:44Z

Having a worktree allows git to run git merge-index, git merge-one-file and git merge-file allowing it to resolve intra file conflicts.

@zeripath Note: git merge-file not need work tree. just request two file.

zeripath · 2021-07-28T00:21:29Z

Sorry - yes git merge-file shows the way that you could avoid having to checkout the entire worktree to assess for mergeability.

giordanolins · 2021-09-20T14:49:05Z

I am the responsible for deploying Gitea on the company. Some devs have found this issue, and after arriving to this thread I tried a couple solutions for the problem. Despite the fact that we have different merge strategies that "are not supported" by gitea for the reasons mentioned above, every time I restart the server, the devs are capable of creating the PRs. Why is that? Here is my error log and the issue seems to be the same. Why after a restart it works if the "merge strategy" is "limited"? :/

[...]
2021/09/20 14:17:24 ...dules/git/command.go:120:RunInDirTimeoutEnvFullPipelineFunc() [D] /data/gitea/tmp/local-repo/pull.git026407621: /usr/bin/git -c credential.helper= -c protocol.version=2 -c filter.lfs.required= -c filter.lfs.smudge= -c filter.lfs.clean= diff -p --binary 29c82abb3763511d55950bcb21be93c0b90561cf tracking,
2021/09/20 14:17:24 ...rvices/pull/patch.go:114:checkConflicts() [E] Unable to get patch file from 29c82abb3763511d55950bcb21be93c0b90561cf to feature/IC-17046 in Company/Repository Error: exit status 128,
2021/09/20 14:17:24 ...ers/web/repo/pull.go:1113:CompareAndPullRequestPost() [E] NewPullRequest: Unable to get patch file from 29c82abb3763511d55950bcb21be93c0b90561cf to feature/IC-17046 in Company/Repository Error: exit status 128,
2021/09/20 14:17:24 ...s/context/context.go:185:HTML() [D] Template: status/500,
2021/09/20 14:17:24 Completed POST /Company/Repository /compare/14.0.0...feature/IC-17046 500 Internal Server Error in 262.213925ms,
2021/09/20 14:17:24 Started GET /avatar/a648882f54a315b38d3678634b264be5?size=96 for 10.0.0.3:50250,
2021/09/20 14:17:24 ...s/context/context.go:740:1() [D] Session ID: eb6b8a26aea8e4bf,
2021/09/20 14:17:24 ...s/context/context.go:741:1() [D] CSRF Token: JX18PgPaRWarf_8-ydBuGpnU6iQ6MTYzMjE0NzMzNzI3NzI0NTQ3NA,
2021/09/20 14:17:24 ...ices/auth/session.go:57:SessionUser() [T] Session Authorization: Found user[3],
[...]

jpraet · 2021-12-16T18:50:05Z

I quite often run into this as well. Our CI applies automatic dependency upgrades to Maven pom.xml.
It does this for PR builds as well as builds of master and release branches.
When the exact same change has been automatically applied on the PR branch, as well as the target branch,
we get "This pull request has changes conflicting with the target branch: pom.xml".

zeripath · 2021-12-16T19:40:38Z

Hmm...

I've just had a thought - maybe we can use git read-tree -m here.

What is really needed though is a few good testcases that reliable replicate the problem because without them we cannot be sure that any improvement we suggest is actually an improvement.

jpraet · 2021-12-16T19:52:45Z

Here's a minimal reproducer for my usecase: https://try.gitea.io/jpraet/conflict-detection/pulls/1

zeripath · 2021-12-16T20:19:25Z

OK, my proposed PR using git read-tree -m works correctly for that.

Can you give an example that should conflict too?

The current TestPatch conflict code uses a plain git apply which does not properly account for 3-way merging. However, we can improve things using `git read-tree -m` to do a three-way merge. We can also use `--patience` to generate a nicer diff for applying patches too. Fix go-gitea#13679 Signed-off-by: Andrew Thornton <[email protected]>

jpraet · 2021-12-16T20:23:40Z

This one should conflict: https://try.gitea.io/jpraet/conflict-detection/pulls/2

zeripath · 2021-12-16T20:26:44Z

yup and #18004 does that correctly too.

jpraet · 2021-12-16T21:00:32Z

Unfortunately OP's https://try.gitea.io/test-gitea3/conflict-test/pulls/1 example is still detected as conflicting with #18004.

zeripath · 2021-12-16T21:30:34Z

OK if we do an git apply with --3way that will work but we can't do that for git <2.32.0.

now git merge-tree will output a reasonable attempt at merge but I'm not sure how to ingest that/ensure it's correct.

zeripath · 2021-12-16T21:49:06Z

To get this working properly means that I would need to reimplement git-merge-one-file but so that it can work without a worktree.

I think that's possible and it would improve the cherry-pick PR I've written too. I'll look at it but I'll leave #18004 up as it's a clear improvement from what we currently have. If #18004 isn't merged by the time I've done that I'll update it.

MarkusAmshove · 2021-12-16T22:08:43Z

My repro from #6417 is no longer on Gitea, but the repository is still on Github if you want to try that one too :-)

zeripath · 2021-12-17T17:04:43Z

OK I've finally implemented git-merge-one-file within gitea - it wasn't actually that bad I can't believe I've been avoiding doing that for 3 almost years.

…-file functionality (#18004) The current TestPatch conflict code uses a plain git apply which does not properly account for 3-way merging. However, we can improve things using `git read-tree -m` to do a three-way merge then follow the algorithm used in merge-one-file. We can also use `--patience` and/or `--histogram` to generate a nicer diff for applying patches too. Fix #13679 Fix #6417 Signed-off-by: Andrew Thornton <[email protected]>

zeripath · 2021-12-19T05:01:04Z

Now that the git-merge-one-file algorithm is implemented, to improve further would require implementing a checkoutless git merge recursive or even ort.

This would be quite a bit more difficult and potentially quite a bit slower.

…-file functionality (go-gitea#18004) The current TestPatch conflict code uses a plain git apply which does not properly account for 3-way merging. However, we can improve things using `git read-tree -m` to do a three-way merge then follow the algorithm used in merge-one-file. We can also use `--patience` and/or `--histogram` to generate a nicer diff for applying patches too. Fix go-gitea#13679 Fix go-gitea#6417 Signed-off-by: Andrew Thornton <[email protected]>

6543 added the type/bug label Nov 23, 2020

jpraet mentioned this issue Jan 27, 2021

Discussion of v1.15.x Roadmap #14477

Closed

12 tasks

stale bot added the issue/stale label Jun 26, 2021

stale bot removed the issue/stale label Jun 26, 2021

zeripath mentioned this issue Dec 16, 2021

Improve TestPatch to use git read-tree -m and implement git-merge-one-file functionality #18004

Merged

zeripath closed this as completed Dec 16, 2021

zeripath reopened this Dec 16, 2021

6543 closed this as completed in #18004 Dec 19, 2021

go-gitea locked and limited conversation to collaborators Apr 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Conflicting files shown on pull request but merging without conflict #13679

Conflicting files shown on pull request but merging without conflict #13679

mschoettle commented Nov 23, 2020

6543 commented Nov 23, 2020 •

edited

Loading

lunny commented Nov 23, 2020 •

edited

Loading

mschoettle commented Nov 24, 2020

a1012112796 commented Nov 24, 2020

a1012112796 commented Nov 24, 2020

zeripath commented Nov 24, 2020 •

edited

Loading

a1012112796 commented Nov 24, 2020 •

edited

Loading

MarkusAmshove commented Apr 24, 2021

stale bot commented Jun 26, 2021

zeripath commented Jun 26, 2021

lunny commented Jun 26, 2021

MarkusAmshove commented Jun 26, 2021

fergusonsa commented Jul 19, 2021 •

edited

Loading

zeripath commented Jul 27, 2021

a1012112796 commented Jul 28, 2021

zeripath commented Jul 28, 2021

giordanolins commented Sep 20, 2021 •

edited

Loading

jpraet commented Dec 16, 2021

zeripath commented Dec 16, 2021

jpraet commented Dec 16, 2021

zeripath commented Dec 16, 2021

jpraet commented Dec 16, 2021

zeripath commented Dec 16, 2021

jpraet commented Dec 16, 2021

zeripath commented Dec 16, 2021

zeripath commented Dec 16, 2021

MarkusAmshove commented Dec 16, 2021

zeripath commented Dec 17, 2021

zeripath commented Dec 19, 2021

Conflicting files shown on pull request but merging without conflict #13679

Conflicting files shown on pull request but merging without conflict #13679

Comments

mschoettle commented Nov 23, 2020

Description

6543 commented Nov 23, 2020 • edited Loading

lunny commented Nov 23, 2020 • edited Loading

mschoettle commented Nov 24, 2020

a1012112796 commented Nov 24, 2020

a1012112796 commented Nov 24, 2020

zeripath commented Nov 24, 2020 • edited Loading

a1012112796 commented Nov 24, 2020 • edited Loading

MarkusAmshove commented Apr 24, 2021

stale bot commented Jun 26, 2021

zeripath commented Jun 26, 2021

lunny commented Jun 26, 2021

MarkusAmshove commented Jun 26, 2021

fergusonsa commented Jul 19, 2021 • edited Loading

zeripath commented Jul 27, 2021

a1012112796 commented Jul 28, 2021

zeripath commented Jul 28, 2021

giordanolins commented Sep 20, 2021 • edited Loading

jpraet commented Dec 16, 2021

zeripath commented Dec 16, 2021

jpraet commented Dec 16, 2021

zeripath commented Dec 16, 2021

jpraet commented Dec 16, 2021

zeripath commented Dec 16, 2021

jpraet commented Dec 16, 2021

zeripath commented Dec 16, 2021

zeripath commented Dec 16, 2021

MarkusAmshove commented Dec 16, 2021

zeripath commented Dec 17, 2021

zeripath commented Dec 19, 2021

6543 commented Nov 23, 2020 •

edited

Loading

lunny commented Nov 23, 2020 •

edited

Loading

zeripath commented Nov 24, 2020 •

edited

Loading

a1012112796 commented Nov 24, 2020 •

edited

Loading

fergusonsa commented Jul 19, 2021 •

edited

Loading

giordanolins commented Sep 20, 2021 •

edited

Loading