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

Gitea not detecting merge conflict properly and failing to merge #29071

Closed
TheBrokenRail opened this issue Feb 7, 2024 · 7 comments · Fixed by #29288
Closed

Gitea not detecting merge conflict properly and failing to merge #29071

TheBrokenRail opened this issue Feb 7, 2024 · 7 comments · Fixed by #29288
Labels
type/bug
Milestone
1.21.6

Comments

@TheBrokenRail
Copy link
Contributor

Description

I'm trying to merge a PR (with the method "Rebase then fast-forward"). But when I click the merge button, it just pops up a message saying "Network error." When I checked the logs it revealed it was an issue with merge conflicts.

The Gitea UI does say that the branch is out-of-date, but it says it should be OK to merge. The "Update branch by merge/rebase" button doesn't do anything and doesn't even appear on https://try.gitea.io.

The PR on https://try.gitea.io: https://try.gitea.io/TheBrokenRail/minecraft-pi-reborn/pulls/1

Gitea Version

v1.21.5

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

https://gist.github.com/TheBrokenRail/e6c0004d81c89d2d4af69688a4e01165

Screenshots

image

Git Version

2.40.1

Operating System

Ubuntu 22.04

How are you running Gitea?

Inside the official Docker container.

Database

SQLite
@yp05327
Copy link
Contributor

yp05327 commented Feb 7, 2024 •
edited
Loading

Missing Redirect -> JSONRedirect changes in #28140
image

As we are using more and more form-fetch-action, maybe we need to add some tests for these responses.

@yp05327
Copy link
Contributor

yp05327 commented Feb 7, 2024

And these shouldn't be JSONError? IIRC, Flash will not work in this case.

@lunny
Copy link
Member

lunny commented Feb 7, 2024 •
edited
Loading

This only affects v1.22, not v1.21.5 I'm wrong. It's backported.

@lunny lunny added this to the 1.21.6 milestone Feb 7, 2024
@TheBrokenRail
Copy link
Contributor Author

So from my understanding, this issue is really two issues?

  1. Gitea isn't detecting the merge conflict correctly, and says it's ready to merge when it isn't.
  2. When it does fails to merge, Gitea doesn't display the error correctly.

@Zettat123
Copy link
Contributor

this issue is really two issues?

Yes. There are two issues.

  1. The issue of error display. Caused by the bug mentioned in comment

  2. As for the conflict detection failure, that's not a bug. Currently Gitea only detects conflicts with the "merge" method, not with the "rebase" method. I think Gitea should also detect conflicts with "rebase" and add some error messages to inform users that there are conflicts when using "rebase", like GitHub does.

@Zettat123 Zettat123 mentioned this issue Feb 21, 2024
Merged
lunny pushed a commit that referenced this issue Feb 21, 2024
@Zettat123
Fix error display when merging PRs (#29288)
79217ea 
Partially fix #29071, regression of Modernize merge button #28140 

Fix some missing `Redirect` -> `JSONRedirect`.

Thanks @yp05327 for the help in
#29071 (comment)
@wxiaoguang wxiaoguang reopened this Feb 21, 2024
@wxiaoguang
Copy link
Contributor

wait for backport

Zettat123 added a commit to Zettat123/gitea that referenced this issue Feb 22, 2024
@Zettat123
Fix error display when merging PRs (go-gitea#29288)
309925d 
Partially fix go-gitea#29071, regression of Modernize merge button go-gitea#28140 

Fix some missing `Redirect` -> `JSONRedirect`.

Thanks @yp05327 for the help in
go-gitea#29071 (comment)
@Zettat123 Zettat123 mentioned this issue Feb 22, 2024
Merged
silverwind pushed a commit that referenced this issue Feb 22, 2024
@Zettat123
Fix error display when merging PRs (#29288) (#29309)
dcb9c38 
Backport #29288
Partially fix #29071, regression of Modernize merge button #28140 

Fix some missing `Redirect` -> `JSONRedirect`.

Thanks @yp05327 for the help in
#29071 (comment)
@lunny
Copy link
Member

lunny commented Feb 22, 2024

Closed by #29309

@lunny lunny closed this as completed Feb 22, 2024
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this issue Feb 22, 2024
@Zettat123 @earl-warren
Fix error display when merging PRs (#29288) (#29309)
9a63c6f 
Backport #29288
Partially fix #29071, regression of Modernize merge button #28140

Fix some missing `Redirect` -> `JSONRedirect`.

Thanks @yp05327 for the help in
go-gitea/gitea#29071 (comment)

(cherry picked from commit dcb9c38568dc4e9502fc416de237cce0eac41cba)
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this issue Feb 27, 2024
@Zettat123 @earl-warren
Fix error display when merging PRs (#29288)
321aaed 
Partially fix #29071, regression of Modernize merge button #28140

Fix some missing `Redirect` -> `JSONRedirect`.

Thanks @yp05327 for the help in
go-gitea/gitea#29071 (comment)

(cherry picked from commit 79217ea63c1f77de7ca79813ae45950724e63d02)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
type/bug
Projects
None yet
Milestone
1.21.6
Development

Successfully merging a pull request may close this issue.

Fix error display when merging PRs Zettat123/gitea
5 participants
@lunny @wxiaoguang @Zettat123 @TheBrokenRail @yp05327