Skip to content

Automerge does not work #24445

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.

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

Closed
EndrII opened this issue Apr 30, 2023 · 7 comments · Fixed by #30780
Closed

Automerge does not work #24445

EndrII opened this issue Apr 30, 2023 · 7 comments · Fixed by #30780
Labels
issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP type/bug
Milestone

Comments

@EndrII
Copy link
Contributor

EndrII commented Apr 30, 2023

Description

I added next rules for merging PR

  • All statuses of commit must be successful before merge
  • PR must have one approve review

Gitea Version

1.19.2

Can you reproduce the bug on the Gitea demo site?

I don't know (I can't check it)

Log Gist

No response

Screenshots

image
image

Git Version

git version 2.25.1

Operating System

ubuntu 20.04 (Linux beloved-pipefish 5.4.0-125-generic #141-Ubuntu SMP Wed Aug 10 13:42:03 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux)

How are you running Gitea?

I run Gitea from prepared binary files

Database

SQLite

@nekrondev
Copy link
Contributor

nekrondev commented Jul 26, 2023

The auto-merge isn't even working on a much simpler workflow by only requesting approval.
I created a demo here https://try.gitea.io/merge_user1/automerge_problem/pulls/1.

merge_user1's repository got branched by merge_user2 (both are repository admins, merge_user2 is a collaborator). Now merge_user2 wants to PR his bugfix. The main branch is protected and needs one approval. merge_user2 submitted his PR with auto merge enabled, i.e. after approval of merge_user1 his change should be auto-merged into main.

Now merge_user1 approved his PR but the commit is not auto-merged into main branch like requested by PR owner.

image

image

image

@nekrondev
Copy link
Contributor

I did some research and it looks like currently automatic merging can only be triggered via API call (GT Actions state when executed successfully aren't counting here either; approval will not trigger this, too).

func NewCommitStatus(ctx *context.APIContext) {

That API function will call the CreateCommitStatus() function that will trigger the automerge checks ...

func CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, creator *user_model.User, sha string, status *git_model.CommitStatus) error {

... that will eventually end in adding a new merge message into the pr_automerge_queue.

func MergeScheduledPullRequest(ctx context.Context, sha string, repo *repo_model.Repository) error {

In the end there are a few missing bits and pieces for actions and approvals that needs to be added.
In your case it's unclear to me why buildbot commit states didn't trigger the automerge (buildbot should use the commit state API which triggers auto merging).

@EndrII
Copy link
Contributor Author

EndrII commented Jul 26, 2023

@nekrondev this is looks as a bug, because you can enable branch protection and block merge without any review. But any users can enable automerge and merge it after successful tests ...
So anyway, the automerge must work as in GitHub. But not like now

@lunny lunny added the issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP label Jul 27, 2023
@nekrondev
Copy link
Contributor

I did a quick test with

  • auto-merge enabled and
  • one approval missing

If I send the API request to commit a successful check the auto-merge is not executed which is fine (the PR still needs to be reviewed).

However if I submit my PR review approval then without any additional calls to the API endpoint the change wouldn't be merged automatically. I think this is the same behavior for Gitea Actions that needs to call the proper automerge function internally after a successful check had been returned from the runner.

@lunny I guess that

func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repository, issue *issues_model.Issue, reviewType issues_model.ReviewType, content, commitID string, attachmentUUIDs []string) (*issues_model.Review, *issues_model.Comment, error) {

is missing a call to MergeScheduledPullRequest() function at module automerge when an approval had been granted and origin PR owner requested an automerge.

@lunny
Copy link
Member

lunny commented Jul 27, 2023

I think the automerge logic should have some refactor. Maybe Gitea could learn some from GH.

@JoaoVictorLouro
Copy link

Same issue on GitTea 1.21

Running on:
Gitea Version
1.21.0 built with GNU Make 4.4.1, go1.21.4 : bindata, timetzdata, sqlite, sqlite_unlock_notify
Database:
mysql (mariadb) 10.6.9
Act Runner:
0.2.6

All through docker (via docker-compose)

image

Any chance we can fix this? Any ideas of where to start looking?

@henrygoodman
Copy link
Contributor

I really hope re-invigorating the auto-merge will also allow us to expose some endpoint to check PR mergeability (from more than just a git perspective, considering reviews/checks/correct merge base etc).

@lunny lunny added this to the 1.21.12 milestone Apr 23, 2024
lunny added a commit that referenced this issue May 21, 2024
…ered (#30780)

Replace #25741
Close #24445
Close #30658
Close #20646
~Depends on #30805~

Since #25741 has been rewritten totally, to make the contribution
easier, I will continue the work in this PR. Thanks @6543

---------

Co-authored-by: 6543 <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>
GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue May 21, 2024
…ered (go-gitea#30780)

Replace go-gitea#25741
Close go-gitea#24445
Close go-gitea#30658
Close go-gitea#20646
~Depends on go-gitea#30805~

Since go-gitea#25741 has been rewritten totally, to make the contribution
easier, I will continue the work in this PR. Thanks @6543

---------

Co-authored-by: 6543 <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>
lunny added a commit that referenced this issue May 22, 2024
…ered (#30780) (#31039)

Backport #30780 by @lunny

Replace #25741
Close #24445
Close #30658
Close #20646
~Depends on #30805~

Since #25741 has been rewritten totally, to make the contribution
easier, I will continue the work in this PR. Thanks @6543

Co-authored-by: Lunny Xiao <[email protected]>
Co-authored-by: 6543 <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP type/bug
Projects
None yet
5 participants