From 9c22c063eba31af0fd575ba4a10529eaa424102a Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 6 Mar 2022 01:58:34 +0200 Subject: [PATCH] feat: the file protocol should always inject the dependency (#4408) --- .changeset/green-trainers-sit.md | 7 + .../core/test/install/injectLocalPackages.ts | 298 ++++++++++++++++++ packages/core/test/install/local.ts | 6 +- .../core/test/install/multipleImporters.ts | 2 +- packages/local-resolver/src/index.ts | 2 +- packages/local-resolver/src/parsePref.ts | 10 +- packages/local-resolver/test/index.ts | 8 +- .../package-requester/src/packageRequester.ts | 2 +- 8 files changed, 324 insertions(+), 11 deletions(-) create mode 100644 .changeset/green-trainers-sit.md diff --git a/.changeset/green-trainers-sit.md b/.changeset/green-trainers-sit.md new file mode 100644 index 00000000000..299f0952d39 --- /dev/null +++ b/.changeset/green-trainers-sit.md @@ -0,0 +1,7 @@ +--- +"@pnpm/local-resolver": major +"@pnpm/package-requester": major +"pnpm": major +--- + +Local dependencies referenced through the `file:` protocol are hard linked (not symlinked) [#4408](https://github.com/pnpm/pnpm/pull/4408). If you need to symlink a dependency, use the `link:` protocol instead. diff --git a/packages/core/test/install/injectLocalPackages.ts b/packages/core/test/install/injectLocalPackages.ts index 14bf2583837..10c9f0aeea0 100644 --- a/packages/core/test/install/injectLocalPackages.ts +++ b/packages/core/test/install/injectLocalPackages.ts @@ -443,6 +443,204 @@ test('inject local packages declared via file protocol', async () => { } }) +test('inject local packages when the file protocol is used', async () => { + const project1Manifest = { + name: 'project-1', + version: '1.0.0', + dependencies: { + 'is-negative': '1.0.0', + }, + devDependencies: { + 'dep-of-pkg-with-1-dep': '100.0.0', + }, + peerDependencies: { + 'is-positive': '>=1.0.0', + }, + } + const project2Manifest = { + name: 'project-2', + version: '1.0.0', + dependencies: { + 'project-1': 'file:../project-1', + }, + devDependencies: { + 'is-positive': '1.0.0', + }, + } + const project3Manifest = { + name: 'project-3', + version: '1.0.0', + dependencies: { + 'project-2': 'file:../project-2', + }, + devDependencies: { + 'is-positive': '2.0.0', + }, + } + const projects = preparePackages([ + { + location: 'project-1', + package: project1Manifest, + }, + { + location: 'project-2', + package: project2Manifest, + }, + { + location: 'project-3', + package: project3Manifest, + }, + ]) + + const importers: MutatedProject[] = [ + { + buildIndex: 0, + manifest: project1Manifest, + mutation: 'install', + rootDir: path.resolve('project-1'), + }, + { + buildIndex: 0, + manifest: project2Manifest, + mutation: 'install', + rootDir: path.resolve('project-2'), + }, + { + buildIndex: 0, + manifest: project3Manifest, + mutation: 'install', + rootDir: path.resolve('project-3'), + }, + ] + const workspacePackages = { + 'project-1': { + '1.0.0': { + dir: path.resolve('project-1'), + manifest: project1Manifest, + }, + }, + 'project-2': { + '1.0.0': { + dir: path.resolve('project-2'), + manifest: project2Manifest, + }, + }, + 'project-3': { + '1.0.0': { + dir: path.resolve('project-3'), + manifest: project2Manifest, + }, + }, + } + await mutateModules(importers, await testDefaults({ + workspacePackages, + })) + + await projects['project-1'].has('is-negative') + await projects['project-1'].has('dep-of-pkg-with-1-dep') + await projects['project-1'].hasNot('is-positive') + + await projects['project-2'].has('is-positive') + await projects['project-2'].has('project-1') + + await projects['project-3'].has('is-positive') + await projects['project-3'].has('project-2') + + expect(fs.readdirSync('node_modules/.pnpm').length).toBe(8) + + const rootModules = assertProject(process.cwd()) + { + const lockfile = await rootModules.readLockfile() + expect(lockfile.packages['file:project-1_is-positive@1.0.0']).toEqual({ + resolution: { + directory: 'project-1', + type: 'directory', + }, + id: 'file:project-1', + name: 'project-1', + version: '1.0.0', + peerDependencies: { + 'is-positive': '>=1.0.0', + }, + dependencies: { + 'is-negative': '1.0.0', + 'is-positive': '1.0.0', + }, + dev: false, + }) + expect(lockfile.packages['file:project-2_is-positive@2.0.0']).toEqual({ + resolution: { + directory: 'project-2', + type: 'directory', + }, + id: 'file:project-2', + name: 'project-2', + version: '1.0.0', + dependencies: { + 'project-1': 'file:project-1_is-positive@2.0.0', + }, + transitivePeerDependencies: ['is-positive'], + dev: false, + }) + + const modulesState = await rootModules.readModulesManifest() + expect(modulesState?.injectedDeps?.['project-1'].length).toEqual(2) + expect(modulesState?.injectedDeps?.['project-1'][0]).toContain(`node_modules${path.sep}.pnpm`) + expect(modulesState?.injectedDeps?.['project-1'][1]).toContain(`node_modules${path.sep}.pnpm`) + } + + await rimraf('node_modules') + await rimraf('project-1/node_modules') + await rimraf('project-2/node_modules') + await rimraf('project-3/node_modules') + + await mutateModules(importers, await testDefaults({ + frozenLockfile: true, + workspacePackages, + })) + + await projects['project-1'].has('is-negative') + await projects['project-1'].has('dep-of-pkg-with-1-dep') + await projects['project-1'].hasNot('is-positive') + + await projects['project-2'].has('is-positive') + await projects['project-2'].has('project-1') + + await projects['project-3'].has('is-positive') + await projects['project-3'].has('project-2') + + expect(fs.readdirSync('node_modules/.pnpm').length).toBe(8) + + // The injected project is updated when one of its dependencies needs to be updated + importers[0].manifest.dependencies!['is-negative'] = '2.0.0' + writeJsonFile('project-1/package.json', importers[0].manifest) + await mutateModules(importers, await testDefaults({ workspacePackages })) + { + const lockfile = await rootModules.readLockfile() + expect(lockfile.packages['file:project-1_is-positive@1.0.0']).toEqual({ + resolution: { + directory: 'project-1', + type: 'directory', + }, + id: 'file:project-1', + name: 'project-1', + version: '1.0.0', + peerDependencies: { + 'is-positive': '>=1.0.0', + }, + dependencies: { + 'is-negative': '2.0.0', + 'is-positive': '1.0.0', + }, + dev: false, + }) + const modulesState = await rootModules.readModulesManifest() + expect(modulesState?.injectedDeps?.['project-1'].length).toEqual(2) + expect(modulesState?.injectedDeps?.['project-1'][0]).toContain(`node_modules${path.sep}.pnpm`) + expect(modulesState?.injectedDeps?.['project-1'][1]).toContain(`node_modules${path.sep}.pnpm`) + } +}) + test('inject local packages and relink them after build', async () => { const project1Manifest = { name: 'project-1', @@ -570,6 +768,106 @@ test('inject local packages and relink them after build', async () => { expect(await pathExists(path.resolve('project-2/node_modules/project-1/main.js'))).toBeTruthy() }) +test('inject local packages and relink them after build (file protocol is used)', async () => { + const project1Manifest = { + name: 'project-1', + version: '1.0.0', + dependencies: { + 'is-negative': '1.0.0', + }, + devDependencies: { + 'dep-of-pkg-with-1-dep': '100.0.0', + }, + peerDependencies: { + 'is-positive': '1.0.0', + }, + scripts: { + prepublishOnly: 'touch main.js', + }, + } + const project2Manifest = { + name: 'project-2', + version: '1.0.0', + dependencies: { + 'is-positive': '1.0.0', + 'project-1': 'file:../project-1', + }, + } + const projects = preparePackages([ + { + location: 'project-1', + package: project1Manifest, + }, + { + location: 'project-2', + package: project2Manifest, + }, + ]) + + const importers: MutatedProject[] = [ + { + buildIndex: 0, + manifest: project1Manifest, + mutation: 'install', + rootDir: path.resolve('project-1'), + }, + { + buildIndex: 0, + manifest: project2Manifest, + mutation: 'install', + rootDir: path.resolve('project-2'), + }, + ] + await mutateModules(importers, await testDefaults()) + + await projects['project-1'].has('is-negative') + await projects['project-1'].has('dep-of-pkg-with-1-dep') + await projects['project-1'].hasNot('is-positive') + + await projects['project-2'].has('is-positive') + await projects['project-2'].has('project-1') + + expect(await pathExists(path.resolve('project-2/node_modules/project-1/main.js'))).toBeTruthy() + + const rootModules = assertProject(process.cwd()) + const lockfile = await rootModules.readLockfile() + expect(lockfile.packages['file:project-1_is-positive@1.0.0']).toEqual({ + resolution: { + directory: 'project-1', + type: 'directory', + }, + id: 'file:project-1', + name: 'project-1', + version: '1.0.0', + peerDependencies: { + 'is-positive': '1.0.0', + }, + dependencies: { + 'is-negative': '1.0.0', + 'is-positive': '1.0.0', + }, + dev: false, + }) + + await rimraf('node_modules') + await rimraf('project-1/main.js') + await rimraf('project-1/node_modules') + await rimraf('project-2/node_modules') + + await mutateModules(importers, await testDefaults({ + frozenLockfile: true, + })) + + await projects['project-1'].has('is-negative') + await projects['project-1'].has('dep-of-pkg-with-1-dep') + await projects['project-1'].hasNot('is-positive') + + await projects['project-2'].has('is-positive') + await projects['project-2'].has('project-1') + + expect(await pathExists(path.resolve('project-2/node_modules/project-1/main.js'))).toBeTruthy() +}) + test('inject local packages when node-linker is hoisted', async () => { const project1Manifest = { name: 'project-1', diff --git a/packages/core/test/install/local.ts b/packages/core/test/install/local.ts index d4535c14dd6..c246932093e 100644 --- a/packages/core/test/install/local.ts +++ b/packages/core/test/install/local.ts @@ -29,7 +29,7 @@ test('local file', async () => { const project = prepareEmpty() f.copy('local-pkg', path.resolve('..', 'local-pkg')) - const manifest = await addDependenciesToPackage({}, ['file:../local-pkg'], await testDefaults()) + const manifest = await addDependenciesToPackage({}, ['link:../local-pkg'], await testDefaults()) const expectedSpecs = { 'local-pkg': `link:..${path.sep}local-pkg` } expect(manifest.dependencies).toStrictEqual(expectedSpecs) @@ -56,7 +56,7 @@ test('local directory with no package.json', async () => { const manifest = await addDependenciesToPackage({}, ['file:./pkg'], await testDefaults()) - const expectedSpecs = { pkg: 'link:pkg' } + const expectedSpecs = { pkg: 'file:pkg' } expect(manifest.dependencies).toStrictEqual(expectedSpecs) await project.has('pkg') @@ -96,7 +96,7 @@ test('local file with symlinked node_modules', async () => { await fs.mkdir(path.join('..', 'node_modules')) await symlinkDir(path.join('..', 'node_modules'), 'node_modules') - const manifest = await addDependenciesToPackage({}, ['file:../local-pkg'], await testDefaults()) + const manifest = await addDependenciesToPackage({}, ['link:../local-pkg'], await testDefaults()) const expectedSpecs = { 'local-pkg': `link:..${path.sep}local-pkg` } expect(manifest.dependencies).toStrictEqual(expectedSpecs) diff --git a/packages/core/test/install/multipleImporters.ts b/packages/core/test/install/multipleImporters.ts index c44cb733f77..22add662637 100644 --- a/packages/core/test/install/multipleImporters.ts +++ b/packages/core/test/install/multipleImporters.ts @@ -359,7 +359,7 @@ test('headless install is used when package linked to another package in the wor dependencies: { 'is-positive': '1.0.0', - 'project-2': 'file:../project-2', + 'project-2': 'link:../project-2', }, } const pkg2 = { diff --git a/packages/local-resolver/src/index.ts b/packages/local-resolver/src/index.ts index a76bc0d591d..2a4fdd7ea2b 100644 --- a/packages/local-resolver/src/index.ts +++ b/packages/local-resolver/src/index.ts @@ -55,7 +55,7 @@ export default async function resolveLocal ( localDependencyManifest = await readProjectManifestOnly(spec.fetchSpec) as DependencyManifest } catch (internalErr: any) { // eslint-disable-line if (!existsSync(spec.fetchSpec)) { - if (wantedDependency.injected) { + if (spec.id.startsWith('file:')) { throw new PnpmError('LINKED_PKG_DIR_NOT_FOUND', `Could not install from "${spec.fetchSpec}" as it does not exist.`) } diff --git a/packages/local-resolver/src/parsePref.ts b/packages/local-resolver/src/parsePref.ts index 18692b9a4c2..3e9bb4ca2eb 100644 --- a/packages/local-resolver/src/parsePref.ts +++ b/packages/local-resolver/src/parsePref.ts @@ -62,7 +62,14 @@ function fromLocal ( .replace(/^(file|link|workspace):[/]*([A-Za-z]:)/, '$2') // drive name paths on windows .replace(/^(file|link|workspace):(?:[/]*([~./]))?/, '$2') - const protocol = type === 'directory' && !injected ? 'link:' : 'file:' + let protocol!: string + if (pref.startsWith('file:')) { + protocol = 'file:' + } else if (pref.startsWith('link:')) { + protocol = 'link:' + } else { + protocol = type === 'directory' && !injected ? 'link:' : 'file:' + } let fetchSpec!: string let normalizedPref!: string if (/^~[/]/.test(spec)) { @@ -78,6 +85,7 @@ function fromLocal ( } } + injected = protocol === 'file:' const dependencyPath = injected ? normalize(path.relative(lockfileDir, fetchSpec)) : normalize(path.resolve(fetchSpec)) diff --git a/packages/local-resolver/test/index.ts b/packages/local-resolver/test/index.ts index 1dabd1d5c4c..57ecebc84f7 100644 --- a/packages/local-resolver/test/index.ts +++ b/packages/local-resolver/test/index.ts @@ -32,10 +32,10 @@ test('resolve workspace directory', async () => { test('resolve directory specified using the file: protocol', async () => { const resolveResult = await resolveFromLocal({ pref: 'file:..' }, { projectDir: __dirname }) - expect(resolveResult!.id).toEqual('link:..') - expect(resolveResult!.normalizedPref).toEqual('link:..') + expect(resolveResult!.id).toEqual('file:..') + expect(resolveResult!.normalizedPref).toEqual('file:..') expect(resolveResult!['manifest']!.name).toEqual('@pnpm/local-resolver') - expect(resolveResult!.resolution['directory']).toEqual(normalize(path.join(__dirname, '..'))) + expect(resolveResult!.resolution['directory']).toEqual('..') expect(resolveResult!.resolution['type']).toEqual('directory') }) @@ -108,7 +108,7 @@ test('fail when resolving tarball specified with the link: protocol', async () = }) test('fail when resolving from not existing directory an injected dependency', async () => { - const wantedDependency = { injected: true, pref: 'link:./dir-does-not-exist' } + const wantedDependency = { pref: 'file:./dir-does-not-exist' } const projectDir = __dirname await expect( resolveFromLocal(wantedDependency, { projectDir }) diff --git a/packages/package-requester/src/packageRequester.ts b/packages/package-requester/src/packageRequester.ts index 40d3ba78b18..95c2a5fa3a4 100644 --- a/packages/package-requester/src/packageRequester.ts +++ b/packages/package-requester/src/packageRequester.ts @@ -195,7 +195,7 @@ async function resolveAndFetch ( const id = pkgId as string - if (resolution.type === 'directory' && !wantedDependency.injected) { + if (resolution.type === 'directory' && !id.startsWith('file:')) { if (manifest == null) { throw new Error(`Couldn't read package.json of local dependency ${wantedDependency.alias ? wantedDependency.alias + '@' : ''}${wantedDependency.pref ?? ''}`) }