diff --git a/__tests__/commands/_helpers.js b/__tests__/commands/_helpers.js index b73f39ed1c..d8fd782b8f 100644 --- a/__tests__/commands/_helpers.js +++ b/__tests__/commands/_helpers.js @@ -10,16 +10,17 @@ import * as fs from '../../src/util/fs.js'; import {Install} from '../../src/cli/commands/install.js'; import Config from '../../src/config.js'; import parsePackagePath from '../../src/util/parse-package-path.js'; - +import type {CLIFunctionReturn} from '../../src/types.js'; +import {run as link} from '../../src/cli/commands/link.js'; const stream = require('stream'); const path = require('path'); -const fixturesLoc = path.join(__dirname, '..', 'fixtures', 'install'); +const installFixturesLoc = path.join(__dirname, '..', 'fixtures', 'install'); export const runInstall = run.bind( null, ConsoleReporter, - fixturesLoc, + installFixturesLoc, async (args, flags, config, reporter, lockfile): Promise => { const install = new Install(flags, config, reporter, lockfile); await install.init(); @@ -30,6 +31,17 @@ export const runInstall = run.bind( [], ); +const linkFixturesLoc = path.join(__dirname, '..', 'fixtures', 'link'); + +export const runLink = run.bind( + null, + ConsoleReporter, + linkFixturesLoc, + (args, flags, config, reporter): CLIFunctionReturn => { + return link(config, reporter, flags, args); + }, +); + export async function createLockfile(dir: string): Promise { const lockfileLoc = path.join(dir, constants.LOCKFILE_FILENAME); let lockfile; @@ -94,7 +106,7 @@ export async function run( ) => Promise | T, args: Array, flags: Object, - name: string | {source: string, cwd: string}, + name: string | {source?: string, cwd: string}, checkInstalled: ?(config: Config, reporter: R, install: T, getStdout: () => string) => ?Promise, beforeInstall: ?(cwd: string) => ?Promise, ): Promise { @@ -113,11 +125,16 @@ export async function run( if (fixturesLoc) { const source = typeof name === 'string' ? name : name.source; - const dir = path.join(fixturesLoc, source); - cwd = await fs.makeTempDir(path.basename(dir)); - await fs.copy(dir, cwd, reporter); - if (typeof name !== 'string') { - cwd = path.join(cwd, name.cwd); + // if source wasn't set then assume we were given a complete path + if (typeof source === 'undefined') { + cwd = typeof name !== 'string' ? name.cwd : await fs.makeTempDir(); + } else { + const dir = path.join(fixturesLoc, source); + cwd = await fs.makeTempDir(path.basename(dir)); + await fs.copy(dir, cwd, reporter); + if (typeof name !== 'string') { + cwd = path.join(cwd, name.cwd); + } } } else { // if fixture loc is not set then CWD is some empty temp dir diff --git a/__tests__/commands/install/integration.js b/__tests__/commands/install/integration.js index 695e1eb91a..c8523bf2f5 100644 --- a/__tests__/commands/install/integration.js +++ b/__tests__/commands/install/integration.js @@ -11,7 +11,7 @@ import {parse} from '../../../src/lockfile'; import {Install, run as install} from '../../../src/cli/commands/install.js'; import Lockfile from '../../../src/lockfile'; import * as fs from '../../../src/util/fs.js'; -import {getPackageVersion, explodeLockfile, runInstall, createLockfile, run as buildRun} from '../_helpers.js'; +import {getPackageVersion, explodeLockfile, runInstall, runLink, createLockfile, run as buildRun} from '../_helpers.js'; jasmine.DEFAULT_TIMEOUT_INTERVAL = 150000; @@ -866,29 +866,6 @@ test('install a scoped module from authed private registry with a missing traili }); }); -test.concurrent('install will not overwrite files in symlinked scoped directories', async (): Promise => { - await runInstall( - {}, - 'install-dont-overwrite-linked-scoped', - async (config): Promise => { - const dependencyPath = path.join(config.cwd, 'node_modules', '@fakescope', 'fake-dependency'); - expect('Symlinked scoped package test').toEqual( - (await fs.readJson(path.join(dependencyPath, 'package.json'))).description, - ); - expect(await fs.exists(path.join(dependencyPath, 'index.js'))).toEqual(false); - }, - async cwd => { - const dirToLink = path.join(cwd, 'dir-to-link'); - - await fs.mkdirp(path.join(cwd, '.yarn-link', '@fakescope')); - await fs.symlink(dirToLink, path.join(cwd, '.yarn-link', '@fakescope', 'fake-dependency')); - - await fs.mkdirp(path.join(cwd, 'node_modules', '@fakescope')); - await fs.symlink(dirToLink, path.join(cwd, 'node_modules', '@fakescope', 'fake-dependency')); - }, - ); -}); - test.concurrent('install of scoped package with subdependency conflict should pass check', (): Promise => { return runInstall({}, 'install-scoped-package-with-subdependency-conflict', async (config, reporter) => { let allCorrect = true; @@ -1134,3 +1111,54 @@ test.concurrent('warns for missing bundledDependencies', (): Promise => { 'missing-bundled-dep', ); }); + +test.concurrent('install will not overwrite linked scoped dependencies', async (): Promise => { + // install only dependencies + await runInstall({production: true}, 'install-dont-overwrite-linked', async (installConfig): Promise => { + // link our fake dep to the registry + await runLink([], {}, 'package-with-name-scoped', async (linkConfig): Promise => { + // link our fake dependency in our node_modules + await runLink( + ['@fakescope/a-package'], + {linkFolder: linkConfig.linkFolder}, + {cwd: installConfig.cwd}, + async (): Promise => { + // check that it exists (just in case) + const existed = await fs.exists(path.join(installConfig.cwd, 'node_modules', '@fakescope', 'a-package')); + expect(existed).toEqual(true); + + // run install to install dev deps which would remove the linked dep if the bug was present + await runInstall({linkFolder: linkConfig.linkFolder}, {cwd: installConfig.cwd}, async (): Promise => { + // if the linked dep is still there is a win :) + const existed = await fs.exists(path.join(installConfig.cwd, 'node_modules', '@fakescope', 'a-package')); + expect(existed).toEqual(true); + }); + }, + ); + }); + }); +}); + +test.concurrent('install will not overwrite linked dependencies', async (): Promise => { + // install only dependencies + await runInstall({production: true}, 'install-dont-overwrite-linked', async (installConfig): Promise => { + // link our fake dep to the registry + await runLink([], {}, 'package-with-name', async (linkConfig): Promise => { + // link our fake dependency in our node_modules + await runLink(['a-package'], {linkFolder: linkConfig.linkFolder}, {cwd: installConfig.cwd}, async (): Promise< + void, + > => { + // check that it exists (just in case) + const existed = await fs.exists(path.join(installConfig.cwd, 'node_modules', 'a-package')); + expect(existed).toEqual(true); + + // run install to install dev deps which would remove the linked dep if the bug was present + await runInstall({linkFolder: linkConfig.linkFolder}, {cwd: installConfig.cwd}, async (): Promise => { + // if the linked dep is still there is a win :) + const existed = await fs.exists(path.join(installConfig.cwd, 'node_modules', 'a-package')); + expect(existed).toEqual(true); + }); + }); + }); + }); +}); diff --git a/__tests__/commands/link.js b/__tests__/commands/link.js index 59fd7db353..a3c1129615 100644 --- a/__tests__/commands/link.js +++ b/__tests__/commands/link.js @@ -1,24 +1,12 @@ /* @flow */ -import {run as buildRun} from './_helpers.js'; -import {run as link} from '../../src/cli/commands/link.js'; +import {runLink} from './_helpers.js'; import {ConsoleReporter} from '../../src/reporters/index.js'; -import type {CLIFunctionReturn} from '../../src/types.js'; import mkdir from './../_temp.js'; import * as fs from '../../src/util/fs.js'; const path = require('path'); -const fixturesLoc = path.join(__dirname, '..', 'fixtures', 'link'); -const runLink = buildRun.bind( - null, - ConsoleReporter, - fixturesLoc, - (args, flags, config, reporter): CLIFunctionReturn => { - return link(config, reporter, flags, args); - }, -); - test.concurrent('creates folder in linkFolder', async (): Promise => { const linkFolder = await mkdir('link-folder'); await runLink([], {linkFolder}, 'package-with-name', async (config, reporter): Promise => { diff --git a/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/.npmrc b/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/.npmrc deleted file mode 100644 index 9465b97ac3..0000000000 --- a/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/.npmrc +++ /dev/null @@ -1 +0,0 @@ -yarn-offline-mirror=./mirror-for-offline diff --git a/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/dir-to-link/package.json b/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/dir-to-link/package.json deleted file mode 100644 index 5de00bef5d..0000000000 --- a/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/dir-to-link/package.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "name": "@fakescope/fake-dependency", - "description": "Symlinked scoped package test", - "version": "1.0.1", - "dependencies": {}, - "license": "MIT" -} diff --git a/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/mirror-for-offline/@fakescope-fake-dependency-1.0.1.tgz b/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/mirror-for-offline/@fakescope-fake-dependency-1.0.1.tgz deleted file mode 100644 index 29e48dd0cd..0000000000 Binary files a/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/mirror-for-offline/@fakescope-fake-dependency-1.0.1.tgz and /dev/null differ diff --git a/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/package.json b/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/package.json deleted file mode 100644 index a0cf66cc64..0000000000 --- a/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/package.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "dependencies": { - "@fakescope/fake-dependency": "1.0.1" - } -} diff --git a/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/yarn.lock b/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/yarn.lock deleted file mode 100644 index ca461377b2..0000000000 --- a/__tests__/fixtures/install/install-dont-overwrite-linked-scoped/yarn.lock +++ /dev/null @@ -1,5 +0,0 @@ -# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. -# yarn lockfile v1 -"@fakescope/fake-dependency@1.0.1": - version "1.0.1" - resolved "@fakescope-fake-dependency-1.0.1.tgz#477dafd486d856af0b3faf5a5f1c895001221609" diff --git a/__tests__/fixtures/install/install-dont-overwrite-linked/package.json b/__tests__/fixtures/install/install-dont-overwrite-linked/package.json new file mode 100644 index 0000000000..86bcb815f7 --- /dev/null +++ b/__tests__/fixtures/install/install-dont-overwrite-linked/package.json @@ -0,0 +1,8 @@ +{ + "dependencies": { + "left-pad": "1.1.3" + }, + "devDependencies": { + "is-buffer": "^1.1.5" + } +} diff --git a/__tests__/fixtures/link/package-with-name-scoped/package.json b/__tests__/fixtures/link/package-with-name-scoped/package.json new file mode 100644 index 0000000000..203ee2bea8 --- /dev/null +++ b/__tests__/fixtures/link/package-with-name-scoped/package.json @@ -0,0 +1,3 @@ +{ + "name": "@fakescope/a-package" +} diff --git a/src/package-linker.js b/src/package-linker.js index c8f06a9dcd..e32fc2480c 100644 --- a/src/package-linker.js +++ b/src/package-linker.js @@ -304,8 +304,13 @@ export default class PackageLinker { const stat = await fs.lstat(entryPath); if (stat.isSymbolicLink()) { - const packageName = entry; - linkTargets.set(packageName, await fs.readlink(entryPath)); + try { + const entryTarget = await fs.realpath(entryPath); + linkTargets.set(entry, entryTarget); + } catch (err) { + this.reporter.warn(this.reporter.lang('linkTargetMissing', entry)); + await fs.unlink(entryPath); + } } else if (stat.isDirectory() && entry[0] === '@') { // if the entry is directory beginning with '@', then we're dealing with a package scope, which // means we must iterate inside to retrieve the package names it contains @@ -317,7 +322,13 @@ export default class PackageLinker { if (stat2.isSymbolicLink()) { const packageName = `${scopeName}/${entry2}`; - linkTargets.set(packageName, await fs.readlink(entryPath2)); + try { + const entryTarget = await fs.realpath(entryPath2); + linkTargets.set(packageName, entryTarget); + } catch (err) { + this.reporter.warn(this.reporter.lang('linkTargetMissing', packageName)); + await fs.unlink(entryPath2); + } } } } @@ -334,7 +345,7 @@ export default class PackageLinker { if ( (await fs.lstat(loc)).isSymbolicLink() && linkTargets.has(packageName) && - linkTargets.get(packageName) === (await fs.readlink(loc)) + linkTargets.get(packageName) === (await fs.realpath(loc)) ) { possibleExtraneous.delete(loc); copyQueue.delete(loc); diff --git a/src/reporters/lang/en.js b/src/reporters/lang/en.js index 5d720788a3..0a781588c7 100644 --- a/src/reporters/lang/en.js +++ b/src/reporters/lang/en.js @@ -172,6 +172,7 @@ const messages = { linkUsing: 'Using linked module for $0.', linkDisusing: 'Removed linked module $0.', linkDisusingMessage: 'You will need to run `yarn` to re-install the package that was linked.', + linkTargetMissing: 'The target of linked module $0 is missing. Removing link.', createInvalidBin: 'Invalid bin entry found in package $0.', createMissingPackage: