From af6ac00e4d6efbba5bf37ff452c6ea39b9338b10 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Fri, 29 Apr 2022 16:21:33 +0300 Subject: [PATCH] fix: global linking (#4641) --- .changeset/pink-llamas-joke.md | 5 ++ .changeset/soft-crews-nail.md | 5 ++ packages/core/package.json | 1 - packages/core/src/install/index.ts | 2 + packages/core/src/link/index.ts | 76 ------------------- packages/core/test/api.ts | 2 - packages/core/test/link.ts | 60 --------------- .../plugin-commands-installation/src/link.ts | 14 ++-- pnpm-lock.yaml | 2 - 9 files changed, 18 insertions(+), 149 deletions(-) create mode 100644 .changeset/pink-llamas-joke.md create mode 100644 .changeset/soft-crews-nail.md diff --git a/.changeset/pink-llamas-joke.md b/.changeset/pink-llamas-joke.md new file mode 100644 index 0000000000..a9ba0d294b --- /dev/null +++ b/.changeset/pink-llamas-joke.md @@ -0,0 +1,5 @@ +--- +"@pnpm/plugin-commands-installation": patch +--- + +Improve global linking. diff --git a/.changeset/soft-crews-nail.md b/.changeset/soft-crews-nail.md new file mode 100644 index 0000000000..435612c50e --- /dev/null +++ b/.changeset/soft-crews-nail.md @@ -0,0 +1,5 @@ +--- +"@pnpm/core": major +--- + +Remove linkFromGlobal and linkToGlobal. diff --git a/packages/core/package.json b/packages/core/package.json index eddc610aa9..a199e243e7 100644 --- a/packages/core/package.json +++ b/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", diff --git a/packages/core/src/install/index.ts b/packages/core/src/install/index.ts index 19f311f1cf..db9697cf56 100644 --- a/packages/core/src/install/index.ts +++ b/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, diff --git a/packages/core/src/link/index.ts b/packages/core/src/link/index.ts index af57b1b126..dfef26c1b7 100644 --- a/packages/core/src/link/index.ts +++ b/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 -} diff --git a/packages/core/test/api.ts b/packages/core/test/api.ts index 31755119c7..a109433ab3 100644 --- a/packages/core/test/api.ts +++ b/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 diff --git a/packages/core/test/link.ts b/packages/core/test/link.ts index ca18340b59..e420f4a3fe 100644 --- a/packages/core/test/link.ts +++ b/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() diff --git a/packages/plugin-commands-installation/src/link.ts b/packages/plugin-commands-installation/src/link.ts index f9fe9f87a6..15215afa8e 100644 --- a/packages/plugin-commands-installation/src/link.ts +++ b/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 } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 36fac8dc45..c3d079b3e0 100644 --- a/pnpm-lock.yaml +++ b/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