fix(ci): rm workspace node_modules (#7490)

Author: reggi

lib/commands/ci.js

@@ -1,9 +1,11 @@
 const reifyFinish = require('../utils/reify-finish.js')
 const runScript = require('@npmcli/run-script')
 const fs = require('fs/promises')
+const path = require('path')
 const { log, time } = require('proc-log')
 const validateLockfile = require('../utils/validate-lockfile.js')
 const ArboristWorkspaceCmd = require('../arborist-cmd.js')
+const getWorkspaces = require('../utils/get-workspaces.js')
 
 class CI extends ArboristWorkspaceCmd {
   static description = 'Clean install a project'
@@ -76,14 +78,22 @@ class CI extends ArboristWorkspaceCmd {
 
     const dryRun = this.npm.config.get('dry-run')
     if (!dryRun) {
+      const workspacePaths = await getWorkspaces([], {
+        path: this.npm.localPrefix,
+        includeWorkspaceRoot: true,
+      })
+
       // Only remove node_modules after we've successfully loaded the virtual
       // tree and validated the lockfile
       await time.start('npm-ci:rm', async () => {
-        const path = `${where}/node_modules`
-        // get the list of entries so we can skip the glob for performance
-        const entries = await fs.readdir(path, null).catch(() => [])
-        return Promise.all(entries.map(f => fs.rm(`${path}/${f}`,
-          { force: true, recursive: true })))
+        return await Promise.all([...workspacePaths.values()].map(async modulePath => {
+          const fullPath = path.join(modulePath, 'node_modules')
+          // get the list of entries so we can skip the glob for performance
+          const entries = await fs.readdir(fullPath, null).catch(() => [])
+          return Promise.all(entries.map(folder => {
+            return fs.rm(path.join(fullPath, folder), { force: true, recursive: true })
+          }))
+        }))
       })
     }
 

mock-registry/lib/index.js

@@ -451,6 +451,50 @@ class MockRegistry {
       ...packument,
     }
   }
+
+  /**
+   * this is a simpler convience method for creating mockable registry with
+   * tarballs for specific versions
+   */
+  async setup (packages) {
+    const format = Object.keys(packages).map(v => {
+      const [name, version] = v.split('@')
+      return { name, version }
+    }).reduce((acc, inc) => {
+      const exists = acc.find(pkg => pkg.name === inc.name)
+      if (exists) {
+        exists.tarballs = {
+          ...exists.tarballs,
+          [inc.version]: packages[`${inc.name}@${inc.version}`],
+        }
+      } else {
+        acc.push({ name: inc.name,
+          tarballs: {
+            [inc.version]: packages[`${inc.name}@${inc.version}`],
+          },
+        })
+      }
+      return acc
+    }, [])
+    const registry = this
+    for (const pkg of format) {
+      const { name, tarballs } = pkg
+      const versions = Object.keys(tarballs)
+      const manifest = await registry.manifest({ name, versions })
+
+      for (const version of versions) {
+        const tarballPath = pkg.tarballs[version]
+        if (!tarballPath) {
+          throw new Error(`Tarball path not provided for version ${version}`)
+        }
+
+        await registry.tarball({
+          manifest: manifest.versions[version],
+          tarball: tarballPath,
+        })
+      }
+    }
+  }
 }
 
 module.exports = MockRegistry

test/fixtures/mock-npm.js

@@ -1,10 +1,12 @@
 const os = require('os')
 const fs = require('fs').promises
+const fsSync = require('fs')
 const path = require('path')
 const tap = require('tap')
 const mockLogs = require('./mock-logs.js')
 const mockGlobals = require('@npmcli/mock-globals')
 const tmock = require('./tmock')
+const MockRegistry = require('@npmcli/mock-registry')
 const defExitCode = process.exitCode
 
 const changeDir = (dir) => {
@@ -288,6 +290,167 @@ const setupMockNpm = async (t, {
   }
 }
 
+const loadNpmWithRegistry = async (t, opts) => {
+  const mock = await setupMockNpm(t, opts)
+  const registry = new MockRegistry({
+    tap: t,
+    registry: mock.npm.config.get('registry'),
+    strict: true,
+  })
+
+  const fileShouldExist = (filePath) => {
+    t.equal(
+      fsSync.existsSync(path.join(mock.npm.prefix, filePath)), true, `${filePath} should exist`
+    )
+  }
+
+  const fileShouldNotExist = (filePath) => {
+    t.equal(
+      fsSync.existsSync(path.join(mock.npm.prefix, filePath)), false, `${filePath} should not exist`
+    )
+  }
+
+  const packageVersionMatches = (filePath, version) => {
+    t.equal(
+      JSON.parse(fsSync.readFileSync(path.join(mock.npm.prefix, filePath), 'utf8')).version, version
+    )
+  }
+
+  const packageInstalled = (target) => {
+    const spec = path.basename(target)
+    const dirname = path.dirname(target)
+    const [name, version = '1.0.0'] = spec.split('@')
+    fileShouldNotExist(`${dirname}/${name}/${name}@${version}.txt`)
+    packageVersionMatches(`${dirname}/${name}/package.json`, version)
+    fileShouldExist(`${dirname}/${name}/index.js`)
+  }
+
+  const packageMissing = (target) => {
+    const spec = path.basename(target)
+    const dirname = path.dirname(target)
+    const [name, version = '1.0.0'] = spec.split('@')
+    fileShouldNotExist(`${dirname}/${name}/${name}@${version}.txt`)
+    fileShouldNotExist(`${dirname}/${name}/package.json`)
+    fileShouldNotExist(`${dirname}/${name}/index.js`)
+  }
+
+  const packageDirty = (target) => {
+    const spec = path.basename(target)
+    const dirname = path.dirname(target)
+    const [name, version = '1.0.0'] = spec.split('@')
+    fileShouldExist(`${dirname}/${name}/${name}@${version}.txt`)
+    packageVersionMatches(`${dirname}/${name}/package.json`, version)
+    fileShouldNotExist(`${dirname}/${name}/index.js`)
+  }
+
+  const assert = {
+    fileShouldExist,
+    fileShouldNotExist,
+    packageVersionMatches,
+    packageInstalled,
+    packageMissing,
+    packageDirty,
+  }
+
+  return { registry, assert, ...mock }
+}
+
+/** breaks down a spec "abbrev@1.1.1" into different parts for mocking */
+function dependencyDetails (spec, opt = {}) {
+  const [name, version = '1.0.0'] = spec.split('@')
+  const { parent, hoist = true, ws, clean = true } = opt
+  const modulePathPrefix = !hoist && parent ? `${parent}/` : ''
+  const modulePath = `${modulePathPrefix}node_modules/${name}`
+  const resolved = `https://registry.npmjs.org/${name}/-/${name}-${version}.tgz`
+  // deps
+  const wsEntries = Object.entries({ ...ws })
+  const depsMap = wsEntries.map(([s, o]) => dependencyDetails(s, { ...o, parent: name }))
+  const dependencies = Object.assign({}, ...depsMap.map(d => d.packageDependency))
+  const spreadDependencies = depsMap.length ? { dependencies } : {}
+  // package and lock objects
+  const packageDependency = { [name]: version }
+  const packageLockEntry = { [modulePath]: { version, resolved } }
+  const packageLockLink = { [modulePath]: { resolved: name, link: true } }
+  const packageLockLocal = { [name]: { version, dependencies } }
+  // build package.js
+  const packageJSON = { name, version, ...spreadDependencies }
+  const packageJSONString = JSON.stringify(packageJSON)
+  const packageJSONFile = { 'package.json': packageJSONString }
+  // build index.js
+  const indexJSString = 'module.exports = "hello world"'
+  const indexJSFile = { 'index.js': indexJSString }
+  // tarball
+  const packageFiles = { ...packageJSONFile, ...indexJSFile }
+  const nodeModules = Object.assign({}, ...depsMap.map(d => d.hoist ? {} : d.dirtyOrCleanDir))
+  const nodeModulesDir = { node_modules: nodeModules }
+  const packageDir = { [name]: { ...packageFiles, ...nodeModulesDir } }
+  const tarballDir = { [`${name}@${version}`]: packageFiles }
+  // dirty files
+  const dirtyFile = { [`${name}@${version}.txt`]: 'dirty file' }
+  const dirtyFiles = { ...packageJSONFile, ...dirtyFile }
+  const dirtyDir = { [name]: dirtyFiles }
+  const dirtyOrCleanDir = clean ? {} : dirtyDir
+
+  return {
+    packageDependency,
+    hoist,
+    depsMap,
+    dirtyOrCleanDir,
+    tarballDir,
+    packageDir,
+    packageLockEntry,
+    packageLockLink,
+    packageLockLocal,
+  }
+}
+
+function workspaceMock (t, opts) {
+  const toObject = [(a, c) => ({ ...a, ...c }), {}]
+  const { workspaces: workspacesDef, ...rest } = { clean: true, ...opts }
+  const workspaces = Object.fromEntries(Object.entries(workspacesDef).map(([name, ws]) => {
+    return [name, Object.fromEntries(Object.entries(ws).map(([wsPackageDep, wsPackageDepOpts]) => {
+      return [wsPackageDep, { ...rest, ...wsPackageDepOpts }]
+    }))]
+  }))
+  const root = 'workspace-root'
+  const version = '1.0.0'
+  const names = Object.keys(workspaces)
+  const ws = Object.entries(workspaces).map(([name, _ws]) => dependencyDetails(name, { ws: _ws }))
+  const deps = ws.map(({ depsMap }) => depsMap).flat()
+  const tarballs = deps.map(w => w.tarballDir).reduce(...toObject)
+  const symlinks = names
+    .map((name) => ({ [name]: t.fixture('symlink', `../${name}`) })).reduce(...toObject)
+  const hoisted = deps.filter(d => d.hoist).map(w => w.dirtyOrCleanDir).reduce(...toObject)
+  const workspaceFolders = ws.map(w => w.packageDir).reduce(...toObject)
+  const packageJSON = { name: root, version, workspaces: names }
+  const packageLockJSON = ({
+    name: root,
+    version,
+    lockfileVersion: 3,
+    requires: true,
+    packages: {
+      '': { name: root, version, workspaces: names },
+      ...deps.filter(d => d.hoist).map(d => d.packageLockEntry).reduce(...toObject),
+      ...ws.map(d => d.packageLockEntry).flat().reduce(...toObject),
+      ...ws.map(d => d.packageLockLink).flat().reduce(...toObject),
+      ...ws.map(d => d.packageLockLocal).flat().reduce(...toObject),
+      ...deps.filter(d => !d.hoist).map(d => d.packageLockEntry).reduce(...toObject),
+    },
+  })
+  return {
+    tarballs,
+    node_modules: {
+      ...hoisted,
+      ...symlinks,
+    },
+    'package-lock.json': JSON.stringify(packageLockJSON),
+    'package.json': JSON.stringify(packageJSON),
+    ...workspaceFolders,
+  }
+}
+
 module.exports = setupMockNpm
 module.exports.load = setupMockNpm
 module.exports.setGlobalNodeModules = setGlobalNodeModules
+module.exports.loadNpmWithRegistry = loadNpmWithRegistry
+module.exports.workspaceMock = workspaceMock