fix: a filtered install should update the lockfile first (#8183)

close #8165

.changeset/early-islands-chew.md

@@ -0,0 +1,13 @@
+---
+"@pnpm/plugin-commands-installation": patch
+"@pnpm/plugin-commands-script-runners": patch
+"@pnpm/read-projects-context": patch
+"@pnpm/plugin-commands-rebuild": patch
+"@pnpm/get-context": patch
+"@pnpm/fs.find-packages": patch
+"@pnpm/core": patch
+"@pnpm/types": patch
+"pnpm": patch
+---
+
+If install is performed on a subset of workspace projects, always create an up-to-date lockfile first. So, a partial install can be performed only on a fully resolved (non-partial) lockfile [#8165](https://github.com/pnpm/pnpm/issues/8165).

exec/plugin-commands-rebuild/src/recursive.ts

@@ -51,7 +51,7 @@ export async function recursiveRebuild (
   if (pkgs.length === 0) {
     return
   }
-  const manifestsByPath: { [dir: string]: Omit<Project, 'dir'> } = {}
+  const manifestsByPath: { [dir: string]: Omit<Project, 'dir' | 'dirRealPath'> } = {}
   for (const { dir, manifest, writeProjectManifest } of pkgs) {
     manifestsByPath[dir] = { manifest, writeProjectManifest }
   }

exec/plugin-commands-script-runners/src/exec.ts

@@ -163,6 +163,7 @@ export async function handler (
           package: {
             ...project,
             dir: opts.dir,
+            dirRealPath: opts.dir,
           } as Project,
         },
       }

exec/plugin-commands-script-runners/test/exec.e2e.ts

@@ -445,6 +445,7 @@ test('pnpm exec shell mode', async () => {
         dependencies: [],
         package: {
           dir: process.cwd(),
+          dirRealPath: process.cwd(),
           writeProjectManifest: async () => {},
           manifest: {
             name: 'test_shell_mode',

fs/find-packages/src/index.ts

@@ -1,3 +1,4 @@
+import { promises as fs } from 'fs'
 import path from 'path'
 import util from 'util'
 import { readExactProjectManifest } from '@pnpm/read-project-manifest'
@@ -21,6 +22,7 @@ export interface Options {
 
 export interface Project {
   dir: string
+  dirRealPath: string
   manifest: ProjectManifest
 
   writeProjectManifest: (manifest: ProjectManifest, force?: boolean | undefined) => Promise<void>
@@ -55,8 +57,10 @@ export async function findPackages (root: string, opts?: Options): Promise<Proje
       ),
       async manifestPath => {
         try {
+          const dir = path.dirname(manifestPath)
           return {
-            dir: path.dirname(manifestPath),
+            dir,
+            dirRealPath: await fs.realpath(dir),
             ...await readExactProjectManifest(manifestPath),
           } as Project
         } catch (err: unknown) {

packages/types/src/project.ts

@@ -2,6 +2,7 @@ import { type ProjectManifest } from './package'
 
 export interface Project {
   dir: string
+  dirRealPath: string
   manifest: ProjectManifest
   writeProjectManifest: (manifest: ProjectManifest, force?: boolean | undefined) => Promise<void>
 }

pkg-manager/core/package.json

@@ -60,6 +60,7 @@
     "@pnpm/which-version-is-pinned": "workspace:*",
     "@zkochan/rimraf": "^2.1.3",
     "is-inner-link": "^4.0.0",
+    "is-subdir": "^1.2.0",
     "load-json-file": "^6.2.0",
     "normalize-path": "^3.0.0",
     "p-every": "^2.0.0",

pkg-manager/core/src/install/allProjectsAreUpToDate.ts

@@ -14,8 +14,8 @@ import {
   DEPENDENCIES_FIELDS,
   DEPENDENCIES_OR_PEER_FIELDS,
   type DependencyManifest,
-  type ProjectManifest,
   type ProjectId,
+  type ProjectManifest,
 } from '@pnpm/types'
 import pEvery from 'p-every'
 import any from 'ramda/src/any'
@@ -23,7 +23,7 @@ import semver from 'semver'
 import getVersionSelectorType from 'version-selector-type'
 
 export async function allProjectsAreUpToDate (
-  projects: Array<ProjectOptions & { id: ProjectId }>,
+  projects: Array<Pick<ProjectOptions, 'manifest' | 'rootDir'> & { id: ProjectId }>,
   opts: {
     autoInstallPeers: boolean
     excludeLinksFromLockfile: boolean

pkg-manager/core/src/install/index.ts

@@ -65,6 +65,7 @@ import {
 } from '@pnpm/types'
 import rimraf from '@zkochan/rimraf'
 import isInnerLink from 'is-inner-link'
+import isSubdir from 'is-subdir'
 import pFilter from 'p-filter'
 import pLimit from 'p-limit'
 import pMapValues from 'p-map-values'
@@ -1404,6 +1405,72 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
 
 const installInContext: InstallFunction = async (projects, ctx, opts) => {
   try {
+    const isPathInsideWorkspace = isSubdir.bind(null, opts.lockfileDir)
+    if (!opts.frozenLockfile && opts.useLockfile) {
+      const allProjectsLocatedInsideWorkspace = Object.values(ctx.projects)
+        .filter((project) => isPathInsideWorkspace(project.rootDirRealPath ?? project.rootDir))
+      if (allProjectsLocatedInsideWorkspace.length > projects.length) {
+        if (
+          await allProjectsAreUpToDate(allProjectsLocatedInsideWorkspace, {
+            autoInstallPeers: opts.autoInstallPeers,
+            excludeLinksFromLockfile: opts.excludeLinksFromLockfile,
+            linkWorkspacePackages: opts.linkWorkspacePackagesDepth >= 0,
+            wantedLockfile: ctx.wantedLockfile,
+            workspacePackages: opts.workspacePackages,
+            lockfileDir: opts.lockfileDir,
+          })
+        ) {
+          return installInContext(projects, ctx, {
+            ...opts,
+            frozenLockfile: true,
+          })
+        } else {
+          const newProjects = [...projects]
+          const getWantedDepsOpts = {
+            autoInstallPeers: opts.autoInstallPeers,
+            includeDirect: opts.includeDirect,
+            updateWorkspaceDependencies: false,
+            nodeExecPath: opts.nodeExecPath,
+          }
+          for (const project of allProjectsLocatedInsideWorkspace) {
+            if (!newProjects.some(({ rootDir }) => rootDir === project.rootDir)) {
+              const wantedDependencies = getWantedDependencies(project.manifest, getWantedDepsOpts)
+                .map((wantedDependency) => ({ ...wantedDependency, updateSpec: true, preserveNonSemverVersionSpec: true }))
+              newProjects.push({
+                mutation: 'install',
+                ...project,
+                wantedDependencies,
+                pruneDirectDependencies: false,
+                updatePackageManifest: false,
+              })
+            }
+          }
+          const result = await installInContext(newProjects, ctx, {
+            ...opts,
+            lockfileOnly: true,
+          })
+          const { stats } = await headlessInstall({
+            ...ctx,
+            ...opts,
+            currentEngine: {
+              nodeVersion: opts.nodeVersion,
+              pnpmVersion: opts.packageManager.name === 'pnpm' ? opts.packageManager.version : '',
+            },
+            currentHoistedLocations: ctx.modulesFile?.hoistedLocations,
+            selectedProjectDirs: projects.map((project) => project.rootDir),
+            allProjects: ctx.projects,
+            prunedAt: ctx.modulesFile?.prunedAt,
+            wantedLockfile: result.newLockfile,
+            useLockfile: opts.useLockfile && ctx.wantedLockfileIsModified,
+            hoistWorkspacePackages: opts.hoistWorkspacePackages,
+          })
+          return {
+            ...result,
+            stats,
+          }
+        }
+      }
+    }
     if (opts.nodeLinker === 'hoisted' && !opts.lockfileOnly) {
       const result = await _installInContext(projects, ctx, {
         ...opts,

pkg-manager/core/test/allProjectsAreUpToDate.test.ts

@@ -21,7 +21,6 @@ const workspacePackages = {
 test('allProjectsAreUpToDate(): works with packages linked through the workspace protocol using relative path', async () => {
   expect(await allProjectsAreUpToDate([
     {
-      buildIndex: 0,
       id: 'bar' as ProjectId,
       manifest: {
         dependencies: {
@@ -31,7 +30,6 @@ test('allProjectsAreUpToDate(): works with packages linked through the workspace
       rootDir: 'bar',
     },
     {
-      buildIndex: 0,
       id: 'foo' as ProjectId,
       manifest: fooManifest,
       rootDir: 'foo',
@@ -64,7 +62,6 @@ test('allProjectsAreUpToDate(): works with packages linked through the workspace
 test('allProjectsAreUpToDate(): works with aliased local dependencies', async () => {
   expect(await allProjectsAreUpToDate([
     {
-      buildIndex: 0,
       id: 'bar' as ProjectId,
       manifest: {
         dependencies: {
@@ -74,7 +71,6 @@ test('allProjectsAreUpToDate(): works with aliased local dependencies', async ()
       rootDir: 'bar',
     },
     {
-      buildIndex: 0,
       id: 'foo' as ProjectId,
       manifest: fooManifest,
       rootDir: 'foo',
@@ -107,7 +103,6 @@ test('allProjectsAreUpToDate(): works with aliased local dependencies', async ()
 test('allProjectsAreUpToDate(): works with aliased local dependencies that specify versions', async () => {
   expect(await allProjectsAreUpToDate([
     {
-      buildIndex: 0,
       id: 'bar' as ProjectId,
       manifest: {
         dependencies: {
@@ -117,7 +112,6 @@ test('allProjectsAreUpToDate(): works with aliased local dependencies that speci
       rootDir: 'bar',
     },
     {
-      buildIndex: 0,
       id: 'foo' as ProjectId,
       manifest: fooManifest,
       rootDir: 'foo',
@@ -150,7 +144,6 @@ test('allProjectsAreUpToDate(): works with aliased local dependencies that speci
 test('allProjectsAreUpToDate(): returns false if the aliased dependency version is out of date', async () => {
   expect(await allProjectsAreUpToDate([
     {
-      buildIndex: 0,
       id: 'bar' as ProjectId,
       manifest: {
         dependencies: {
@@ -160,7 +153,6 @@ test('allProjectsAreUpToDate(): returns false if the aliased dependency version
       rootDir: 'bar',
     },
     {
-      buildIndex: 0,
       id: 'foo' as ProjectId,
       manifest: fooManifest,
       rootDir: 'foo',
@@ -195,7 +187,6 @@ test('allProjectsAreUpToDate(): use link and registry version if linkWorkspacePa
     await allProjectsAreUpToDate(
       [
         {
-          buildIndex: 0,
           id: 'bar' as ProjectId,
           manifest: {
             dependencies: {
@@ -207,7 +198,6 @@ test('allProjectsAreUpToDate(): use link and registry version if linkWorkspacePa
           rootDir: 'bar',
         },
         {
-          buildIndex: 0,
           id: 'bar2' as ProjectId,
           manifest: {
             dependencies: {
@@ -217,13 +207,11 @@ test('allProjectsAreUpToDate(): use link and registry version if linkWorkspacePa
           rootDir: 'bar2',
         },
         {
-          buildIndex: 0,
           id: 'foo' as ProjectId,
           manifest: fooManifest,
           rootDir: 'foo',
         },
         {
-          buildIndex: 0,
           id: 'foo2' as ProjectId,
           manifest: {
             name: 'foo2',
@@ -232,7 +220,6 @@ test('allProjectsAreUpToDate(): use link and registry version if linkWorkspacePa
           rootDir: 'foo2',
         },
         {
-          buildIndex: 0,
           id: 'foo3' as ProjectId,
           manifest: {
             name: 'foo3',
@@ -289,7 +276,6 @@ test('allProjectsAreUpToDate(): use link and registry version if linkWorkspacePa
 test('allProjectsAreUpToDate(): returns false if dependenciesMeta differs', async () => {
   expect(await allProjectsAreUpToDate([
     {
-      buildIndex: 0,
       id: 'bar' as ProjectId,
       manifest: {
         dependencies: {
@@ -304,7 +290,6 @@ test('allProjectsAreUpToDate(): returns false if dependenciesMeta differs', asyn
       rootDir: 'bar',
     },
     {
-      buildIndex: 0,
       id: 'foo' as ProjectId,
       manifest: fooManifest,
       rootDir: 'foo',
@@ -337,7 +322,6 @@ test('allProjectsAreUpToDate(): returns false if dependenciesMeta differs', asyn
 test('allProjectsAreUpToDate(): returns true if dependenciesMeta matches', async () => {
   expect(await allProjectsAreUpToDate([
     {
-      buildIndex: 0,
       id: 'bar' as ProjectId,
       manifest: {
         dependencies: {
@@ -352,7 +336,6 @@ test('allProjectsAreUpToDate(): returns true if dependenciesMeta matches', async
       rootDir: 'bar',
     },
     {
-      buildIndex: 0,
       id: 'foo' as ProjectId,
       manifest: fooManifest,
       rootDir: 'foo',
@@ -401,7 +384,6 @@ describe('local file dependency', () => {
   })
   const projects = [
     {
-      buildIndex: 0,
       id: 'bar' as ProjectId,
       manifest: {
         dependencies: {
@@ -411,7 +393,6 @@ describe('local file dependency', () => {
       rootDir: 'bar',
     },
     {
-      buildIndex: 0,
       id: 'foo' as ProjectId,
       manifest: fooManifest,
       rootDir: 'foo',
@@ -502,7 +483,6 @@ describe('local file dependency', () => {
 test('allProjectsAreUpToDate(): returns true if workspace dependency\'s version type is tag', async () => {
   const projects = [
     {
-      buildIndex: 0,
       id: 'bar' as ProjectId,
       manifest: {
         dependencies: {
@@ -512,7 +492,6 @@ test('allProjectsAreUpToDate(): returns true if workspace dependency\'s version
       rootDir: 'bar',
     },
     {
-      buildIndex: 0,
       id: 'foo' as ProjectId,
       manifest: fooManifest,
       rootDir: 'foo',

pkg-manager/core/test/install/multipleImporters.ts

@@ -1,11 +1,11 @@
 import fs from 'fs'
 import path from 'path'
 import { assertProject } from '@pnpm/assert-project'
-import { LOCKFILE_VERSION } from '@pnpm/constants'
+import { LOCKFILE_VERSION, WANTED_LOCKFILE } from '@pnpm/constants'
 import { readCurrentLockfile } from '@pnpm/lockfile-file'
 import { prepareEmpty, preparePackages } from '@pnpm/prepare'
 import { addDistTag } from '@pnpm/registry-mock'
-import { type ProjectManifest } from '@pnpm/types'
+import { type ProjectManifest, type ProjectId } from '@pnpm/types'
 import {
   addDependenciesToPackage,
   type MutatedProject,
@@ -16,6 +16,7 @@ import { sync as rimraf } from '@zkochan/rimraf'
 import { createPeersDirSuffix } from '@pnpm/dependency-path'
 import loadJsonFile from 'load-json-file'
 import pick from 'ramda/src/pick'
+import { sync as readYamlFile } from 'read-yaml-file'
 import sinon from 'sinon'
 import { sync as writeYamlFile } from 'write-yaml-file'
 import { testDefaults } from '../utils'
@@ -68,7 +69,6 @@ test('install only the dependencies of the specified importer', async () => {
       rootDir: path.resolve('project-2'),
     },
   ]
-  await mutateModules(importers, testDefaults({ allProjects, lockfileOnly: true }))
 
   await mutateModules(importers.slice(0, 1), testDefaults({ allProjects }))
 
@@ -78,6 +78,130 @@ test('install only the dependencies of the specified importer', async () => {
   const rootModules = assertProject(process.cwd())
   rootModules.has('.pnpm/is-positive@1.0.0')
   rootModules.hasNot('.pnpm/is-negative@1.0.0')
+
+  const lockfile: any = readYamlFile(WANTED_LOCKFILE) // eslint-disable-line
+  expect(lockfile.importers?.['project-2' as ProjectId].dependencies?.['is-negative'].version).toBe('1.0.0')
+})
+
+test('install only the dependencies of the specified importer, when node-linker is hoisted', async () => {
+  preparePackages([
+    {
+      location: 'project-1',
+      package: { name: 'project-1' },
+    },
+    {
+      location: 'project-2',
+      package: { name: 'project-2' },
+    },
+  ])
+
+  const importers: MutatedProject[] = [
+    {
+      mutation: 'install',
+      rootDir: path.resolve('project-1'),
+    },
+    {
+      mutation: 'install',
+      rootDir: path.resolve('project-2'),
+    },
+  ]
+  const allProjects = [
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project-1',
+        version: '1.0.0',
+
+        dependencies: {
+          'is-positive': '1.0.0',
+        },
+      },
+      rootDir: path.resolve('project-1'),
+    },
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project-2',
+        version: '1.0.0',
+
+        dependencies: {
+          'is-negative': '1.0.0',
+        },
+      },
+      rootDir: path.resolve('project-2'),
+    },
+  ]
+
+  await mutateModules(importers.slice(0, 1), testDefaults({ allProjects, nodeLinker: 'hoisted' }))
+
+  const rootModules = assertProject(process.cwd())
+  rootModules.has('is-positive')
+  // rootModules.hasNot('is-negative') // TODO: fix
+
+  const lockfile: any = readYamlFile(WANTED_LOCKFILE) // eslint-disable-line
+  expect(lockfile.importers?.['project-2' as ProjectId].dependencies?.['is-negative'].version).toBe('1.0.0')
+})
+
+test('install only the dependencies of the specified importer, when no lockfile is used', async () => {
+  const projects = preparePackages([
+    {
+      location: 'project-1',
+      package: { name: 'project-1' },
+    },
+    {
+      location: 'project-2',
+      package: { name: 'project-2' },
+    },
+  ])
+
+  const importers: MutatedProject[] = [
+    {
+      mutation: 'install',
+      rootDir: path.resolve('project-1'),
+    },
+    {
+      mutation: 'install',
+      rootDir: path.resolve('project-2'),
+    },
+  ]
+  const allProjects = [
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project-1',
+        version: '1.0.0',
+
+        dependencies: {
+          'is-positive': '1.0.0',
+        },
+      },
+      rootDir: path.resolve('project-1'),
+    },
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project-2',
+        version: '1.0.0',
+
+        dependencies: {
+          'is-negative': '1.0.0',
+        },
+      },
+      rootDir: path.resolve('project-2'),
+    },
+  ]
+
+  await mutateModules(importers.slice(0, 1), testDefaults({ allProjects, useLockfile: false }))
+
+  projects['project-1'].has('is-positive')
+  projects['project-2'].hasNot('is-negative')
+
+  const rootModules = assertProject(process.cwd())
+  rootModules.has('.pnpm/is-positive@1.0.0')
+  rootModules.hasNot('.pnpm/is-negative@1.0.0')
+
+  const lockfile: any = readYamlFile(path.resolve('node_modules/.pnpm/lock.yaml')) // eslint-disable-line
+  expect(lockfile.importers?.['project-2' as ProjectId]).toStrictEqual({})
 })
 
 test('install only the dependencies of the specified importer. The current lockfile has importers that do not exist anymore', async () => {

pkg-manager/get-context/src/index.ts

@@ -68,6 +68,7 @@ export interface ProjectOptions {
   manifest: ProjectManifest
   modulesDir?: string
   rootDir: string
+  rootDirRealPath?: string
 }
 
 interface HookOptions {

pkg-manager/plugin-commands-installation/src/recursive.ts

@@ -169,7 +169,7 @@ export async function recursive (
     let importers = getImporters(opts)
     const calculatedRepositoryRoot = await fs.realpath(calculateRepositoryRoot(opts.workspaceDir, importers.map(x => x.rootDir)))
     const isFromWorkspace = isSubdir.bind(null, calculatedRepositoryRoot)
-    importers = await pFilter(importers, async ({ rootDir }: { rootDir: string }) => isFromWorkspace(await fs.realpath(rootDir)))
+    importers = await pFilter(importers, async ({ rootDirRealPath }) => isFromWorkspace(rootDirRealPath))
     if (importers.length === 0) return true
     let mutation!: string
     switch (cmdFullName) {
@@ -519,22 +519,23 @@ function getAllProjects (manifestsByPath: ManifestsByPath, allProjectsGraph: Pro
     buildIndex,
     manifest: manifestsByPath[rootDir].manifest,
     rootDir,
+    rootDirRealPath: allProjectsGraph[rootDir].package.dirRealPath,
   }))).flat()
 }
 
-interface ManifestsByPath { [dir: string]: Omit<Project, 'dir'> }
+interface ManifestsByPath { [dir: string]: Omit<Project, 'dir' | 'dirRealPath'> }
 
-function getManifestsByPath (projects: Project[]): Record<string, Omit<Project, 'dir'>> {
+function getManifestsByPath (projects: Project[]): Record<string, Omit<Project, 'dir' | 'dirRealPath'>> {
   return projects.reduce((manifestsByPath, { dir, manifest, writeProjectManifest }) => {
     manifestsByPath[dir] = { manifest, writeProjectManifest }
     return manifestsByPath
-  }, {} as Record<string, Omit<Project, 'dir'>>)
+  }, {} as Record<string, Omit<Project, 'dir' | 'dirRealPath'>>)
 }
 
-function getImporters (opts: Pick<RecursiveOptions, 'selectedProjectsGraph' | 'ignoredPackages'>): Array<{ rootDir: string }> {
+function getImporters (opts: Pick<RecursiveOptions, 'selectedProjectsGraph' | 'ignoredPackages'>): Array<{ rootDir: string, rootDirRealPath: string }> {
   let rootDirs = Object.keys(opts.selectedProjectsGraph)
   if (opts.ignoredPackages != null) {
     rootDirs = rootDirs.filter((rootDir) => !opts.ignoredPackages!.has(rootDir))
   }
-  return rootDirs.map((rootDir) => ({ rootDir }))
+  return rootDirs.map((rootDir) => ({ rootDir, rootDirRealPath: opts.selectedProjectsGraph[rootDir].package.dirRealPath }))
 }

pkg-manager/read-projects-context/src/index.ts

@@ -1,3 +1,5 @@
+import { promises as fs } from 'fs'
+import util from 'util'
 import path from 'path'
 import { getLockfileImporterId } from '@pnpm/lockfile-file'
 import { type Modules, readModulesManifest } from '@pnpm/modules-yaml'
@@ -9,6 +11,7 @@ export interface ProjectOptions {
   binsDir?: string
   modulesDir?: string
   rootDir: string
+  rootDirRealPath?: string
 }
 
 export async function readProjectsContext<T> (
@@ -54,6 +57,7 @@ export async function readProjectsContext<T> (
           binsDir: project.binsDir ?? path.join(project.rootDir, relativeModulesDir, '.bin'),
           id: importerId,
           modulesDir,
+          rootDirRealPath: project.rootDirRealPath ?? await realpath(project.rootDir),
         }
       })),
     registries: ((modules?.registries) != null) ? normalizeRegistries(modules.registries) : undefined,
@@ -62,3 +66,14 @@ export async function readProjectsContext<T> (
     virtualStoreDirMaxLength: modules?.virtualStoreDirMaxLength,
   }
 }
+
+async function realpath (path: string): Promise<string> {
+  try {
+    return await fs.realpath(path)
+  } catch (err: unknown) {
+    if (util.types.isNativeError(err) && 'code' in err && err.code === 'ENOENT') {
+      return path
+    }
+    throw err
+  }
+}

pnpm-lock.yaml

@@ -3204,6 +3204,9 @@ importers:
       is-inner-link:
         specifier: ^4.0.0
         version: 4.0.0
+      is-subdir:
+        specifier: ^1.2.0
+        version: 1.2.0
       load-json-file:
         specifier: ^6.2.0
         version: 6.2.0