Skip to content

git-node: add release promotion step - #402

Closed
codebytere wants to merge 12 commits into
nodejs:mainfrom
codebytere:git-node-release-promote
Closed

git-node: add release promotion step#402
codebytere wants to merge 12 commits into
nodejs:mainfrom
codebytere:git-node-release-promote

Conversation

@codebytere

@codebyterecodebytere commented Apr 9, 2020

Copy link
Copy Markdown
Member

Refs #388.

Some of this hard to test, since it involved taking real steps that we take during releases.

I've chosen to seek active confirmation more here than in the prep stage, since many of the steps are irreversible, but I'm open to any and all thoughts about how much we might want.

This adds the secondary portion of release automation, for the promotion step. Specifically, we want to:

  1. Verify that the release PR has green CI and an approval
  2. Create and sign the release tag
  3. Set up for next release
  4. Merge the release proposal branch into the release branch
  5. Cherry pick release commit to master
  6. Push release tag
  7. Promote and sign the release builds

cc @nodejs/releasers

@codebytere
codebytereforce-pushed the git-node-release-promote branch 2 times, most recently from b44d683 to 84b0a27CompareApril 9, 2020 05:52
@codecov

codecovBot commented Apr 9, 2020

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.34%. Comparing base (f58051c) to head (3377b44).
Report is 363 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #402 +/- ##
=======================================
Coverage 76.34% 76.34% =======================================
Files 21 21 Lines 1484 1484 =======================================
Hits 1133 1133 Misses 351 351 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@targos

Copy link
Copy Markdown
Member

I'll review and try this when I do the 13.x release next week

@codebytere
codebytereforce-pushed the git-node-release-promote branch 3 times, most recently from 0666c26 to 13191bdCompareApril 9, 2020 17:44
@codebytere
codebytere marked this pull request as ready for review April 9, 2020 17:48
@targos

Copy link
Copy Markdown
Member

I'm trying git-node release --prepare but it runs out of memory while "Updating CHANGELOG_V13.md".

@targos

Copy link
Copy Markdown
Member

Is it supposed to ask me for my GitHub credentials?
It blocks immediately at:

$ /home/mzasso/git/nodejs/node-core-utils/bin/git-node release --promote 32813 ⠋ Verifying Releaser statusIf this is your first time running this command, follow the instructions to create an access token. If you prefer to create it yourself on Github, see https://github.com/nodejs/node-core-utils/blob/master/README.md.
⠸ Verifying Releaser status

@targos

Copy link
Copy Markdown
Member

Okay, I found that I can enter my username/password/otp code and it continues.
Then:

⠴ Verifying Releaser status
✔ Received member information of nodejs/releasers
✔ undefined is not a Releaser; aborting release

@targos

Copy link
Copy Markdown
Member

The output of the next steps is weird:

✔ Done loading data for nodejs/node/pull/32813
... skipped some lines
✔ Approvals: 6
✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/32813#pullrequestreview-393019854
✔ - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/32813#pullrequestreview-393040785
✔ - Richard Lau (@richardlau): https://github.com/nodejs/node/pull/32813#pullrequestreview-393075642
✔ - Shelley Vohr (@codebytere) (TSC): https://github.com/nodejs/node/pull/32813#pullrequestreview-393107238
✔ - Jiawen Geng (@gengjiawen): https://github.com/nodejs/node/pull/32813#pullrequestreview-393109215
✔ - Matheus Marchini (@mmarchini) (TSC): https://github.com/nodejs/node/pull/32813#pullrequestreview-393134711
✖ This PR needs to wait 19 more hours to land
✖ Jenkins CI is failing for #32813
--------------------------------------------------------------------------------
? Do you want to proceed? Yes
✔ Jenkins CI is passing
✖ GitHub CI is failing for #32813
--------------------------------------------------------------------------------
? Do you want to proceed? Yes
✔ GitHub CI is passing
✖ #32813 does not have sufficient approvals
--------------------------------------------------------------------------------
? Do you want to proceed? Yes
✔ #32813 has necessary approvals

@targos

Copy link
Copy Markdown
Member

Then it crashes on the secure tag step:

? Tag and sign the release? Yes
Error: /home/mzasso/git/nodejs/node-core-utils/node_modules/git-secure-tag/lib/git-secure-tag/batch.js:83
throw new Error(`Unexpected reply for ${object}`);
^
Error: Unexpected reply for at Batch.getEntry (/home/mzasso/git/nodejs/node-core-utils/node_modules/git-secure-tag/lib/git-secure-tag/batch.js:83:11)
at Batch.onMissing (/home/mzasso/git/nodejs/node-core-utils/node_modules/git-secure-tag/lib/git-secure-tag/batch.js:89:22)
at Batch.onHeader (/home/mzasso/git/nodejs/node-core-utils/node_modules/git-secure-tag/lib/git-secure-tag/batch.js:73:10)
at Batch.onData (/home/mzasso/git/nodejs/node-core-utils/node_modules/git-secure-tag/lib/git-secure-tag/batch.js:56:12)
at Socket.<anonymous> (/home/mzasso/git/nodejs/node-core-utils/node_modules/git-secure-tag/lib/git-secure-tag/batch.js:17:44)
at Socket.emit (events.js:315:20)
at addChunk (_stream_readable.js:297:12)
at readableAddChunk (_stream_readable.js:273:9)
at Socket.Readable.push (_stream_readable.js:214:10)
at Pipe.onStreamRead (internal/stream_base_commons.js:186:23)
at exports.runSync (/home/mzasso/git/nodejs/node-core-utils/lib/run.js:60:11)
at ReleasePromotion.secureTagRelease (/home/mzasso/git/nodejs/node-core-utils/lib/promote_release.js:246:12)
at ReleasePromotion.promote (/home/mzasso/git/nodejs/node-core-utils/lib/promote_release.js:85:10)
at processTicksAndRejections (internal/process/task_queues.js:97:5)

