Feat: check changed packages list in CI (#10392)

.github/workflows/changelog-check.yml

@@ -0,0 +1,34 @@
+name: Changelog
+
+# only pull requests targeting `master`: the check compares the branch to
+# `origin/master`, it would report unrelated packages on a PR based on another
+# branch
+on:
+  pull_request:
+    branches:
+      - master
+
+jobs:
+  changed-packages:
+    runs-on: ubuntu-latest
+    steps:
+      # https://github.com/actions/checkout
+      - uses: actions/checkout@v4
+        with:
+          # the whole history is required to compute the merge base with `master`
+          fetch-depth: 0
+
+          # the check only needs commits and trees: `git diff --name-only
+          # --no-renames` compares tree entries and never reads file contents,
+          # skipping the historical blobs halves the fetch (~112 MiB → ~53 MiB)
+          filter: blob:none
+
+      # https://github.com/actions/setup-node
+      - name: Setup Node environment
+        uses: actions/setup-node@v4
+        with:
+          node-version: '>=22.12'
+
+      # the script only uses Node builtins, no `yarn install` necessary
+      - name: Ensure changed packages are listed in CHANGELOG.unreleased.md
+        run: ./scripts/check-changed-pkgs.js

package.json

@@ -72,7 +72,7 @@
     "test": "npm run test-lint && npm run test-unit",
     "test-integration": "scripts/run-script.js --parallel test-integration",
     "test-lint": "eslint --ignore-path .gitignore --ignore-pattern packages/xo-web .",
-    "test-unit": "scripts/run-script.js --bail test"
+    "test-unit": "node --test 'scripts/**/*.test.mjs' && scripts/run-script.js --bail test"
   },
   "workspaces": [
     "@*/*",

scripts/_readChangelogPackages.js

@@ -0,0 +1,55 @@
+'use strict'
+
+const fs = require('fs').promises
+const joinPath = require('path').join
+
+// this module is used by `check-changed-pkgs.js`, which runs in the CI without
+// `yarn install`, it must therefore only depend on Node builtins
+const escapeRegExp = string => string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
+
+const CHANGELOG_PATH = joinPath(__dirname, '../CHANGELOG.unreleased.md')
+const START_TAG = '<!--packages-start-->'
+const END_TAG = '<!--packages-end-->'
+
+/**
+ * Read the `Packages to release` block of `CHANGELOG.unreleased.md`
+ *
+ * @returns {Promise<{ [packageName: string]: 'patch' | 'minor' | 'major' }>} A mapping of package names to their release type
+ */
+async function readChangelogPackages() {
+  const content = await fs.readFile(CHANGELOG_PATH, 'utf8')
+  const changelogRegex = new RegExp(`${escapeRegExp(START_TAG)}(.*)${escapeRegExp(END_TAG)}`, 's')
+  const block = changelogRegex.exec(content)?.[1].trim()
+
+  if (block === undefined) {
+    throw new Error(`Could not find changelog block in ${CHANGELOG_PATH}`)
+  }
+
+  const toRelease = { __proto__: null }
+
+  block.split('\n').forEach(rawLine => {
+    const line = rawLine.trim()
+
+    if (!line) {
+      return
+    }
+
+    const match = line.match(/^-\s*(?<name>\S+)\s+(?<releaseType>patch|minor|major)$/)
+
+    if (!match) {
+      throw new Error(`Invalid line: "${rawLine}"`)
+    }
+
+    const { name, releaseType } = match.groups
+    if (name in toRelease) {
+      throw new Error('duplicate package to release in CHANGELOG.unreleased.md: ' + name)
+    }
+
+    toRelease[name] = releaseType
+  })
+
+  return toRelease
+}
+
+exports.CHANGELOG_PATH = CHANGELOG_PATH
+exports.readChangelogPackages = readChangelogPackages

scripts/check-changed-pkgs.js

@@ -0,0 +1,186 @@
+#!/usr/bin/env node
+
+// Fail if a package modified by the current branch is not listed in the
+// `Packages to release` section of `CHANGELOG.unreleased.md`.
+//
+// Meant to run in the CI, on a branch already rebased on `master`.
+//
+// Only depends on Node builtins so that it can run without `yarn install`.
+
+'use strict'
+
+const { execFileSync } = require('child_process')
+const { existsSync, readdirSync } = require('fs')
+const joinPath = require('path').join
+
+const { CHANGELOG_PATH, readChangelogPackages } = require('./_readChangelogPackages.js')
+
+const ROOT_DIR = joinPath(__dirname, '..')
+
+// branches are rebased before being merged, the merge base with `master` is
+// therefore the commit the branch starts from
+const BASE_REF = 'origin/master'
+
+// how many changed files are shown per package in the error report
+const MAX_REPORTED_FILES = 5
+
+// packages which are never listed in `CHANGELOG.unreleased.md`
+const IGNORED_PACKAGES = new Set([
+  // build-time only configuration, never published
+  '@xen-orchestra/babel-config',
+
+  // released separately, `gen-deps-list.js` rejects it if listed
+  '@xen-orchestra/lite',
+])
+
+// files which don't change what is shipped to users: a package whose changes
+// are limited to them does not need to be released
+const IGNORED_FILES = [
+  // tests
+  //
+  // both `test/` and `tests/` are in use in this repository, they hold test
+  // files, fixtures and helpers which are never shipped
+  /(?:^|\/)(?:__snapshots__|tests?)\//,
+  /\.(?:integ|load|spec|test)\.[cm]?[jt]sx?$/,
+
+  // docs and repository metadata
+  /(?:^|\/)docs\//,
+  /(?:^|\/)(?:README\.md|\.USAGE\.md|\.gitignore|\.npmignore)$/,
+]
+
+/**
+ * @param {string} file A path relative to the repository root
+ * @returns {boolean}
+ */
+function isIgnoredFile(file) {
+  return IGNORED_FILES.some(regex => regex.test(file))
+}
+
+/**
+ * @param {readonly string[]} changedFiles Paths relative to the repository root
+ * @param {ReadonlyMap<string, string>} packageNamesByDir A mapping of package directories (e.g. `@vates/types`) to package names
+ * @returns {Map<string, string[]>} A mapping of package names to the changed files which require their release
+ */
+function getChangedPackages(changedFiles, packageNamesByDir) {
+  const changedPackages = new Map()
+
+  for (const file of changedFiles) {
+    const parts = file.split('/')
+
+    // a package file is always at least at the third level, e.g. `@vates/types/index.mts`
+    if (parts.length < 3) {
+      continue
+    }
+
+    const name = packageNamesByDir.get(`${parts[0]}/${parts[1]}`)
+    if (name === undefined || IGNORED_PACKAGES.has(name) || isIgnoredFile(file)) {
+      continue
+    }
+
+    let files = changedPackages.get(name)
+    if (files === undefined) {
+      changedPackages.set(name, (files = []))
+    }
+    files.push(file)
+  }
+
+  return changedPackages
+}
+
+/**
+ * Mirrors `getPackages()` from `./utils`, without its dependencies
+ *
+ * @returns {Map<string, string>} A mapping of package directories (e.g. `@vates/types`) to package names
+ */
+function getPackageNamesByDir() {
+  const packageNamesByDir = new Map()
+
+  for (const scope of [undefined, '@vates', '@xen-orchestra']) {
+    const scopeDir = scope ?? 'packages'
+
+    for (const entry of readdirSync(joinPath(ROOT_DIR, scopeDir))) {
+      const relativeDir = `${scopeDir}/${entry}`
+
+      // a directory without a `package.json` is not a package
+      if (existsSync(joinPath(ROOT_DIR, relativeDir, 'package.json'))) {
+        packageNamesByDir.set(relativeDir, scope === undefined ? entry : `${scope}/${entry}`)
+      }
+    }
+  }
+
+  return packageNamesByDir
+}
+
+function git(...args) {
+  return execFileSync('git', args, { cwd: ROOT_DIR, encoding: 'utf8', maxBuffer: 1e8 }).trimEnd()
+}
+
+/**
+ * @returns {string[]} The files changed by the current branch, relative to the repository root
+ */
+function getChangedFiles() {
+  let mergeBase
+  try {
+    mergeBase = git('merge-base', BASE_REF, 'HEAD')
+  } catch (error) {
+    const err = new Error(
+      `could not compute the merge base with ${BASE_REF}, the whole history must be fetched (\`fetch-depth: 0\` with \`actions/checkout\`)`
+    )
+    err.cause = error
+    throw err
+  }
+
+  // `--no-renames` so that the original path of a renamed file is reported as well
+  const output = git('diff', '--name-only', '--no-renames', mergeBase, 'HEAD')
+
+  return output === '' ? [] : output.split('\n')
+}
+
+async function main() {
+  const changedPackages = getChangedPackages(getChangedFiles(), getPackageNamesByDir())
+
+  const listedPackages = await readChangelogPackages()
+
+  const missing = Array.from(changedPackages.keys())
+    .filter(name => !(name in listedPackages))
+    .sort()
+
+  // `console` is how the other scripts of this directory report to the user
+  if (missing.length === 0) {
+    console.log(`✓ the ${changedPackages.size} changed package(s) are listed in CHANGELOG.unreleased.md`)
+    return
+  }
+
+  console.error(
+    `✗ ${missing.length} changed package(s) missing from the \`Packages to release\` list of ${CHANGELOG_PATH}:`
+  )
+  console.error('')
+  for (const name of missing) {
+    const files = changedPackages.get(name)
+    console.error(`- ${name}`)
+    for (const file of files.slice(0, MAX_REPORTED_FILES)) {
+      console.error(`    ${file}`)
+    }
+    if (files.length > MAX_REPORTED_FILES) {
+      console.error(`    … and ${files.length - MAX_REPORTED_FILES} other file(s)`)
+    }
+  }
+  console.error('')
+  console.error('Add them to the `Packages to release` section, keeping the list alphabetically ordered:')
+  console.error('')
+  for (const name of missing) {
+    console.error(`- ${name} <patch|minor|major>`)
+  }
+
+  process.exitCode = 1
+}
+
+exports.getChangedPackages = getChangedPackages
+exports.isIgnoredFile = isIgnoredFile
+
+if (require.main === module) {
+  main().catch(error => {
+    console.error(error)
+    process.exit(1)
+  })
+}

scripts/check-changed-pkgs.test.mjs

@@ -0,0 +1,120 @@
+import assert from 'node:assert/strict'
+import { describe, it } from 'node:test'
+
+import { getChangedPackages, isIgnoredFile } from './check-changed-pkgs.js'
+
+const PACKAGE_NAMES_BY_DIR = new Map([
+  ['@vates/types', '@vates/types'],
+  ['@xen-orchestra/babel-config', '@xen-orchestra/babel-config'],
+  ['@xen-orchestra/backups', '@xen-orchestra/backups'],
+  ['@xen-orchestra/lite', '@xen-orchestra/lite'],
+  ['packages/xo-server', 'xo-server'],
+])
+
+const changedPackages = changedFiles => getChangedPackages(changedFiles, PACKAGE_NAMES_BY_DIR)
+const changedPackageNames = changedFiles => Array.from(changedPackages(changedFiles).keys()).sort()
+
+describe('isIgnoredFile', function () {
+  for (const file of [
+    '@vates/nbd-client/tests/ca-cert.pem',
+    // a test file which does not follow the `*.test.*` naming
+    '@vates/xml/test/parse.js',
+    '@xen-orchestra/backups/_runners/_vmRunners/__snapshots__/IncrementalXapi.mjs.snap',
+    '@xen-orchestra/backups/RemoteAdapter.integ.mjs',
+    '@xen-orchestra/backups/RemoteAdapter.test.mjs',
+    '@xen-orchestra/backups/docs/design.md',
+    '@xen-orchestra/backups/README.md',
+    '@xen-orchestra/web-core/src/composables/foo.spec.ts',
+    // a test directory nested in the package sources
+    '@xen-orchestra/web/src/test/create-vm.ts',
+    'packages/xo-cli/.USAGE.md',
+    'packages/xo-server/.npmignore',
+    'packages/xo-server/perf.load.mjs',
+    'packages/xo-web/src/common/index.spec.js',
+  ]) {
+    it(`ignores ${file}`, function () {
+      assert.equal(isIgnoredFile(file), true)
+    })
+  }
+
+  for (const file of [
+    '@vates/types/index.mts',
+    '@xen-orchestra/backups/RemoteAdapter.mjs',
+    '@xen-orchestra/backups/package.json',
+    // not a test file, only a file *about* tests
+    '@xen-orchestra/backups/testUtils.mjs',
+    // `docs` and `test` as file names, not as directories
+    'packages/xo-server/docs.md',
+    'packages/xo-server/test.mjs',
+    'packages/xo-web/src/common/index.js',
+  ]) {
+    it(`does not ignore ${file}`, function () {
+      assert.equal(isIgnoredFile(file), false)
+    })
+  }
+})
+
+describe('getChangedPackages', function () {
+  it('maps a file to the package which owns its directory', function () {
+    assert.deepEqual(changedPackageNames(['packages/xo-server/src/api/vm.mjs', '@vates/types/index.mts']), [
+      '@vates/types',
+      'xo-server',
+    ])
+  })
+
+  it('reports the files which require the release of each package', function () {
+    assert.deepEqual(
+      Object.fromEntries(
+        changedPackages([
+          'packages/xo-server/src/api/vm.mjs',
+          'packages/xo-server/src/api/vm.spec.mjs',
+          'packages/xo-server/package.json',
+        ])
+      ),
+      { 'xo-server': ['packages/xo-server/src/api/vm.mjs', 'packages/xo-server/package.json'] }
+    )
+  })
+
+  it('ignores files which are not in a package', function () {
+    assert.deepEqual(
+      changedPackageNames([
+        'CHANGELOG.unreleased.md',
+        '.github/workflows/ci.yml',
+        'scripts/check-changed-pkgs.js',
+        'docs/backups.md',
+        'packages/not-a-package/index.js',
+      ]),
+      []
+    )
+  })
+
+  it('ignores files directly in a package scope directory', function () {
+    assert.deepEqual(changedPackageNames(['@vates/README.md', 'packages/tsconfig.json']), [])
+  })
+
+  it('ignores packages which are never listed in the changelog', function () {
+    assert.deepEqual(
+      changedPackageNames([
+        '@xen-orchestra/babel-config/index.js',
+        '@xen-orchestra/lite/src/App.vue',
+        '@vates/types/index.mts',
+      ]),
+      ['@vates/types']
+    )
+  })
+
+  it('ignores a package whose changes are limited to ignored files', function () {
+    assert.deepEqual(
+      changedPackageNames([
+        '@xen-orchestra/backups/README.md',
+        '@xen-orchestra/backups/tests/fixture.json',
+        '@vates/types/types.test.mts',
+      ]),
+      []
+    )
+  })
+
+  it('handles an empty list of changed files', function () {
+    assert.deepEqual(changedPackageNames([]), [])
+  })
+})

scripts/gen-deps-list.js

@@ -1,23 +1,15 @@
 #!/usr/bin/env node
 'use strict'
 
-const fs = require('fs').promises
-const joinPath = require('path').join
 const semver = require('semver')
 const { getPackages } = require('./utils')
-const escapeRegExp = require('lodash/escapeRegExp')
+const { readChangelogPackages } = require('./_readChangelogPackages.js')
 const invert = require('lodash/invert')
 const keyBy = require('lodash/keyBy')
 
 const { debug } = require('../@xen-orchestra/log').createLogger('gen-deps-list')
 const computeDepOrder = require('./_computeDepOrder.js')
 
-const changelogConfig = {
-  path: joinPath(__dirname, '../CHANGELOG.unreleased.md'),
-  startTag: '<!--packages-start-->',
-  endTag: '<!--packages-end-->',
-}
-
 const RELEASE_WEIGHT = { PATCH: 1, MINOR: 2, MAJOR: 3 }
 const RELEASE_TYPE = invert(RELEASE_WEIGHT)
 
@@ -126,37 +118,7 @@ async function main(args, scriptName) {
 async function readPackagesFromChangelog(toRelease) {
   debug('reading packages from CHANGELOG.unreleased.md')
 
-  const content = await fs.readFile(changelogConfig.path)
-  const changelogRegex = new RegExp(
-    `${escapeRegExp(changelogConfig.startTag)}(.*)${escapeRegExp(changelogConfig.endTag)}`,
-    's'
-  )
-  const block = changelogRegex.exec(content)?.[1].trim()
-
-  if (block === undefined) {
-    throw new Error(`Could not find changelog block in ${changelogConfig.path}`)
-  }
-
-  block.split('\n').forEach(rawLine => {
-    const line = rawLine.trim()
-
-    if (!line) {
-      return
-    }
-
-    const match = line.match(/^-\s*(?<name>\S+)\s+(?<releaseType>patch|minor|major)$/)
-
-    if (!match) {
-      throw new Error(`Invalid line: "${rawLine}"`)
-    }
-
-    const { name, releaseType } = match.groups
-    if (name in toRelease) {
-      throw new Error('duplicate package to release in CHANGELOG.unreleased.md: ' + name)
-    }
-
-    toRelease[name] = releaseType
-  })
+  Object.assign(toRelease, await readChangelogPackages())
 }
 
 /**