chore(node-pr): various fixes and updates for the node PR script (#5818)

.github/workflows/create-node-pr.yml

@@ -9,6 +9,10 @@ on:
         description: "The npm spec to create the PR from"
         required: true
         default: 'latest'
+      branch:
+        description: "The major node version to serve as the base of the PR. Should be `main` or a number like `18`, `19`, etc."
+        required: true
+        default: 'main'
       dryRun:
         description: "Setting this to anything will run all the steps except opening the PR"
 
@@ -23,6 +27,8 @@ jobs:
     steps:
       - name: Checkout
         uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
       - name: Setup Git User
         run: |
           git config --global user.email "npm-cli+bot@github.com"
@@ -46,4 +52,4 @@ jobs:
           GITHUB_TOKEN: ${{ secrets.NODE_PULL_REQUEST_TOKEN }}
         run: |
           DRY_RUN=$([ -z "${{ inputs.dryRun }}" ] && echo "" || echo "--dry-run")
-          node scripts/create-node-pr.js "${{ inputs.spec }}" "$DRY_RUN"
+          node scripts/create-node-pr.js ${{ inputs.spec }} ${{ inputs.branch }} "$DRY_RUN"

DEPENDENCIES.md

@@ -536,6 +536,9 @@ graph LR;
   npm-->read-package-json;
   npm-->read;
   npm-->readdir-scoped-modules;
+  npm-->remark-gfm;
+  npm-->remark-github;
+  npm-->remark;
   npm-->rimraf;
   npm-->semver;
   npm-->smoke-tests;

package.json

@@ -206,6 +206,9 @@
     "licensee": "^9.0.0",
     "nock": "^13.2.4",
     "npm-packlist": "^7.0.1",
+    "remark": "^14.0.2",
+    "remark-gfm": "^3.0.1",
+    "remark-github": "^11.2.4",
     "spawk": "^1.7.1",
     "tap": "^16.0.1"
   },

scripts/create-node-pr.js

@@ -1,4 +1,4 @@
-const { join } = require('path')
+const { join, basename } = require('path')
 const fsp = require('fs/promises')
 const hgi = require('hosted-git-info')
 const semver = require('semver')
@@ -8,11 +8,12 @@ const tar = require('tar')
 const { cp, withTempDir } = require('@npmcli/fs')
 const { CWD, run, spawn, git, fs, gh } = require('./util.js')
 
+const NODE_FORK = 'npm/node'
 // this script expects node to already be cloned to a directory at the cli root named "node"
 const NODE_DIR = join(CWD, 'node')
 const gitNode = spawn.create('git', { cwd: NODE_DIR })
 
