fix: global linking (#4641)

.changeset/pink-llamas-joke.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/plugin-commands-installation": patch
+---
+
+Improve global linking.

.changeset/soft-crews-nail.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/core": major
+---
+
+Remove linkFromGlobal and linkToGlobal.

packages/core/package.json

@@ -58,7 +58,6 @@
     "p-every": "^2.0.0",
     "p-filter": "^2.1.0",
     "p-limit": "^3.1.0",
-    "path-absolute": "^1.0.1",
     "path-exists": "^4.0.0",
     "ramda": "^0.27.1",
     "run-groups": "^3.0.1",

packages/core/src/install/index.ts

@@ -576,6 +576,7 @@ export async function addDependenciesToPackage (
   manifest: ProjectManifest,
   dependencySelectors: string[],
   opts: InstallOptions & {
+    bin?: string
     allowNew?: boolean
     peer?: boolean
     pinnedVersion?: 'major' | 'minor' | 'patch'
@@ -585,6 +586,7 @@ export async function addDependenciesToPackage (
   const projects = await mutateModules(
     [
       {
+        binsDir: opts.bin,
         allowNew: opts.allowNew,
         dependencySelectors,
         manifest,

packages/core/src/link/index.ts

@@ -19,7 +19,6 @@ import {
   PackageSpecObject,
   updateProjectManifestObject,
 } from '@pnpm/manifest-utils'
-import { prune } from '@pnpm/modules-cleaner'
 import { pruneSharedLockfile } from '@pnpm/prune-lockfile'
 import readProjectManifest from '@pnpm/read-project-manifest'
 import { symlinkDirectRootDependency } from '@pnpm/symlink-dependency'
@@ -30,8 +29,6 @@ import {
   ProjectManifest,
 } from '@pnpm/types'
 import normalize from 'normalize-path'
-import pathAbsolute from 'path-absolute'
-import clone from 'ramda/src/clone'
 import {
   extendOptions,
   LinkOptions,
@@ -60,7 +57,6 @@ export default async function link (
   }, true)
 
   const importerId = getLockfileImporterId(ctx.lockfileDir, opts.dir)
-  const currentLockfile = clone(ctx.currentLockfile)
   const linkedPkgs: Array<{path: string, manifest: DependencyManifest, alias: string}> = []
   const specsToUpsert = [] as PackageSpecObject[]
 
@@ -107,30 +103,6 @@ export default async function link (
   const warn = (message: string) => logger.warn({ message, prefix: opts.dir })
   const updatedWantedLockfile = pruneSharedLockfile(ctx.wantedLockfile, { warn })
 
-  await prune(
-    [
-      {
-        binsDir: opts.binsDir,
-        id: importerId,
-        modulesDir: ctx.modulesDir,
-        rootDir: opts.dir,
-      },
-    ],
-    {
-      currentLockfile,
-      hoistedDependencies: ctx.hoistedDependencies,
-      hoistedModulesDir: (opts.hoistPattern != null) ? ctx.hoistedModulesDir : undefined,
-      include: ctx.include,
-      lockfileDir: opts.lockfileDir,
-      publicHoistedModulesDir: (opts.publicHoistPattern != null) ? ctx.rootModulesDir : undefined,
-      registries: ctx.registries,
-      skipped: ctx.skipped,
-      storeController: opts.storeController,
-      virtualStoreDir: ctx.virtualStoreDir,
-      wantedLockfile: updatedCurrentLockfile,
-    }
-  )
-
   // Linking should happen after removing orphans
   // Otherwise would've been removed
   for (const { alias, manifest, path } of linkedPkgs) {
@@ -209,51 +181,3 @@ function addLinkToLockfile (
     delete projectSnapshot.specifiers[opts.linkedPkgName]
   }
 }
-
-export async function linkFromGlobal (
-  pkgNames: string[],
-  linkTo: string,
-  maybeOpts: LinkOptions & {globalDir: string}
-) {
-  const reporter = maybeOpts?.reporter
-  if ((reporter != null) && typeof reporter === 'function') {
-    streamParser.on('data', reporter)
-  }
-  const opts = await extendOptions(maybeOpts)
-  const globalPkgPath = pathAbsolute(maybeOpts.globalDir)
-  const linkFromPkgs = pkgNames.map((pkgName) => path.join(globalPkgPath, 'node_modules', pkgName))
-  const newManifest = await link(linkFromPkgs, path.join(linkTo, 'node_modules'), opts)
-
-  if ((reporter != null) && typeof reporter === 'function') {
-    streamParser.removeListener('data', reporter)
-  }
-
-  return newManifest
-}
-
-export async function linkToGlobal (
-  linkFrom: string,
-  maybeOpts: LinkOptions & {
-    globalBin: string
-    globalDir: string
-  }
-) {
-  const reporter = maybeOpts?.reporter
-  if ((reporter != null) && typeof reporter === 'function') {
-    streamParser.on('data', reporter)
-  }
-  maybeOpts.lockfileDir = maybeOpts.lockfileDir ?? maybeOpts.globalDir
-  const opts = await extendOptions(maybeOpts)
-  const globalPkgPath = pathAbsolute(maybeOpts.globalDir)
-  const newManifest = await link([linkFrom], path.join(globalPkgPath, 'node_modules'), {
-    ...opts,
-    dir: maybeOpts.globalDir,
-    linkToBin: maybeOpts.globalBin,
-  })
-
-  if ((reporter != null) && typeof reporter === 'function') {
-    streamParser.removeListener('data', reporter)
-  }
-
-  return newManifest
-}

packages/core/test/api.ts

@@ -3,9 +3,7 @@ import { testDefaults } from './utils'
 
 test('API', () => {
   expect(typeof pnpm.install).toBe('function')
-  expect(typeof pnpm.linkFromGlobal).toBe('function')
   expect(typeof pnpm.link).toBe('function')
-  expect(typeof pnpm.linkToGlobal).toBe('function')
 })
 
 // TODO: some sort of this validation might need to exist

packages/core/test/link.ts

@@ -4,15 +4,11 @@ import {
   addDependenciesToPackage,
   install,
   link,
-  linkFromGlobal,
-  linkToGlobal,
 } from '@pnpm/core'
 import fixtures from '@pnpm/test-fixtures'
 import { prepareEmpty } from '@pnpm/prepare'
 import { addDistTag } from '@pnpm/registry-mock'
 import { RootLog } from '@pnpm/core-loggers'
-import { isExecutable } from '@pnpm/assert-project'
-import exists from 'path-exists'
 import sinon from 'sinon'
 import writeJsonFile from 'write-json-file'
 import symlink from 'symlink-dir'
@@ -156,62 +152,6 @@ test('relative link is rewritten by named installation to regular dependency', a
   expect(currentLockfile.dependencies['hello-world-js-bin']).toBe('1.0.0')
 })
 
-test('global link', async () => {
-  const project = prepareEmpty()
-  const projectPath = process.cwd()
-
-  const linkedPkgName = 'hello-world-js-bin'
-  const linkedPkgPath = path.resolve('..', linkedPkgName)
-
-  f.copy(linkedPkgName, linkedPkgPath)
-
-  const opts = await testDefaults()
-
-  process.chdir(linkedPkgPath)
-  const globalDir = path.resolve('..', 'global')
-  const globalBin = path.resolve('..', 'global', 'bin')
-  await linkToGlobal(process.cwd(), { ...opts, globalDir, globalBin, manifest: {} }) // eslint-disable-line @typescript-eslint/no-explicit-any
-
-  await isExecutable((value, comment) => expect(value).toBeTruthy(), path.join(globalBin, 'hello-world-js-bin'))
-
-  // bins of dependencies should not be linked, see issue https://github.com/pnpm/pnpm/issues/905
-  expect(await exists(path.join(globalBin, 'cowsay'))).toBeFalsy() // cowsay not linked
-  expect(await exists(path.join(globalBin, 'cowthink'))).toBeFalsy() // cowthink not linked
-
-  process.chdir(projectPath)
-
-  await linkFromGlobal([linkedPkgName], process.cwd(), { ...opts, globalDir, manifest: {} }) // eslint-disable-line @typescript-eslint/no-explicit-any
-
-  await project.isExecutable('.bin/hello-world-js-bin')
-})
-
-test('failed linking should not create empty folder', async () => {
-  prepareEmpty()
-
-  const globalDir = path.resolve('..', 'global')
-
-  try {
-    await linkFromGlobal(['does-not-exist'], process.cwd(), await testDefaults({ globalDir, manifest: {} }))
-    throw new Error('should have failed')
-  } catch (err: any) { // eslint-disable-line
-    expect(await exists(path.join(globalDir, 'node_modules', 'does-not-exist'))).toBeFalsy()
-  }
-})
-
-test('node_modules is pruned after linking', async () => {
-  prepareEmpty()
-
-  await writeJsonFile('../is-positive/package.json', { name: 'is-positive', version: '1.0.0' })
-
-  const manifest = await addDependenciesToPackage({}, ['is-positive@1.0.0'], await testDefaults())
-
-  expect(await exists('node_modules/.pnpm/is-positive@1.0.0/node_modules/is-positive/package.json')).toBeTruthy()
-
-  await link(['../is-positive'], path.resolve('node_modules'), await testDefaults({ manifest, dir: process.cwd() }))
-
-  expect(await exists('node_modules/.pnpm/is-positive@1.0.0/node_modules/is-positive/package.json')).toBeFalsy()
-})
-
 test('relative link uses realpath when contained in a symlinked dir', async () => {
   prepareEmpty()
 

packages/plugin-commands-installation/src/link.ts

@@ -14,11 +14,11 @@ import findWorkspacePackages, { arrayOfWorkspacePackagesToMap } from '@pnpm/find
 import { StoreController } from '@pnpm/package-store'
 import { createOrConnectStoreControllerCached, CreateStoreControllerOptions } from '@pnpm/store-connection-manager'
 import {
+  addDependenciesToPackage,
   install,
   InstallOptions,
   link,
   LinkFunctionOptions,
-  linkToGlobal,
   WorkspacePackages,
 } from '@pnpm/core'
 import pLimit from 'p-limit'
@@ -114,13 +114,11 @@ export async function handler (
       throw new PnpmError('LINK_BAD_PARAMS', 'You must provide a parameter')
     }
     const { manifest, writeProjectManifest } = await tryReadProjectManifest(opts.dir, opts)
-    const newManifest = await linkToGlobal(cwd, {
-      ...linkOpts,
-      dir: cwd,
-      globalBin: linkOpts.bin,
-      globalDir: linkOpts.dir,
-      manifest: manifest ?? {},
-    })
+    const newManifest = await addDependenciesToPackage(
+      manifest ?? {},
+      [`link:${cwd}`],
+      linkOpts
+    )
     await writeProjectManifest(newManifest)
     return
   }

pnpm-lock.yaml

@@ -454,7 +454,6 @@ importers:
       p-every: ^2.0.0
       p-filter: ^2.1.0
       p-limit: ^3.1.0
-      path-absolute: ^1.0.1
       path-exists: ^4.0.0
       path-name: ^1.0.0
       ramda: ^0.27.1
@@ -511,7 +510,6 @@ importers:
       p-every: 2.0.0
       p-filter: 2.1.0
       p-limit: 3.1.0
-      path-absolute: 1.0.1
       path-exists: 4.0.0
       ramda: 0.27.2
       run-groups: 3.0.1