fix: only remove node_modules if the user confirms to do so (#6458)

This commit is contained in:
Zoltan Kochan
2023-04-30 11:55:25 +03:00
committed by GitHub
parent 3a1a1385d8
commit 497b0a79c0
12 changed files with 144 additions and 80 deletions

View File

@@ -0,0 +1,6 @@
---
"@pnpm/get-context": patch
"pnpm": patch
---
Ask the user to confirm the removal of node_modules directory unless the `--force` option is passed.

View File

@@ -9,4 +9,9 @@ packages:
- '!has-package-lock-json'
- '!hello-world-js-bin'
- '!has-yarn-lock'
- '!has-yarn2-lock'
- '!has-yarn2-lock'
- '!workspace-with-lockfile-dupes'
- '!workspace-with-lockfile-subdep-dupes'
- '!workspace-has-shared-yarn-lock'
- '!workspace-has-shared-package-lock-json'
- '!workspace-has-shared-npm-shrinkwrap-json'

View File

@@ -31,6 +31,8 @@ const basePatchOption = {
describe('patch and commit', () => {
let defaultPatchOption: patch.PatchCommandOptions
let cacheDir: string
let storeDir: string
beforeEach(async () => {
prepare({
@@ -38,8 +40,8 @@ describe('patch and commit', () => {
'is-positive': '1.0.0',
},
})
const cacheDir = path.resolve('cache')
const storeDir = path.resolve('store')
cacheDir = path.resolve('cache')
storeDir = path.resolve('store')
defaultPatchOption = {
...basePatchOption,
cacheDir,
@@ -73,9 +75,11 @@ describe('patch and commit', () => {
await patchCommit.handler({
...DEFAULT_OPTS,
cacheDir,
dir: process.cwd(),
frozenLockfile: false,
fixLockfile: true,
storeDir,
}, [patchDir])
const { manifest } = await readProjectManifest(process.cwd())
@@ -104,9 +108,11 @@ describe('patch and commit', () => {
await patchCommit.handler({
...DEFAULT_OPTS,
cacheDir,
dir: process.cwd(),
frozenLockfile: false,
fixLockfile: true,
storeDir,
}, [patchDir])
expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).toContain('// test patching')
@@ -124,10 +130,12 @@ describe('patch and commit', () => {
await patchCommit.handler({
...DEFAULT_OPTS,
cacheDir,
dir: process.cwd(),
frozenLockfile: false,
fixLockfile: true,
patchesDir,
storeDir,
}, [patchDir])
const { manifest } = await readProjectManifest(process.cwd())
@@ -160,9 +168,11 @@ describe('patch and commit', () => {
await patchCommit.handler({
...DEFAULT_OPTS,
cacheDir,
dir: process.cwd(),
frozenLockfile: false,
fixLockfile: true,
storeDir,
}, [patchDir])
expect(fs.readFileSync('node_modules/is-positive/index.js', 'utf8')).toContain('// test patching')
@@ -177,9 +187,11 @@ describe('patch and commit', () => {
await patchCommit.handler({
...DEFAULT_OPTS,
cacheDir,
dir: process.cwd(),
frozenLockfile: false,
fixLockfile: true,
storeDir,
}, [patchDir])
const { manifest } = await readProjectManifest(process.cwd())
@@ -224,9 +236,11 @@ describe('patch and commit', () => {
await patchCommit.handler({
...DEFAULT_OPTS,
cacheDir,
dir: process.cwd(),
frozenLockfile: false,
fixLockfile: true,
storeDir,
}, [patchDir])
const { manifest } = await readProjectManifest(process.cwd())
@@ -318,9 +332,11 @@ describe('prompt to choose version', () => {
fs.appendFileSync(path.join(patchDir, 'source/index.js'), '// test patching', 'utf8')
await patchCommit.handler({
...DEFAULT_OPTS,
cacheDir,
dir: process.cwd(),
frozenLockfile: false,
fixLockfile: true,
storeDir,
}, [patchDir])
const { manifest } = await readProjectManifest(process.cwd())

View File

@@ -65,7 +65,10 @@ test('do not fail on non-compatible node_modules when forced with a named instal
}
expect(err.code).toBe('ERR_PNPM_MODULES_BREAKING_CHANGE')
await install({}, opts)
await install({}, {
...opts,
force: true, // Don't ask for prompt
})
})
test("don't fail on non-compatible store when forced", async () => {
@@ -91,7 +94,10 @@ test('do not fail on non-compatible store when forced during named installation'
}
expect(err.code).toBe('ERR_PNPM_MODULES_BREAKING_CHANGE')
await install({}, opts)
await install({}, {
...opts,
force: true, // Don't ask for prompt
})
})
async function saveModulesYaml (pnpmVersion: string, storeDir: string) {

View File

@@ -137,6 +137,7 @@ test('fail if installing different types of dependencies in a project that uses
await project.hasNot('once')
const newOpts = await testDefaults({
force: true, // Don't ask for prompt
include: {
dependencies: true,
devDependencies: true,

View File

@@ -1061,7 +1061,10 @@ test('lockfile is not getting broken if the used registry changes', async () =>
manifest,
mutation: 'install',
rootDir: process.cwd(),
}, newOpts)
}, {
...newOpts,
force: true, // Don't ask for prompt
})
await addDependenciesToPackage(manifest, ['is-negative@1'], newOpts)
expect(Object.keys((await project.readLockfile()).packages)).toStrictEqual([

View File

@@ -46,6 +46,7 @@
"@pnpm/types": "workspace:*",
"@zkochan/rimraf": "^2.1.2",
"ci-info": "^3.8.0",
"enquirer": "^2.3.6",
"path-absolute": "^1.0.1",
"ramda": "npm:@pnpm/ramda@0.28.1"
},

View File

@@ -17,6 +17,7 @@ import {
type Registries,
} from '@pnpm/types'
import rimraf from '@zkochan/rimraf'
import enquirer from 'enquirer'
import pathAbsolute from 'path-absolute'
import clone from 'ramda/src/clone'
import equals from 'ramda/src/equals'
@@ -114,6 +115,7 @@ export async function getContext (
registries: opts.registries,
storeDir: opts.storeDir,
virtualStoreDir,
confirmModulesPurge: !opts.force,
forceHoistPattern: opts.forceHoistPattern,
hoistPattern: opts.hoistPattern,
@@ -211,6 +213,7 @@ async function validateModules (
registries: Registries
storeDir: string
virtualStoreDir: string
confirmModulesPurge?: boolean
hoistPattern?: string[] | undefined
forceHoistPattern?: boolean
@@ -227,7 +230,7 @@ async function validateModules (
!equals(modules.publicHoistPattern, opts.publicHoistPattern || undefined)
) {
if (opts.forceNewModules && (rootProject != null)) {
await purgeModulesDirsOfImporter(opts.virtualStoreDir, rootProject)
await purgeModulesDirsOfImporter(opts, rootProject)
return { purged: true }
}
throw new PnpmError(
@@ -249,7 +252,7 @@ async function validateModules (
}
} catch (err: any) { // eslint-disable-line
if (!opts.forceNewModules) throw err
await purgeModulesDirsOfImporter(opts.virtualStoreDir, rootProject)
await purgeModulesDirsOfImporter(opts, rootProject)
purged = true
}
}
@@ -272,19 +275,19 @@ async function validateModules (
}
} catch (err: any) { // eslint-disable-line
if (!opts.forceNewModules) throw err
await purgeModulesDirsOfImporter(opts.virtualStoreDir, project)
await purgeModulesDirsOfImporter(opts, project)
purged = true
}
}))
if ((modules.registries != null) && !equals(opts.registries, modules.registries)) {
if (opts.forceNewModules) {
await Promise.all(projects.map(purgeModulesDirsOfImporter.bind(null, opts.virtualStoreDir)))
await purgeModulesDirsOfImporters(opts, projects)
return { purged: true }
}
throw new PnpmError('REGISTRIES_MISMATCH', `This modules directory was created using the following registries configuration: ${JSON.stringify(modules.registries)}. The current configuration is ${JSON.stringify(opts.registries)}. To recreate the modules directory using the new settings, run "pnpm install${opts.global ? ' -g' : ''}".`)
}
if (purged && (rootProject == null)) {
await purgeModulesDirsOfImporter(opts.virtualStoreDir, {
await purgeModulesDirsOfImporter(opts, {
modulesDir: path.join(opts.lockfileDir, opts.modulesDir),
rootDir: opts.lockfileDir,
})
@@ -293,23 +296,54 @@ async function validateModules (
}
async function purgeModulesDirsOfImporter (
virtualStoreDir: string,
opts: {
confirmModulesPurge?: boolean
virtualStoreDir: string
},
importer: {
modulesDir: string
rootDir: string
}
) {
logger.info({
message: `Recreating ${importer.modulesDir}`,
prefix: importer.rootDir,
})
try {
// We don't remove the actual modules directory, just the contents of it.
// 1. we will need the directory anyway.
// 2. in some setups, pnpm won't even have permission to remove the modules directory.
await removeContentsOfDir(importer.modulesDir, virtualStoreDir)
} catch (err: any) { // eslint-disable-line
if (err.code !== 'ENOENT') throw err
return purgeModulesDirsOfImporters(opts, [importer])
}
async function purgeModulesDirsOfImporters (
opts: {
confirmModulesPurge?: boolean
virtualStoreDir: string
},
importers: Array<{
modulesDir: string
rootDir: string
}>
) {
if (opts.confirmModulesPurge ?? true) {
const confirmed = await enquirer.prompt({
type: 'confirm',
name: 'question',
message: importers.length === 1
? `The modules directory at "${importers[0].modulesDir}" will be removed and reinstalled from scratch. Proceed?`
: 'The modules directories will be removed and reinstalled from scratch. Proceed?',
initial: true,
})
if (!confirmed) {
throw new PnpmError('ABORTED_REMOVE_MODULES_DIR', 'Aborted removal of modules directory')
}
}
for (const importer of importers) {
logger.info({
message: `Recreating ${importer.modulesDir}`,
prefix: importer.rootDir,
})
try {
// We don't remove the actual modules directory, just the contents of it.
// 1. we will need the directory anyway.
// 2. in some setups, pnpm won't even have permission to remove the modules directory.
await removeContentsOfDir(importer.modulesDir, opts.virtualStoreDir)
} catch (err: any) { // eslint-disable-line
if (err.code !== 'ENOENT') throw err
}
}
}
@@ -437,6 +471,7 @@ export async function getContextForSingleImporter (
registries: opts.registries,
storeDir: opts.storeDir,
virtualStoreDir,
confirmModulesPurge: !opts.force,
forceHoistPattern: opts.forceHoistPattern,
hoistPattern: opts.hoistPattern,

29
pnpm-lock.yaml generated
View File

@@ -3032,6 +3032,9 @@ importers:
ci-info:
specifier: ^3.8.0
version: 3.8.0
enquirer:
specifier: ^2.3.6
version: 2.3.6
path-absolute:
specifier: ^1.0.1
version: 1.0.1
@@ -5150,12 +5153,18 @@ importers:
'@pnpm/plugin-commands-licenses':
specifier: workspace:*
version: 'link:'
'@pnpm/prepare':
specifier: workspace:*
version: link:../../__utils__/prepare
'@pnpm/read-package-json':
specifier: workspace:*
version: link:../../pkg-manifest/read-package-json
'@pnpm/registry-mock':
specifier: 3.8.0
version: 3.8.0(typanion@3.12.1)
'@pnpm/test-fixtures':
specifier: workspace:*
version: link:../../__utils__/test-fixtures
'@types/ramda':
specifier: 0.28.20
version: 0.28.20
@@ -5168,9 +5177,6 @@ importers:
strip-ansi:
specifier: ^6.0.1
version: 6.0.1
tempy:
specifier: ^1.0.1
version: 1.0.1
reviewing/plugin-commands-listing:
dependencies:
@@ -8871,7 +8877,7 @@ packages:
/@types/byline@4.2.33:
resolution: {integrity: sha512-LJYez7wrWcJQQDknqZtrZuExMGP0IXmPl1rOOGDqLbu+H7UNNRfKNuSxCBcQMLH1EfjeWidLedC/hCc5dDfBog==}
dependencies:
'@types/node': 18.16.2
'@types/node': 14.18.42
dev: true
/@types/cacheable-request@6.0.3:
@@ -8879,7 +8885,7 @@ packages:
dependencies:
'@types/http-cache-semantics': 4.0.1
'@types/keyv': 3.1.4
'@types/node': 18.16.0
'@types/node': 14.18.42
'@types/responselike': 1.0.0
/@types/concat-stream@2.0.0:
@@ -8907,7 +8913,7 @@ packages:
resolution: {integrity: sha512-IO+MJPVhoqz+28h1qLAcBEH2+xHMK6MTyHJc7MTnnYb6wsoLR29POVGJ7LycmVXIqyy/4/2ShP5sUwTXuOwb/w==}
dependencies:
'@types/minimatch': 5.1.2
'@types/node': 18.16.0
'@types/node': 14.18.42
dev: true
/@types/graceful-fs@4.1.6:
@@ -8981,7 +8987,7 @@ packages:
/@types/keyv@3.1.4:
resolution: {integrity: sha512-BQ5aZNSCpj7D6K2ksrRCTmKRLEpnPvWDiLPfoGyhZ++8YtiK9d/3DBKPJgry359X/P1PfruyYwvnvwFjuEiEIg==}
dependencies:
'@types/node': 18.16.0
'@types/node': 14.18.42
/@types/lodash@4.14.181:
resolution: {integrity: sha512-n3tyKthHJbkiWhDZs3DkhkCzt2MexYHXlX0td5iMplyfwketaOeKboEVBqzceH7juqvEg3q5oUoBFxSLu7zFag==}
@@ -9024,15 +9030,10 @@ packages:
/@types/node@14.18.42:
resolution: {integrity: sha512-xefu+RBie4xWlK8hwAzGh3npDz/4VhF6icY/shU+zv/1fNn+ZVG7T7CRwe9LId9sAYRPxI+59QBPuKL3WpyGRg==}
dev: true
/@types/node@18.16.0:
resolution: {integrity: sha512-BsAaKhB+7X+H4GnSjGhJG9Qi8Tw+inU9nJDwmD5CgOmBLEI6ArdhikpLX7DjbjDRDTbqZzU2LSQNZg8WGPiSZQ==}
/@types/node@18.16.2:
resolution: {integrity: sha512-GQW/JL/5Fz/0I8RpeBG9lKp0+aNcXEaVL71c0D2Q0QHDTFvlYKT7an0onCUXj85anv7b4/WesqdfchLc0jtsCg==}
dev: true
/@types/normalize-package-data@2.4.1:
resolution: {integrity: sha512-Gj7cI7z+98M282Tqmp2K5EIsoouUEzbBJhQQzDE3jSIRk6r9gsz0oUokqIUR4u1R3dMHo0pDHM7sNOHyhulypw==}
dev: true
@@ -9065,7 +9066,7 @@ packages:
/@types/responselike@1.0.0:
resolution: {integrity: sha512-85Y2BjiufFzaMIlvJDvTTB8Fxl2xfLo4HgmHzVBz08w4wDePCTjYw66PdrolO0kzli3yam/YCgRufyo1DdQVTA==}
dependencies:
'@types/node': 18.16.0
'@types/node': 14.18.42
/@types/retry@0.12.2:
resolution: {integrity: sha512-XISRgDJ2Tc5q4TRqvgJtzsRkFYNJzZrhTdtMoGVBttwzzQJkPnS3WWTFc7kuDRoPtPakl+T+OfdEUjYJj7Jbow==}
@@ -13577,7 +13578,7 @@ packages:
jws: 3.2.2
lodash: 4.17.21
ms: 2.1.3
semver: 7.3.8
semver: 7.5.0
/jsprim@1.4.2:
resolution: {integrity: sha512-P2bSOMAc/ciLz6DzgjVlGJP9+BrJWu5UDGK70C2iweC5QBIeFf0ZXRvGjEj2uYgrY2MkAAhsSWHDWlFtEroZWw==}

View File

@@ -39,11 +39,12 @@
"@pnpm/plugin-commands-licenses": "workspace:*",
"@pnpm/read-package-json": "workspace:*",
"@pnpm/registry-mock": "3.8.0",
"@pnpm/test-fixtures": "workspace:*",
"@pnpm/prepare": "workspace:*",
"@types/ramda": "0.28.20",
"@types/wrap-ansi": "^8.0.1",
"@types/zkochan__table": "npm:@types/table@6.0.0",
"strip-ansi": "^6.0.1",
"tempy": "^1.0.1"
"strip-ansi": "^6.0.1"
},
"dependencies": {
"@pnpm/cli-utils": "workspace:*",

View File

@@ -2,35 +2,18 @@
import path from 'path'
import { licenses } from '@pnpm/plugin-commands-licenses'
import { install } from '@pnpm/plugin-commands-installation'
import { REGISTRY_MOCK_PORT } from '@pnpm/registry-mock'
import { tempDir } from '@pnpm/prepare'
import { fixtures } from '@pnpm/test-fixtures'
import stripAnsi from 'strip-ansi'
import { DEFAULT_OPTS } from './utils'
import tempy from 'tempy'
const REGISTRY_URL = `http://localhost:${REGISTRY_MOCK_PORT}`
const LICENSES_OPTIONS = {
cacheDir: 'cache',
fetchRetries: 1,
fetchRetryFactor: 1,
fetchRetryMaxtimeout: 60,
fetchRetryMintimeout: 10,
global: false,
networkConcurrency: 16,
offline: false,
rawConfig: { registry: REGISTRY_URL },
registries: { default: REGISTRY_URL },
strictSsl: false,
tag: 'latest',
userAgent: '',
userConfig: {},
}
const f = fixtures(__dirname)
test('pnpm licenses', async () => {
const workspaceDir = path.resolve('./test/fixtures/complex-licenses')
const workspaceDir = tempDir()
f.copy('complex-licenses', workspaceDir)
const tmp = tempy.directory()
const storeDir = path.join(tmp, 'store')
const storeDir = path.join(workspaceDir, 'store')
await install.handler({
...DEFAULT_OPTS,
dir: workspaceDir,
@@ -40,7 +23,7 @@ test('pnpm licenses', async () => {
// Attempt to run the licenses command now
const { output, exitCode } = await licenses.handler({
...LICENSES_OPTIONS,
...DEFAULT_OPTS,
dir: workspaceDir,
pnpmHomeDir: '',
long: false,
@@ -54,10 +37,10 @@ test('pnpm licenses', async () => {
})
test('pnpm licenses: show details', async () => {
const workspaceDir = path.resolve('./test/fixtures/simple-licenses')
const workspaceDir = tempDir()
f.copy('simple-licenses', workspaceDir)
const tmp = tempy.directory()
const storeDir = path.join(tmp, 'store')
const storeDir = path.join(workspaceDir, 'store')
await install.handler({
...DEFAULT_OPTS,
dir: workspaceDir,
@@ -67,7 +50,7 @@ test('pnpm licenses: show details', async () => {
// Attempt to run the licenses command now
const { output, exitCode } = await licenses.handler({
...LICENSES_OPTIONS,
...DEFAULT_OPTS,
dir: workspaceDir,
pnpmHomeDir: '',
long: true,
@@ -81,10 +64,10 @@ test('pnpm licenses: show details', async () => {
})
test('pnpm licenses: output as json', async () => {
const workspaceDir = path.resolve('./test/fixtures/simple-licenses')
const workspaceDir = tempDir()
f.copy('simple-licenses', workspaceDir)
const tmp = tempy.directory()
const storeDir = path.join(tmp, 'store')
const storeDir = path.join(workspaceDir, 'store')
await install.handler({
...DEFAULT_OPTS,
dir: workspaceDir,
@@ -94,7 +77,7 @@ test('pnpm licenses: output as json', async () => {
// Attempt to run the licenses command now
const { output, exitCode } = await licenses.handler({
...LICENSES_OPTIONS,
...DEFAULT_OPTS,
dir: workspaceDir,
pnpmHomeDir: '',
long: false,
@@ -126,7 +109,7 @@ test('pnpm licenses: output as json', async () => {
test('pnpm licenses: fails when lockfile is missing', async () => {
await expect(
licenses.handler({
...LICENSES_OPTIONS,
...DEFAULT_OPTS,
dir: path.resolve('./test/fixtures/invalid'),
pnpmHomeDir: '',
long: true,
@@ -137,10 +120,10 @@ test('pnpm licenses: fails when lockfile is missing', async () => {
})
test('pnpm licenses: should correctly read LICENSE file with executable file mode', async () => {
const workspaceDir = path.resolve('./test/fixtures/file-mode-test')
const workspaceDir = tempDir()
f.copy('file-mode-test', workspaceDir)
const tmp = tempy.directory()
const storeDir = path.join(tmp, 'store')
const storeDir = path.join(workspaceDir, 'store')
await install.handler({
...DEFAULT_OPTS,
dir: workspaceDir,
@@ -150,7 +133,7 @@ test('pnpm licenses: should correctly read LICENSE file with executable file mod
// Attempt to run the licenses command now
const { output, exitCode } = await licenses.handler({
...LICENSES_OPTIONS,
...DEFAULT_OPTS,
dir: workspaceDir,
pnpmHomeDir: '',
long: true,
@@ -164,10 +147,10 @@ test('pnpm licenses: should correctly read LICENSE file with executable file mod
})
test('pnpm licenses should work with file protocol dependency', async () => {
const workspaceDir = path.resolve('./test/fixtures/with-file-protocol')
const workspaceDir = tempDir()
f.copy('with-file-protocol', workspaceDir)
const tmp = tempy.directory()
const storeDir = path.join(tmp, 'store')
const storeDir = path.join(workspaceDir, 'store')
await install.handler({
...DEFAULT_OPTS,
dir: workspaceDir,
@@ -176,7 +159,7 @@ test('pnpm licenses should work with file protocol dependency', async () => {
})
const { output, exitCode } = await licenses.handler({
...LICENSES_OPTIONS,
...DEFAULT_OPTS,
dir: workspaceDir,
pnpmHomeDir: '',
long: false,

View File

@@ -9,6 +9,12 @@
"../../__typings__/**/*.d.ts"
],
"references": [
{
"path": "../../__utils__/prepare"
},
{
"path": "../../__utils__/test-fixtures"
},
{
"path": "../../cli/cli-utils"
},