-const createNodeTarball = async ({ mani, registryOnly, tag, dir: extractDir }) => {
+const createNodeTarball = async ({ mani, registryOnly, localTest, tag, dir: extractDir }) => {
   const tarball = join(extractDir, 'npm-node.tgz')
   await pacote.tarball.file(mani._from, tarball, { resolved: mani._resolved })
 
@@ -28,12 +29,26 @@ const createNodeTarball = async ({ mani, registryOnly, tag, dir: extractDir }) =
   await fs.rimraf(tarball)
 
   // checkout the tag since we need to get files from source.
-  await git.dirty()
-  tag && await git('checkout', tag)
-  for (const path of ['.npmrc', 'tap-snapshots/', 'test/']) {
+  if (!localTest) {
+    try {
+      await git('checkout', tag)
+    } catch (err) {
+      log.error('Use the `--local-test` flag to avoid checking out the tag')
+      throw err
+    }
+  }
+  // currently there is an empty .npmrc file in the deps/npm dir in the node repo
+  // i do not know why and it might not be used but in order to minimize any
+  // unnecessary churn, let's create that file to match the old process
+  await fsp.writeFile(join(extractDir, '.npmrc'), '', 'utf-8')
+
+  // copy our test dirs so that tests can be run
+  for (const path of ['tap-snapshots/', 'test/']) {
     await cp(join(CWD, path), join(extractDir, path), { recursive: true })
   }
 
+  // recreate the tarball as closely as possible to how we would before publishing
+  // to the registry. the only difference here is the extra files we put in the dir
   await tar.c({
     ...pacote.DirFetcher.tarCreateOptions(mani),
     cwd: extractDir,
@@ -43,82 +58,227 @@ const createNodeTarball = async ({ mani, registryOnly, tag, dir: extractDir }) =
   return tarball
 }
 
-const main = async (spec, opts) => withTempDir(CWD, async (tmpDir) => {
-  const { dryRun, registryOnly, skipCheckout } = opts
+const getPrBody = async ({ releases, closePrs }) => {
+  const useSummary = releases.length > 1
+  const releasePath = (v) => `/npm/cli/releases/tag/v${v}`
 
-  const mani = await pacote.manifest(`npm@${spec}`, { preferOnline: true })
+  // XXX: add links to relevant CI and CITGM runs once we no longer include our tests
+  let prBody = ''
+
+  if (useSummary) {
+    const summary = releases.map(r => {
+      return `[\`npm@${r.version}\`](https://github.com${releasePath(r.version)})`
+    })
+    prBody += `This PR contains changes from: ${summary.join(' ')}\n\n`
+  }
+
+  if (closePrs.length) {
+    prBody += `This PR replaces: ${closePrs.map(pr => pr.url).join(' ')}\n\n`
+  }
 
-  const head = {
-    tag: `v${mani.version}`,
-    branch: `npm-v${mani.version}`,
-    host: hgi.fromUrl('npm/node'),
-    message: `deps: upgrade npm to ${mani.version}`,
+  if (prBody) {
+    prBody += '---\n\n'
   }
-  log.silly(head)
+
+  for (const { version, body } of releases) {
+    prBody += useSummary
+      ? `<details><summary>${version}</summary>\n<p>\n\n${body}\n\n</p>\n</details>`
+      : body
+    prBody += '\n'
+  }
+
+  // These comes from the releases so those link to the raw comparison between tags.
+  // Since we are putting this in a PR we can change those links back to the releases.
+  prBody = prBody.replace(/\/npm\/cli\/compare\/v[\w.-]+\.\.\.v([\w.-]+)/g, releasePath('$1'))
+
+  const { remark } = await import('remark')
+  const { default: remarkGfm } = await import('remark-gfm')
+  const { default: remarkGithub } = await import('remark-github')
+
+  return remark()
+    .use(remarkGfm)
+    .use(remarkGithub, {
+      repository: 'npm/cli',
+      // dont link mentions, but anything else make the link an explicit referance to npm/cli
+      buildUrl: (values, buildUrl) => values.type === 'mention' ? false : buildUrl(values),
+    })
+    .process(prBody)
+    .then(v => String(v))
+}
+
+const tokenRemoteUrl = ({ host, token }) => {
+  // this is a remote url that uses a github token as the username
+  // in order to authenticate with github
+  const headRemoteUrl = new URL(host.https())
+  headRemoteUrl.username = token
+  // we have to manually change the protocol. the whatwg url spec
+  // does not allow changing a special protocol to another one
+  // but the protocol has to be `https:` without the `git+`
+  return headRemoteUrl.toString().replace('git+https:', 'https:')
+}
+
+const main = async (spec, branch = 'main', opts) => withTempDir(CWD, async (tmpDir) => {
+  const { GITHUB_TOKEN } = process.env
+  const { dryRun, registryOnly, localTest } = opts
+
+  if (!spec) {
+    throw new Error('`spec` is required as the first argument')
+  }
+
+  if (!branch) {
+    throw new Error('`branch` is required as the second argument')
+  }
+
+  if (!GITHUB_TOKEN) {
+    throw new Error(`process.env.GITHUB_TOKEN is required`)
+  }
+
+  await fsp.access(NODE_DIR, fsp.constants.F_OK).catch(() => {
+    throw new Error(`node repo must be checked out to \`${NODE_DIR}\` to continue`)
+  })
+
+  await gh.json('repo', 'view', NODE_FORK, 'url').catch(() => {
+    throw new Error(`node repo must be forked to ${NODE_FORK}`)
+  })
+
+  await git.dirty().catch((er) => {
+    if (localTest) {
+      return log.info('Skipping git dirty check due to `--local-test` flag')
+    }
+    throw er
+  })
+
+  const mani = await pacote.manifest(`npm@${spec}`, { preferOnline: true })
+  const packument = await pacote.packument('npm', { preferOnline: true })
+  const npmVersions = Object.keys(packument.versions).sort(semver.rcompare)
+
+  const npmVersion = semver.parse(mani.version)
+  const npmHost = hgi.fromUrl(NODE_FORK)
+  const npmTag = `v${npmVersion}`
+  const npmBranch = `npm-${npmTag}`
+  const npmRemoteUrl = tokenRemoteUrl({ host: npmHost, token: GITHUB_TOKEN })
+  const npmMessage = (v = npmVersion) => `deps: upgrade npm to ${v}`
 
   const tarball = await createNodeTarball({
     mani,
+    tag: npmTag,
     dir: tmpDir,
     registryOnly,
-    // the only reason this is optional is for testing when updating this script.
-    // if we checkout an older tag, it won't have the updates we are testing.
-    tag: skipCheckout ? null : head.tag,
+    localTest,
   })
+  log.info('tarball path', tarball)
 
-  await fsp.access(NODE_DIR, fsp.constants.F_OK).catch(() => {
-    throw new Error(`node repo must be checked out to \`${NODE_DIR}\` to continue`)
-  })
+  const nodeRemote = 'origin'
+  const nodeBranch = /^\d+$/.test(branch) ? `v${branch}.x-staging` : branch
+  const nodeHost = hgi.fromUrl(await gitNode('remote', 'get-url', nodeRemote, { out: true }))
+  const nodePrArgs = ['pr', '-R', nodeHost.path()]
+
+  await gitNode('fetch', nodeRemote)
+  await gitNode('checkout', nodeBranch)
+  await gitNode('reset', '--hard', `${nodeRemote}/${nodeBranch}`)
+
+  const nodeNpmPath = join('deps', 'npm')
+  const nodeNpmDir = join(NODE_DIR, nodeNpmPath)
+  const nodeNpmVersion = require(join(nodeNpmDir, 'package.json')).version
 
-  const base = {
-    // we used to send PRs sometimes for old versions to the 14.x staging
-    // branch. this might not be needed anymore, but this is how we
-    // would do it, if we needed to send a PR for backport fixes
-    branch: semver.major(mani.version) <= 8 ? '14.x-staging' : 'main',
-    remote: 'origin',
-    host: hgi.fromUrl(await gitNode('remote', 'get-url', 'origin', { out: true })),
+  // this is the range of all versions included in this update based
+  // on the current version of npm in node currently. we use this
+  // to build a list of all release notes and to close any existing PRs
+  const newNpmVersions = npmVersions.slice(
+    npmVersions.indexOf(npmVersion.toString()),
+    npmVersions.indexOf(nodeNpmVersion)
+  )
+    .reverse()
+    .map((v) => semver.parse(v))
+    .filter((version) => version.major === npmVersion.major)
+
+  // get a list of all versions changelogs to add to the body of the PR
+  // do this before we checkout our branch and make any changes
+  const npmReleases = await Promise.all(newNpmVersions.map(async (v) => {
+    // dont include prereleases unless we are updating to a prerlease since we
+    // manually put all prerelease notes into the first stable major version
+    if (v.prerelease.length && !npmVersion.prerelease.length) {
+      return null
+    }
+    return {
+      version: v,
+      body: await gh.json('release', 'view', `v${v}`, 'body', { quiet: true }).then(r => r.trim()),
+    }
+  })).then(r => r.filter(Boolean))
+
+  log.info('npm versions', newNpmVersions.map(v => v.toString()))
+  log.info('npm releases', npmReleases.map(u => u.version.toString()))
+
+  await gitNode('branch', '-D', npmBranch, { ok: true })
+  await gitNode('checkout', '-b', npmBranch)
+  await fs.clean(nodeNpmDir)
+  await tar.x({ strip: 1, file: tarball, cwd: nodeNpmDir })
+  await fs.rimraf(join(nodeNpmDir, basename(tarball)))
+
+  await gitNode('add', '-A', nodeNpmPath)
+  await gitNode('commit', '-m', npmMessage())
+  await gitNode('rebase', '--whitespace', 'fix', nodeBranch)
+
+  await gitNode('remote', 'rm', npmHost.user, { ok: true })
+  await gitNode('remote', 'add', npmHost.user, npmRemoteUrl)
+  if (!dryRun) {
+    await gitNode('push', npmHost.user, npmBranch, '--force')
   }
-  log.silly(base)
 
-  await gh('repo', 'fork', base.host.path(), '--org', head.host.user, { quiet: true, ok: true })
-  await gitNode('fetch', base.remote)
-  await gitNode('checkout', base.branch)
-  await gitNode('reset', '--hard', `${base.remote}/${base.branch}`)
-  await gitNode('branch', '-D', head.branch, { ok: true })
-  await gitNode('checkout', '-b', head.branch)
+  const npmPrs = await gh.json(
+    ...nodePrArgs, 'list',
+    '-S', `in:title "${npmMessage('')}"`,
+    'number,title,url'
+  )
+
+  log.info('Found other npm PRs', npmPrs)
 
-  const npmPath = join('deps', 'npm')
-  const npmDir = join(NODE_DIR, npmPath)
-  await fs.clean(npmDir)
-  await tar.x({ strip: 1, file: tarball, cwd: npmDir })
+  let existingPr = null
+  const closePrs = []
 
-  await gitNode('add', '-A', npmPath)
-  await gitNode('commit', '-m', head.message)
-  await gitNode('rebase', '--whitespace', 'fix', base.branch)
+  for (const pr of npmPrs) {
+    const prVersion = pr.title.replace(npmMessage(''), '').trim()
+    log.silly('checking existing PR', prVersion, pr)
 
-  await gitNode('remote', 'add', head.host.user, head.host.ssh(), { ok: true })
-  await gitNode('push', head.host.user, head.branch, '--force')
+    if (!existingPr && prVersion === npmVersion.toString()) {
+      existingPr = pr
+    } else if (newNpmVersions.some(v => v.toString() === prVersion)) {
+      closePrs.push(pr)
+    }
+  }
+
+  log.info('Found exisiting PR', existingPr)
+  log.info('Found PRs to close', closePrs)
 
-  const notes = await gh.json('release', 'view', head.tag, 'body')
-  log.silly('body', notes)
+  const prBody = await getPrBody({ releases: npmReleases, closePrs })
 
   const prArgs = [
-    'pr', 'create',
-    '-R', base.host.path(),
-    '-B', base.branch,
-    '-H', `${head.host.user}:${head.branch}`,
-    '-t', head.message,
-  ]
+    nodePrArgs,
+    (existingPr ? ['edit', existingPr.number] : ['create', '-H', `${npmHost.user}:${npmBranch}`]),
+    '-B', nodeBranch,
+    '-t', npmMessage(),
+  ].flat()
 
   if (dryRun) {
     log.info(`gh ${prArgs.join(' ')}`)
-    const url = new URL(base.host.browse())
-    const compare = `${base.branch}...${head.host.user}:${head.host.project}:${head.branch}`
-    url.pathname += `/compare/${compare}`
-    url.searchParams.set('expand', '1')
-    return url.toString()
+    return prBody
+  }
+
+  const newOrUpdatedPr = await gh(prArgs, '-F', '-', { input: prBody, out: true })
+  const closeMessage = `Closing in favor of ${newOrUpdatedPr}`
+
+  for (const closePr of closePrs) {
+    log.info('Attempting to close PR', closePr.url)
+    try {
+      await gh(nodePrArgs, 'close', closePr.number, '-c', closeMessage)
+    } catch (err) {
+      log.error('Could not close PR', err)
+    }
   }
 
-  return gh(...prArgs, '-F', '-', { cwd: NODE_DIR, input: notes, out: true })
+  return newOrUpdatedPr
 })
 
-run(({ argv, ...opts }) => main(argv.remain[0], opts))
+run(({ argv, ...opts }) => main(argv.remain[0], argv.remain[1], opts), {
+  redact: new RegExp(process.env.GITHUB_TOKEN, 'g'),
+})