Help wanted: Backporting to v11.x

[addaleax]

Hi @nodejs/collaborators ! Backport requests to v11.x-staging are queuing up and it’s getting pretty problematic.

If you feel like you can pick up items from https://github.com/nodejs/node/issues?page=2&q=label%3Abackport-requested-v11.x+is%3Aclosed, in particular some of the older ones, that would help a lot. The backporting guide is here, for those who want to revisit :)

In the meantime, I would consider measures like marking all PRs that mostly move code around as blocked, so that at least the problem doesn’t get worse. What do you think?

[addaleax]

Oh, and for anybody who wants to help: Really, please take a look at the oldest PRs first. These PRs tend to touch a lot of common code, so first backporting one of the more recent ones is going to make backporting the older ones even harder.

Partial backports would also help tremendously.

[BridgeAR]

The numbers of open backport requests is now down by ~40%. Most still open ones do not seem to be trivial to backport though.

[sam-github]

Looks like the ones left are mostly @joyeecheung , a handful of @addaleax , and one @cjihrig --- the master refactors are great, but if they aren't backported they might form a wall beyond which even small changes won't cherry-pick without conflict.

[joyeecheung]

One observation: the relationships between the PRs pending backports seem to be similar. If the commits touching a particular file are landed on master as A-B-C-D-E...-Z, oftentimes is no point trying to cherry-pick B/C...Z without backporting A first where A:

• May happen to require a manual backport in nature, e.g. it's semver-major but with some small refactoring
• Touches a significant amount of code in that file (say >30%)
• Involves a file move or split

It usually makes the situation worse if one of the later commits is backported manually out-of-order, say, H - then every commit in the chain may not be landed cleanly even if someone manage to backport A on top of H, to make the subsequent cherry-picking easier the prematurely-backported H has to be reverted first.

(It should be possible to make some smarter tools to at least be aware of the dependencies of commits for a particular file, e.g. as demonstrated by git log <sha> /path/to/file)

[joyeecheung]

Is the current cherry-picking process done manually (as per https://github.com/nodejs/node/blob/master/doc/releases.md#1-update-the-staging-branch ) or is that streamlined somehow? If so the streamlining should take #25403 (comment) into account, adding some additional labels (like backport-blocked-vx.y) may help.

[sam-github]

I've noticed the issue with out of order backports as well, things can get progressively worse.

I haven't worked actively on LTS branches in the last year, but the process is pretty flexible. The -staging branches can be rewritten, its not necessary to "git revert" commits on them, for example, they can be dropped. I'm not sure what the meaning of backport-blocked-vx.y would be. The meaning of the current backport labels is documented, somewhere, but I'm having trouble finding it.

/cc @BethGriggs

[joyeecheung]

The -staging branches can be rewritten, its not necessary to "git revert" commits on them, for example, they can be dropped.

It is possible to reorder the commits after the backports are merged, but when someone who is not a releaser trying to backport locally (and open backport PRs afterwards), reverting those commits would help getting the tree into a shape that is easier to cherry-pick on cleanly.

[BridgeAR]

We ran into issues like these before and we also spoke about these things a couple of times (e.g., on the collaborator summit).

For me there's really just one way to solve this issue (that I am aware of): as soon as a releaser hits a PR that they are not able to backport we block the next release until it's backported (or what ever it relies upon). This normally only applies to PRs which refactor a lot or which imply a deep understanding of a specific code part. It could get more difficult after a while if we have big breaking changes but there's also often also a remedy for that: we could start backporting semver-major PRs in a non-breaking fashion. It often makes sense to have bigger code changes than the actual breaking change (as in: the other code changes might not be necessary without the actual breaking part). So backporting those non-breaking parts would be great as well.

But enforcing this is difficult as we can not expect everyone who opens a PR to always be available for backporting and we someone else might have to do it instead at times (which is not always that easy).

We could give that a try if others agree to it as well. I believe in that case, not only the @nodejs/releasers but all @nodejs/collaborators should agree to it.

[joyeecheung]

BTW I am writing a tool to help analyzing the dependencies of commits

Though currently it is quite slow..(also it does not handle ill-formatted commit messages, like in the screenshot the console commit is backported, just that the backported commit has PR-URL: #24709, I could grep for those as well but that would double the search time.

[sam-github]

we can not expect everyone who opens a PR to always be available for backporting

Definitely true for non-collaborator PRs, and for people who make occaisonal contributions, but I think its reasonable to encourage (strongly encourage?) regular contributors to node to do the work to get their PRs into shape to be released. If they can't, they can also respond with a request for help.

It would be nice if some tooling auto-labelled and posted into PRs telling people after they merge whether they land clean in the release staging branches.

Would adding this to git node land be unreasonable? I assume its deliberate that the actual final stage of posting the Landed in XXXX comment and pushing the commit is done by hand, but if it was automated, the Landed in ... could also include a "this will need backporting, please do ..." note. Or the note could be output as part of what needs to be pasted manually?

[joyeecheung]

@sam-github I think this should be preferably be done by the bot, as switching between branches locally may break people's workflows. Also as I explained in #25403 (comment) it's not a simple matter of backporting PRs as soon as possible, they should also be backported in the correct order to reduce conflicts, otherwise the backports may be even less productive.

[sam-github]

Agreed. Still, the correct order is likely the order they land in master... so reminding people to backport once it lands is close to the correct order. It may be useful to at least give them (or whoever is running git node land?) a heads up that at the time it landed, this PR was not automatically a candidate for release in Current.

In the case that their PR doesn't land clean in staging because it depends on a previous PR to master that hasn't been backported yet, then yes - their backport is blocked, they shouldn't start backporting yet. That's useful information.

What kind of conditions can bots respond to?

As commits land in staging branches, the status of PRs in terms of whether they do or do not need backporting changes... they might initially appear to need backporting, but are better described as blocked by a previous PR that has not (yet?) backported -- once the previous PR is backported the PR in question may change from "backport needed" to "cherry picks clean".

[targos]

I think we need a sort of simple status website that reports which is the next PR that either needs to be backported or marked don't land.