Improve release proposal script (#5338)

This introduces two significant changes:

1. Don't suppress important errors. Some errors generated by the
   commands in the script should be ignored as they are just noise.
   Others however are important clues to setup and permission issues.

2. Improve requirements check, to ensure branch-diff has the expected
   permissions.

Author: watson

LICENSE-3rdparty.csv

@@ -37,6 +37,7 @@ dev,@eslint/eslintrc,MIT,Copyright OpenJS Foundation and other contributors, <ww
 dev,@eslint/js,MIT,Copyright OpenJS Foundation and other contributors, <www.openjsf.org>
 dev,@msgpack/msgpack,ISC,Copyright 2019 The MessagePack Community
 dev,@stylistic/eslint-plugin-js,MIT,Copyright OpenJS Foundation and other contributors, <www.openjsf.org>
+dev,application-config-path,MIT,Copyright (c) 2015, 2023 Linus Unnebäck
 dev,autocannon,MIT,Copyright 2016 Matteo Collina
 dev,aws-sdk,Apache 2.0,Copyright 2012-2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 dev,axios,MIT,Copyright 2014-present Matt Zabriskie

package.json

@@ -122,6 +122,7 @@
     "@msgpack/msgpack": "^3.0.0-beta3",
     "@stylistic/eslint-plugin-js": "^3.0.1",
     "@types/node": "^16.0.0",
+    "application-config-path": "^1.0.0",
     "autocannon": "^4.5.2",
     "aws-sdk": "^2.1446.0",
     "axios": "^1.7.4",

scripts/release/helpers/requirements.js

@@ -2,14 +2,15 @@
 
 /* eslint-disable @stylistic/js/max-len */
 
+const { join } = require('path')
+const { existsSync, readFileSync } = require('fs')
+const { default: getApplicationConfigPath } = require('application-config-path')
 const { capture, fatal, run } = require('./terminal')
 
-const requiredScopes = ['public_repo', 'read:org']
-
 // Check that the `git` CLI is installed.
 function checkGit () {
   try {
-    run('git --version')
+    run('git --version', false)
   } catch (e) {
     fatal(
       'The "git" CLI could not be found.',
@@ -21,7 +22,7 @@ function checkGit () {
 // Check that the `branch-diff` CLI is installed.
 function checkBranchDiff () {
   try {
-    run('branch-diff --version')
+    run('branch-diff --version', false)
   } catch (e) {
     const link = [
       'https://datadoghq.atlassian.net/wiki/spaces/DL/pages/3125511269/Node.js+Tracer+Release+Process',
@@ -32,53 +33,72 @@ function checkBranchDiff () {
       `Please visit ${link} for instructions to install.`
     )
   }
+
+  const branchDiffConfigPath = join(getApplicationConfigPath('changelog-maker'), 'config.json')
+
+  if (!existsSync(branchDiffConfigPath)) {
+    const link = 'https://github.com/nodejs/changelog-maker?tab=readme-ov-file#development'
+    fatal(
+      'The "branch-diff" configuration file is missing.',
+      `Please visit ${link} for instructions to configure.`
+    )
+  }
+
+  const requiredScopes = ['public_repo']
+  const { token } = JSON.parse(readFileSync(branchDiffConfigPath, 'utf8'))
+
+  checkGitHubScopes(token, requiredScopes, 'branch-diff')
 }
 
 // Check that the `gh` CLI is installed and authenticated.
 function checkGitHub () {
-  if (!process.env.GITHUB_TOKEN && !process.env.GH_TOKEN) {
+  const token = process.env.GITHUB_TOKEN || process.env.GH_TOKEN
+  const requiredScopes = ['public_repo', 'read:org']
+
+  if (!token) {
     const link = 'https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens#creating-a-personal-access-token-classic'
 
     fatal(
-      'The GITHUB_TOKEN environment variable is missing.',
+      'The GITHUB_TOKEN | GH_TOKEN environment variable is missing.',
       `Please visit ${link} for instructions to generate a personal access token.`,
       `The following scopes are required when generating the token: ${requiredScopes.join(', ')}`
     )
   }
 
   try {
-    run('gh --version')
+    run('gh --version', false)
   } catch (e) {
     fatal(
       'The "gh" CLI could not be found.',
       'Please visit https://github.com/cli/cli#installation for instructions to install.'
     )
   }
 
-  checkGitHubScopes()
+  checkGitHubScopes(token, requiredScopes, 'GITHUB_TOKEN | GH_TOKEN')
 }
 
-// Check that the active GITHUB_TOKEN has the required scopes.
-function checkGitHubScopes () {
+function checkGitHubScopes (token, requiredScopes, source) {
   const url = 'https://api.github.com'
   const headers = [
     'Accept: application/vnd.github.v3+json',
-    `Authorization: Bearer ${process.env.GITHUB_TOKEN || process.env.GH_TOKEN}`,
+    `Authorization: Bearer ${token}`,
     'X-GitHub-Api-Version: 2022-11-28'
   ].map(h => `-H "${h}"`).join(' ')
 
-  const lines = capture(`curl -sS -I ${headers} ${url}`).trim().split(/\r?\n/g)
+  const lines = capture(`curl -sS -I ${headers} ${url}`).split(/\r?\n/g)
   const scopeLine = lines.find(line => line.startsWith('x-oauth-scopes:')) || ''
   const scopes = scopeLine.replace('x-oauth-scopes:', '').trim().split(', ')
-  const link = 'https://github.com/settings/tokens'
+  const missingScopes = []
 
   for (const req of requiredScopes) {
-    if (!scopes.includes(req)) {
-      fatal(
-        `Missing "${req}" scope for GITHUB_TOKEN.`,
-        `Please visit ${link} and make sure the following scopes are enabled: ${requiredScopes.join(' ,')}.`
-      )
-    }
+    if (!scopes.includes(req)) missingScopes.push(req)
+  }
+
+  if (missingScopes.length !== 0) {
+    fatal(
+      `Missing scopes for ${source}: ${missingScopes.join(', ')}`,
+      'Please visit https://github.com/settings/tokens and make sure they are enabled!'
+    )
   }
 }
 

scripts/release/helpers/terminal.js

@@ -1,6 +1,6 @@
 'use strict'
 
-const { execSync, spawnSync } = require('child_process')
+const { spawnSync } = require('child_process')
 
 const { params, flags } = parse()
 
@@ -23,19 +23,20 @@ let timer
 let current
 
 // Output a command to the terminal and execute it.
-function run (cmd) {
-  capture(cmd)
+function run (cmd, treatStderrAsFailure = true) {
+  capture(cmd, treatStderrAsFailure)
 }
 
 // Ask a question in terminal and return the response.
 function prompt (question) {
   print(`${BOLD}${CYAN}?${RESET} ${BOLD}${question}${RESET} `)
 
-  const child = spawnSync('bash', ['-c', 'read answer && echo $answer'], {
+  const { stdout } = spawnSync('bash', ['-c', 'read answer && echo $answer'], {
+    encoding: 'utf8',
     stdio: ['inherit']
   })
 
-  return child.stdout.toString()
+  return stdout
 }
 
 // Ask whether to continue and otherwise exit the process.
@@ -54,18 +55,49 @@ function checkpoint (question) {
 }
 
 // Run a command and capture its output to return it to the caller.
-function capture (cmd) {
+function capture (cmd, treatStderrAsFailure = true) {
   if (flags.debug) {
     log(`${GRAY}> ${cmd}${RESET}`)
   }
 
-  const output = execSync(cmd, { encoding: 'utf8', stdio: 'pipe' }).toString().trim()
+  const result = spawnSync(cmd, { encoding: 'utf8', shell: true })
+
+  if (result.error) throw result.error
+  if (result.status !== 0) {
+    const err = new Error(`Command failed: ${cmd}\n${result.stderr}`)
+    for (const [k, v] of Object.entries(result)) {
+      err[k] = v
+    }
+    throw err
+  }
+
+  const stdout = result.stdout.trim()
+  const stderr = result.stderr.trim()
 
   if (flags.debug) {
-    log(output)
+    log(stdout)
+    if (stderr) {
+      if (flags['no-abort-on-error']) {
+        log(`${RED}${stderr}${RESET}`)
+      } else {
+        fatal(
+          stderr,
+          'Aborting due to error! Use --no-abort-on-error to ignore and continue.'
+        )
+      }
+    }
+  } else if (treatStderrAsFailure && stderr) {
+    if (flags['no-abort-on-error']) {
+      log(`${RED}${stderr}${RESET}`)
+    } else {
+      fatal(
+        stderr,
+        'Aborting due to error! Use --no-abort-on-error to ignore and continue.'
+      )
+    }
   }
 
-  return output
+  return stdout
 }
 
 // Start an operation and show a spinner until it reports as passing or failing.

scripts/release/proposal.js

@@ -50,40 +50,39 @@ try {
   // Make sure the release branch is up to date to prepare for new proposal.
   // The main branch is not automatically pulled to avoid inconsistencies between
   // release lines if new commits are added to it during a release.
-  run(`git checkout v${releaseLine}.x`)
-  run('git pull --ff-only')
+  run(`git checkout --quiet v${releaseLine}.x`)
+  run('git pull --quiet --ff-only')
 
   pass(`v${releaseLine}.x`)
 
   const diffCmd = [
     'branch-diff',
     '--user DataDog',
     '--repo dd-trace-js',
-  `--exclude-label=semver-major,dont-land-on-v${releaseLine}.x`
+    `--exclude-label=semver-major,dont-land-on-v${releaseLine}.x`
   ].join(' ')
 
   start('Determine version increment')
 
-  const lastVersion = require('../../package.json').version
-  const [, lastMinor, lastPatch] = lastVersion.split('.').map(Number)
+  const { DD_MAJOR, DD_MINOR, DD_PATCH } = require('../../version')
   const lineDiff = capture(`${diffCmd} --markdown=true v${releaseLine}.x master`)
   const isMinor = flags.minor || (!flags.patch && lineDiff.includes('SEMVER-MINOR'))
   const newVersion = isMinor
-    ? `${releaseLine}.${lastMinor + 1}.0`
-    : `${releaseLine}.${lastMinor}.${lastPatch + 1}`
+    ? `${releaseLine}.${DD_MINOR + 1}.0`
+    : `${releaseLine}.${DD_MINOR}.${DD_PATCH + 1}`
   const notesDir = path.join(os.tmpdir(), 'release_notes')
   const notesFile = path.join(notesDir, `${newVersion}.md`)
 
-  pass(`${isMinor ? 'minor' : 'patch'} (${lastVersion} -> ${newVersion})`)
+  pass(`${isMinor ? 'minor' : 'patch'} (${DD_MAJOR}.${DD_MINOR}.${DD_PATCH} -> ${newVersion})`)
 
   start('Checkout release proposal branch')
 
   // Checkout new or existing branch.
-  run(`git checkout v${newVersion}-proposal || git checkout -b v${newVersion}-proposal`)
+  run(`git checkout --quiet v${newVersion}-proposal &> /dev/null || git checkout --quiet -b v${newVersion}-proposal`)
 
   try {
     // Pull latest changes in case the release was started by someone else.
-    run(`git remote show origin | grep v${newVersion} && git pull --ff-only`)
+    run(`git remote show origin | grep v${newVersion} && git pull --ff-only`, false)
   } catch (e) {
     // Either there is no remote to pull from or the local and remote branches
     // have diverged. In both cases we ignore the error and will just use our
@@ -95,7 +94,7 @@ try {
   start('Check for new changes')
 
   // Get the hashes of the last version and the commits to add.
-  const lastCommit = capture('git log -1 --pretty=%B').trim()
+  const lastCommit = capture('git log -1 --pretty=%B')
   const proposalDiff = capture(`${diffCmd} --format=sha --reverse v${newVersion}-proposal master`)
     .replace(/\n/g, ' ').trim()
 
@@ -114,7 +113,7 @@ try {
 
     // Cherry pick all new commits to the proposal branch.
     try {
-      run(`echo "${proposalDiff}" | xargs git cherry-pick`)
+      run(`echo "${proposalDiff}" | xargs git cherry-pick`, false)
 
       pass()
     } catch (err) {
@@ -148,7 +147,7 @@ try {
 
   // Create or edit the PR. This will also automatically output a link to the PR.
   try {
-    run(`gh pr create -d -B v${releaseLine}.x -t "v${newVersion} proposal" -F ${notesFile}`)
+    run(`gh pr create -d -B v${releaseLine}.x -t "v${newVersion} proposal" -F ${notesFile}`, false)
   } catch (e) {
     // PR already exists so update instead.
     // TODO: Keep existing non-release-notes PR description if there is one.

yarn.lock

@@ -1151,6 +1151,11 @@ append-transform@^2.0.0:
   dependencies:
     default-require-extensions "^3.0.0"
 
+application-config-path@^1.0.0:
+  version "1.0.0"
+  resolved "https://registry.yarnpkg.com/application-config-path/-/application-config-path-1.0.0.tgz#9c25b8c00ac9a342db27275abd3f38c67bbe5a05"
+  integrity sha512-6ZDlLTlfqrTybVzZJDpX2K2ZufqyMyiTbOG06GpxmkmczFgTN+YYRGcTcMCXv/F5P5SrZijVjzzpPUE9BvheLg==
+
 archy@^1.0.0:
   version "1.0.0"
   resolved "https://registry.npmjs.org/archy/-/archy-1.0.0.tgz"