Skip to content

Commit

Permalink
fix: don't print that lockfile is up to date when it is not (#6574)
Browse files Browse the repository at this point in the history
close #6544
  • Loading branch information
zkochan committed May 23, 2023
1 parent 78e8392 commit a53ef4d
Show file tree
Hide file tree
Showing 14 changed files with 67 additions and 62 deletions.
7 changes: 7 additions & 0 deletions .changeset/curly-starfishes-end.md
@@ -0,0 +1,7 @@
---
"@pnpm/headless": major
"@pnpm/core": patch
"pnpm": patch
---

Don't print "Lockfile is up-to-date" message before finishing all the lockfile checks [#6544](https://github.com/pnpm/pnpm/issues/6544).
6 changes: 6 additions & 0 deletions .changeset/metal-turkeys-know.md
@@ -0,0 +1,6 @@
---
"@pnpm/get-context": major
---

New property returned: `existsNonEmptyWantedLockfile`.
The `existsWantedLockfile` now means only that a file existed.
2 changes: 1 addition & 1 deletion hooks/types/src/index.ts
Expand Up @@ -5,7 +5,7 @@ export interface PreResolutionHookContext {
wantedLockfile: Lockfile
currentLockfile: Lockfile
existsCurrentLockfile: boolean
existsWantedLockfile: boolean
existsNonEmptyWantedLockfile: boolean
lockfileDir: string
storeDir: string
registries: Registries
Expand Down
2 changes: 1 addition & 1 deletion lockfile/lockfile-file/README.md
Expand Up @@ -28,7 +28,7 @@ if the lockfile file format is not compatible with the current library.

Reads the lockfile file from `<virtualStoreDir>/lock.yaml`.

### `existsWantedLockfile(pkgPath) => Promise<Boolean>`
### `existsNonEmptyWantedLockfile(pkgPath) => Promise<Boolean>`

Returns `true` if a `pnpm-lock.yaml` exists in the root of the package.

Expand Down
4 changes: 2 additions & 2 deletions lockfile/lockfile-file/src/existsWantedLockfile.ts
Expand Up @@ -2,12 +2,12 @@ import fs from 'fs'
import path from 'path'
import { getWantedLockfileName } from './lockfileName'

interface ExistsWantedLockfileOptions {
interface existsNonEmptyWantedLockfileOptions {
useGitBranchLockfile?: boolean
mergeGitBranchLockfiles?: boolean
}

export async function existsWantedLockfile (pkgPath: string, opts: ExistsWantedLockfileOptions = {
export async function existsNonEmptyWantedLockfile (pkgPath: string, opts: existsNonEmptyWantedLockfileOptions = {
useGitBranchLockfile: false,
mergeGitBranchLockfiles: false,
}) {
Expand Down
2 changes: 1 addition & 1 deletion lockfile/lockfile-file/src/index.ts
Expand Up @@ -4,7 +4,7 @@ export {
writeCurrentLockfile,
writeWantedLockfile,
} from './write'
export { existsWantedLockfile } from './existsWantedLockfile'
export { existsNonEmptyWantedLockfile } from './existsWantedLockfile'
export { getLockfileImporterId } from './getLockfileImporterId'
export * from '@pnpm/lockfile-types'
export * from './read'
Expand Down
8 changes: 4 additions & 4 deletions lockfile/lockfile-file/test/read.test.ts
@@ -1,7 +1,7 @@
import path from 'path'
import { getCurrentBranch } from '@pnpm/git-utils'
import {
existsWantedLockfile,
existsNonEmptyWantedLockfile,
readCurrentLockfile,
readWantedLockfile,
writeCurrentLockfile,
Expand Down Expand Up @@ -154,9 +154,9 @@ test('writeCurrentLockfile()', async () => {
expect(await readCurrentLockfile(projectPath, { ignoreIncompatible: false })).toEqual(wantedLockfile)
})

test('existsWantedLockfile()', async () => {
test('existsNonEmptyWantedLockfile()', async () => {
const projectPath = tempy.directory()
expect(await existsWantedLockfile(projectPath)).toBe(false)
expect(await existsNonEmptyWantedLockfile(projectPath)).toBe(false)
await writeWantedLockfile(projectPath, {
importers: {
'.': {
Expand Down Expand Up @@ -192,7 +192,7 @@ test('existsWantedLockfile()', async () => {
},
},
})
expect(await existsWantedLockfile(projectPath)).toBe(true)
expect(await existsNonEmptyWantedLockfile(projectPath)).toBe(true)
})

test('readWantedLockfile() when useGitBranchLockfile', async () => {
Expand Down
43 changes: 34 additions & 9 deletions pkg-manager/core/src/install/index.ts
Expand Up @@ -32,7 +32,7 @@ import {
type PatchFile,
} from '@pnpm/lockfile-file'
import { writePnpFile } from '@pnpm/lockfile-to-pnp'
import { extendProjectsWithTargetDirs } from '@pnpm/lockfile-utils'
import { extendProjectsWithTargetDirs, satisfiesPackageManifest } from '@pnpm/lockfile-utils'
import { logger, globalInfo, streamParser } from '@pnpm/logger'
import { getAllDependenciesFromManifest } from '@pnpm/manifest-utils'
import { writeModulesManifest } from '@pnpm/modules-yaml'
Expand Down Expand Up @@ -251,7 +251,7 @@ export async function mutateModules (
currentLockfile: ctx.currentLockfile,
wantedLockfile: ctx.wantedLockfile,
existsCurrentLockfile: ctx.existsCurrentLockfile,
existsWantedLockfile: ctx.existsWantedLockfile,
existsNonEmptyWantedLockfile: ctx.existsNonEmptyWantedLockfile,
lockfileDir: ctx.lockfileDir,
storeDir: ctx.storeDir,
registries: ctx.registries,
Expand Down Expand Up @@ -327,7 +327,7 @@ export async function mutateModules (
}), patchedDependencies)
: undefined
const frozenLockfile = opts.frozenLockfile ||
opts.frozenLockfileIfExists && ctx.existsWantedLockfile
opts.frozenLockfileIfExists && ctx.existsNonEmptyWantedLockfile
let outdatedLockfileSettings = false
if (!opts.ignorePackageManifest) {
const outdatedLockfileSettingName = getOutdatedLockfileSetting(ctx.wantedLockfile, {
Expand Down Expand Up @@ -375,7 +375,7 @@ export async function mutateModules (
!needsFullResolution &&
opts.preferFrozenLockfile &&
(!opts.pruneLockfileImporters || Object.keys(ctx.wantedLockfile.importers).length === Object.keys(ctx.projects).length) &&
ctx.existsWantedLockfile &&
ctx.existsNonEmptyWantedLockfile &&
(
ctx.wantedLockfile.lockfileVersion === LOCKFILE_VERSION ||
ctx.wantedLockfile.lockfileVersion === LOCKFILE_VERSION_V6
Expand All @@ -401,14 +401,39 @@ Note that in CI environments, this setting is enabled by default.`,
}
)
}
if (!opts.ignorePackageManifest) {
const _satisfiesPackageManifest = satisfiesPackageManifest.bind(null, {
autoInstallPeers: opts.autoInstallPeers,
excludeLinksFromLockfile: opts.excludeLinksFromLockfile,
})
for (const { id, manifest, rootDir } of Object.values(ctx.projects)) {
const { satisfies, detailedReason } = _satisfiesPackageManifest(ctx.wantedLockfile.importers[id], manifest)
if (!satisfies) {
if (!ctx.existsWantedLockfile) {
throw new PnpmError('NO_LOCKFILE',
`Cannot install with "frozen-lockfile" because ${WANTED_LOCKFILE} is present`, {
hint: 'Note that in CI environments this setting is true by default. If you still need to run install in such cases, use "pnpm install --no-frozen-lockfile"',
})
}
throw new PnpmError('OUTDATED_LOCKFILE',
`Cannot install with "frozen-lockfile" because ${WANTED_LOCKFILE} is not up to date with ` +
path.relative(opts.lockfileDir, path.join(rootDir, 'package.json')), {
hint: `Note that in CI environments this setting is true by default. If you still need to run install in such cases, use "pnpm install --no-frozen-lockfile"
Failure reason:
${detailedReason ?? ''}`,
})
}
}
}
if (opts.lockfileOnly) {
// The lockfile will only be changed if the workspace will have new projects with no dependencies.
await writeWantedLockfile(ctx.lockfileDir, ctx.wantedLockfile)
return {
updatedProjects: projects.map((mutatedProject) => ctx.projects[mutatedProject.rootDir]),
}
}
if (!ctx.existsWantedLockfile) {
if (!ctx.existsNonEmptyWantedLockfile) {
if (Object.values(ctx.projects).some((project) => pkgHasDependencies(project.manifest))) {
throw new Error(`Headless installation requires a ${WANTED_LOCKFILE} file`)
}
Expand Down Expand Up @@ -463,7 +488,7 @@ Note that in CI environments, this setting is enabled by default.`,
error.code !== 'ERR_PNPM_LOCKFILE_MISSING_DEPENDENCY' &&
!BROKEN_LOCKFILE_INTEGRITY_ERRORS.has(error.code)
) ||
(!ctx.existsWantedLockfile && !ctx.existsCurrentLockfile)
(!ctx.existsNonEmptyWantedLockfile && !ctx.existsCurrentLockfile)
) throw error
if (BROKEN_LOCKFILE_INTEGRITY_ERRORS.has(error.code)) {
needsFullResolution = true
Expand Down Expand Up @@ -641,12 +666,12 @@ Note that in CI environments, this setting is enabled by default.`,
// Unfortunately, the private lockfile may differ from the public one.
// A user might run named installations on a project that has a pnpm-lock.yaml file before running a noop install
const makePartialCurrentLockfile = !installsOnly && (
ctx.existsWantedLockfile && !ctx.existsCurrentLockfile ||
ctx.existsNonEmptyWantedLockfile && !ctx.existsCurrentLockfile ||
!ctx.currentLockfileIsUpToDate
)
const result = await installInContext(projectsToInstall, ctx, {
...opts,
currentLockfileIsUpToDate: !ctx.existsWantedLockfile || ctx.currentLockfileIsUpToDate,
currentLockfileIsUpToDate: !ctx.existsNonEmptyWantedLockfile || ctx.currentLockfileIsUpToDate,
makePartialCurrentLockfile,
needsFullResolution,
pruneVirtualStore,
Expand Down Expand Up @@ -1377,7 +1402,7 @@ const installInContext: InstallFunction = async (projects, ctx, opts) => {
} catch (error: any) { // eslint-disable-line
if (
!BROKEN_LOCKFILE_INTEGRITY_ERRORS.has(error.code) ||
(!ctx.existsWantedLockfile && !ctx.existsCurrentLockfile)
(!ctx.existsNonEmptyWantedLockfile && !ctx.existsCurrentLockfile)
) throw error
opts.needsFullResolution = true
// Ideally, we would not update but currently there is no other way to redownload the integrity of the package
Expand Down
2 changes: 1 addition & 1 deletion pkg-manager/core/test/install/frozenLockfile.ts
Expand Up @@ -124,7 +124,7 @@ test(`frozen-lockfile: should fail if no ${WANTED_LOCKFILE} is present`, async (
'is-positive': '^3.0.0',
},
}, await testDefaults({ frozenLockfile: true }))
).rejects.toThrow(`Headless installation requires a ${WANTED_LOCKFILE} file`)
).rejects.toThrow(`Cannot install with "frozen-lockfile" because ${WANTED_LOCKFILE} is present`)
})

test(`prefer-frozen-lockfile: should prefer headless installation when ${WANTED_LOCKFILE} satisfies package.json`, async () => {
Expand Down
2 changes: 2 additions & 0 deletions pkg-manager/get-context/src/index.ts
Expand Up @@ -34,6 +34,7 @@ export interface PnpmContext {
currentLockfileIsUpToDate: boolean
existsCurrentLockfile: boolean
existsWantedLockfile: boolean
existsNonEmptyWantedLockfile: boolean
extraBinPaths: string[]
extraNodePaths: string[]
lockfileHadConflicts: boolean
Expand Down Expand Up @@ -383,6 +384,7 @@ export interface PnpmSingleContext {
currentLockfileIsUpToDate: boolean
existsCurrentLockfile: boolean
existsWantedLockfile: boolean
existsNonEmptyWantedLockfile: boolean
extraBinPaths: string[]
extraNodePaths: string[]
lockfileHadConflicts: boolean
Expand Down
10 changes: 7 additions & 3 deletions pkg-manager/get-context/src/readLockfiles.ts
Expand Up @@ -5,7 +5,7 @@ import {
} from '@pnpm/constants'
import {
createLockfileObject,
existsWantedLockfile,
existsNonEmptyWantedLockfile,
isEmptyLockfile,
type Lockfile,
readCurrentLockfile,
Expand All @@ -21,6 +21,7 @@ export interface PnpmContext {
currentLockfile: Lockfile
existsCurrentLockfile: boolean
existsWantedLockfile: boolean
existsNonEmptyWantedLockfile: boolean
wantedLockfile: Lockfile
}

Expand All @@ -47,6 +48,7 @@ export async function readLockfiles (
currentLockfileIsUpToDate: boolean
existsCurrentLockfile: boolean
existsWantedLockfile: boolean
existsNonEmptyWantedLockfile: boolean
wantedLockfile: Lockfile
wantedLockfileIsModified: boolean
lockfileHadConflicts: boolean
Expand Down Expand Up @@ -83,7 +85,7 @@ export async function readLockfiles (
fileReads.push(readWantedLockfile(opts.lockfileDir, lockfileOpts))
}
} else {
if (await existsWantedLockfile(opts.lockfileDir, lockfileOpts)) {
if (await existsNonEmptyWantedLockfile(opts.lockfileDir, lockfileOpts)) {
logger.warn({
message: `A ${WANTED_LOCKFILE} file exists. The current configuration prohibits to read or write a lockfile`,
prefix: opts.lockfileDir,
Expand Down Expand Up @@ -131,11 +133,13 @@ export async function readLockfiles (
}
}
}
const existsWantedLockfile = files[0] != null
return {
currentLockfile,
currentLockfileIsUpToDate: equals(currentLockfile, wantedLockfile),
existsCurrentLockfile: files[1] != null,
existsWantedLockfile: files[0] != null && !isEmptyLockfile(wantedLockfile),
existsWantedLockfile,
existsNonEmptyWantedLockfile: existsWantedLockfile && !isEmptyLockfile(wantedLockfile),
wantedLockfile,
wantedLockfileIsModified,
lockfileHadConflicts,
Expand Down
22 changes: 0 additions & 22 deletions pkg-manager/headless/src/index.ts
Expand Up @@ -13,7 +13,6 @@ import {
statsLogger,
summaryLogger,
} from '@pnpm/core-loggers'
import { PnpmError } from '@pnpm/error'
import {
filterLockfileByImportersAndEngine,
} from '@pnpm/filter-lockfile'
Expand All @@ -36,7 +35,6 @@ import { writePnpFile } from '@pnpm/lockfile-to-pnp'
import {
extendProjectsWithTargetDirs,
nameVerFromPkgSnapshot,
satisfiesPackageManifest,
} from '@pnpm/lockfile-utils'
import {
type LogBase,
Expand Down Expand Up @@ -194,26 +192,6 @@ export async function headlessInstall (opts: HeadlessOptions): Promise<Installat
const publicHoistedModulesDir = rootModulesDir
const selectedProjects = Object.values(pick(opts.selectedProjectDirs, opts.allProjects))

if (!opts.ignorePackageManifest) {
const _satisfiesPackageManifest = satisfiesPackageManifest.bind(null, {
autoInstallPeers: opts.autoInstallPeers,
excludeLinksFromLockfile: opts.excludeLinksFromLockfile,
})
for (const { id, manifest, rootDir } of selectedProjects) {
const { satisfies, detailedReason } = _satisfiesPackageManifest(wantedLockfile.importers[id], manifest)
if (!satisfies) {
throw new PnpmError('OUTDATED_LOCKFILE',
`Cannot install with "frozen-lockfile" because ${WANTED_LOCKFILE} is not up to date with ` +
path.relative(lockfileDir, path.join(rootDir, 'package.json')), {
hint: `Note that in CI environments this setting is true by default. If you still need to run install in such cases, use "pnpm install --no-frozen-lockfile"
Failure reason:
${detailedReason ?? ''}`,
})
}
}
}

const scriptsOpts = {
optional: false,
extraBinPaths: opts.extraBinPaths,
Expand Down
17 changes: 0 additions & 17 deletions pkg-manager/headless/test/index.ts
Expand Up @@ -457,23 +457,6 @@ test('available packages are relinked during forced install', async () => {
})).toBeTruthy()
})

test(`fail when ${WANTED_LOCKFILE} is not up to date with package.json`, async () => {
const projectDir = tempDir()

const simpleDir = f.find('simple')
await fs.copyFile(path.join(simpleDir, 'package.json'), path.join(projectDir, 'package.json'))

const simpleWithMoreDepsDir = f.find('simple-with-more-deps')
await fs.copyFile(path.join(simpleWithMoreDepsDir, WANTED_LOCKFILE), path.join(projectDir, WANTED_LOCKFILE))

try {
await headlessInstall(await testDefaults({ lockfileDir: projectDir }))
throw new Error()
} catch (err: any) { // eslint-disable-line
expect(err.message).toBe(`Cannot install with "frozen-lockfile" because ${WANTED_LOCKFILE} is not up to date with package.json`)
}
})

test('installing local dependency', async () => {
let prefix = f.prepare('has-local-dep')
prefix = path.join(prefix, 'pkg')
Expand Down
2 changes: 1 addition & 1 deletion pnpm/test/install/hooks.ts
Expand Up @@ -646,7 +646,7 @@ test('preResolution hook', async () => {
expect(ctx.lockfileDir).toBeDefined()
expect(ctx.storeDir).toBeDefined()
expect(ctx.existsCurrentLockfile).toBe(false)
expect(ctx.existsWantedLockfile).toBe(false)
expect(ctx.existsNonEmptyWantedLockfile).toBe(false)

expect(ctx.registries).toEqual({
default: 'http://localhost:7776/',
Expand Down

0 comments on commit a53ef4d

Please sign in to comment.