perf: skip redundant GVS internal linking on warm reinstall (#11073)

* perf: skip redundant GVS internal linking on warm reinstall

When GVS is enabled and the store is warm (added === 0), skip
re-creating internal symlinks, re-linking bins inside the GVS store,
and re-importing packages since they already persist outside
node_modules/. Also filter directPkgDirs by hasBin to avoid
unnecessary package.json reads when linking direct dep bins.

* fix: preserve link: deps in hasBin filter for bin linking

The hasBin filter was dropping directories not present in the dep graph
(e.g. link: dependencies), which would silently break bin linking for
linked local packages that expose binaries.

---------

Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/skip-gvs-warm-reinstall.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/installing.deps-restorer": patch
+"pnpm": patch
+---
+
+Skip redundant internal linking during GVS warm reinstall when no packages were added. Also filter direct dependency directories by `hasBin` before reading manifests to avoid unnecessary package.json reads.

installing/deps-restorer/src/index.ts

@@ -387,6 +387,7 @@ export async function headlessInstall (opts: HeadlessOptions): Promise<Installat
   const depNodes = Object.values(graph)
 
   const added = depNodes.filter(({ fetching }) => fetching).length
+  const skipGvsInternalLinking = opts.enableGlobalVirtualStore === true && added === 0
   statsLogger.debug({
     added,
     prefix: lockfileDir,
@@ -429,28 +430,30 @@ export async function headlessInstall (opts: HeadlessOptions): Promise<Installat
       })
     }
   } else if (opts.enableModulesDir !== false || opts.enableGlobalVirtualStore) {
-    if (opts.enableModulesDir !== false) {
-      await Promise.all(depNodes.map(async (depNode) => fs.mkdir(depNode.modules, { recursive: true })))
-    }
-    await Promise.all([
-      opts.symlink === false || opts.enableModulesDir === false
-        ? Promise.resolve()
-        : linkAllModules(depNodes, {
-          optional: opts.include.optionalDependencies,
+    if (!skipGvsInternalLinking) {
+      if (opts.enableModulesDir !== false) {
+        await Promise.all(depNodes.map(async (depNode) => fs.mkdir(depNode.modules, { recursive: true })))
+      }
+      await Promise.all([
+        opts.symlink === false || opts.enableModulesDir === false
+          ? Promise.resolve()
+          : linkAllModules(depNodes, {
+            optional: opts.include.optionalDependencies,
+          }),
+        linkAllPkgs(opts.storeController, depNodes, {
+          allowBuild,
+          force: opts.force,
+          disableRelinkLocalDirDeps: opts.disableRelinkLocalDirDeps,
+          depGraph: graph,
+          depsStateCache,
+          enableGlobalVirtualStore: opts.enableGlobalVirtualStore,
+          ignoreScripts: opts.ignoreScripts,
+          lockfileDir: opts.lockfileDir,
+          sideEffectsCacheRead: opts.sideEffectsCacheRead,
+          storeDir: opts.storeDir,
         }),
-      linkAllPkgs(opts.storeController, depNodes, {
-        allowBuild,
-        force: opts.force,
-        disableRelinkLocalDirDeps: opts.disableRelinkLocalDirDeps,
-        depGraph: graph,
-        depsStateCache,
-        enableGlobalVirtualStore: opts.enableGlobalVirtualStore,
-        ignoreScripts: opts.ignoreScripts,
-        lockfileDir: opts.lockfileDir,
-        sideEffectsCacheRead: opts.sideEffectsCacheRead,
-        storeDir: opts.storeDir,
-      }),
-    ])
+      ])
+    }
 
     stageLogger.debug({
       prefix: lockfileDir,
@@ -493,7 +496,7 @@ export async function headlessInstall (opts: HeadlessOptions): Promise<Installat
       newHoistedDependencies = {}
     }
 
-    if (!skipPostImportLinking) {
+    if (!skipPostImportLinking && !skipGvsInternalLinking) {
       await linkAllBins(graph, {
         extraNodePaths: opts.extraNodePaths,
         optional: opts.include.optionalDependencies,
@@ -618,6 +621,9 @@ export async function headlessInstall (opts: HeadlessOptions): Promise<Installat
                 }
               }
             }
+            // Skip packages without bins to avoid unnecessary manifest reads.
+            // Dirs not in graph (e.g. link: deps) are kept since they may expose bins.
+            directPkgDirs = directPkgDirs.filter((dir) => graph[dir] == null || graph[dir].hasBin)
             await linkBinsOfPackages(
               (
                 await Promise.all(

pnpm/test/install/globalVirtualStore.ts

@@ -75,3 +75,34 @@ test('approve-builds updates GVS symlinks and runs builds at correct hash direct
   const workspaceManifest = readYamlFileSync<any>(path.resolve('pnpm-workspace.yaml')) // eslint-disable-line
   expect(workspaceManifest.allowBuilds?.['@pnpm.e2e/pre-and-postinstall-scripts-example']).toBe(true)
 })
+
+test('warm GVS reinstall skips internal linking', async () => {
+  prepare({
+    dependencies: {
+      '@pnpm.e2e/hello-world-js-bin': '*',
+      '@pnpm.e2e/pkg-with-1-dep': '100.0.0',
+    },
+  })
+  const storeDir = path.resolve('store')
+  writeYamlFileSync(path.resolve('pnpm-workspace.yaml'), {
+    enableGlobalVirtualStore: true,
+    storeDir,
+    privateHoistPattern: '*',
+  })
+
+  // First install — warms GVS store
+  await execPnpm(['install'])
+
+  // Delete node_modules
+  fs.rmSync(path.resolve('node_modules'), { recursive: true })
+
+  // Reinstall with frozen lockfile — should skip internal GVS linking
+  await execPnpm(['install', '--frozen-lockfile'])
+
+  // Verify structure is correct after warm reinstall
+  expect(fs.existsSync(path.resolve('node_modules/@pnpm.e2e/hello-world-js-bin/package.json'))).toBeTruthy()
+  expect(fs.existsSync(path.resolve('node_modules/@pnpm.e2e/pkg-with-1-dep/package.json'))).toBeTruthy()
+  expect(fs.existsSync(path.resolve('node_modules/.pnpm/node_modules/@pnpm.e2e/dep-of-pkg-with-1-dep'))).toBeTruthy()
+  expect(fs.existsSync(path.resolve('node_modules/.bin/hello-world-js-bin'))).toBeTruthy()
+  expect(fs.existsSync(path.resolve('node_modules/.pnpm/lock.yaml'))).toBeTruthy()
+})