@codebytere

codebytere commented Apr 14, 2020

Copy link
Copy Markdown
MemberAuthor

hmm @targos - it pulls information from your local ncu configuration 🤔 i have the normal setup for myself and it finds that i'm codebytere successfully.

In re. the second issue - looks like cli doesn't handle nested spinners very well, i'll refactor.

And re. secure-tag - i added it to deps but we might need to simply require that a releaser has globally installed it instead and then execute it that way. What do you think?

Comment threadlib/promote_release.js Outdated
Comment threadlib/promote_release.js Outdated
Comment threadlib/promote_release.js Outdated
@targos

Copy link
Copy Markdown
Member

@codebytere yeah, I actually did not have my .ncurc (it doesn't work well with the npm-check-updates lib and I have to move the file when I use it). I put it back in my home directory and it worked

@targos

Copy link
Copy Markdown
Member

I wonder why it said that the PR does not have sufficient approvals. It has 6 of them 🤔

@targos

Copy link
Copy Markdown
Member

This is what is passed to runSync for the secure tag:

/home/mzasso/git/nodejs/node-core-utils/node_modules/.bin/git-secure-tag [
'v13.13.0',
'813052119e9b73411534e2c50b027781e8882e10\n',
'-sm',
`"'2020-04-14, Node.js v13.13.0 (Current) Release"`
]

Comment threadlib/promote_release.js Outdated
Comment threadlib/promote_release.js Outdated
Comment threadlib/promote_release.js Outdated
@targos

Copy link
Copy Markdown
Member

The branch switching doesn't work:

? Merge proposal branch into staging branch? Yes
⠋ Merging proposal branchError: Switched to branch 'v13.x'
at exports.runSync (/home/mzasso/git/nodejs/node-core-utils/lib/run.js:60:11)
at ReleasePromotion.mergeProposalBranch (/home/mzasso/git/nodejs/node-core-utils/lib/promote_release.js:292:5)
at ReleasePromotion.promote (/home/mzasso/git/nodejs/node-core-utils/lib/promote_release.js:101:16)
at processTicksAndRejections (internal/process/task_queues.js:97:5)

@targos

Copy link
Copy Markdown
Member

^

That's because runSync throws child.stderr as an error if it's not empty. But git sometimes outputs its messages to stderr.
What should we do here?

Comment threadlib/promote_release.js
if (!didResolveConflicts) {
cli.warn(`Aborting release promotion for version ${version}`);
return;
}

@targostargosApr 14, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else, we need to:

git add src/node_version.h # or ask the user to add conflicted files manually
git cherry-pick --continue
git push upstream master
git checkout branch-staging

Comment threadlib/promote_release.js
Comment threadlib/promote_release.js Outdated
@targos

Copy link
Copy Markdown
Member

The promotion step doesn't work. It stops immediately with a success message, but the promotion did not happen (note that the promotion script is interactive so we may have to run it in a special way)

@codebytere
codebytereforce-pushed the git-node-release-promote branch from 946d16d to d944859CompareMay 5, 2020 19:17
@codebytere
codebytereforce-pushed the git-node-release-promote branch from d944859 to c1ab158CompareMay 20, 2020 18:52
@codebytere

Copy link
Copy Markdown
MemberAuthor

@targos rebased and updated a few things if you're willing to give this another spin on your next release!

@targos

Copy link
Copy Markdown
Member

I'm giving it a try now with 12.17.0

Comment threadlib/promote_release.js Outdated
Comment threadlib/promote_release.js Outdated
@codebytere
codebytereforce-pushed the git-node-release-promote branch from 0702242 to 3377b44CompareMay 26, 2020 16:41
@codebyterecodebytere added the Work In Progress PR's that are in progress label Jun 27, 2020
Comment on lines +118 to +125
} else {
if (!releasers.some(r => r.login === release.username)) {
cli.stopSpinner(
`${release.username} is not a Releaser; aborting release`);
return;
}
cli.stopSpinner('Verified Releaser status');
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

Suggested change
}else{
if(!releasers.some(r=>r.login===release.username)){
cli.stopSpinner(
`${release.username} is not a Releaser; aborting release`);
return;
}
cli.stopSpinner('Verified Releaser status');
}
}elseif(releasers.every(r=>r.login!==release.username)){
cli.stopSpinner(
`${release.username} is not a Releaser; aborting release`);
return;
}
cli.stopSpinner('Verified Releaser status');

@github-actions

Copy link
Copy Markdown
Contributor

This PR is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@github-actions

Copy link
Copy Markdown
Contributor

This PR is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@aduh95

Copy link
Copy Markdown
Contributor

Superseded by #835

@aduh95aduh95 closed this Mar 28, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Work In ProgressPR's that are in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@codebytere@targos@joyeecheung@aduh95@lundibundi