Proposal: Action to verify if the PR lands cleanly on active release lines

[RafaelGSS]

Currently, it's painful for the releaser and for the contributor to land commits on non-current release lines due to conflicts. It's an annoying task to ask for a backport, and it's also bad for the contributor to create a backport PR after a month or two and land this feature in 2/3 months.

My suggestion is to have an action that, when the author-ready flag is placed, it will attempt to cherry-pick this commit on active release lines and inform the user when it fails.

It will make the life of releasers easier, and the contributor could create a backport without having to wait 2 months~

cc: @nodejs/releasers

[aduh95]

I was thinking about that as I was working on the next v22.x release, another pain point is that the current labelling system gives too little info.
From a backporter perspective, I am under the impression that it would be worth to always try to cherry-pick PRs that did not land cleanly on a previous session (and just skip it if it still doesn't land cleanly), while it's almost never worth trying to cherry-pick PRs that e.g. land cleanly but make some test fail.

For example, I've just landed the node config backport, which unblocked the backport of several other PRs, but it was kinda tricky to find which ones.

when the author-ready flag is placed, it will attempt to cherry-pick this commit on active release lines and inform the user when it fails.
and the contributor could create a backport without having to wait 2 months~

How do you foresee this working? LTS release lines are always weeks behind main, most likely the "cherry-pickability" of a commit before it has even landed on main would not be very interesting, I'm not sure what the contributor could do with that info 🤔

[joyeecheung]

My suggestion is to have an action that, when the author-ready flag is placed, it will attempt to cherry-pick this commit on active release lines and inform the user when it fails.

I think trying PRs individually on release branches may incur a lot more false positives if the release branches isn't kept up with the main branch fast enough - from my experience doing backports, if PR B is touching code already touched by PR A, then the automation needs to know that it must cherry-pick PR A first before trying PR B, otherwise it's going to encounter conflicts needlessly (this was what I tried to reduce some churn for with git node backport but I didn't put enough work to it so the algorithm was very naive and only look at file overlaps for dependents, not more specific code range overlaps). Sometimes the entire train can get blocked once a particular PR is having a conflict, even though the conflicts of that particular PR might be superficial (e.g. some mass stylistic refactoring), cherry-picking subsequent PRs without that one would cause a lot of problems, but once the superficial conflicts are resolved for that PR the train can move forward smoothly. In that case if the automation isn't aware of the bottleneck, it may send false alarms to all the later PRs in the train, even though it's only the backport of the bottleneck PR that's actionable.

[aduh95]

In #56143, I've raised the idea of using "staging-staging" branches where commits that land on main would be attempted to be cherry-picked right away so the automation can tell us¹ if a commit doesn't land cleanly or if the linter is not happy.

Footnotes

1. either by opening an issue, or by adding a comment to the PR, depending what level of noise we're happy with. ↩

[RafaelGSS]

I'm sorry for the late reply. I missed the notifications.

I think the "staging-staging" branches approach might help a bit, but it won't solve the problem of alerting the PR author that their commit won't land on v20.x for instance.

So far, I don't have a solution for that.

[aduh95]

it won't solve the problem of alerting the PR author that their commit won't land on v20.x for instance.

Are you saying 20.x because it's on Maintenance, or did you mean something else? I'm not sure it's a problem worth solving for Maintenance lines tbh, we're quite picky about which commit should be backported to those, it seems OK to have it be a manual thing.

I think the "staging-staging" branches approach might help a bit,

Another approach the Nix project is taking: every backport goes through a PR, and you can trigger a backport by adding a backport (and if a commit doesn't land cleanly, the automation adds a comment to the original PR; all the tests are re-run for each backport, which solves other problems). Then at some point, the staging branch is cutoff into a staging-next branch, where commits can "mature" before being actually merged into the release. I'm not saying we should do that (I'm not sure it would work well for us), just sharing in case we want to take some inspiration.

[RafaelGSS]

No, I mentioned v20.x just for example, let's assume active LTS.

I'm not saying we should do that (I'm not sure it would work well for us)

Due to how extensive and difficult is for us to get a green CI due to flakiness, I think that won't solve either

[joyeecheung]

Due to how extensive and difficult is for us to get a green CI due to flakiness, I think that won't solve either

Not about that idea per-se (I am not sure if I fully follow what it is describing), but if we are thinking about experimenting with branches for staging maybe not starting a full CI would help, or only trigger it on specific platforms with GitHub actions for this, since we don't need it to pass all the platforms if we are just testing whether the backport lands at all.

[RafaelGSS]

I wrote a small script that does this verification

check-landing() {
  if [ "$#" -lt 1 ]; then
    echo "Usage: check-landing <target-branch> [<target-branch> ...]"
    return 1
  fi

  local current_commit
  current_commit=$(git rev-parse HEAD)
  local current_branch
  current_branch=$(git rev-parse --abbrev-ref HEAD)

  for branch in "$@"; do
    echo "Checking if HEAD applies cleanly to $branch..."

    local tmp_branch="tmp-cherry-pick-${branch//\//_}"
    git fetch origin "$branch" >/dev/null 2>&1 || {
      echo "⚠️  Failed to fetch origin/$branch"
      continue
    }

    git switch -c "$tmp_branch" "origin/$branch" >/dev/null 2>&1 || {
      echo "⚠️  Failed to create tmp branch from origin/$branch"
      continue
    }

    if git cherry-pick --no-commit "$current_commit" >/dev/null 2>&1; then
      echo "✅ HEAD applies cleanly on $branch"
    else
      echo "❌ Conflict when applying on $branch"
    fi

    git cherry-pick --abort >/dev/null 2>&1 || true
    git reset --hard >/dev/null 2>&1 || true
    git clean -f -d >/dev/null 2>&1 || true
    git switch "$current_branch" >/dev/null 2>&1
    git branch -D "$tmp_branch" >/dev/null 2>&1
  done
}

It's working fine so far (with some limitations - such as using only HEAD)...

[RafaelGSS]

Maybe we should make it more generic and include as git node land --check $PR_ID [...list-of-branches]?