mirror of
https://github.com/pnpm/pnpm.git
synced 2026-06-01 04:28:22 -04:00
fix(env-installer): suppress 'Installing config dependencies...' on no-op installs (#11766)
* fix(env-installer): only print "Installing config dependencies..." when work is actually being done Previously the message was emitted unconditionally for every config dependency, before any of the "do we need to fetch / re-symlink?" checks. As a result the banner printed on every install even when everything was already cached and correctly linked. Emit the started event lazily — at most once per install, and only when an orphan is being removed, a parent or subdep needs fetching, a parent symlink needs (re)creating, or orphan subdep siblings are being pruned. --- Written by an agent (Claude Code, claude-opus-4-7). * test(env-installer): assert installing-config-deps events fire only when work happens Captures `streamParser` events around `resolveAndInstallConfigDeps` to verify the lazy emission introduced in the previous commit: - fresh install emits both `started` and `done`, - a follow-up no-op install emits neither, - removing a config dep still emits `started` (orphan cleanup work). --- Written by an agent (Claude Code, claude-opus-4-7). * test(env-installer): subscribe to streamParser once at module load `streamParser` is a `split2` Transform stream that buffers writes until the first 'data' listener attaches and then drains the whole buffer into it. Subscribing per-test made the new install-config-deps test capture events from every earlier test in the file. Move the subscription to module load and have each test drain the accumulated events around its own call. Also drop the "removal" assertion: `resolveAndInstallConfigDeps` does not prune entries that disappear from the configDeps argument (lockfile pruning happens at a higher layer), so the scenario it claimed to test never actually fired the orphan-cleanup path. --- Written by an agent (Claude Code, claude-opus-4-7). * fix(env-installer): emit started when only the sibling symlink needs relinking If a config dep's optional subdep is already cached in the global virtual store but the sibling symlink under the parent's node_modules is missing or points at a stale target, symlinkDir() does real work without reportStarted ever firing. Check whether the link already points at the expected target and only fire reportStarted + symlinkDir when it doesn't, mirroring the parentSymlinkAlreadyCorrect path. Also clean up the test-level streamParser listener in afterAll so the subscription doesn't outlive the test file. --- Written by an agent (Claude Code, claude-opus-4-7).
This commit is contained in:
6
.changeset/quiet-config-deps.md
Normal file
6
.changeset/quiet-config-deps.md
Normal file
@@ -0,0 +1,6 @@
|
||||
---
|
||||
"@pnpm/installing.env-installer": patch
|
||||
"pnpm": patch
|
||||
---
|
||||
|
||||
Don't print "Installing config dependencies..." when config dependencies are already installed and nothing needs to be fetched, re-linked, or removed.
|
||||
@@ -39,8 +39,17 @@ export async function installConfigDeps (
|
||||
|
||||
const configModulesDir = path.join(opts.rootDir, 'node_modules/.pnpm-config')
|
||||
const existingConfigDeps: string[] = await readModulesDir(configModulesDir) ?? []
|
||||
|
||||
let startedEmitted = false
|
||||
const reportStarted = (): void => {
|
||||
if (startedEmitted) return
|
||||
startedEmitted = true
|
||||
installingConfigDepsLogger.debug({ status: 'started' })
|
||||
}
|
||||
|
||||
await Promise.all(existingConfigDeps.map(async (existingConfigDep) => {
|
||||
if (!normalizedDeps[existingConfigDep]) {
|
||||
reportStarted()
|
||||
await rimraf(path.join(configModulesDir, existingConfigDep))
|
||||
}
|
||||
}))
|
||||
@@ -68,8 +77,8 @@ export async function installConfigDeps (
|
||||
// get pruned and relinked.
|
||||
const parentSymlinkAlreadyCorrect = existingConfigDeps.includes(pkgName) &&
|
||||
await symlinkPointsTo(configDepPath, pkgDirInGlobalVirtualStore)
|
||||
installingConfigDepsLogger.debug({ status: 'started' })
|
||||
if (!fs.existsSync(path.join(pkgDirInGlobalVirtualStore, 'package.json'))) {
|
||||
reportStarted()
|
||||
const { fetching } = await opts.store.fetchPackage({
|
||||
force: true,
|
||||
lockfileDir: opts.rootDir,
|
||||
@@ -96,11 +105,13 @@ export async function installConfigDeps (
|
||||
globalVirtualStoreDir,
|
||||
rootDir: opts.rootDir,
|
||||
store: opts.store,
|
||||
reportStarted,
|
||||
})
|
||||
}
|
||||
if (parentSymlinkAlreadyCorrect) {
|
||||
return
|
||||
}
|
||||
reportStarted()
|
||||
if (existingConfigDeps.includes(pkgName)) {
|
||||
await rimraf(configDepPath)
|
||||
}
|
||||
@@ -240,6 +251,7 @@ interface InstallOptionalSubdepsOpts {
|
||||
globalVirtualStoreDir: string
|
||||
rootDir: string
|
||||
store: StoreController
|
||||
reportStarted: () => void
|
||||
}
|
||||
|
||||
async function installOptionalSubdeps (opts: InstallOptionalSubdepsOpts): Promise<void> {
|
||||
@@ -268,15 +280,18 @@ async function installOptionalSubdeps (opts: InstallOptionalSubdepsOpts): Promis
|
||||
|
||||
const expectedSiblings = new Set([opts.parentName, ...compatibleSubdeps.map((s) => s.name)])
|
||||
const existingSiblings = await readModulesDir(opts.parentNodeModulesDir) ?? []
|
||||
await Promise.all(existingSiblings
|
||||
.filter((name) => !expectedSiblings.has(name))
|
||||
.map((name) => rimraf(path.join(opts.parentNodeModulesDir, name))))
|
||||
const orphanSiblings = existingSiblings.filter((name) => !expectedSiblings.has(name))
|
||||
if (orphanSiblings.length > 0) {
|
||||
opts.reportStarted()
|
||||
}
|
||||
await Promise.all(orphanSiblings.map((name) => rimraf(path.join(opts.parentNodeModulesDir, name))))
|
||||
|
||||
await Promise.all(compatibleSubdeps.map(async (subdep) => {
|
||||
const subdepFullPkgId = `${subdep.name}@${subdep.version}:${subdep.resolution.integrity}`
|
||||
const subdepRelPath = calcLeafGlobalVirtualStorePath(subdepFullPkgId, subdep.name, subdep.version)
|
||||
const subdepDirInGlobalVirtualStore = path.join(opts.globalVirtualStoreDir, subdepRelPath, 'node_modules', subdep.name)
|
||||
if (!fs.existsSync(path.join(subdepDirInGlobalVirtualStore, 'package.json'))) {
|
||||
opts.reportStarted()
|
||||
const { fetching } = await opts.store.fetchPackage({
|
||||
force: true,
|
||||
lockfileDir: opts.rootDir,
|
||||
@@ -293,6 +308,10 @@ async function installOptionalSubdeps (opts: InstallOptionalSubdepsOpts): Promis
|
||||
})
|
||||
}
|
||||
const linkPath = path.join(opts.parentNodeModulesDir, subdep.name)
|
||||
if (await symlinkPointsTo(linkPath, subdepDirInGlobalVirtualStore)) {
|
||||
return
|
||||
}
|
||||
opts.reportStarted()
|
||||
await fs.promises.mkdir(path.dirname(linkPath), { recursive: true })
|
||||
await symlinkDir(subdepDirInGlobalVirtualStore, linkPath)
|
||||
}))
|
||||
|
||||
@@ -1,8 +1,9 @@
|
||||
import path from 'node:path'
|
||||
|
||||
import { expect, test } from '@jest/globals'
|
||||
import { afterAll, expect, test } from '@jest/globals'
|
||||
import { resolveAndInstallConfigDeps } from '@pnpm/installing.env-installer'
|
||||
import { createEnvLockfile, readEnvLockfile, writeEnvLockfile } from '@pnpm/lockfile.fs'
|
||||
import { type LogBase, streamParser } from '@pnpm/logger'
|
||||
import { prepareEmpty } from '@pnpm/prepare'
|
||||
import { getIntegrity, REGISTRY_MOCK_PORT } from '@pnpm/registry-mock'
|
||||
import { createTempStore } from '@pnpm/testing.temp-store'
|
||||
@@ -22,6 +23,28 @@ function createOpts () {
|
||||
}
|
||||
}
|
||||
|
||||
interface InstallingConfigDepsEvent { status: string, deps?: Array<{ name: string, version: string }> }
|
||||
|
||||
// `streamParser` is a `split2` Transform stream that buffers writes until the
|
||||
// first 'data' listener attaches, then drains the whole buffer into it.
|
||||
// Subscribing per-test would therefore replay events from earlier tests into
|
||||
// the current test's listener. Subscribe once at module load and let each test
|
||||
// take only the events accumulated since its last drain.
|
||||
const accumulatedConfigDepEvents: InstallingConfigDepsEvent[] = []
|
||||
const configDepsListener = (msg: LogBase): void => {
|
||||
const log = msg as { name?: string, status?: string, deps?: Array<{ name: string, version: string }> }
|
||||
if (log.name !== 'pnpm:installing-config-deps' || log.status == null) return
|
||||
accumulatedConfigDepEvents.push({ status: log.status, deps: log.deps })
|
||||
}
|
||||
streamParser.on('data', configDepsListener)
|
||||
afterAll(() => {
|
||||
streamParser.removeListener('data', configDepsListener)
|
||||
})
|
||||
|
||||
function takeConfigDepEvents (): InstallingConfigDepsEvent[] {
|
||||
return accumulatedConfigDepEvents.splice(0, accumulatedConfigDepEvents.length)
|
||||
}
|
||||
|
||||
test('resolves and installs config dep when no env lockfile exists', async () => {
|
||||
prepareEmpty()
|
||||
const opts = createOpts()
|
||||
@@ -222,6 +245,29 @@ test('fails with frozenLockfile when new-format deps need resolution', async ()
|
||||
}, { ...opts, frozenLockfile: true })).rejects.toThrow('Cannot update configDependencies with "frozen-lockfile"')
|
||||
})
|
||||
|
||||
test('emits installing-config-deps events only when work is needed', async () => {
|
||||
prepareEmpty()
|
||||
const opts = createOpts()
|
||||
|
||||
takeConfigDepEvents()
|
||||
await resolveAndInstallConfigDeps({
|
||||
'@pnpm.e2e/foo': '100.0.0',
|
||||
}, opts)
|
||||
const firstRunEvents = takeConfigDepEvents()
|
||||
|
||||
expect(firstRunEvents.map(e => e.status)).toEqual(['started', 'done'])
|
||||
expect(firstRunEvents.find(e => e.status === 'done')?.deps).toEqual([
|
||||
{ name: '@pnpm.e2e/foo', version: '100.0.0' },
|
||||
])
|
||||
|
||||
await resolveAndInstallConfigDeps({
|
||||
'@pnpm.e2e/foo': '100.0.0',
|
||||
}, opts)
|
||||
const secondRunEvents = takeConfigDepEvents()
|
||||
|
||||
expect(secondRunEvents).toStrictEqual([])
|
||||
})
|
||||
|
||||
test('succeeds with frozenLockfile when env lockfile is up-to-date', async () => {
|
||||
prepareEmpty()
|
||||
const opts = createOpts()
|
||||
|
||||
Reference in New Issue
Block a user