diff --git a/.changeset/weak-pets-share.md b/.changeset/weak-pets-share.md new file mode 100644 index 0000000000..951c0ffbc4 --- /dev/null +++ b/.changeset/weak-pets-share.md @@ -0,0 +1,6 @@ +--- +"@pnpm/hoist": patch +"pnpm": patch +--- + +Hoisting with symlinks should not override external symlinks and directories in the root of node_modules. diff --git a/config/config/package.json b/config/config/package.json index 2c1cf00100..5765d9fac0 100644 --- a/config/config/package.json +++ b/config/config/package.json @@ -61,7 +61,7 @@ "@types/is-windows": "^1.0.0", "@types/ramda": "0.28.15", "@types/which": "^2.0.1", - "symlink-dir": "^5.0.1" + "symlink-dir": "^5.1.0" }, "funding": "https://opencollective.com/pnpm", "exports": { diff --git a/fs/symlink-dependency/package.json b/fs/symlink-dependency/package.json index 726189ea2e..29f5e5acf9 100644 --- a/fs/symlink-dependency/package.json +++ b/fs/symlink-dependency/package.json @@ -42,7 +42,7 @@ "dependencies": { "@pnpm/core-loggers": "workspace:*", "@pnpm/types": "workspace:*", - "symlink-dir": "^5.0.1" + "symlink-dir": "^5.1.0" }, "funding": "https://opencollective.com/pnpm", "exports": { diff --git a/pkg-manager/core/package.json b/pkg-manager/core/package.json index d3e8340d32..c3ded1617b 100644 --- a/pkg-manager/core/package.json +++ b/pkg-manager/core/package.json @@ -97,7 +97,7 @@ "read-yaml-file": "^2.1.0", "resolve-link-target": "^2.0.0", "sinon": "^14.0.2", - "symlink-dir": "^5.0.1", + "symlink-dir": "^5.1.0", "write-json-file": "^4.3.0", "write-yaml-file": "^4.2.0" }, diff --git a/pkg-manager/core/test/install/hoist.ts b/pkg-manager/core/test/install/hoist.ts index f9cdef5bdf..671ec55c17 100644 --- a/pkg-manager/core/test/install/hoist.ts +++ b/pkg-manager/core/test/install/hoist.ts @@ -13,6 +13,7 @@ import rimraf from '@zkochan/rimraf' import resolveLinkTarget from 'resolve-link-target' import { WANTED_LOCKFILE } from '@pnpm/constants' import { addDistTag } from '@pnpm/registry-mock' +import symlinkDir from 'symlink-dir' import writeYamlFile from 'write-yaml-file' import { testDefaults } from '../utils' @@ -50,6 +51,24 @@ test('should hoist dependencies to the root of node_modules when publicHoistPatt await project.isExecutable('.bin/mime') }) +test('public hoist should not override directories that are already in the root of node_modules', async () => { + const project = prepareEmpty() + fs.mkdirSync('node_modules/debug', { recursive: true }) + fs.writeFileSync('node_modules/debug/pnpm-test.txt', '') + fs.mkdirSync('cookie') + fs.writeFileSync('cookie/pnpm-test.txt', '') + await symlinkDir('cookie', 'node_modules/cookie') + + await addDependenciesToPackage({}, + ['express@4.18.2'], + await testDefaults({ fastUnpack: false, publicHoistPattern: '*' })) + + await project.has('express') + await project.has('debug/pnpm-test.txt') + await project.has('cookie/pnpm-test.txt') + await project.has('mime') +}) + test('should hoist some dependencies to the root of node_modules when publicHoistPattern is used and others to the virtual store directory', async () => { const project = prepareEmpty() diff --git a/pkg-manager/hoist/package.json b/pkg-manager/hoist/package.json index d6bcd71c46..940b157668 100644 --- a/pkg-manager/hoist/package.json +++ b/pkg-manager/hoist/package.json @@ -40,16 +40,19 @@ }, "dependencies": { "@pnpm/constants": "workspace:*", + "@pnpm/core-loggers": "workspace:*", "@pnpm/link-bins": "workspace:*", "@pnpm/lockfile-types": "workspace:*", "@pnpm/lockfile-utils": "workspace:*", "@pnpm/lockfile-walker": "workspace:*", "@pnpm/matcher": "workspace:*", - "@pnpm/symlink-dependency": "workspace:*", "@pnpm/types": "workspace:*", "@pnpm/util.lex-comparator": "1.0.0", "dependency-path": "workspace:*", - "ramda": "npm:@pnpm/ramda@0.28.1" + "is-subdir": "^1.2.0", + "ramda": "npm:@pnpm/ramda@0.28.1", + "resolve-link-target": "^2.0.0", + "symlink-dir": "^5.1.0" }, "funding": "https://opencollective.com/pnpm", "exports": { diff --git a/pkg-manager/hoist/src/index.ts b/pkg-manager/hoist/src/index.ts index f3091c3db2..e45c9d6d00 100644 --- a/pkg-manager/hoist/src/index.ts +++ b/pkg-manager/hoist/src/index.ts @@ -1,4 +1,6 @@ +import fs from 'fs' import path from 'path' +import { linkLogger } from '@pnpm/core-loggers' import { WANTED_LOCKFILE } from '@pnpm/constants' import { linkBins, WarnFunction } from '@pnpm/link-bins' import { @@ -8,11 +10,13 @@ import { import { lockfileWalker, LockfileWalkerStep } from '@pnpm/lockfile-walker' import { logger } from '@pnpm/logger' import { createMatcher } from '@pnpm/matcher' -import { symlinkDependency } from '@pnpm/symlink-dependency' import { HoistedDependencies } from '@pnpm/types' import { lexCompare } from '@pnpm/util.lex-comparator' import * as dp from 'dependency-path' +import isSubdir from 'is-subdir' import mapObjIndexed from 'ramda/src/mapObjIndexed' +import resolveLinkTarget from 'resolve-link-target' +import symlinkDir from 'symlink-dir' const hoistLogger = logger('hoist') @@ -202,6 +206,7 @@ async function symlinkHoistedDependencies ( virtualStoreDir: string } ) { + const symlink = symlinkHoistedDependency.bind(null, opts) await Promise.all( Object.entries(hoistedDependencies) .map(async ([depPath, pkgAliases]) => { @@ -218,8 +223,44 @@ async function symlinkHoistedDependencies ( const targetDir = hoistType === 'public' ? opts.publicHoistedModulesDir : opts.privateHoistedModulesDir - await symlinkDependency(depLocation, targetDir, pkgAlias) + const dest = path.join(targetDir, pkgAlias) + return symlink(depLocation, dest) })) - } - )) + }) + ) +} + +async function symlinkHoistedDependency ( + opts: { virtualStoreDir: string }, + depLocation: string, + dest: string +) { + try { + await symlinkDir(depLocation, dest, { overwrite: false }) + linkLogger.debug({ target: dest, link: depLocation }) + return + } catch (err: any) { // eslint-disable-line + if (err.code !== 'EEXIST' && err.code !== 'EISDIR') throw err + } + let existingSymlink!: string + try { + existingSymlink = await resolveLinkTarget(dest) + } catch (err) { + hoistLogger.debug({ + skipped: dest, + reason: 'a directory is present at the target location', + }) + return + } + if (!isSubdir(opts.virtualStoreDir, existingSymlink)) { + hoistLogger.debug({ + skipped: dest, + existingSymlink, + reason: 'an external symlink is present at the target location', + }) + return + } + await fs.promises.unlink(dest) + await symlinkDir(depLocation, dest) + linkLogger.debug({ target: dest, link: depLocation }) } diff --git a/pkg-manager/hoist/tsconfig.json b/pkg-manager/hoist/tsconfig.json index fbe1f3e514..9d72521ae2 100644 --- a/pkg-manager/hoist/tsconfig.json +++ b/pkg-manager/hoist/tsconfig.json @@ -12,9 +12,6 @@ { "path": "../../config/matcher" }, - { - "path": "../../fs/symlink-dependency" - }, { "path": "../../lockfile/lockfile-types" }, @@ -27,6 +24,9 @@ { "path": "../../packages/constants" }, + { + "path": "../../packages/core-loggers" + }, { "path": "../../packages/dependency-path" }, diff --git a/pkg-manager/link-bins/package.json b/pkg-manager/link-bins/package.json index 518e57db27..26291262e9 100644 --- a/pkg-manager/link-bins/package.json +++ b/pkg-manager/link-bins/package.json @@ -46,7 +46,7 @@ "normalize-path": "^3.0.0", "p-settle": "^4.1.1", "ramda": "npm:@pnpm/ramda@0.28.1", - "symlink-dir": "^5.0.1" + "symlink-dir": "^5.1.0" }, "devDependencies": { "@pnpm/link-bins": "workspace:*", diff --git a/pkg-manager/plugin-commands-installation/package.json b/pkg-manager/plugin-commands-installation/package.json index 9ca9af5471..5f1691b6c8 100644 --- a/pkg-manager/plugin-commands-installation/package.json +++ b/pkg-manager/plugin-commands-installation/package.json @@ -51,7 +51,7 @@ "proxyquire": "^2.1.3", "read-yaml-file": "^2.1.0", "sinon": "^14.0.2", - "symlink-dir": "^5.0.1", + "symlink-dir": "^5.1.0", "tempy": "^1.0.1", "write-json-file": "^4.3.0", "write-pkg": "4.0.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 78c9f0957d..f9638dfae1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -607,8 +607,8 @@ importers: specifier: ^2.0.1 version: 2.0.1 symlink-dir: - specifier: ^5.0.1 - version: 5.0.1 + specifier: ^5.1.0 + version: 5.1.0 config/matcher: dependencies: @@ -1466,8 +1466,8 @@ importers: specifier: workspace:* version: link:../../packages/types symlink-dir: - specifier: ^5.0.1 - version: 5.0.1 + specifier: ^5.1.0 + version: 5.1.0 devDependencies: '@pnpm/prepare': specifier: workspace:* @@ -2703,8 +2703,8 @@ importers: specifier: ^14.0.2 version: 14.0.2 symlink-dir: - specifier: ^5.0.1 - version: 5.0.1 + specifier: ^5.1.0 + version: 5.1.0 write-json-file: specifier: ^4.3.0 version: 4.3.0 @@ -2920,6 +2920,9 @@ importers: '@pnpm/constants': specifier: workspace:* version: link:../../packages/constants + '@pnpm/core-loggers': + specifier: workspace:* + version: link:../../packages/core-loggers '@pnpm/link-bins': specifier: workspace:* version: link:../link-bins @@ -2938,9 +2941,6 @@ importers: '@pnpm/matcher': specifier: workspace:* version: link:../../config/matcher - '@pnpm/symlink-dependency': - specifier: workspace:* - version: link:../../fs/symlink-dependency '@pnpm/types': specifier: workspace:* version: link:../../packages/types @@ -2950,9 +2950,18 @@ importers: dependency-path: specifier: workspace:* version: link:../../packages/dependency-path + is-subdir: + specifier: ^1.2.0 + version: 1.2.0 ramda: specifier: npm:@pnpm/ramda@0.28.1 version: /@pnpm/ramda/0.28.1 + resolve-link-target: + specifier: ^2.0.0 + version: 2.0.0 + symlink-dir: + specifier: ^5.1.0 + version: 5.1.0 devDependencies: '@pnpm/hoist': specifier: workspace:* @@ -3012,8 +3021,8 @@ importers: specifier: npm:@pnpm/ramda@0.28.1 version: /@pnpm/ramda/0.28.1 symlink-dir: - specifier: ^5.0.1 - version: 5.0.1 + specifier: ^5.1.0 + version: 5.1.0 devDependencies: '@pnpm/link-bins': specifier: workspace:* @@ -3453,8 +3462,8 @@ importers: specifier: ^14.0.2 version: 14.0.2 symlink-dir: - specifier: ^5.0.1 - version: 5.0.1 + specifier: ^5.1.0 + version: 5.1.0 tempy: specifier: ^1.0.1 version: 1.0.1 @@ -4060,8 +4069,8 @@ importers: specifier: ^6.0.1 version: 6.0.1 symlink-dir: - specifier: ^5.0.1 - version: 5.0.1 + specifier: ^5.1.0 + version: 5.1.0 tempy: specifier: ^1.0.1 version: 1.0.1 @@ -16115,6 +16124,15 @@ packages: dependencies: better-path-resolve: 1.0.0 rename-overwrite: 4.0.3 + dev: true + + /symlink-dir/5.1.0: + resolution: {integrity: sha512-nrwSDbmMYGwc+wu6gzmTdQIV9GzItMG/0LX4c9Co0bhtx2KK1WUKjNIyhjhAdIKaA5eIwVhB2tqFsn8ooUsRKA==} + engines: {node: '>=12.10'} + hasBin: true + dependencies: + better-path-resolve: 1.0.0 + rename-overwrite: 4.0.3 /syncpack/8.3.9: resolution: {integrity: sha512-oDaEvgZAoU40WXX4okQ7DiCfVEcO2antl+jV+Av+FbS4Hyh8VoJ8F6HGo2G8eT9H+AyBBM1iDUdbtR1MQRimoQ==} @@ -17529,7 +17547,7 @@ time: /string.prototype.replaceall/1.0.6: '2021-10-05T01:00:00.092Z' /strip-ansi/6.0.1: '2021-09-23T16:34:41.798Z' /strip-bom/4.0.0: '2019-04-28T04:40:47.887Z' - /symlink-dir/5.0.1: '2021-05-03T15:54:10.299Z' + /symlink-dir/5.1.0: '2022-11-20T16:48:15.918Z' /syncpack/8.3.9: '2022-10-28T16:58:42.312Z' /tar-stream/2.2.0: '2020-12-29T10:22:57.508Z' /tar/6.1.12: '2022-11-01T16:33:12.294Z' diff --git a/pnpm/package.json b/pnpm/package.json index e310c56cb4..f20f7a5a9b 100644 --- a/pnpm/package.json +++ b/pnpm/package.json @@ -101,7 +101,7 @@ "semver": "^7.3.8", "split-cmd": "^1.0.1", "strip-ansi": "^6.0.1", - "symlink-dir": "^5.0.1", + "symlink-dir": "^5.1.0", "tempy": "^1.0.1", "tree-kill": "^1.2.2", "which": "^2.0.2",