diff --git a/.changeset/sixty-walls-sparkle.md b/.changeset/sixty-walls-sparkle.md new file mode 100644 index 0000000000..d582b84e5c --- /dev/null +++ b/.changeset/sixty-walls-sparkle.md @@ -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. diff --git a/__fixtures__/pnpm-workspace.yaml b/__fixtures__/pnpm-workspace.yaml index dbe84125f7..8587c3deea 100644 --- a/__fixtures__/pnpm-workspace.yaml +++ b/__fixtures__/pnpm-workspace.yaml @@ -9,4 +9,9 @@ packages: - '!has-package-lock-json' - '!hello-world-js-bin' - '!has-yarn-lock' - - '!has-yarn2-lock' \ No newline at end of file + - '!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' diff --git a/patching/plugin-commands-patching/test/patch.test.ts b/patching/plugin-commands-patching/test/patch.test.ts index 75c39c2611..9f13e0d27f 100644 --- a/patching/plugin-commands-patching/test/patch.test.ts +++ b/patching/plugin-commands-patching/test/patch.test.ts @@ -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()) diff --git a/pkg-manager/core/test/breakingChanges.ts b/pkg-manager/core/test/breakingChanges.ts index 4c0c8cc9fb..620b9adb1f 100644 --- a/pkg-manager/core/test/breakingChanges.ts +++ b/pkg-manager/core/test/breakingChanges.ts @@ -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) { diff --git a/pkg-manager/core/test/install/only.ts b/pkg-manager/core/test/install/only.ts index f0fa2f4488..73c19ff24b 100644 --- a/pkg-manager/core/test/install/only.ts +++ b/pkg-manager/core/test/install/only.ts @@ -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, diff --git a/pkg-manager/core/test/lockfile.ts b/pkg-manager/core/test/lockfile.ts index 106bebbfa9..89a75f27b4 100644 --- a/pkg-manager/core/test/lockfile.ts +++ b/pkg-manager/core/test/lockfile.ts @@ -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([ diff --git a/pkg-manager/get-context/package.json b/pkg-manager/get-context/package.json index e589fb4ec3..5bf3502b40 100644 --- a/pkg-manager/get-context/package.json +++ b/pkg-manager/get-context/package.json @@ -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" }, diff --git a/pkg-manager/get-context/src/index.ts b/pkg-manager/get-context/src/index.ts index 5badb55b75..80ecbb6dfa 100644 --- a/pkg-manager/get-context/src/index.ts +++ b/pkg-manager/get-context/src/index.ts @@ -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, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 807109f6e1..2a4494ba40 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -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==} diff --git a/reviewing/plugin-commands-licenses/package.json b/reviewing/plugin-commands-licenses/package.json index c40dee5867..0644249ea5 100644 --- a/reviewing/plugin-commands-licenses/package.json +++ b/reviewing/plugin-commands-licenses/package.json @@ -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:*", diff --git a/reviewing/plugin-commands-licenses/test/index.ts b/reviewing/plugin-commands-licenses/test/index.ts index 9e1389cfa2..7563d0578c 100644 --- a/reviewing/plugin-commands-licenses/test/index.ts +++ b/reviewing/plugin-commands-licenses/test/index.ts @@ -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, diff --git a/reviewing/plugin-commands-licenses/tsconfig.json b/reviewing/plugin-commands-licenses/tsconfig.json index bd22331ccc..d5c0a39f14 100644 --- a/reviewing/plugin-commands-licenses/tsconfig.json +++ b/reviewing/plugin-commands-licenses/tsconfig.json @@ -9,6 +9,12 @@ "../../__typings__/**/*.d.ts" ], "references": [ + { + "path": "../../__utils__/prepare" + }, + { + "path": "../../__utils__/test-fixtures" + }, { "path": "../../cli/cli-utils" },