Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: don't print that lockfile is up to date when it is not #6574

Merged
merged 3 commits into from
May 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/curly-starfishes-end.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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