node-core-utils feat(git-node): add release promotion step

You can try it with git node release -S --promote https://github.com/nodejs/node/pull/53945.

Supersedes https://github.com/nodejs/node-core-utils/pull/402

/cc @nodejs/releasers

Jul 22 '24 12:07 aduh95

Github CI is green, but it says failing

? Do you want to proceed anyway? Yes
✔  Jenkins CI is passing
✘  GitHub CI is failing for #54123

You probably already know that, but in case someone else doesn't know: that's not an issue introduced by this PR, it's a recurring issue that affects all PRs open from nodejs/node (sometimes it says "CI is failing", sometimes it says "CI is still running")

Aug 06 '24 17:08 aduh95

When an error happens, we should at least print what are the remaining steps so users can do it manually

That seems quite hard to do, because either it's a error case we can predict (and in this case, we should have the code account for it, not hard fail), either it's something we can't predict (and in which case, the steps to be taken are also hard to predict).

I though the script would do the git cherry-pick --continue for me, so it failed. Maybe we should clarify that in the prompt

We can have the script check if .git/CHERRY_PICK_HEAD exists, and run git cherry-pick --continue at this point. I'll push something for that EDIT: that's done in https://github.com/nodejs/node-core-utils/pull/835/commits/8aa8c08c7a3556d8d8bbf5aa6a1542dd5c7fc86f

Aug 06 '24 18:08 aduh95

It doesn't store state, so after the above error I couldn't start from where I stopped.

Hopefully that's not a blocking concern, as I don't think I'll be willing in implementing that in this PR.

It could be a bit more clear that you should run the commands in a separate terminal

Do you have a suggestion? Is this a blocking concern?

Aug 06 '24 19:08 aduh95

exciting 🤩

Aug 06 '24 20:08 ruyadorno

Thanks for working on this! I will try it the next time I make a release.

Aug 07 '24 06:08 targos

What's missing to have it landed?

Sep 17 '24 19:09 RafaelGSS

@targos said he wanted to give it a shot, so I'm fine waiting on his input – but also fine landing it if folks think it's ready to land.

Sep 17 '24 20:09 aduh95

So, I gave a shot today for v22.9.0 and something got wrong. It conflicts (as expected) but on v22.x-staging branch instead of main branch. I think you merge with main got wrong

? Do you want to try reset the local v22.x-staging branch to upstream/v22.x-staging? Yes
⠦ Bringing upstream/v22.x-staging up to date...From github.com:nodejs/node
 * branch                    v22.x-staging -> FETCH_HEAD
✔  upstream/v22.x-staging is now up-to-date
v22.x-staging is out of sync with upstream/v22.x-staging. Mismatched commits:
 - 4631be0311f 2024-09-17, Version 22.9.0 (Current)
 - 8a3482ec5e4 Working on v22.9.1
--------------------------------------------------------------------------------
? Reset to upstream/v22.x-staging? Yes
HEAD is now at 8a3482ec5e4 Working on v22.9.1
   ✔  Reset to upstream/v22.x-staging
Auto-merging src/node_version.h
CONFLICT (content): Merge conflict in src/node_version.h
error: could not apply 4631be0311f... 2024-09-17, Version 22.9.0 (Current)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
--------------------------------------------------------------------------------
   ℹ  Resolve the conflicts and commit the result
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
? Finished resolving cherry-pick conflicts? Yes
   ℹ  You are running in dry-run mode, meaning NCU will not run the `git push` commands, you would need to copy-paste the following command in another terminal window. Alternatively, pass `--run` flag to ask NCU to run the command for you (might not work if you need to type a passphrase to push to the remote).
   ℹ  Run the following commands to push to remote:
   ℹ  git push upstream v22.x-staging v22.9.0

Sep 17 '24 21:09 RafaelGSS

So it created 2 release commits,

1 - Release commit
2 - Working on v22.9.1
3 - Release commit

Sep 17 '24 21:09 RafaelGSS

It expects ncu-config get branch to return main, could it be that it is set to v22.x-staging on your local machine?

Sep 17 '24 21:09 aduh95

It expects ncu-config get branch to return main, could it be that it is set to v22.x-staging on your local machine?

It was null

➜  nodejs-internal-benchmark git:(main) ✗ ncu-config get branch
undefined

Sep 18 '24 04:09 RafaelGSS

Oh! My bad, I didn't test that case, tbh I thought main would be the default already.

Sep 18 '24 09:09 aduh95

So I tried reproducing by removing the setting for the branch name, and I immediately got an error:

$ git node release -S --promote https://github.com/nodejs/node/pull/54966
   ⚠  You have not told git-node what branch you are trying to land commits on.
--------------------------------------------------------------------------------
   ℹ  For example, if your want to land commits on the `main` branch, you can run:

  $ ncu-config set branch main
