diff --git a/build/shared/landing.mts b/build/shared/landing.mts new file mode 100644 index 000000000..1b3d9ede5 --- /dev/null +++ b/build/shared/landing.mts @@ -0,0 +1,94 @@ +/** + * @file Building the commit message a pull request lands as. + * @author The OpenINF Authors & Friends + * @license MIT OR Apache-2.0 OR BlueOak-1.0.0 + * @module {type ES6Module} build/shared/landing + * + * Nothing here knows what a subject is supposed to look like. Composing a + * landed message -- keeping every commit's words, gathering the trailers into + * the one paragraph git reads, adding the pull request it came through -- is + * the same job whatever house style a repository writes its subjects in, and + * the repositories in this organization do not agree on that yet. + */ + +import { TRAILER_ORDER } from '@openinf/portal/build/commit-message'; + +/** One commit's message, split into the parts a landed message reuses. */ +export type CommitParts = { + subject: string; + body: string[]; + trailers: string[]; +}; + +const tokenOf = (line: string) => line.match(/^([A-Za-z][\w-]*):/)?.[1] ?? ''; + +/** + * Splits a commit message into the parts a landed message reuses. Trailers + * are lifted out wherever they were written, because a squashed message can + * only have one trailer block and it has to be at the end. + * @param {string} message One commit's whole message. + * @returns {CommitParts} Its subject, its body, and the trailers it carried. + */ +export function partsOfMessage(message: string): CommitParts { + const [subject = '', ...rest] = message + .replace(/[\r\n]+$/, '') + .split(/\r?\n/); + const body: string[] = []; + const trailers: string[] = []; + + for (const line of rest) { + if (TRAILER_ORDER.includes(tokenOf(line))) trailers.push(line); + else body.push(line); + } + + while (body.at(-1)?.trim() === '') body.pop(); + while (body.at(0)?.trim() === '') body.shift(); + + return { subject, body, trailers }; +} + +/** + * Builds the message a pull request should land as. One commit lands as + * itself. Several land as one, every message kept whole so that nothing + * written down is lost -- each subject becomes a heading in the body, the + * first included, since a pull request title describes the whole and need not + * be any single commit's subject. + * @param {CommitParts[]} parts Each commit on the branch, oldest first. + * @param {string} prUrl The pull request the commits are landing through. + * @returns {string} Everything below the subject line. + */ +export function composeLandingMessage(parts: CommitParts[], prUrl: string) { + const paragraphs = + parts.length === 1 + ? (parts[0]?.body ?? []) + : parts.flatMap((part, index) => [ + ...(index === 0 ? [] : ['']), + part.subject, + '', + ...part.body, + ]); + + const rank = (line: string) => TRAILER_ORDER.indexOf(tokenOf(line)); + const gathered = [ + ...new Set([...parts.flatMap((part) => part.trailers), `PR-URL: ${prUrl}`]), + ].sort((one, other) => rank(one) - rank(other)); + + // `Fixes:` says everything `Refs:` would about the same issue. + const fixed = new Set( + gathered + .filter((line) => line.startsWith('Fixes:')) + .map((line) => line.slice('Fixes:'.length).trim()) + ); + + return [ + ...paragraphs, + '', + ...gathered.filter( + (line) => + !( + line.startsWith('Refs:') && + fixed.has(line.slice('Refs:'.length).trim()) + ) + ), + ].join('\n'); +} diff --git a/build/shared/landing.test.mts b/build/shared/landing.test.mts new file mode 100644 index 000000000..0da47c21e --- /dev/null +++ b/build/shared/landing.test.mts @@ -0,0 +1,177 @@ +/** + * @file Tests for building the message a pull request lands as. + * @author The OpenINF Authors & Friends + * @license MIT OR Apache-2.0 OR BlueOak-1.0.0 + * @module {type ES6Module} build/shared/landing.test + */ + +import { deepStrictEqual, match, ok } from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; +import { describe, test } from 'node:test'; +import { validateCommitMessage } from '@openinf/portal/build/commit-message'; +import { + composeLandingMessage, + partsOfMessage, +} from '@openinf/portal/build/landing'; + +const URL_ = 'https://github.com/OpenINF/openinf.github.io/pull/1234'; + +describe('partsOfMessage', () => { + test('separates subject, body and trailers', () => { + deepStrictEqual( + partsOfMessage( + '🏗️🔧:fix it\n\nWhy it needed fixing.\n\nSigned-off-by: A \n' + ), + { + subject: '🏗️🔧:fix it', + body: ['Why it needed fixing.'], + trailers: ['Signed-off-by: A '], + } + ); + }); + + test('lifts a trailer out of the middle of a body', () => { + // A squashed message can only have one trailer block, at the end, so one + // written half way up has to be found wherever it is. + const { body, trailers } = partsOfMessage( + '🏗️🔧:fix it\n\nRefs: https://x/1\n\nMore explanation.' + ); + + deepStrictEqual(trailers, ['Refs: https://x/1']); + deepStrictEqual(body, ['More explanation.']); + }); + + test('reads a message written with carriage returns', () => { + deepStrictEqual( + partsOfMessage('🏗️🔧:fix it\r\n\r\nBody.\r\n\r\nFixes: https://x/1\r\n') + .trailers, + ['Fixes: https://x/1'] + ); + }); +}); + +describe('composeLandingMessage', () => { + test('a single commit lands as itself, plus where it came from', () => { + const parts = [partsOfMessage('🏗️🔧:fix it\n\nWhy.\n\nRefs: https://x/1')]; + + // `PR-URL` comes before `Refs` in the documented order, so the trailer + // the commit carried moves below the one the landing adds. + deepStrictEqual( + composeLandingMessage(parts, URL_), + `Why.\n\nPR-URL: ${URL_}\nRefs: https://x/1` + ); + }); + + test('several commits keep every word, subjects as headings', () => { + const parts = [ + partsOfMessage('🏗️✨:the first thing\n\nWhy the first.'), + partsOfMessage('🏗️🔧:the second thing\n\nWhy the second.'), + ]; + + deepStrictEqual( + composeLandingMessage(parts, URL_), + [ + '🏗️✨:the first thing', + '', + 'Why the first.', + '', + '🏗️🔧:the second thing', + '', + 'Why the second.', + '', + `PR-URL: ${URL_}`, + ].join('\n') + ); + }); + + test('gathers scattered trailers into one block, in order', () => { + const parts = [ + partsOfMessage('🏗️✨:one\n\nA.\n\nFixes: https://x/9'), + partsOfMessage( + '🏗️🔧:two\n\nB.\n\nAssisted-by: Claude-Code:claude-opus-5\nSigned-off-by: D ' + ), + ]; + const message = composeLandingMessage(parts, URL_); + const block = message.slice(message.lastIndexOf('\n\n') + 2).split('\n'); + + deepStrictEqual(block, [ + 'Signed-off-by: D ', + 'Assisted-by: Claude-Code:claude-opus-5', + `PR-URL: ${URL_}`, + 'Fixes: https://x/9', + ]); + }); + + test('keeps one copy of a trailer both commits carried', () => { + const signed = 'Signed-off-by: D '; + const parts = [ + partsOfMessage(`🏗️✨:one\n\nA.\n\n${signed}`), + partsOfMessage(`🏗️🔧:two\n\nB.\n\n${signed}`), + ]; + + deepStrictEqual( + composeLandingMessage(parts, URL_) + .split('\n') + .filter((l) => l === signed).length, + 1 + ); + }); + + test('drops a Refs that duplicates a Fixes', () => { + const parts = [ + partsOfMessage('🏗️✨:one\n\nA.\n\nRefs: https://x/9'), + partsOfMessage('🏗️🔧:two\n\nB.\n\nFixes: https://x/9'), + ]; + const message = composeLandingMessage(parts, URL_); + + ok(message.includes('Fixes: https://x/9')); + ok(!message.includes('Refs: https://x/9')); + }); +}); + +describe('the message that comes out', () => { + test('passes the rules a commit answers to', () => { + const parts = [ + partsOfMessage( + '🏗️✨:one\n\nA reason.\n\nSigned-off-by: D \nAssisted-by: Claude-Code:claude-opus-5' + ), + partsOfMessage('🏗️🔧:two\n\nAnother reason.'), + ]; + const subject = '🏗️✨:land two things at once'; + + deepStrictEqual( + validateCommitMessage( + `${subject}\n\n${composeLandingMessage(parts, URL_)}` + ), + [] + ); + }); + + test('ends in a block git reads as trailers', () => { + const parts = [ + partsOfMessage('🏗️✨:one\n\nA reason.\n\nSigned-off-by: D '), + ]; + const message = `🏗️✨:one\n\n${composeLandingMessage(parts, URL_)}`; + const parsed = execFileSync('git', ['interpret-trailers', '--parse'], { + encoding: 'utf8', + input: message, + }) + .split('\n') + .filter(Boolean); + + deepStrictEqual(parsed, ['Signed-off-by: D ', `PR-URL: ${URL_}`]); + }); + + test('a body that would be too wide is still reported', () => { + // The rules are applied to the composed message, not to the commits it + // came from, so an over-wide line cannot slip through the join. + const parts = [partsOfMessage(`🏗️✨:one\n\n${'word '.repeat(20)}`)]; + + match( + validateCommitMessage( + `🏗️✨:one\n\n${composeLandingMessage(parts, URL_)}` + ).join(), + /the limit is 72/ + ); + }); +}); diff --git a/build/tasks/land-pull-request.mts b/build/tasks/land-pull-request.mts new file mode 100644 index 000000000..8941b58ed --- /dev/null +++ b/build/tasks/land-pull-request.mts @@ -0,0 +1,177 @@ +/** + * @file Land a pull request as one commit, with a message worth keeping. + * @author The OpenINF Authors & Friends + * @license MIT OR Apache-2.0 OR BlueOak-1.0.0 + * @module {type ES6Module} build/tasks/land-pull-request + * + * Run as `nps land `, or `nps "land --dry-run"` to see the + * message without landing anything. Squash merging through the GitHub user + * interface would take the pull request body, or nothing, and leave the + * trailers where git cannot read them; this builds the message instead, holds + * it to the same rules a commit answers to, and only then merges. + */ + +import { execFileSync } from 'node:child_process'; +import { validateCommitMessage } from '@openinf/portal/build/commit-message'; +import { + composeLandingMessage, + partsOfMessage, +} from '@openinf/portal/build/landing'; + +/** What this task needs to know about a pull request. */ +type PullRequest = { + title: string; + base: string; + head: string; + state: string; + draft: boolean; + mergeableState: string; +}; + +const REPOSITORY = 'OpenINF/openinf.github.io'; +const DEFAULT_BRANCH = 'live'; + +const [number, ...flags] = process.argv.slice(2); +const dryRun = flags.includes('--dry-run'); + +const gh = (...args: string[]) => + execFileSync('gh', args, { encoding: 'utf8', maxBuffer: 1 << 24 }).trim(); +const git = (...args: string[]) => + execFileSync('git', args, { encoding: 'utf8', maxBuffer: 1 << 24 }).trim(); + +/** + * Reads the pull request, waiting for GitHub to work out whether it merges. + * The answer is `unknown` for a moment after anything lands on the base, and + * acting on that would be acting on a stale one. + * @param {string} pull The pull request number. + * @returns {Promise} The fields this task needs. + */ +const readPull = async (pull: string): Promise => { + const fetchPull = () => + JSON.parse( + gh( + 'api', + `repos/${REPOSITORY}/pulls/${pull}`, + '--jq', + '{title, base: .base.ref, head: .head.sha, state, draft, mergeableState: .mergeable_state}' + ) + ); + let found = fetchPull(); + + // Only an open pull request is worth waiting on. A closed one reports + // `unknown` for ever, and waiting twenty seconds to say so helps nobody. + for ( + let attempt = 0; + attempt < 10 && + found.state === 'open' && + !found.draft && + found.mergeableState === 'unknown'; + attempt += 1 + ) { + await new Promise((resolve) => setTimeout(resolve, 2000)); + found = fetchPull(); + } + + return found; +}; + +/** + * Says why a pull request cannot be landed as it stands. + * @param {PullRequest} pull What GitHub reports about it. + * @returns {string} The reason, or an empty string if there is none. + */ +const refuse = (pull: PullRequest) => { + if (pull.draft || pull.state !== 'open') { + return `it is ${pull.draft ? 'a draft' : pull.state}`; + } + + if (pull.mergeableState === 'dirty') { + return `it conflicts with ${DEFAULT_BRANCH}; rebase the branch onto it and push first`; + } + + if (pull.base !== DEFAULT_BRANCH) { + return `it targets ${pull.base}; retarget it, or land its base first`; + } + + if (pull.mergeableState === 'unknown') { + return 'GitHub has not worked out whether it merges yet; try again shortly'; + } + + return ''; +}; + +if (number === undefined || !/^\d+$/.test(number)) { + console.error('Usage: nps "land [--dry-run]"'); + process.exitCode = 1; +} else { + const pull = await readPull(number); + const reason = refuse(pull); + + if (reason !== '') { + console.error(`#${number} cannot be landed: ${reason}.`); + process.exitCode = 1; + } else { + // Oldest first, so the landed message reads in the order the work was + // done rather than the order git lists it. + const shas = git( + 'rev-list', + '--reverse', + '--no-merges', + `origin/${DEFAULT_BRANCH}..${pull.head}` + ) + .split('\n') + .filter(Boolean); + const parts = shas.map((sha) => + partsOfMessage(git('log', '-1', '--format=%B', sha)) + ); + const message = composeLandingMessage( + parts, + `https://github.com/${REPOSITORY}/pull/${number}` + ); + const problems = validateCommitMessage(`${pull.title}\n\n${message}`); + const rule = '='.repeat(72); + + console.log(`${rule}\n${pull.title}\n\n${message}\n${rule}`); + console.log( + `#${number}: ${shas.length} commit${shas.length === 1 ? '' : 's'}, ${pull.mergeableState}` + ); + + if (shas.length === 0) { + console.error(`\n#${number} has no commits over ${DEFAULT_BRANCH}.`); + process.exitCode = 1; + } else if (problems.length > 0) { + console.error( + '\nThe message this would land does not pass its own rules:' + ); + for (const problem of problems) console.error(` ${problem}`); + process.exitCode = 1; + } else if (dryRun) { + console.log('\nDry run; nothing landed.'); + } else { + const merged = JSON.parse( + gh( + 'api', + '-X', + 'PUT', + `repos/${REPOSITORY}/pulls/${number}/merge`, + '-f', + 'merge_method=squash', + '-f', + `commit_title=${pull.title}`, + '-f', + `commit_message=${message}`, + '-f', + `sha=${pull.head}`, + '--jq', + '{merged, sha}' + ) + ); + + if (merged.merged === true) console.log(`\nLanded as ${merged.sha}`); + else { + console.error(`\n#${number} did not merge.`); + process.exitCode = 1; + } + } + } +} diff --git a/package-scripts.yml b/package-scripts.yml index c58790260..84ea5962c 100644 --- a/package-scripts.yml +++ b/package-scripts.yml @@ -39,6 +39,9 @@ scripts: toml: node build/tasks/format/format-toml.mts ts: node build/tasks/format/format-ts.mts yaml: node build/tasks/format/format-yaml.mts + # Squash a pull request into one commit with a message built from the + # commits it contains. Takes a number, and --dry-run to see it first. + land: node build/tasks/land-pull-request.mts build: nps compile.buildPortal test: nps verify.all start: eleventy --serve diff --git a/package.json b/package.json index affd224a6..0227bb74a 100644 --- a/package.json +++ b/package.json @@ -11,6 +11,7 @@ "exports": { "./build/commit-message": "./build/shared/commit-message.mts", "./build/constants": "./build/shared/constants.mts", + "./build/landing": "./build/shared/landing.mts", "./build/utils": "./build/utils.mts" }, "homepage": "open.inf.is",