From dfa9c9201366e9f025034cd40cb5ec9a8968dc9e Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 13 Nov 2024 14:07:06 +0000 Subject: [PATCH 1/7] feat(git-node): add release promotion step (#835) Co-authored-by: Shelley Vohr --- components/git/release.js | 80 ++++-- lib/promote_release.js | 495 ++++++++++++++++++++++++++++++++++++++ lib/session.js | 4 + package.json | 1 + 4 files changed, 565 insertions(+), 15 deletions(-) create mode 100644 lib/promote_release.js diff --git a/components/git/release.js b/components/git/release.js index d87e92b4..51ea89a5 100644 --- a/components/git/release.js +++ b/components/git/release.js @@ -1,14 +1,31 @@ +import auth from '../../lib/auth.js'; import CLI from '../../lib/cli.js'; import ReleasePreparation from '../../lib/prepare_release.js'; +import ReleasePromotion from '../../lib/promote_release.js'; +import TeamInfo from '../../lib/team_info.js'; +import Request from '../../lib/request.js'; import { runPromise } from '../../lib/run.js'; -export const command = 'release [newVersion|options]'; +export const command = 'release [prid|options]'; export const describe = 'Manage an in-progress release or start a new one.'; const PREPARE = 'prepare'; const PROMOTE = 'promote'; +const RELEASERS = 'releasers'; const releaseOptions = { + filterLabel: { + describe: 'Labels separated by "," to filter security PRs', + type: 'string' + }, + 'gpg-sign': { + describe: 'GPG-sign commits, will be passed to the git process', + alias: 'S' + }, + newVersion: { + describe: 'Version number of the release to be prepared', + type: 'string' + }, prepare: { describe: 'Prepare a new release of Node.js', type: 'boolean' @@ -21,14 +38,16 @@ const releaseOptions = { describe: 'Default relase date when --prepare is used. It must be YYYY-MM-DD', type: 'string' }, + run: { + describe: 'Run steps that involve touching more than the local clone, ' + + 'including `git push` commands. Might not work if a passphrase ' + + 'required to push to the remote clone.', + type: 'boolean' + }, security: { describe: 'Demarcate the new security release as a security release', type: 'boolean' }, - filterLabel: { - describe: 'Labels separated by "," to filter security PRs', - type: 'string' - }, skipBranchDiff: { describe: 'Skips the initial branch-diff check when preparing releases', type: 'boolean' @@ -49,11 +68,16 @@ let yargsInstance; export function builder(yargs) { yargsInstance = yargs; return yargs - .options(releaseOptions).positional('newVersion', { - describe: 'Version number of the release to be prepared or promoted' + .options(releaseOptions).positional('prid', { + describe: 'PR number or URL of the release proposal to be promoted', + type: 'string' }) - .example('git node release --prepare 1.2.3', - 'Prepare a release of Node.js tagged v1.2.3') + .example('git node release --prepare --security', + 'Prepare a new security release of Node.js with auto-determined version') + .example('git node release --prepare --newVersion=1.2.3', + 'Prepare a new release of Node.js tagged v1.2.3') + .example('git node release --promote 12345', + 'Promote a prepared release of Node.js with PR #12345') .example('git node --prepare --startLTS', 'Prepare the first LTS release'); } @@ -88,17 +112,21 @@ function release(state, argv) { } async function main(state, argv, cli, dir) { + const prID = /^(?:https:\/\/github\.com\/nodejs\/node\/pull\/)?(\d+)$/.exec(argv.prid); + if (prID) { + argv.prid = Number(prID[1]); + } if (state === PREPARE) { - const prep = new ReleasePreparation(argv, cli, dir); + const release = new ReleasePreparation(argv, cli, dir); - await prep.prepareLocalBranch(); + await release.prepareLocalBranch(); - if (prep.warnForWrongBranch()) return; + if (release.warnForWrongBranch()) return; // If the new version was automatically calculated, confirm it. if (!argv.newVersion) { const create = await cli.prompt( - `Create release with new version ${prep.newVersion}?`, + `Create release with new version ${release.newVersion}?`, { defaultAnswer: true }); if (!create) { @@ -107,8 +135,30 @@ async function main(state, argv, cli, dir) { } } - return prep.prepare(); + return release.prepare(); } else if (state === PROMOTE) { - // TODO(codebytere): implement release promotion. + const credentials = await auth({ github: true }); + const request = new Request(credentials); + const release = new ReleasePromotion(argv, request, cli, dir); + + cli.startSpinner('Verifying Releaser status'); + const info = new TeamInfo(cli, request, 'nodejs', RELEASERS); + + const releasers = await info.getMembers(); + if (release.username === undefined) { + cli.stopSpinner('Failed to verify Releaser status'); + cli.info( + 'Username was undefined - do you have your .ncurc set up correctly?'); + return; + } else if (releasers.every(r => r.login !== release.username)) { + cli.stopSpinner(`${release.username} is not a Releaser`, 'failed'); + if (!argv.dryRun) { + throw new Error('aborted'); + } + } else { + cli.stopSpinner(`${release.username} is a Releaser`); + } + + return release.promote(); } } diff --git a/lib/promote_release.js b/lib/promote_release.js new file mode 100644 index 00000000..a83f088d --- /dev/null +++ b/lib/promote_release.js @@ -0,0 +1,495 @@ +import path from 'node:path'; +import fs from 'node:fs/promises'; +import semver from 'semver'; +import * as gst from 'git-secure-tag'; + +import { forceRunAsync } from './run.js'; +import PRData from './pr_data.js'; +import PRChecker from './pr_checker.js'; +import Session from './session.js'; +import { existsSync } from 'node:fs'; + +const dryRunMessage = '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).'; + +export default class ReleasePromotion extends Session { + constructor(argv, req, cli, dir) { + super(cli, dir, argv.prid); + this.req = req; + this.dryRun = !argv.run; + this.isLTS = false; + this.ltsCodename = ''; + this.date = ''; + this.gpgSign = argv?.['gpg-sign'] + ? (argv['gpg-sign'] === true ? ['-S'] : ['-S', argv['gpg-sign']]) + : []; + } + + get branch() { + return this.defaultBranch ?? this.config.branch; + } + + async getDefaultBranch() { + const { repository: { defaultBranchRef } } = await this.req.gql( + 'DefaultBranchRef', + { owner: this.owner, repo: this.repo }); + return defaultBranchRef.name; + } + + async promote() { + const { prid, cli } = this; + + // In the promotion stage, we can pull most relevant data + // from the release commit created in the preparation stage. + // Verify that PR is ready to promote. + const { + githubCIReady, + isApproved, + jenkinsReady, + releaseCommitSha + } = await this.verifyPRAttributes(); + + this.releaseCommitSha = releaseCommitSha; + + let localCloneIsClean = true; + const currentHEAD = await forceRunAsync('git', ['rev-parse', 'HEAD'], + { captureStdout: true, ignoreFailure: false }); + if (currentHEAD.trim() !== releaseCommitSha) { + cli.warn('Current HEAD is not the release commit'); + localCloneIsClean = false; + } + try { + await forceRunAsync('git', ['--no-pager', 'diff', '--exit-code'], { ignoreFailure: false }); + } catch { + cli.warn('Some local changes have not been committed'); + localCloneIsClean = false; + } + if (!localCloneIsClean) { + if (await cli.prompt('Should we reset the local HEAD to be the release proposal?')) { + cli.startSpinner('Fetching the proposal upstream...'); + await forceRunAsync('git', ['fetch', this.upstream, releaseCommitSha], + { ignoreFailure: false }); + await forceRunAsync('git', ['reset', releaseCommitSha, '--hard'], { ignoreFailure: false }); + cli.stopSpinner('Local HEAD is now in sync with the proposal'); + } else { + cli.error('Local clone is not ready'); + throw new Error('Aborted'); + } + } + + await this.parseDataFromReleaseCommit(); + + const { version } = this; + cli.startSpinner('Verifying Jenkins CI status'); + if (!jenkinsReady) { + cli.stopSpinner( + `Jenkins CI is failing for #${prid}`, cli.SPINNER_STATUS.FAILED); + const proceed = await cli.prompt('Do you want to proceed?'); + if (!proceed) { + cli.warn(`Aborting release promotion for version ${version}`); + throw new Error('Aborted'); + } + } else { + cli.stopSpinner('Jenkins CI is passing'); + } + + cli.startSpinner('Verifying GitHub CI status'); + if (!githubCIReady) { + cli.stopSpinner( + `GitHub CI is failing for #${prid}`, cli.SPINNER_STATUS.FAILED); + const proceed = await cli.prompt('Do you want to proceed?'); + if (!proceed) { + cli.warn(`Aborting release promotion for version ${version}`); + throw new Error('Aborted'); + } + } else { + cli.stopSpinner('GitHub CI is passing'); + } + + cli.startSpinner('Verifying PR approval status'); + if (!isApproved) { + cli.stopSpinner( + `#${prid} does not have sufficient approvals`, + cli.SPINNER_STATUS.FAILED); + const proceed = await cli.prompt('Do you want to proceed?'); + if (!proceed) { + cli.warn(`Aborting release promotion for version ${version}`); + throw new Error('Aborted'); + } + } else { + cli.stopSpinner(`#${prid} has necessary approvals`); + } + + // Create and sign the release tag. + const shouldTagAndSignRelease = await cli.prompt( + 'Tag and sign the release?'); + if (!shouldTagAndSignRelease) { + cli.warn(`Aborting release promotion for version ${version}`); + throw new Error('Aborted'); + } + await this.secureTagRelease(); + + // Set up for next release. + cli.startSpinner('Setting up for next release'); + await this.setupForNextRelease(); + cli.stopSpinner('Successfully set up for next release'); + + // Merge vX.Y.Z-proposal into vX.x. + await this.mergeProposalBranch(); + + // Cherry pick release commit to master. + const shouldCherryPick = await cli.prompt( + 'Cherry-pick release commit to the default branch?', { defaultAnswer: true }); + if (!shouldCherryPick) { + cli.warn(`Aborting release promotion for version ${version}`); + throw new Error('Aborted'); + } + await this.cherryPickToDefaultBranch(); + + // Update `node_version.h` + await forceRunAsync('git', ['checkout', 'HEAD', '--', 'src/node_version.h'], + { ignoreFailure: false }); + + // There will be remaining cherry-pick conflicts the Releaser will + // need to resolve, so confirm they've been resolved before + // proceeding with next steps. + cli.separator(); + cli.info('Resolve the conflicts and commit the result'); + cli.separator(); + const didResolveConflicts = await cli.prompt( + 'Finished resolving cherry-pick conflicts?', { defaultAnswer: true }); + if (!didResolveConflicts) { + cli.warn(`Aborting release promotion for version ${version}`); + throw new Error('Aborted'); + } + + if (existsSync('.git/CHERRY_PICK_HEAD')) { + cli.info('Cherry-pick is still in progress, attempting to continue it.'); + await forceRunAsync('git', ['cherry-pick', ...this.gpgSign, '--continue'], + { ignoreFailure: false }); + } + + // Validate release commit on the default branch + const releaseCommitOnDefaultBranch = + await forceRunAsync('git', ['show', 'HEAD', '--name-only', '--pretty=format:%s'], + { captureStdout: true, ignoreFailure: false }); + const [commitTitle, ...modifiedFiles] = releaseCommitOnDefaultBranch.trim().split('\n'); + await this.validateReleaseCommit(commitTitle); + if (modifiedFiles.some(file => !file.endsWith('.md'))) { + cli.warn('Some modified files are not markdown, that\'s unusual.'); + cli.info(`The list of modified files: ${modifiedFiles.map(f => `- ${f}`).join('\n')}`); + if (!await cli.prompt('Do you want to proceed anyway?', { defaultAnswer: false })) { + throw new Error('Aborted'); + } + } + + // Push to the remote default branch and release tag. + await this.pushTagAndDefaultBranchToRemote(); + + // Promote and sign the release builds. + await this.promoteAndSignRelease(); + + cli.separator(); + cli.ok(`Release promotion for ${version} complete.\n`); + cli.info( + 'To finish this release, you\'ll need to: \n' + + ` 1. Check the release at: https://nodejs.org/dist/v${version}\n` + + ' 2. Create the blog post for nodejs.org.\n' + + ' 3. Create the release on GitHub.\n' + + ' 4. Optionally, announce the release on your social networks.\n' + + ' 5. Tag @nodejs-social-team on #nodejs-release Slack channel.\n'); + + cli.separator(); + cli.info('Use the following command to create the GitHub release:'); + cli.separator(); + cli.info( + 'awk \'' + + `/^## ${this.date}, Version ${this.version.replaceAll('.', '\\.')} /,` + + '/^<\\x2fa>$/{' + + 'print buf; if(firstLine == "") firstLine = $0; else buf = $0' + + `}' doc/changelogs/CHANGELOG_V${ + this.versionComponents.major}.md | gh release create v${this.version} --verify-tag --latest${ + this.isLTS ? '=false' : ''} --title=${JSON.stringify(this.releaseTitle)} --notes-file -`); + } + + async verifyPRAttributes() { + const { cli, prid, owner, repo, req } = this; + + const data = new PRData({ prid, owner, repo }, cli, req); + await data.getAll(); + + const checker = new PRChecker(cli, data, { prid, owner, repo }, { maxCommits: 0 }); + const jenkinsReady = checker.checkJenkinsCI(); + const githubCIReady = checker.checkGitHubCI(); + const isApproved = checker.checkReviewsAndWait(new Date(), false); + + return { + githubCIReady, + isApproved, + jenkinsReady, + releaseCommitSha: data.commits.at(-1).commit.oid + }; + } + + async validateReleaseCommit(releaseCommitMessage) { + const { cli } = this; + const data = {}; + // Parse out release date. + if (!/^\d{4}-\d{2}-\d{2}, Version \d/.test(releaseCommitMessage)) { + cli.error(`Invalid Release commit message: ${releaseCommitMessage}`); + throw new Error('Aborted'); + } + data.date = releaseCommitMessage.slice(0, 10); + const systemDate = new Date().toISOString().slice(0, 10); + if (data.date !== systemDate) { + cli.warn( + `The release date (${data.date}) does not match the system date for today (${systemDate}).` + ); + if (!await cli.prompt('Do you want to proceed anyway?', { defaultAnswer: false })) { + throw new Error('Aborted'); + } + } + + // Parse out release version. + data.version = releaseCommitMessage.slice(20, releaseCommitMessage.indexOf(' ', 20)); + const version = semver.parse(data.version); + if (!version) { + cli.error(`Release commit contains invalid semantic version: ${data.version}`); + throw new Error('Aborted'); + } + + const { major, minor, patch } = version; + data.stagingBranch = `v${major}.x-staging`; + data.versionComponents = { + major, + minor, + patch + }; + + // Parse out LTS status and codename. + if (!releaseCommitMessage.endsWith(' (Current)')) { + const match = /'([^']+)' \(LTS\)$/.exec(releaseCommitMessage); + if (match == null) { + cli.error('Invalid release commit, it should match either Current or LTS release format'); + throw new Error('Aborted'); + } + data.isLTS = true; + data.ltsCodename = match[1]; + } + return data; + } + + async parseDataFromReleaseCommit() { + const { cli, releaseCommitSha } = this; + + const releaseCommitMessage = await forceRunAsync('git', [ + '--no-pager', 'log', '-1', + releaseCommitSha, + '--pretty=format:%s'], { + captureStdout: true, + ignoreFailure: false + }); + + const releaseCommitData = await this.validateReleaseCommit(releaseCommitMessage); + + this.date = releaseCommitData.date; + this.version = releaseCommitData.version; + this.stagingBranch = releaseCommitData.stagingBranch; + this.versionComponents = releaseCommitData.versionComponents; + this.isLTS = releaseCommitData.isLTS; + this.ltsCodename = releaseCommitData.ltsCodename; + + // Check if CHANGELOG show the correct releaser for the current release + const changeLogDiff = await forceRunAsync('git', [ + '--no-pager', 'diff', + `${this.releaseCommitSha}^..${this.releaseCommitSha}`, + '--', + `doc/changelogs/CHANGELOG_V${this.versionComponents.major}.md` + ], { captureStdout: true, ignoreFailure: false }); + const headingLine = /^\+## \d{4}-\d{2}-\d{2}, Version \d.+$/m.exec(changeLogDiff); + if (headingLine == null) { + cli.error('Cannot find section for the new release in CHANGELOG'); + throw new Error('Aborted'); + } + this.releaseTitle = headingLine[0].slice(4); + const expectedLine = `+## ${releaseCommitMessage}, @${this.username}`; + if (headingLine[0] !== expectedLine && + !headingLine[0].startsWith(`${expectedLine} prepared by @`)) { + cli.error( + `Invalid section heading for CHANGELOG. Expected "${ + expectedLine.slice(1) + }", found "${headingLine[0].slice(1)}` + ); + if (!await cli.prompt('Do you want to proceed anyway?', { defaultAnswer: false })) { + throw new Error('Aborted'); + } + } + } + + async secureTagRelease() { + const { version, isLTS, ltsCodename, releaseCommitSha } = this; + + const releaseInfo = isLTS ? `${ltsCodename} (LTS)` : '(Current)'; + + try { + await new Promise((resolve, reject) => { + const api = new gst.API(process.cwd()); + api.sign(`v${version}`, releaseCommitSha, { + insecure: false, + m: `${this.date} Node.js v${version} ${releaseInfo} Release` + }, (err) => err ? reject(err) : resolve()); + }); + } catch (err) { + const tagCommitSHA = await forceRunAsync('git', [ + 'rev-parse', `refs/tags/v${version}^0` + ], { captureStdout: true, ignoreFailure: false }); + if (tagCommitSHA.trim() !== releaseCommitSha) { + throw new Error( + `Existing version tag points to ${tagCommitSHA.trim()} instead of ${releaseCommitSha}`, + { cause: err } + ); + } + await forceRunAsync('git', ['tag', '--verify', `v${version}`], { ignoreFailure: false }); + this.cli.info('Using the existing tag'); + } + } + + // Set up the branch so that nightly builds are produced with the next + // version number and a pre-release tag. + async setupForNextRelease() { + const { versionComponents, prid } = this; + + // Update node_version.h for next patch release. + const filePath = path.resolve('src', 'node_version.h'); + const nodeVersionFile = await fs.open(filePath, 'r+'); + + const patchVersion = versionComponents.patch + 1; + let cursor = 0; + for await (const line of nodeVersionFile.readLines({ autoClose: false })) { + cursor += line.length + 1; + if (line === `#define NODE_PATCH_VERSION ${versionComponents.patch}`) { + await nodeVersionFile.write(`${patchVersion}`, cursor - 2, 'ascii'); + } else if (line === '#define NODE_VERSION_IS_RELEASE 1') { + await nodeVersionFile.write('0', cursor - 2, 'ascii'); + break; + } + } + + await nodeVersionFile.close(); + + const workingOnVersion = + `v${versionComponents.major}.${versionComponents.minor}.${patchVersion}`; + + // Create 'Working On' commit. + await forceRunAsync('git', ['add', filePath], { ignoreFailure: false }); + return forceRunAsync('git', [ + 'commit', + ...this.gpgSign, + '-m', + `Working on ${workingOnVersion}`, + '-m', + `PR-URL: https://github.com/nodejs/node/pull/${prid}` + ], { ignoreFailure: false }); + } + + async mergeProposalBranch() { + const { cli, dryRun, stagingBranch, versionComponents } = this; + const releaseBranch = `v${versionComponents.major}.x`; + + let prompt = 'Merge proposal branch into staging branch?'; + if (dryRun) { + cli.info(dryRunMessage); + cli.info('Run the following commands to merge the staging branch:'); + cli.info(`git push ${this.upstream} HEAD:refs/heads/${releaseBranch + } HEAD:refs/heads/${stagingBranch}`); + prompt = 'Ready to continue?'; + } + + const shouldMergeProposalBranch = await cli.prompt(prompt, { defaultAnswer: true }); + if (!shouldMergeProposalBranch) { + cli.warn('Aborting release promotion'); + throw new Error('Aborted'); + } else if (dryRun) { + return; + } + + // TODO: find a solution for key passphrase from the terminal + cli.startSpinner('Merging proposal branch'); + await forceRunAsync('git', ['push', this.upstream, `HEAD:refs/heads/${releaseBranch}`, + `HEAD:refs/heads/${stagingBranch}`], + { ignoreFailure: false }); + cli.stopSpinner('Merged proposal branch'); + } + + async pushTagAndDefaultBranchToRemote() { + const { cli, dryRun, version } = this; + const tagVersion = `v${version}`; + + this.defaultBranch ??= await this.getDefaultBranch(); + + let prompt = `Push release tag and ${this.defaultBranch} to ${this.upstream}?`; + if (dryRun) { + cli.info(dryRunMessage); + cli.info('Run the following commands to push to remote:'); + cli.info(`git push ${this.upstream} ${this.defaultBranch} ${tagVersion}`); + prompt = 'Ready to continue?'; + } + + const shouldPushTag = await cli.prompt(prompt, { defaultAnswer: true }); + if (!shouldPushTag) { + cli.warn('Aborting release promotion'); + throw new Error('Aborted'); + } else if (dryRun) { + return; + } + + cli.startSpinner('Pushing to remote'); + await forceRunAsync('git', ['push', this.upstream, this.defaultBranch, tagVersion], + { ignoreFailure: false }); + cli.stopSpinner(`Pushed ${tagVersion} and ${this.defaultBranch} to remote`); + } + + async promoteAndSignRelease() { + const { cli, dryRun } = this; + let prompt = 'Promote and sign release builds?'; + + if (dryRun) { + cli.info(dryRunMessage); + cli.info('Run the following command to sign and promote the release:'); + cli.info('./tools/release.sh -i '); + prompt = 'Ready to continue?'; + } + const shouldPromote = await cli.prompt(prompt, { defaultAnswer: true }); + if (!shouldPromote) { + cli.warn('Aborting release promotion'); + throw new Error('Aborted'); + } else if (dryRun) { + return; + } + + // TODO: move this to .ncurc + const defaultKeyPath = '~/.ssh/node_id_rsa'; + const keyPath = await cli.prompt( + `Please enter the path to your ssh key (Default ${defaultKeyPath}): `, + { questionType: 'input', defaultAnswer: defaultKeyPath }); + + cli.startSpinner('Signing and promoting the release'); + await forceRunAsync('./tools/release.sh', ['-i', keyPath], { ignoreFailure: false }); + cli.stopSpinner('Release has been signed and promoted'); + } + + async cherryPickToDefaultBranch() { + this.defaultBranch ??= await this.getDefaultBranch(); + const releaseCommitSha = this.releaseCommitSha; + await forceRunAsync('git', ['checkout', this.defaultBranch], { ignoreFailure: false }); + + await this.tryResetBranch(); + + // There will be conflicts, we do not want to treat this as a failure. + await forceRunAsync('git', ['cherry-pick', ...this.gpgSign, releaseCommitSha], + { ignoreFailure: true }); + } +} diff --git a/lib/session.js b/lib/session.js index 937de253..c6bba9a6 100644 --- a/lib/session.js +++ b/lib/session.js @@ -87,6 +87,10 @@ export default class Session { return this.config.branch; } + get username() { + return this.config.username; + } + get readme() { return this.config.readme; } diff --git a/package.json b/package.json index 28e21165..4289a7a7 100644 --- a/package.json +++ b/package.json @@ -46,6 +46,7 @@ "core-validate-commit": "^4.1.0", "figures": "^6.1.0", "ghauth": "^6.0.7", + "git-secure-tag": "^2.3.1", "js-yaml": "^4.1.0", "listr2": "^8.2.4", "lodash": "^4.17.21", From 391487bb977fe122d322637c21c6f7ee8881f6b4 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 19 Nov 2024 16:16:17 +0000 Subject: [PATCH 2/7] feat(git-node): prompt before attempting branch-diff (#869) --- lib/prepare_release.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/prepare_release.js b/lib/prepare_release.js index 612ab233..ea4a89eb 100644 --- a/lib/prepare_release.js +++ b/lib/prepare_release.js @@ -167,7 +167,11 @@ export default class ReleasePreparation extends Session { return this.prepareSecurity(); } - if (this.runBranchDiff) { + const runBranchDiff = await cli.prompt( + 'Do you want to check if any additional commits could be backported ' + + '(recommended except for Maintenance releases)?', + { defaultAnswer: this.runBranchDiff }); + if (runBranchDiff) { // TODO: UPDATE re-use // Check the branch diff to determine if the releaser // wants to backport any more commits before proceeding. From 82527ad29271b9a07d58bffa6bb48c77127a089a Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 19 Nov 2024 16:16:36 +0000 Subject: [PATCH 3/7] feat(git-node): use a single `git push` command (#870) --- lib/promote_release.js | 67 +++++++++++++++--------------------------- 1 file changed, 24 insertions(+), 43 deletions(-) diff --git a/lib/promote_release.js b/lib/promote_release.js index a83f088d..e3f919a3 100644 --- a/lib/promote_release.js +++ b/lib/promote_release.js @@ -134,12 +134,9 @@ export default class ReleasePromotion extends Session { // Set up for next release. cli.startSpinner('Setting up for next release'); - await this.setupForNextRelease(); + const workingOnNewReleaseCommit = await this.setupForNextRelease(); cli.stopSpinner('Successfully set up for next release'); - // Merge vX.Y.Z-proposal into vX.x. - await this.mergeProposalBranch(); - // Cherry pick release commit to master. const shouldCherryPick = await cli.prompt( 'Cherry-pick release commit to the default branch?', { defaultAnswer: true }); @@ -186,8 +183,8 @@ export default class ReleasePromotion extends Session { } } - // Push to the remote default branch and release tag. - await this.pushTagAndDefaultBranchToRemote(); + // Push to the remote the release tag, and default, release, and staging branch. + await this.pushToRemote(workingOnNewReleaseCommit); // Promote and sign the release builds. await this.promoteAndSignRelease(); @@ -385,7 +382,7 @@ export default class ReleasePromotion extends Session { // Create 'Working On' commit. await forceRunAsync('git', ['add', filePath], { ignoreFailure: false }); - return forceRunAsync('git', [ + await forceRunAsync('git', [ 'commit', ...this.gpgSign, '-m', @@ -393,48 +390,28 @@ export default class ReleasePromotion extends Session { '-m', `PR-URL: https://github.com/nodejs/node/pull/${prid}` ], { ignoreFailure: false }); + const workingOnNewReleaseCommit = await forceRunAsync('git', ['rev-parse', 'HEAD'], + { ignoreFailure: false, captureStdout: true }); + return workingOnNewReleaseCommit.trim(); } - async mergeProposalBranch() { - const { cli, dryRun, stagingBranch, versionComponents } = this; + async pushToRemote(workingOnNewReleaseCommit) { + const { cli, dryRun, version, versionComponents, stagingBranch } = this; const releaseBranch = `v${versionComponents.major}.x`; - - let prompt = 'Merge proposal branch into staging branch?'; - if (dryRun) { - cli.info(dryRunMessage); - cli.info('Run the following commands to merge the staging branch:'); - cli.info(`git push ${this.upstream} HEAD:refs/heads/${releaseBranch - } HEAD:refs/heads/${stagingBranch}`); - prompt = 'Ready to continue?'; - } - - const shouldMergeProposalBranch = await cli.prompt(prompt, { defaultAnswer: true }); - if (!shouldMergeProposalBranch) { - cli.warn('Aborting release promotion'); - throw new Error('Aborted'); - } else if (dryRun) { - return; - } - - // TODO: find a solution for key passphrase from the terminal - cli.startSpinner('Merging proposal branch'); - await forceRunAsync('git', ['push', this.upstream, `HEAD:refs/heads/${releaseBranch}`, - `HEAD:refs/heads/${stagingBranch}`], - { ignoreFailure: false }); - cli.stopSpinner('Merged proposal branch'); - } - - async pushTagAndDefaultBranchToRemote() { - const { cli, dryRun, version } = this; const tagVersion = `v${version}`; this.defaultBranch ??= await this.getDefaultBranch(); - let prompt = `Push release tag and ${this.defaultBranch} to ${this.upstream}?`; + let prompt = `Push release tag and commits to ${this.upstream}?`; if (dryRun) { cli.info(dryRunMessage); - cli.info('Run the following commands to push to remote:'); - cli.info(`git push ${this.upstream} ${this.defaultBranch} ${tagVersion}`); + cli.info('Run the following command to push to remote:'); + cli.info(`git push ${this.upstream} ${ + this.defaultBranch} ${ + tagVersion} ${ + workingOnNewReleaseCommit}:refs/heads/${releaseBranch} ${ + workingOnNewReleaseCommit}:refs/heads/${stagingBranch}`); + cli.warn('Once pushed, you must not delete the local tag'); prompt = 'Ready to continue?'; } @@ -447,9 +424,13 @@ export default class ReleasePromotion extends Session { } cli.startSpinner('Pushing to remote'); - await forceRunAsync('git', ['push', this.upstream, this.defaultBranch, tagVersion], - { ignoreFailure: false }); - cli.stopSpinner(`Pushed ${tagVersion} and ${this.defaultBranch} to remote`); + await forceRunAsync('git', ['push', this.upstream, this.defaultBranch, tagVersion, + `${workingOnNewReleaseCommit}:refs/heads/${releaseBranch}`, + `${workingOnNewReleaseCommit}:refs/heads/${stagingBranch}`], + { ignoreFailure: false }); + cli.stopSpinner(`Pushed ${tagVersion}, ${this.defaultBranch}, ${ + releaseBranch}, and ${stagingBranch} to remote`); + cli.warn('Now that it has been pushed, you must not delete the local tag'); } async promoteAndSignRelease() { From ec6c6cbf23d90738e4e73a17c245d24ac331de7d Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 19 Nov 2024 18:21:59 +0000 Subject: [PATCH 4/7] fix(git-node): do not assume release commit will conflict (#871) --- lib/promote_release.js | 55 +++++++++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/lib/promote_release.js b/lib/promote_release.js index e3f919a3..e8aeaa25 100644 --- a/lib/promote_release.js +++ b/lib/promote_release.js @@ -144,23 +144,34 @@ export default class ReleasePromotion extends Session { cli.warn(`Aborting release promotion for version ${version}`); throw new Error('Aborted'); } - await this.cherryPickToDefaultBranch(); - - // Update `node_version.h` - await forceRunAsync('git', ['checkout', 'HEAD', '--', 'src/node_version.h'], - { ignoreFailure: false }); + const appliedCleanly = await this.cherryPickToDefaultBranch(); + + // Ensure `node_version.h`'s `NODE_VERSION_IS_RELEASE` bit is not updated + await forceRunAsync('git', ['checkout', + appliedCleanly + ? 'HEAD^' // In the absence of conflict, the top of the remote branch is the commit before. + : 'HEAD', // In case of conflict, HEAD is still the top of the remove branch. + '--', 'src/node_version.h'], + { ignoreFailure: false }); - // There will be remaining cherry-pick conflicts the Releaser will - // need to resolve, so confirm they've been resolved before - // proceeding with next steps. - cli.separator(); - cli.info('Resolve the conflicts and commit the result'); - cli.separator(); - const didResolveConflicts = await cli.prompt( - 'Finished resolving cherry-pick conflicts?', { defaultAnswer: true }); - if (!didResolveConflicts) { - cli.warn(`Aborting release promotion for version ${version}`); - throw new Error('Aborted'); + if (appliedCleanly) { + // There were no conflicts, we have to amend the commit to revert the + // `node_version.h` changes. + await forceRunAsync('git', ['commit', ...this.gpgSign, '--amend', '--no-edit', '-n'], + { ignoreFailure: false }); + } else { + // There will be remaining cherry-pick conflicts the Releaser will + // need to resolve, so confirm they've been resolved before + // proceeding with next steps. + cli.separator(); + cli.info('Resolve the conflicts and commit the result'); + cli.separator(); + const didResolveConflicts = await cli.prompt( + 'Finished resolving cherry-pick conflicts?', { defaultAnswer: true }); + if (!didResolveConflicts) { + cli.warn(`Aborting release promotion for version ${version}`); + throw new Error('Aborted'); + } } if (existsSync('.git/CHERRY_PICK_HEAD')) { @@ -469,8 +480,14 @@ export default class ReleasePromotion extends Session { await this.tryResetBranch(); - // There will be conflicts, we do not want to treat this as a failure. - await forceRunAsync('git', ['cherry-pick', ...this.gpgSign, releaseCommitSha], - { ignoreFailure: true }); + // There might be conflicts, we do not want to treat this as a hard failure, + // but we want to retain that information. + try { + await forceRunAsync('git', ['cherry-pick', ...this.gpgSign, releaseCommitSha], + { ignoreFailure: false }); + return true; + } catch { + return false; + } } } From 4eaad654126a6ea7f4c7e684b8e04f9d94ae8d08 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 19 Nov 2024 18:22:51 +0000 Subject: [PATCH 5/7] feat(git-v8): preserve original author when backporting (#872) --- components/git/v8.js | 18 +++++++-- lib/update-v8/backport.js | 84 ++++++++++++++++++++++++++++++++++----- lib/update-v8/index.js | 1 + 3 files changed, 90 insertions(+), 13 deletions(-) diff --git a/components/git/v8.js b/components/git/v8.js index b0c2879e..716718ab 100644 --- a/components/git/v8.js +++ b/components/git/v8.js @@ -44,10 +44,22 @@ export function builder(yargs) { describe: 'Bump V8 embedder version number or patch version', default: true }) + .option('gpg-sign', { + alias: 'S', + type: 'boolean', + describe: 'GPG-sign commits', + default: false + }) + .option('preserve-original-author', { + type: 'boolean', + describe: 'Preserve original commit author and date', + default: true + }) .option('squash', { type: 'boolean', describe: - 'If multiple commits are backported, squash them into one', + 'If multiple commits are backported, squash them into one. When ' + + '`--squash` is passed, `--preserve-original-author` will be ignored', default: false }); } @@ -88,7 +100,7 @@ export function handler(argv) { input, spawnArgs: { cwd: options.nodeDir, - stdio: input ? ['pipe', 'ignore', 'ignore'] : 'ignore' + stdio: input ? ['pipe', 'inherit', 'inherit'] : 'inherit' } }); }; @@ -97,7 +109,7 @@ export function handler(argv) { return forceRunAsync('git', args, { ignoreFailure: false, captureStdout: true, - spawnArgs: { cwd: options.v8Dir, stdio: ['ignore', 'pipe', 'ignore'] } + spawnArgs: { cwd: options.v8Dir, stdio: ['ignore', 'pipe', 'inherit'] } }); }; diff --git a/lib/update-v8/backport.js b/lib/update-v8/backport.js index e3ff224e..6df9a6c0 100644 --- a/lib/update-v8/backport.js +++ b/lib/update-v8/backport.js @@ -9,6 +9,7 @@ import { ListrEnquirerPromptAdapter } from '@listr2/prompt-adapter-enquirer'; import { shortSha } from '../utils.js'; import { getCurrentV8Version } from './common.js'; +import { forceRunAsync } from '../run.js'; export async function checkOptions(options) { if (options.sha.length > 1 && options.squash) { @@ -41,6 +42,8 @@ export function doBackport(options) { } } todo.push(commitSquashedBackport()); + } else if (options.preserveOriginalAuthor) { + todo.push(cherryPickV8Commits(options)); } else { todo.push(applyAndCommitPatches()); } @@ -76,18 +79,47 @@ function commitSquashedBackport() { }; }; -function commitPatch(patch) { +const commitTask = (patch, extraArgs, trailers) => async(ctx) => { + const messageTitle = formatMessageTitle([patch]); + const messageBody = formatMessageBody(patch, false, trailers); + await ctx.execGitNode('add', ['deps/v8']); + await ctx.execGitNode('commit', [ + ...ctx.gpgSign, ...extraArgs, + '-m', messageTitle, '-m', messageBody + ]); +}; + +function amendHEAD(patch) { return { - title: 'Commit patch', + title: 'Amend/commit', task: async(ctx) => { - const messageTitle = formatMessageTitle([patch]); - const messageBody = formatMessageBody(patch, false); - await ctx.execGitNode('add', ['deps/v8']); - await ctx.execGitNode('commit', ['-m', messageTitle, '-m', messageBody]); + let coAuthor; + if (patch.hadConflicts) { + const getGitConfigEntry = async(configKey) => { + const output = await forceRunAsync('git', ['config', configKey], { + ignoreFailure: false, + captureStdout: true, + spawnArgs: { cwd: ctx.nodeDir } + }); + return output.trim(); + }; + await ctx.execGitNode('am', [...ctx.gpgSign, '--continue']); + coAuthor = `\nCo-authored-by: ${ + await getGitConfigEntry('user.name')} <${ + await getGitConfigEntry('user.email')}>`; + } + await commitTask(patch, ['--amend'], coAuthor)(ctx); } }; } +function commitPatch(patch) { + return { + title: 'Commit patch', + task: commitTask(patch) + }; +} + function formatMessageTitle(patches) { const action = patches.some(patch => patch.hadConflicts) ? 'backport' : 'cherry-pick'; @@ -106,12 +138,12 @@ function formatMessageTitle(patches) { } } -function formatMessageBody(patch, prefixTitle) { +function formatMessageBody(patch, prefixTitle, trailers = '') { const indentedMessage = patch.message.replace(/\n/g, '\n '); const body = 'Original commit message:\n\n' + ` ${indentedMessage}\n\n` + - `Refs: https://github.com/v8/v8/commit/${patch.sha}`; + `Refs: https://github.com/v8/v8/commit/${patch.sha}${trailers}`; if (prefixTitle) { const action = patch.hadConflicts ? 'Backport' : 'Cherry-pick'; @@ -167,6 +199,15 @@ function applyAndCommitPatches() { }; } +function cherryPickV8Commits() { + return { + title: 'Cherry-pick commit from V8 clone to deps/v8', + task: (ctx, task) => { + return task.newListr(ctx.patches.map(cherryPickV8CommitTask)); + } + }; +} + function applyPatchTask(patch) { return { title: `Commit ${shortSha(patch.sha)}`, @@ -190,10 +231,33 @@ function applyPatchTask(patch) { }; } -async function applyPatch(ctx, task, patch) { +function cherryPickV8CommitTask(patch) { + return { + title: `Commit ${shortSha(patch.sha)}`, + task: (ctx, task) => { + const todo = [ + { + title: 'Cherry-pick', + task: (ctx, task) => applyPatch(ctx, task, patch, 'am') + } + ]; + if (ctx.bump !== false) { + if (ctx.nodeMajorVersion < 9) { + todo.push(incrementV8Version()); + } else { + todo.push(incrementEmbedderVersion()); + } + } + todo.push(amendHEAD(patch)); + return task.newListr(todo); + } + }; +} + +async function applyPatch(ctx, task, patch, method = 'apply') { try { await ctx.execGitNode( - 'apply', + method, ['-p1', '--3way', '--directory=deps/v8'], patch.data /* input */ ); diff --git a/lib/update-v8/index.js b/lib/update-v8/index.js index 4f999e90..1c9afca1 100644 --- a/lib/update-v8/index.js +++ b/lib/update-v8/index.js @@ -26,6 +26,7 @@ export function minor(options) { export async function backport(options) { const shouldStop = await checkOptions(options); if (shouldStop) return; + options.gpgSign = options.gpgSign ? ['-S'] : []; const tasks = new Listr( [updateV8Clone(), doBackport(options)], getOptions(options) From 7822d08b3c91bdc2d2859d111bef28ac17448da6 Mon Sep 17 00:00:00 2001 From: Aviv Keller Date: Tue, 19 Nov 2024 13:23:38 -0500 Subject: [PATCH 6/7] fix(git-node): followup fix for supported wpt jsons (#855) --- components/git/wpt.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/git/wpt.js b/components/git/wpt.js index f5d1b22c..66c8784b 100644 --- a/components/git/wpt.js +++ b/components/git/wpt.js @@ -52,7 +52,7 @@ async function main(argv) { if (fs.existsSync(statusFolder)) { const jsons = fs.readdirSync(statusFolder); supported = supported.concat( - jsons.map(item => item.replace('.json', ''))); + jsons.map(item => path.basename(item, path.extname(item)))); } else { cli.warn(`Please create the status JSON files in ${statusFolder}`); } From cc6f1d4111bc523378839278ff3b0dc3be0a7b22 Mon Sep 17 00:00:00 2001 From: "Node.js GitHub Bot" Date: Tue, 19 Nov 2024 13:25:58 -0500 Subject: [PATCH 7/7] chore(main): release 5.7.0 (#868) --- CHANGELOG.md | 16 ++++++++++++++++ package.json | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fb3c683..90a7f23d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # Changelog +## [5.7.0](https://github.com/nodejs/node-core-utils/compare/v5.6.0...v5.7.0) (2024-11-19) + + +### Features + +* **git-node:** add release promotion step ([#835](https://github.com/nodejs/node-core-utils/issues/835)) ([dfa9c92](https://github.com/nodejs/node-core-utils/commit/dfa9c9201366e9f025034cd40cb5ec9a8968dc9e)) +* **git-node:** prompt before attempting branch-diff ([#869](https://github.com/nodejs/node-core-utils/issues/869)) ([391487b](https://github.com/nodejs/node-core-utils/commit/391487bb977fe122d322637c21c6f7ee8881f6b4)) +* **git-node:** use a single `git push` command ([#870](https://github.com/nodejs/node-core-utils/issues/870)) ([82527ad](https://github.com/nodejs/node-core-utils/commit/82527ad29271b9a07d58bffa6bb48c77127a089a)) +* **git-v8:** preserve original author when backporting ([#872](https://github.com/nodejs/node-core-utils/issues/872)) ([4eaad65](https://github.com/nodejs/node-core-utils/commit/4eaad654126a6ea7f4c7e684b8e04f9d94ae8d08)) + + +### Bug Fixes + +* **git-node:** do not assume release commit will conflict ([#871](https://github.com/nodejs/node-core-utils/issues/871)) ([ec6c6cb](https://github.com/nodejs/node-core-utils/commit/ec6c6cbf23d90738e4e73a17c245d24ac331de7d)) +* **git-node:** followup fix for supported wpt jsons ([#855](https://github.com/nodejs/node-core-utils/issues/855)) ([7822d08](https://github.com/nodejs/node-core-utils/commit/7822d08b3c91bdc2d2859d111bef28ac17448da6)) + ## [5.6.0](https://github.com/nodejs/node-core-utils/compare/v5.5.1...v5.6.0) (2024-11-08) diff --git a/package.json b/package.json index 4289a7a7..ea1e0420 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@node-core/utils", - "version": "5.6.0", + "version": "5.7.0", "description": "Utilities for Node.js core collaborators", "type": "module", "engines": {