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

@@ -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 });
   }

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<string, { size: number, sha256: string }>} next every file of the new version (the update files left out)
  * @param {{ version: string, files: Map<string, { size: number, sha256: string }> }[]} 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,
   };
 }

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';
 }