From 1563b4fd511c99e3d6fe3725701d09021fa79fc0 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Mon, 17 Jun 2024 02:46:18 +0100 Subject: [PATCH] Build: Repair reproducible builds since Node 16>18 upgrade It stopped working on GitHub Actions after because at some point a Node 18 minor release upgraded from npm 8 to npm 10, which slightly changed the gzip binary encoding of npm-pack tarballs (tgz file). Fix by comparing the hash of the raw .tar contents instead of the tgz file. This is still fully strict on the extracted contents, including file order, file metadata (chmod, mtime) and byte-for-byte of every file contents (JS/CSS/TXT/JSON, etc.). --- .github/workflows/reproducible.yaml | 2 +- build/reproducible-builds.js | 164 +++++++++++++++------------- 2 files changed, 92 insertions(+), 74 deletions(-) diff --git a/.github/workflows/reproducible.yaml b/.github/workflows/reproducible.yaml index 8ac61a667..01b58d1d2 100644 --- a/.github/workflows/reproducible.yaml +++ b/.github/workflows/reproducible.yaml @@ -13,7 +13,7 @@ on: jobs: run: name: Verify releases - if: ${{ github.repository == 'qunitjs/qunit' }} # skip on forks, noisy cron + if: ${{ github.repository == 'qunitjs/qunit' }} # skip noisy cron on forks runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v4 diff --git a/build/reproducible-builds.js b/build/reproducible-builds.js index d55b336d2..165bc6c04 100644 --- a/build/reproducible-builds.js +++ b/build/reproducible-builds.js @@ -1,10 +1,9 @@ // Helper for the "Reproducible builds" job. // // Prerequisites: -// * Node.js 14+ -// * npm 7.7.0+ -// * tar (preinstalled on Linux/macOS) -// * shasum (preinstalled on Linux/macOS) +// * Node.js 18+, npm 10+ +// * Git 2.11+ +// * tar, shasum, gunzip (preinstalled on Linux/macOS) const cp = require('child_process'); const fs = require('fs'); @@ -17,21 +16,25 @@ const tempDir = path.join(__dirname, '../temp', 'reproducible-builds'); const SRC_REPO = 'https://github.com/qunitjs/qunit.git'; /** - * How many past releases to verify. + * Known caveats prior to QUnit 2.17.0: * - * Note that qunit@<2.16.0 were not fully reproducible. + * - Prior to QUnit 2.14.1, file headers included an uncontrolled "current" timestamp. + * This would have to be ignored or replaced prior to comparison. + * - Prior to QUnit 2.14.1, the build wrote files to "/dist" instead of "/qunit". + * - QUnit 2.15.0 contained some CR (\r) characters in comments from fuzzysort.js, + * which get normalized to LF (\n) by Git and npm, but not in the actual builds + * and in what we publish to the CDN. This was fixed in qunit@2.16.0 and qunit@2.17.0. * - * qunit@<=2.14.1 embedded a timestamp in the file header. This would have to be - * ignored (or replaced with the timestamp found in the files you compare against). - * In the 2.14.1, timestamps were removed from the output. Also, prior to 2.14.1, - * the build wrote files to "/dist" instead of "/qunit". + * QUnit 2.17.0 and later are fully reproducible with this script. Notes: * - * qunit@2.15.0 contained some CR (\r) characters in comments from fuzzysort.js, - * which got normalized to LF (\n) by Git, npm, and the CDN on their own. This was - * fixed in qunit@2.16.0 by removing the comment in question, and qunit@2.17.0 - * normalizes CRLF during the build. + * - qunit@2.17.0 to 2.21.0 were built and published using npm 8 or npm 9. + * + * In npm 10, upstream changed gzip encoding slightly for the npm-pack tarball (.tgz). This + * means a tarball from npm 10+ is not byte-for-byte identical to ones generated by npm 8 or 9. + * After gzip-decompression, however, the tar stream is byte-for-byte identical. + * Either use npm 8 or 9 to verify these, or verify the tarball after gzip decompression. */ -const VERIFY_COUNT = 2; +const VERIFY_COUNT = 3; async function buildRelease (version, cacheDir = null) { console.log(`... ${version}: checking out the source`); @@ -44,7 +47,6 @@ async function buildRelease (version, cacheDir = null) { // Remove any artefacts that were checked into Git utils.cleanDir(gitDir + '/qunit/'); - // Use sync for npm-ci to avoid concurrency bugs with shared cache console.log(`... ${version}: installing development dependencies from npm`); const npmEnv = { npm_config_cache: cacheDir, @@ -52,6 +54,7 @@ async function buildRelease (version, cacheDir = null) { PATH: process.env.PATH, PUPPETEER_DOWNLOAD_PATH: path.join(cacheDir, 'puppeteer_download') }; + // Use sync for npm-ci to avoid concurrency bugs with shared cache cp.execFileSync('npm', ['ci'], { env: npmEnv, cwd: gitDir @@ -70,26 +73,33 @@ async function buildRelease (version, cacheDir = null) { }); return { - js: fs.readFileSync(gitDir + '/qunit/qunit.js', 'utf8'), - css: fs.readFileSync(gitDir + '/qunit/qunit.css', 'utf8'), - tgz: cp.execFileSync( - 'shasum', ['-a', '256', '-b', `qunit-${version}.tgz`], - { encoding: 'utf8', cwd: gitDir } - ) + js: { + name: gitDir + '/qunit/qunit.js', + contents: fs.readFileSync(gitDir + '/qunit/qunit.js', 'utf8') + }, + css: { + name: gitDir + '/qunit/qunit.css', + contents: fs.readFileSync(gitDir + '/qunit/qunit.css', 'utf8') + }, + tgz: { + name: gitDir + `/qunit-${version}.tgz`, + contents: cp.execSync( + `gunzip --stdout qunit-${version}.tgz | shasum -a 256 -b`, + { encoding: 'utf8', cwd: gitDir } + ) + } }; } const Reproducible = { async fetch () { - // Keep the stuff that matters in memory. Below, we will run unaudited npm dev deps - // as part of build commands, which can modify anything on disk. + // Fetch official releases first and store them in memory (not on disk). Only after that will + // we run the build commands (which involve unaudited npm packages as dev deps) which could + // modify anything on disk. Hence don't store what we want to compare against on disk. const releases = {}; { - console.log('Setting up temp directory...'); - - // This can take a while when running it locally (not CI), - // as it first need to remove any old builds. + // This may take a while locally, when removing previous builds. utils.cleanDir(tempDir); } { @@ -100,8 +110,14 @@ const Reproducible = { for (const release of data.qunit.all.slice(0, VERIFY_COUNT)) { releases[release.version] = { cdn: { - js: await utils.download(`https://code.jquery.com/${release.filename}`), - css: await utils.download(`https://code.jquery.com/${release.theme}`) + js: { + name: `https://code.jquery.com/${release.filename}`, + contents: await utils.download(`https://code.jquery.com/${release.filename}`) + }, + css: { + name: `https://code.jquery.com/${release.theme}`, + contents: await utils.download(`https://code.jquery.com/${release.theme}`) + } } }; } @@ -111,28 +127,36 @@ const Reproducible = { const npmIndexUrl = 'https://registry.npmjs.org/qunit'; const data = JSON.parse(await utils.download(npmIndexUrl)); - for (const version of Object.keys(data.versions).slice(-VERIFY_COUNT)) { - if (!releases[version]) { - releases[version] = {}; + for (const version in releases) { + if (!data.versions[version]) { + throw new Error(`QUnit ${version} is missing from https://www.npmjs.com/package/qunit`); } - const tarball = data.versions[version].dist.tarball; const tarFile = path.join(tempDir, path.basename(tarball)); await utils.downloadFile(tarball, tarFile); releases[version].npm = { - js: cp.execFileSync( - 'tar', ['-xOf', tarFile, 'package/qunit/qunit.js'], - { encoding: 'utf8' } - ), - css: cp.execFileSync( - 'tar', ['-xOf', tarFile, 'package/qunit/qunit.css'], - { encoding: 'utf8' } - ), - tgz: cp.execFileSync( - 'shasum', ['-a', '256', '-b', path.basename(tarball)], - { encoding: 'utf8', cwd: tempDir } - ) + js: { + name: `npm:${path.basename(tarball)}#package/qunit/qunit.js`, + contents: cp.execFileSync( + 'tar', ['-xOf', tarFile, 'package/qunit/qunit.js'], + { encoding: 'utf8' } + ) + }, + css: { + name: `npm:${path.basename(tarball)}#package/qunit/qunit.css`, + contents: cp.execFileSync( + 'tar', ['-xOf', tarFile, 'package/qunit/qunit.css'], + { encoding: 'utf8' } + ) + }, + tgz: { + name: `npm:${path.basename(tarball)}`, + contents: cp.execSync( + `gunzip --stdout ${path.basename(tarball)} | shasum -a 256 -b`, + { encoding: 'utf8', cwd: tempDir } + ) + } }; } } @@ -142,12 +166,12 @@ const Reproducible = { const cacheDir = path.join(tempDir, 'cache'); utils.cleanDir(cacheDir); - // Start the builds in parallel and await results. - // Let the first error propagate and ignore others (avoids "Unhandled rejection" later). + // Start builds in parallel and await results. const buildPromises = []; for (const version in releases) { - releases[version].buildPromise = buildRelease(version, cacheDir); - buildPromises.push(releases[version].buildPromise); + buildPromises.push( + (releases[version].buildPromise = buildRelease(version, cacheDir)) + ); } await Promise.all(buildPromises); @@ -156,29 +180,29 @@ const Reproducible = { const release = releases[version]; const build = await release.buildPromise; - // For qunit@2.15.0, normalize CRLF to match what Git and npm did during upload. - if (version === '2.15.0') { - build.js = utils.normalizeEOL(build.js); - } - let verified = true; for (const distro in release) { for (const file in release[distro]) { - if (release[distro][file] !== build[file]) { + if (release[distro][file].contents === build[file].contents) { + console.log( + `... ${version}: ${release[distro][file].name} matches ${build[file].name}` + ); + } else { verified = false; console.error( `QUnit ${version} ${file} from ${distro} differs from build` ); - diffs.push([ - { - name: `qunit-${version}-build.${file}`, - contents: build[file] - }, - { - name: `qunit-${version}-${distro}.${file}`, - contents: release[distro][file] - } - ]); + const buildFile = `qunit-${version}-build.${file}`; + const releaseFile = `qunit-${version}-${distro}.${file}`; + fs.writeFileSync(buildFile, utils.verboseNonPrintable(build[file].contents)); + fs.writeFileSync(releaseFile, utils.verboseNonPrintable(release[distro][file].contents)); + diffs.push( + `--- ${build[file].name}\n+++ ${release[distro][file].name}\n`, + utils.getDiff(buildFile, releaseFile, { ignoreWhitespace: false }) + .split('\n').slice(2).join('\n') + ); + fs.rmSync(buildFile); + fs.rmSync(releaseFile); } } } @@ -189,13 +213,7 @@ const Reproducible = { } diffs.forEach(diff => { - const fromFile = path.join(tempDir, diff[0].name); - const toFile = path.join(tempDir, diff[1].name); - fs.writeFileSync(fromFile, utils.verboseNonPrintable(diff[0].contents)); - fs.writeFileSync(toFile, utils.verboseNonPrintable(diff[1].contents)); - process.stdout.write( - utils.getDiff(fromFile, toFile, { ignoreWhitespace: false }) - ); + process.stdout.write(diff); }); if (diffs.length) { throw new Error('One or more distributions differ from the reproduced build');