diff --git a/components/git/land.js b/components/git/land.js index dc6aa9df..049f05c0 100644 --- a/components/git/land.js +++ b/components/git/land.js @@ -37,6 +37,11 @@ const landOptions = { describe: 'Assume "yes" as answer to all prompts and run ' + 'non-interactively. If an undesirable situation occurs, such as a pull ' + 'request or commit check fails, then git node land will abort.' + }, + backport: { + describe: 'Land a backport PR onto a staging branch', + default: false, + type: 'boolean' } }; @@ -152,8 +157,14 @@ async function main(state, argv, cli, req, dir) { cli.log('run `git node land --abort` before starting a new session'); return; } - session = new LandingSession(cli, req, dir, argv.prid); + session = new LandingSession(cli, req, dir, argv.prid, argv.backport); const metadata = await getMetadata(session.argv, cli); + if (argv.backport) { + const split = metadata.metadata.split('\n')[0]; + if (split === 'PR-URL: ') { + cli.error('Commit message is missing PR-URL'); + } + } return session.start(metadata); } else if (state === APPLY) { return session.apply(); diff --git a/docs/git-node.md b/docs/git-node.md index c0e8879c..ef91c883 100644 --- a/docs/git-node.md +++ b/docs/git-node.md @@ -40,6 +40,7 @@ Options: --continue, -c Continue the landing session [boolean] --final Verify the landed PR and clean up [boolean] --abort Abort the current landing session [boolean] + --backport Land a backport PR on a staging branch [boolean] --yes Assume "yes" as answer to all prompts and run non-interactively. If an undesirable situation occurs, such as a pull request or commit check fails, then git node land will @@ -47,12 +48,14 @@ Options: Examples: - git node land 12344 Land https://github.com/nodejs/node/pull/12344 in - the current directory - git node land --abort Abort the current session - git node land --amend Append metadata to the current commit message - git node land --final Verify the landed PR and clean up - git node land --continue Continue the current landing session + git node land 12344 Land https://github.com/nodejs/node/pull/12344 + in the current directory + git node land --abort Abort the current session + git node land --amend Append metadata to the current commit message + git node land --final Verify the landed PR and clean up + git node land --continue Continue the current landing session + git node land --backport 30072 Land https://github.com/nodejs/node/pull/30072 + as a backport in the current directory ``` diff --git a/lib/landing_session.js b/lib/landing_session.js index bf0b97f5..5a209735 100644 --- a/lib/landing_session.js +++ b/lib/landing_session.js @@ -13,9 +13,16 @@ const { const isWindows = process.platform === 'win32'; class LandingSession extends Session { - constructor(cli, req, dir, prid) { + constructor(cli, req, dir, prid, backport) { super(cli, dir, prid); this.req = req; + this.backport = backport; + } + + get argv() { + const args = super.argv; + args.backport = this.backport; + return args; } async start(metadata) { @@ -163,13 +170,25 @@ class LandingSession extends Session { amended.push(''); } + const BACKPORT_RE = /BACKPORT-PR-URL\s*:\s*(\S+)/i; + const PR_RE = /PR-URL\s*:\s*(\S+)/i; + const REVIEW_RE = /Reviewed-By\s*:\s*(\S+)/i; + for (const line of metadata) { if (original.includes(line)) { if (line) { cli.warn(`Found ${line}, skipping..`); } } else { - amended.push(line); + if (line.match(BACKPORT_RE)) { + let prIndex = amended.findIndex(datum => datum.match(PR_RE)); + if (prIndex === -1) { + prIndex = amended.findIndex(datum => datum.match(REVIEW_RE)) - 1; + } + amended.splice(prIndex + 1, 0, line); + } else { + amended.push(line); + } } } diff --git a/lib/links.js b/lib/links.js index 1243dee0..99cc68d5 100644 --- a/lib/links.js +++ b/lib/links.js @@ -4,6 +4,7 @@ const FIXES_RE = /(Close[ds]?|Fix(e[ds])?|Resolve[sd]?)\s*:\s*(\S+)/mgi; const FIX_RE = /(Close[ds]?|Fix(e[ds])?|Resolve[sd]?)\s*:\s*(\S+)/i; const REFS_RE = /Refs?\s*:\s*(\S+)/mgi; const REF_RE = /Refs?\s*:\s*(\S+)/i; +const PR_RE = /PR-URL\s*:\s*(\S+)/i; const cheerio = require('cheerio'); /** @@ -36,7 +37,19 @@ class LinkParser { const m = item.match(REF_RE); if (!m) continue; const ref = m[1]; - const url = this.getRefUrlFromOP(ref); + const url = this.getUrlFromOP(ref); + if (url) result.push(url); + } + return result; + } + + getPRUrlsFromArray(arr) { + const result = []; + for (const item of arr) { + const m = item.match(PR_RE); + if (!m) continue; + const prUrl = m[1]; + const url = this.getUrlFromOP(prUrl); if (url) result.push(url); } return result; @@ -44,7 +57,7 @@ class LinkParser { // Do this so we can reliably get the correct url. // Otherwise, the number could reference a PR or an issue. - getRefUrlFromOP(ref) { + getUrlFromOP(ref) { const as = this.$('a'); const links = as.map((i, el) => this.$(el)).get(); for (const link of links) { @@ -58,22 +71,22 @@ class LinkParser { getFixes() { const text = this.$.text(); - const fixes = text.match(FIXES_RE); - if (fixes) { - return this.getFixesUrlsFromArray(fixes); - } - return []; + const fixes = text.match(FIXES_RE) || []; + return this.getFixesUrlsFromArray(fixes); } getRefs() { const text = this.$.text(); - const refs = text.match(REFS_RE); - if (refs) { - return this.getRefsUrlsFromArray(refs); - } - return []; + const refs = text.match(REFS_RE) || []; + return this.getRefsUrlsFromArray(refs); } -}; + + getAltPrUrl() { + const text = this.$.text(); + const refs = text.match(PR_RE) || []; + return this.getPRUrlsFromArray(refs); + } +} const GITHUB_PULL_REQUEST_URL = /github.com\/([^/]+)\/([^/]+)\/pull\/(\d+)/; diff --git a/lib/metadata_gen.js b/lib/metadata_gen.js index 51aa3b3b..a22b3a88 100644 --- a/lib/metadata_gen.js +++ b/lib/metadata_gen.js @@ -10,11 +10,12 @@ class MetadataGenerator { * @param {PRData} data */ constructor(data) { - const { owner, repo, pr, reviewers } = data; + const { owner, repo, pr, reviewers, argv } = data; this.owner = owner; this.repo = repo; this.pr = pr; this.reviewers = reviewers; + this.argv = argv; } /** @@ -31,15 +32,24 @@ class MetadataGenerator { const parser = new LinkParser(owner, repo, op); const fixes = parser.getFixes(); const refs = parser.getRefs(); - + const altPrUrl = parser.getAltPrUrl(); const meta = [ - `PR-URL: ${prUrl}`, ...fixes.map((fix) => `Fixes: ${fix}`), - ...refs.map((ref) => `Refs: ${ref}`), - ...reviewedBy.map((r) => `Reviewed-By: ${r.reviewer.getContact()}`), - '' // creates final EOL + ...refs.map((ref) => `Refs: ${ref}`) ]; - + const backport = this.argv ? this.argv.backport : undefined; + if (backport) { + meta.unshift(`Backport-PR-URL: ${prUrl}`); + meta.unshift(`PR-URL: ${altPrUrl}`); + } else { + // Reviews are only added here as backports should not contain reviews + // for the backport itself in the metadata + meta.unshift(`PR-URL: ${prUrl}`); + meta.push( + ...reviewedBy.map((r) => `Reviewed-By: ${r.reviewer.getContact()}`) + ); + } + meta.push(''); // creates final EOL return meta.join('\n'); } } diff --git a/lib/session.js b/lib/session.js index 2a5d7f3b..0666ac0c 100644 --- a/lib/session.js +++ b/lib/session.js @@ -369,6 +369,7 @@ class Session { ` $ ncu-config set branch ${rev}`); cli.separator(); return true; + // TODO warn if backporting onto master branch } } diff --git a/test/fixtures/data.js b/test/fixtures/data.js index 3ca3e42f..9c85dfb4 100644 --- a/test/fixtures/data.js +++ b/test/fixtures/data.js @@ -75,6 +75,7 @@ const firstTimerPrivatePR = readJSON('first_timer_pr_with_private_email.json'); const semverMajorPR = readJSON('semver_major_pr.json'); const fixAndRefPR = readJSON('pr_with_fixes_and_refs.json'); const fixCrossPR = readJSON('pr_with_fixes_cross.json'); +const backportPR = readJSON('pr_with_backport.json'); const conflictingPR = readJSON('conflicting_pr.json'); const emptyProfilePR = readJSON('empty_profile_pr.json'); const closedPR = readJSON('./closed_pr.json'); @@ -114,6 +115,7 @@ module.exports = { semverMajorPR, fixAndRefPR, fixCrossPR, + backportPR, conflictingPR, emptyProfilePR, readme, diff --git a/test/fixtures/pr_with_backport.json b/test/fixtures/pr_with_backport.json new file mode 100644 index 00000000..fe83adb1 --- /dev/null +++ b/test/fixtures/pr_with_backport.json @@ -0,0 +1,22 @@ +{ + "createdAt": "2019-10-22T22:42:25Z", + "authorAssociation": "CONTRIBUTOR", + "author": { + "login": "gabrielschulhof", + "email": "gabriel.schulhof@intel.com", + "name": "Gabriel Schulhof" + }, + "url": "https://github.com/nodejs/node/pull/30072", + "bodyHTML": "
Build the addons for benchmarks in the same way that the addons for
\ntests are built.
PR-URL: #29995
\nFixes: nodejs/build#1961
\nRefs: 53ca0b9#commitcomment-35494896
\nReviewed-By: @addaleax
\nReviewed-By: @Trott
\nReviewed-By: @BethGriggs
\nReviewed-By: @gengjiawen
make -j4 test
(UNIX), or vcbuild test
(Windows) passesWe need this PR along with #30070 because 53ca0b9#commitcomment-35494896
", + "bodyText": "Build the addons for benchmarks in the same way that the addons for\ntests are built.\nPR-URL: #29995\nFixes: nodejs/build#1961\nRefs: 53ca0b9#commitcomment-35494896\nReviewed-By: @addaleax\nReviewed-By: @Trott\nReviewed-By: @BethGriggs\nReviewed-By: @gengjiawen\n\nChecklist\n\n\n make -j4 test (UNIX), or vcbuild test (Windows) passes\n commit message follows commit guidelines\n\n\nWe need this PR along with #30070 because 53ca0b9#commitcomment-35494896", + "labels": { + "nodes": [ + { + "name": "build" + }, + { + "name": "v12.x" + } + ] + } +} diff --git a/test/unit/metadata_gen.test.js b/test/unit/metadata_gen.test.js index ac2755ab..df7a05c1 100644 --- a/test/unit/metadata_gen.test.js +++ b/test/unit/metadata_gen.test.js @@ -4,6 +4,7 @@ const MetadataGenerator = require('../../lib/metadata_gen'); const { fixAndRefPR, fixCrossPR, + backportPR, allGreenReviewers } = require('../fixtures/data'); @@ -15,6 +16,21 @@ const data = { reviewers: allGreenReviewers }; const crossData = Object.assign({}, data, { pr: fixCrossPR }); +const backportArgv = { + argv: { + owner: 'nodejs', + repo: 'node', + upstream: 'upstream', + branch: 'v12.x-staging', + readme: undefined, + waitTimeSingleApproval: undefined, + waitTimeMultiApproval: undefined, + prid: 30072, + backport: true + } +}; + +const backportData = Object.assign({}, data, { pr: backportPR }, backportArgv); const expected = `PR-URL: https://github.com/nodejs/node/pull/16438 Fixes: https://github.com/nodejs/node/issues/16437 @@ -31,6 +47,11 @@ Reviewed-By: Quux User