fix: don't create a broken lockfile

When registries config settings change, don't create a broken
lockfile.

PR #2384 
close #2379
This commit is contained in:
Zoltan Kochan
2020-02-29 01:48:40 +02:00
committed by GitHub
parent 0fdb99bf77
commit 84fc4c20ea
6 changed files with 100 additions and 30 deletions

View File

@@ -86,7 +86,7 @@ function reportUnexpectedStore (err: Error, msg: object) {
pnpm now wants to use the store at "${msg['actualStorePath']}" to link dependencies.
If you want to use the new store location, reinstall your dependencies with "pnpm install --force".
If you want to use the new store location, reinstall your dependencies with "pnpm install".
You may change the global store location by running "pnpm config set store-dir <dir>".
(This error may happen if the node_modules was installed with a different major version of pnpm)
@@ -101,7 +101,7 @@ function reportUnexpectedVirtualStoreDir (err: Error, msg: object) {
pnpm now wants to use the virtual store at "${msg['actual']}" to link dependencies from the store.
If you want to use the new virtual store location, reinstall your dependencies with "pnpm install --force".
If you want to use the new virtual store location, reinstall your dependencies with "pnpm install".
You may change the virtual store location by changing the value of the virtual-store-dir config.
`
@@ -112,7 +112,7 @@ function reportStoreBreakingChange (msg: object) {
${formatErrorSummary(`The store used for the current node_modules is incomatible with the current version of pnpm`)}
Store path: ${colorPath(msg['storePath'])}
Try running the same command with the ${highlight('--force')} parameter.
Run "pnpm install" to recreate node_modules.
`
if (msg['additionalInformation']) {
@@ -128,7 +128,7 @@ function reportModulesBreakingChange (msg: object) {
${formatErrorSummary(`The current version of pnpm is not compatible with the available node_modules structure`)}
node_modules path: ${colorPath(msg['modulesPath'])}
Run ${highlight('pnpm install --force')} to recreate node_modules.
Run ${highlight('pnpm install')} to recreate node_modules.
`
if (msg['additionalInformation']) {

View File

@@ -57,6 +57,7 @@ export default async function getContext<T> (
projects: (ProjectOptions & T)[],
opts: {
force: boolean,
forceNewNodeModules?: boolean,
forceSharedLockfile: boolean,
extraBinPaths: string[],
lockfileDir: string,
@@ -85,9 +86,10 @@ export default async function getContext<T> (
if (importersContext.modules) {
await validateNodeModules(importersContext.modules, importersContext.projects, {
currentHoistPattern: importersContext.currentHoistPattern,
force: opts.force,
forceNewNodeModules: opts.forceNewNodeModules === true,
include: opts.include,
lockfileDir: opts.lockfileDir,
registries: opts.registries,
storeDir: opts.storeDir,
virtualStoreDir,
@@ -170,9 +172,10 @@ async function validateNodeModules (
}>,
opts: {
currentHoistPattern?: string[],
force: boolean,
forceNewNodeModules: boolean,
include?: IncludedDependencies,
lockfileDir: string,
registries: Registries,
storeDir: string,
virtualStoreDir: string,
@@ -188,7 +191,7 @@ async function validateNodeModules (
) {
const rootProject = projects.find(({ id }) => id === '.')
if (opts.forceShamefullyHoist && modules.shamefullyHoist !== opts.shamefullyHoist) {
if (opts.force && rootProject) {
if (opts.forceNewNodeModules && rootProject) {
await purgeModulesDirsOfImporter(rootProject)
return
}
@@ -196,17 +199,17 @@ async function validateNodeModules (
throw new PnpmError(
'SHAMEFULLY_HOIST_WANTED',
'This "node_modules" folder was created using the --shamefully-hoist option.'
+ ' You must add that option, or else run "pnpm install --force" to recreate the "node_modules" folder.',
+ ' You must add that option, or else run "pnpm install" to recreate the "node_modules" folder.',
)
}
throw new PnpmError(
'SHAMEFULLY_HOIST_NOT_WANTED',
'This "node_modules" folder was created without the --shamefully-hoist option.'
+ ' You must remove that option, or else "pnpm install --force" to recreate the "node_modules" folder.',
+ ' You must remove that option, or else "pnpm install" to recreate the "node_modules" folder.',
)
}
if (opts.forceIndependentLeaves && Boolean(modules.independentLeaves) !== opts.independentLeaves) {
if (opts.force) {
if (opts.forceNewNodeModules) {
// TODO: remove the node_modules in the lockfile directory
await Promise.all(projects.map(purgeModulesDirsOfImporter))
return
@@ -215,13 +218,13 @@ async function validateNodeModules (
throw new PnpmError(
'INDEPENDENT_LEAVES_WANTED',
'This "node_modules" folder was created using the --independent-leaves option.'
+ ' You must add that option, or else run "pnpm install --force" to recreate the "node_modules" folder.',
+ ' You must add that option, or else run "pnpm install" to recreate the "node_modules" folder.',
)
}
throw new PnpmError(
'INDEPENDENT_LEAVES_NOT_WANTED',
'This "node_modules" folder was created without the --independent-leaves option.'
+ ' You must remove that option, or else "pnpm install --force" to recreate the "node_modules" folder.',
+ ' You must remove that option, or else "pnpm install" to recreate the "node_modules" folder.',
)
}
if (opts.forceHoistPattern && rootProject) {
@@ -237,11 +240,11 @@ async function validateNodeModules (
throw new PnpmError(
'HOISTING_NOT_WANTED',
'This "node_modules" folder was created without the --hoist-pattern option.'
+ ' You must remove that option, or else add the --force option to recreate the "node_modules" folder.',
+ ' You must remove that option, or else run "pnpm install" to recreate the "node_modules" folder.',
)
}
} catch (err) {
if (!opts.force) throw err
if (!opts.forceNewNodeModules) throw err
await purgeModulesDirsOfImporter(rootProject)
}
}
@@ -263,10 +266,17 @@ async function validateNodeModules (
}
}
} catch (err) {
if (!opts.force) throw err
if (!opts.forceNewNodeModules) throw err
await purgeModulesDirsOfImporter(project)
}
}))
if (modules.registries && !R.equals(opts.registries, modules.registries)) {
if (opts.forceNewNodeModules) {
await Promise.all(projects.map(purgeModulesDirsOfImporter))
return
}
throw new PnpmError('REGISTRIES_MISMATCH', `This "node_modules" directory was created using the following registries configuration: ${JSON.stringify(modules.registries)}. The current configuration is ${JSON.stringify(opts.registries)}. To recreate "node_modules" using the new settings, run "pnpm install".`)
}
}
async function purgeModulesDirsOfImporter (
@@ -320,6 +330,7 @@ export async function getContextForSingleImporter (
manifest: ProjectManifest,
opts: {
force: boolean,
forceNewNodeModules?: boolean,
forceSharedLockfile: boolean,
extraBinPaths: string[],
lockfileDir: string,
@@ -375,9 +386,10 @@ export async function getContextForSingleImporter (
if (modules) {
await validateNodeModules(modules, projects, {
currentHoistPattern,
force: opts.force,
forceNewNodeModules: opts.forceNewNodeModules === true,
include: opts.include,
lockfileDir: opts.lockfileDir,
registries: opts.registries,
storeDir: opts.storeDir,
virtualStoreDir,

View File

@@ -149,6 +149,8 @@ export async function mutateModules (
throw new PnpmError('OPTIONAL_DEPS_REQUIRE_PROD_DEPS', 'Optional dependencies cannot be installed without production dependencies')
}
const installsOnly = projects.every((project) => project.mutation === 'install')
opts['forceNewNodeModules'] = installsOnly
const ctx = await getContext(projects, opts)
for (const { manifest, rootDir } of ctx.projects) {
@@ -180,7 +182,6 @@ export async function mutateModules (
return result
async function _install (): Promise<Array<{ rootDir: string, manifest: ProjectManifest }>> {
const installsOnly = projects.every((project) => project.mutation === 'install')
if (
!opts.lockfileOnly &&
!opts.update &&

View File

@@ -1,4 +1,5 @@
import { WANTED_LOCKFILE } from '@pnpm/constants'
import PnpmError from '@pnpm/error'
import { prepareEmpty, preparePackages } from '@pnpm/prepare'
import rimraf = require('@zkochan/rimraf')
import isCI = require('is-ci')
@@ -61,11 +62,20 @@ test("don't fail on non-compatible node_modules when forced in a workspace", asy
test('do not fail on non-compatible node_modules when forced with a named installation', async (t: tape.Test) => {
prepareEmpty(t)
const opts = await testDefaults({ force: true })
const opts = await testDefaults()
await saveModulesYaml('0.50.0', opts.storeDir)
await addDependenciesToPackage({}, ['is-negative'], opts)
let err!: PnpmError
try {
await addDependenciesToPackage({}, ['is-negative'], opts)
} catch (_err) {
err = _err
}
t.ok(err)
t.equal(err.code, 'ERR_PNPM_MODULES_BREAKING_CHANGE')
await install({}, opts)
})
test("don't fail on non-compatible store when forced", async (t: tape.Test) => {
@@ -81,11 +91,20 @@ test("don't fail on non-compatible store when forced", async (t: tape.Test) => {
test('do not fail on non-compatible store when forced during named installation', async (t: tape.Test) => {
prepareEmpty(t)
const opts = await testDefaults({ force: true })
const opts = await testDefaults()
await saveModulesYaml('0.32.0', opts.storeDir)
await addDependenciesToPackage({}, ['is-negative'], opts)
let err!: PnpmError
try {
await addDependenciesToPackage({}, ['is-negative'], opts)
} catch (_err) {
err = _err
}
t.ok(err)
t.equal(err.code, 'ERR_PNPM_MODULES_BREAKING_CHANGE')
await install({}, opts)
})
async function saveModulesYaml (pnpmVersion: string, storeDir: string) {

View File

@@ -2,7 +2,7 @@ import { WANTED_LOCKFILE } from '@pnpm/constants'
import { prepareEmpty } from '@pnpm/prepare'
import fs = require('mz/fs')
import path = require('path')
import { install } from 'supi'
import { addDependenciesToPackage, install } from 'supi'
import tape = require('tape')
import promisifyTape from 'tape-promise'
import { testDefaults } from '../utils'
@@ -101,16 +101,17 @@ test('fail if installing different types of dependencies in a project that uses
await project.hasNot('once')
let err!: Error & { code: string }
const newOpts = await testDefaults({
include: {
dependencies: true,
devDependencies: true,
optionalDependencies: true,
},
lockfileDir,
})
try {
await install(manifest, await testDefaults({
include: {
dependencies: true,
devDependencies: true,
optionalDependencies: true,
},
lockfileDir,
}))
await addDependenciesToPackage(manifest, ['is-negative'], newOpts)
} catch (_) {
err = _
}
@@ -118,4 +119,6 @@ test('fail if installing different types of dependencies in a project that uses
t.ok(err, 'installation failed')
t.equal(err.code, 'ERR_PNPM_INCLUDED_DEPS_CONFLICT', 'error has correct error code')
t.ok(err.message.includes('was installed with devDependencies. Current install wants optionalDependencies, dependencies, devDependencies.'), 'correct error message')
await install(manifest, newOpts)
})

View File

@@ -1,5 +1,6 @@
import { WANTED_LOCKFILE } from '@pnpm/constants'
import { RootLog } from '@pnpm/core-loggers'
import PnpmError from '@pnpm/error'
import { Lockfile, TarballResolution } from '@pnpm/lockfile-file'
import { prepareEmpty, preparePackages } from '@pnpm/prepare'
import { fromDir as readPackageJsonFromDir } from '@pnpm/read-package-json'
@@ -1052,3 +1053,37 @@ test('existing dependencies are preserved when updating a lockfile to a newer fo
t.deepEqual(initialLockfile.packages, updatedLockfile.packages, 'dependency versions preserved')
})
test('lockfile is not getting broken if the used registry changes', async (t: tape.Test) => {
const project = prepareEmpty(t)
const manifest = await addDependenciesToPackage({}, ['is-positive@1'], await testDefaults())
const newOpts = await testDefaults({ registries: { default: 'https://registry.npmjs.org/' } })
let err!: PnpmError
try {
await addDependenciesToPackage(manifest, ['is-negative@1'], newOpts)
} catch (_err) {
err = _err
}
t.ok(err)
t.equal(err.code, 'ERR_PNPM_REGISTRIES_MISMATCH')
await mutateModules([
{
buildIndex: 0,
manifest,
mutation: 'install',
rootDir: process.cwd(),
},
], newOpts)
await addDependenciesToPackage(manifest, ['is-negative@1'], newOpts)
t.deepEqual(
Object.keys((await project.readLockfile()).packages),
[
'/is-negative/1.0.1',
'/is-positive/1.0.0',
],
)
})