--------------------------------------------------------------------------------
Error: Failed to create new session
    at new Session (file:///…/node-core-utils/lib/session.js:27:15)
    at new ReleasePromotion (file:///…/node-core-utils/lib/promote_release.js:22:5)
    at main (file:///…/node-core-utils/components/git/release.js:127:21)
…

Did you modify that check locally maybe? You should not be able to proceed with an empty value.

Sep 18 '24 10:09 aduh95

I did not. I have just gh pr checkout 835 and run the script.

Actually, I found why. I normally use a local ncu configuration and it was set to v22.x-staging indeed. Can we automatically change it to main or at least exit the script if it wasn't set properly?

Sep 18 '24 18:09 RafaelGSS

So, I tried the command for v20.18.0. Unfortunately, I didn't go far. At some point it tried to update my local staging branch from upstream but it was already pushed in an earlier step (the manual one, since I ran in dry-run mode by mistake, but the dry-run had already created the tag so it wasn't completely dry). I guess I should have answered "no" to the questions about v20.x-staging but I was afraid that it would cancel the entire run.

$ node ../node-core-utils/bin/git-node.js release --promote https://github.com/nodejs/node/pull/55170
✔  Received member information of nodejs/releasers
✔  targos is a Releaser
✔  Done loading data for nodejs/node/pull/55170
   ℹ  Last Full PR CI on 2024-10-02T14:11:11Z: https://ci.nodejs.org/job/node-test-pull-request/62893/
   ℹ  Last CITGM CI on 2024-10-02T14:11:11Z: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3484/
⠋ Querying data for job/node-test-pull-request/62893/   ✘  Last GitHub CI failed
   ℹ  This PR was created on Mon, 30 Sep 2024 08:59:29 GMT
   ✔  Approvals: 4
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/55170#pullrequestreview-2342018931
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/55170#pullrequestreview-2343657161
   ✔  - Richard Lau (@richardlau) (TSC): https://github.com/nodejs/node/pull/55170#pullrequestreview-2345406712
   ✔  - Ruy Adorno (@ruyadorno) (TSC): https://github.com/nodejs/node/pull/55170#pullrequestreview-2345570558
   ✔  Last Jenkins CI successful
✔  Jenkins CI is passing
✘  GitHub CI is failing for #55170
--------------------------------------------------------------------------------
? Do you want to proceed? Yes
✔  #55170 has necessary approvals
--------------------------------------------------------------------------------
? Tag and sign the release? Yes
⠼ Setting up for next release[v20.18.0-proposal 985262a4a82] Working on v20.18.1
 1 file changed, 2 insertions(+), 2 deletions(-)
✔  Successfully set up for next release
   ℹ  You are running in dry-run mode, meaning NCU will not run the `git push` commands, you would need to copy-paste the following command in another terminal window. Alternatively, pass `--run` flag to ask NCU to run the command for you (might not work if you need to type a passphrase to push to the remote).
   ℹ  Run the following commands to merge the staging branch:
   ℹ  git push upstream HEAD:refs/heads/v20.x HEAD:refs/heads/v20.x-staging
--------------------------------------------------------------------------------
? Ready to continue? Yes
--------------------------------------------------------------------------------
? Cherry-pick release commit to the default branch? Yes
Switched to branch 'v20.x-staging'
Your branch is behind 'upstream/v20.x-staging' by 2 commits, and can be fast-forwarded.
  (use "git pull" to update your local branch)
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
? Do you want to try reset the local v20.x-staging branch to upstream/v20.x-staging? Yes
⠙ Bringing upstream/v20.x-staging up to date...From github.com:nodejs/node
 * branch                    v20.x-staging -> FETCH_HEAD
✔  upstream/v20.x-staging is now up-to-date
v20.x-staging is out of sync with upstream/v20.x-staging. Mismatched commits:
 - 7eebd17fa2c 2024-10-03, Version 20.18.0 'Iron' (LTS)
 - 985262a4a82 Working on v20.18.1
--------------------------------------------------------------------------------
? Reset to upstream/v20.x-staging? Yes
HEAD is now at 985262a4a82 Working on v20.18.1
   ✔  Reset to upstream/v20.x-staging
Auto-merging src/node_version.h
CONFLICT (content): Merge conflict in src/node_version.h
error: could not apply 7eebd17fa2c... 2024-10-03, Version 20.18.0 'Iron' (LTS)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config advice.mergeConflict false"
--------------------------------------------------------------------------------
   ℹ  Resolve the conflicts and commit the result
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
? Finished resolving cherry-pick conflicts? Yes
   ✘  Invalid Release commit message: Working on v20.18.1
Error: Aborted
    at ReleasePromotion.validateReleaseCommit (file:///Users/mzasso/git/nodejs/node-core-utils/lib/promote_release.js:234:13)
    at ReleasePromotion.promote (file:///Users/mzasso/git/nodejs/node-core-utils/lib/promote_release.js:168:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)

Oct 03 '24 18:10 targos

FWIW you can delete the local tag as long as you haven't pushed it.

? Cherry-pick release commit to the default branch? Yes Switched to branch 'v20.x-staging'

It looks like it thinks the default branch is v20.x-staging and not main, which is unlikely to produce good results. What does ncu-config get branch return?

Oct 03 '24 20:10 aduh95

It is v20.x-staging, which is normal from my pov. I work on a v20.x release in my v20.x workspace

Oct 03 '24 20:10 targos

It is v20.x-staging, which is normal from my pov

Does it need to be v20.x-staging for something else? Landing backport PR maybe? Asking because I'm wondering if we should rename it to defaultBranch to avoid the confusion – or if I choose just make an API call to get the name of the default branch (and assume there's a local branch with that name)

Oct 04 '24 12:10 aduh95

I believe it's for backport PRs, but I'm not sure.

Oct 04 '24 12:10 targos

I've confirmed that to land a (backport or not) PR to a staging branch, NCU request you to set a different branch in your config. I've updated the promotion script to make an API call to get the name of the default branch instead of relying on the config. I've also added a fallback when the version tag already exists to not crash if it points to the right commit and is correctly signed.

Oct 12 '24 08:10 aduh95

I've tried it with the v18.20.5 release, and did not get any blocker. I'm tempted to land it, we might still want to iterate on it but I think it's at a point where we can rely on it. @nodejs/releasers, wdyt?

Nov 13 '24 10:11 aduh95

LGTM

Nov 13 '24 10:11 targos