GiteaBot / gitea-backporter

:tea: A script that looks for PRs in Gitea that need to be backported and creates the backport PR for them
MIT License
17 stars 8 forks source link

Check for existence of backport branch before trying to create it #125

Closed yardenshoham closed 5 months ago

yardenshoham commented 6 months ago
silverwind commented 6 months ago

Do we read the backport/done label elsewhere which could be removed with this? Setting it is fine, but just not reading it to determine done status.

yardenshoham commented 6 months ago

https://github.com/GiteaBot/gitea-backporter/blob/e863e8b9bac2b47a8d29a5b481ef11b58bfb675f/src/github.ts#L22

silverwind commented 6 months ago

Hmm I think we may have to remove the -label:backport/done filter. Imagine a scenario:

So the mechanism will work as long as the bot does both backports in the same run, but that likely can not be guaranteed, right?

silverwind commented 6 months ago

I guess above could be fixed if backport/done is only set only after all backports were done. Did not check the code in detail yet.

yardenshoham commented 6 months ago

The bot never sets a backport/done when there is more than 1 backport/v* label

yardenshoham commented 6 months ago

https://github.com/GiteaBot/gitea-backporter/blob/e863e8b9bac2b47a8d29a5b481ef11b58bfb675f/src/github.ts#L424-L431

silverwind commented 6 months ago

https://github.com/GiteaBot/gitea-backporter/blob/e863e8b9bac2b47a8d29a5b481ef11b58bfb675f/src/github.ts#L424-L431

Hmm I think that is the problem, it should have set backport/done after is has completed all backport PRs. I think we have to move the label setting out of createBackportPr and into the main run function. Then once, the run has completed, verify that all backport PRs exist and if so, set backport/done. That should at least fix the issue seen at https://github.com/go-gitea/gitea/pull/30245.

yardenshoham commented 6 months ago

I could give it a shot next week

silverwind commented 6 months ago

I think my suggestion with the branch was bad because a backport is only truly done once the PR exists, the branch creation is just one of the steps.

yardenshoham commented 6 months ago

That's the same thing anyway, we only push if the cherry-pick succeeds

silverwind commented 6 months ago

Here's a pseudo-code how I would do it:


- fetch candidates, excluding done label
- const todo = Set([`${prnum}-${branch}`],...)
- for each candidate
  - check if branch exists, if not, create it
  - check if pr exists, if not, create it
  - todo.delete(`${prnum}-${branch}`) # at this point we know a pr is done
- if (todo.size === 0) setLabel(`backport/done`)
silverwind commented 6 months ago

Only question is whether GH API can help with "check if pr exists", e.g. determine PR number from branch name alone, likely there must be some way.

silverwind commented 5 months ago

I don't think this fix was right and it shouldn't have merged imho.

yardenshoham commented 5 months ago

This fix is correct but causes too many API calls, which leads to rate limiting. The easiest solution I can think of is memorization of the backportPrExists function