lab132 / buildbot-gitea

Buildbot plugin for integration with gitea.
MIT License
62 stars 21 forks source link

Add parameter to ignore mergeable check #31

Closed r7l closed 1 year ago

r7l commented 1 year ago

Hello, there is currently a bug within Gitea that might cause a race condition effect when pushing additional commits into existing PRs. This leads to Gitea sending mergeable as false if that's not the case.

The situation got even worse with recent versions and Gitea people won't address this issue and it seems only be an issue with Buildbot. There are not other complains about it beside this linked issue.

But even without this issue it would be a better in our setup to test PRs even if they're not mergeable just yet.

Would it be possible to add this as a parameter in order to be able to skip this check?

pampersrocker commented 1 year ago

But even without this issue it would be a better in our setup to test PRs even if they're not mergeable just yet.

What exactly should be tested here then? If it's not mergeable (due to a bug or actually not mergeable), buildbot cannot simply merge the pull request and test the result, because it does not know how to resolve the conflicts.

If you just want to trigger builds on the newly pushed commits, the push action from the webhook should be just fine. The pull_request and related actions specifically in combination with the gitea source step first merge the pull request and then test the result.

r7l commented 1 year ago

Thanks allot for the quick response.

We run tests on anything that is pushed to a PR. People might still push to branches not related to any PR. We don't run our Gitea projects like it is done on Github (with people forking and pushing their fork changes). Everyone is just pushing into one repo into their own branches. Buildbot is not merging anything in our setup. Merges are done manually after reviews and testing.

So this is the kind of actions we have:

I would have to look into the push webhook but i would asume that i have to filter out anything not related to a PR manually while this is already done by Gitea when using the pull_request webhook. I've had a look into the code and it seems like one or two lines that need to be changed for a new parameter to skip the mergeable flag. Pretty much like it is done here.

If you're ok with this, i could provide a PR.

pampersrocker commented 1 year ago

Sorry, I think there is a misunderstanding here.

To be clear:

I hope this makes the pull_request feature more clear. But you are correct, there is currently no way to filter for branches only in a pull/merge request status without (ab-)using the pull_request merge test feature.

Feel free to create a PR, I can ammend the unit tests, as they can be tricky to get right.

r7l commented 1 year ago

I've created the PR. While doing so, i was testing this further and i still don't understand why this shouldn't be default behaviour or why it wouldn't be a bug from Gitea.

Here is what i did:

In this case, the PR was mergeable all the time. Technically it's just 3 commits in line. Maybe i have a wrong idea about the usage of Git but i see additional commits to PRs being tested on Github all the time. New commit triggers actions on Github but does not trigger actions on Gitea.

Whatever it is. It might not be everyones use case and i am still greatful if the PR goes through. Thanks allot.

pampersrocker commented 1 year ago

This is definitely a bug with Gitea, that even trivial merge requests are considered not mergeable. Thanks for the PR 👍 Closing this.