Check for existence of backport branch before trying to create it

[yardenshoham]

• Fixes (?) Done tracking with multiple branches #124

[silverwind]

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]

gitea-backporter/src/github.ts

Line 22 in e863e8b

`is:pr is:merged base:main label:backport/v${giteaMajorMinorVersion} -label:backport/done -label:backport/manual repo:go-gitea/gitea`,

[silverwind]

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

• PR with two backport tags merges
• Bot creates first backport, sets backport/done, bot stops for whatever reason
• Bot runs again does not see that one backport is still outstanding on the PR because of that filter

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]

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]

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

[yardenshoham]

gitea-backporter/src/github.ts

Lines 424 to 431 in e863e8b

	// if the original PR had exactly one backport/* label, add the backport/done label to it
	const backportLabels = originalPr.labels
	.filter((label) => label.name.startsWith("backport/"));
	if (backportLabels.length === 1) {
	await addLabels(originalPr.number, ["backport/done"]);
	console.log(`Added backport/done label to PR #${originalPr.number}`);
	}
	};

[silverwind]

gitea-backporter/src/github.ts

Lines 424 to 431 in e863e8b

	// if the original PR had exactly one backport/* label, add the backport/done label to it
	const backportLabels = originalPr.labels
	.filter((label) => label.name.startsWith("backport/"));
	if (backportLabels.length === 1) {
	await addLabels(originalPr.number, ["backport/done"]);
	console.log(`Added backport/done label to PR #${originalPr.number}`);
	}
	};

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 go-gitea/gitea#30245.

[yardenshoham]

I could give it a shot next week

[silverwind]

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]

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

[silverwind]

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]

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]

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

[yardenshoham]

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