From 9fc7e38a01f404f3d3e7ebb92ad610c2901431c7 Mon Sep 17 00:00:00 2001 From: sganggs Date: Wed, 7 Oct 2026 11:31:36 +0800 Subject: [PATCH] fb5-patch-pkg: an update build refuses a stage without node_modules or public/vendor when a base has them (a --no-install build would list every dependency file as removed and break each install it is applied to); the names no update deletes are split off in diffBases (`left`, named in the summary); test/update-package.test.js (9): the base-folder filters and the refusal --- test/update-package.test.js | 35 ++++++++++++++++++++++------------- tools/package-update.mjs | 10 +++++++--- tools/package.mjs | 13 +++++++++---- 3 files changed, 38 insertions(+), 20 deletions(-) diff --git a/test/update-package.test.js b/test/update-package.test.js index 06554b52..9fcf81bc 100644 --- a/test/update-package.test.js +++ b/test/update-package.test.js @@ -13,7 +13,7 @@ import os from 'node:os'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import zlib from 'node:zlib'; -import { APPLIED_FILE, MANIFEST_FILE, UPDATE_FILE, applyPendingUpdate, checkInstall, parseUpdate } from '../server/update.js'; +import { APPLIED_FILE, MANIFEST_FILE, UPDATE_FILE, applyPendingUpdate, checkInstall, parseUpdate, removalProblem } from '../server/update.js'; import { compareVersions, diffBases, readBase, readZip } from '../tools/package-update.mjs'; import { FOLDER, readBases, scanFiles, stageProblems } from '../tools/package.mjs'; @@ -72,6 +72,14 @@ test('diff with two bases: cumulative — one update works over each; a file cha ]); }); +test('diff: a name no update deletes (node_modules/x/.env.example) is left out of removed and listed as left', () => { + const base = { version: '0.2.0', files: files({ 'node_modules/x/index.js': 'x', 'node_modules/x/.env.example': 'E=1', 'server/a.js': 'a' }) }; + const d = diffBases(files({ 'server/a.js': 'a' }), [base], { removable: (rel) => !removalProblem(rel) }); + assert.deepEqual(d.removed.map((r) => r.path), ['node_modules/x/index.js']); + assert.deepEqual(d.left, ['node_modules/x/.env.example']); + assert.deepEqual(diffBases(files({}), [base]).left, [], 'everything is removable by default'); +}); + test('versions compare numerically, a pre-release before its release', () => { assert.equal(compareVersions('0.2.0', '0.2.1'), -1); assert.equal(compareVersions('0.10.0', '0.9.9'), 1); @@ -262,26 +270,27 @@ test('bases are refused when they cannot be one: lite, an update zip, not older, } }); -test('a base folder: npm\'s files are compared like the stage\'s; a name an update never deletes stays out of removed, named', { skip: !hasZipTool && 'no zip / tar' }, () => { +test('a base folder: npm\'s files are compared like the stage\'s, per-machine and clutter files left out; no update without npm ci', { skip: !hasZipTool && 'no zip / tar' }, () => { const co = fakeCheckout('9.9.8'); const out = tmp('out'); const base = tmp('base'); try { assert.equal(runTool(['--root', co.dir, '--out', out, '--no-install', '--allow-dirty']).status, 0); extract(path.join(out, `${FOLDER}-v9.9.8.zip`), base); - // as if 9.9.8's npm ci had installed a package 9.9.9 no longer has, one file of it with a per-machine name - put(base, `${FOLDER}/node_modules/gone/index.js`, 'module.exports = 1;\n'); - put(base, `${FOLDER}/node_modules/gone/.env.example`, 'X=1\n'); - put(base, `${FOLDER}/logs/server.log`, 'a log'); // per-machine: never part of a base + const root = path.join(base, FOLDER); + put(root, 'logs/server.log', 'a log'); // per-machine: never part of a base + put(root, 'scripts/service.env.cmd', 'set PORT=3000'); + put(root, 'public/.DS_Store', 'finder'); + put(root, 'node_modules/ws/.DS_Store', 'npm ships what it ships'); + const b = readBases([root], '9.9.9')[0]; + assert.ok(![...b.files.keys()].some((f) => /^(?:logs|scripts\/service|public\/\.DS)/.test(f))); + assert.ok(b.files.has('node_modules/ws/.DS_Store'), 'node_modules is taken as npm wrote it, like the stage'); + // a base with node_modules and a build without npm ci: refused, it would delete them everywhere bump(co, '9.9.9'); assert.equal(co.git('add', '-A').status, 0); - const r = runTool(['--root', co.dir, '--out', out, '--no-install', '--allow-dirty', '--update', '--from', path.join(base, FOLDER), '--keep-stage']); - assert.equal(r.status, 0, r.stdout + r.stderr); - assert.match(r.stdout, /^removed: 1 file\(s\) \(node_modules\/gone\/index\.js\)$/m); - assert.match(r.stdout, /^not removed \(an update never deletes these names\): node_modules\/gone\/\.env\.example$/m); - const u = parseUpdate(read(path.join(out, `${FOLDER}-v9.9.9-update`, FOLDER), UPDATE_FILE)); - assert.deepEqual(u.removed.map((x) => x.path), ['node_modules/gone/index.js']); - assert.ok(has(path.join(out, `${FOLDER}-v9.9.9-update`, '.full', FOLDER), MANIFEST_FILE), '--keep-stage keeps the full stage too'); + const r = runTool(['--root', co.dir, '--out', out, '--no-install', '--allow-dirty', '--update', '--from', root]); + assert.equal(r.status, 1, r.stdout); + assert.match(r.stderr, /v9\.9\.8 ships node_modules\/ but this build has none \(--no-install\?\): an update built so would delete it from every install/); } finally { for (const d of [co.dir, out, base]) fs.rmSync(d, { recursive: true, force: true }); } diff --git a/tools/package-update.mjs b/tools/package-update.mjs index 03b37cc2..c500f46c 100644 --- a/tools/package-update.mjs +++ b/tools/package-update.mjs @@ -190,13 +190,14 @@ export function readBase(source, { folder = 'Stronghold-Protocol', skip = () => } /** - * Compare the new full stage with the bases. + * Compare the new full stage with the bases. `removable(rel)`: false for a path no update deletes (it goes to `left`). * @param {Map} next every file of the new version (the update files left out) * @param {{ version: string, files: Map }[]} bases - * @returns {{ ship: string[], removed: { path: string, sha256: string[] }[], caseOnly: string[], + * @param {{ removable?: (rel: string) => boolean }} [opts] + * @returns {{ ship: string[], removed: { path: string, sha256: string[] }[], caseOnly: string[], left: string[], * perBase: { version: string, added: number, changed: number, unchanged: number, removed: number }[] }} */ -export function diffBases(next, bases) { +export function diffBases(next, bases, { removable = () => true } = {}) { const ship = new Set(); const perBase = []; for (const b of bases) { @@ -211,10 +212,12 @@ export function diffBases(next, bases) { const lower = new Set([...next.keys()].map((f) => f.toLowerCase())); const removed = new Map(); const caseOnly = new Set(); + const left = new Set(); for (const b of bases) { for (const [rel, d] of b.files) { if (next.has(rel)) continue; if (lower.has(rel.toLowerCase())) { caseOnly.add(rel); continue; } + if (!removable(rel)) { left.add(rel); continue; } if (!removed.has(rel)) removed.set(rel, new Set()); removed.get(rel).add(d.sha256); } @@ -223,6 +226,7 @@ export function diffBases(next, bases) { ship: [...ship].sort(), removed: [...removed.keys()].sort().map((rel) => ({ path: rel, sha256: [...removed.get(rel)].sort() })), caseOnly: [...caseOnly].sort(), + left: [...left].sort(), perBase, }; } diff --git a/tools/package.mjs b/tools/package.mjs index 972ed4b8..b9fbea7e 100644 --- a/tools/package.mjs +++ b/tools/package.mjs @@ -585,14 +585,19 @@ export function buildUpdate(root, p, bases, { out, install = true, force = false log(`comparing ${staged.length} files with ${bases.map((b) => `v${b.version}`).join(', ')} …`); const next = new Map(); for (const f of staged) if (f !== MANIFEST_FILE) next.set(f, digestOf(full, f)); - const diff = diffBases(next, bases); + // without npm ci the stage has no node_modules / public/vendor, and the update would delete the bases' copies + for (const lib of ['node_modules/', 'public/vendor/']) { + const inBase = bases.find((b) => [...b.files.keys()].some((f) => f.startsWith(lib))); + if (inBase && ![...next.keys()].some((f) => f.startsWith(lib))) { + throw new Error(`v${inBase.version} ships ${lib} but this build has none (--no-install?): an update built so would delete it from every install`); + } + } // a path no update deletes (a per-machine name such as node_modules/x/.env.example) stays behind, named in the summary - diff.left = diff.removed.filter((r) => removalProblem(r.path)).map((r) => r.path); + const diff = diffBases(next, bases, { removable: (rel) => !removalProblem(rel) }); for (const rel of diff.left) { const bad = pathProblem(rel); if (bad) throw new Error(`a base ships ${rel}, which is ${bad}`); } - diff.removed = diff.removed.filter((r) => !removalProblem(r.path)); for (const rel of diff.ship) copyInto(full, stage, rel); copyInto(full, stage, MANIFEST_FILE); const update = { app: p.version, from: bases.map((b) => b.version), files: new Map(diff.ship.map((f) => [f, next.get(f)])), removed: diff.removed }; @@ -632,7 +637,7 @@ export function formatUpdate(r, version) { const gone = r.diff.removed.map((x) => x.path); lines.push(`removed: ${gone.length} file(s)${gone.length ? ` (${gone.slice(0, 8).join(', ')}${gone.length > 8 ? ' …' : ''})` : ''}`); if (r.diff.caseOnly.length) lines.push(`not removed (a new file differs only in case): ${r.diff.caseOnly.join(', ')}`); - if (r.diff.left?.length) lines.push(`not removed (an update never deletes these names): ${r.diff.left.join(', ')}`); + if (r.diff.left.length) lines.push(`not removed (an update never deletes these names): ${r.diff.left.join(', ')}`); return lines.join('\n') + '\n'; }