Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(linker): use lockfileFolder when creating bin links #4730

Merged
merged 2 commits into from
Oct 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions __tests__/commands/add.js
Original file line number Diff line number Diff line change
Expand Up @@ -957,3 +957,32 @@ test.concurrent('installing with --pure-lockfile and then adding should keep bui
expect(await fs.exists(path.join(config.cwd, 'node_modules', 'package-a', 'temp.txt'))).toBe(true);
});
});

test.concurrent('preserves unaffected bin links after adding to workspace package', async () => {
await runInstall({binLinks: true}, 'workspaces-install-bin', async (config): Promise<void> => {
const reporter = new ConsoleReporter({});

expect(await fs.exists(`${config.cwd}/node_modules/.bin/rimraf`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/node_modules/.bin/touch`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/node_modules/.bin/workspace-1`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/packages/workspace-2/node_modules/.bin/rimraf`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/packages/workspace-2/node_modules/.bin/workspace-1`)).toEqual(true);

// add package
const childConfig = await makeConfigFromDirectory(`${config.cwd}/packages/workspace-1`, reporter, {binLinks: true});
await add(childConfig, reporter, {}, ['max-safe-integer@1.0.0']);

expect(
JSON.parse(await fs.readFile(path.join(config.cwd, 'packages/workspace-1/package.json'))).dependencies,
).toEqual({
'max-safe-integer': '1.0.0',
});

// bin links should be preserved
expect(await fs.exists(`${config.cwd}/node_modules/.bin/rimraf`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/node_modules/.bin/touch`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/node_modules/.bin/workspace-1`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/packages/workspace-2/node_modules/.bin/rimraf`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/packages/workspace-2/node_modules/.bin/workspace-1`)).toEqual(true);
});
});
33 changes: 31 additions & 2 deletions __tests__/commands/install/workspaces-install.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/* @flow */

import {run as check} from '../../../src/cli/commands/check.js';
import {Install} from '../../../src/cli/commands/install.js';
import {Install, run as install} from '../../../src/cli/commands/install.js';
import * as reporters from '../../../src/reporters/index.js';
import * as fs from '../../../src/util/fs.js';
import {runInstall, run as buildRun} from '../_helpers.js';
import {runInstall, run as buildRun, makeConfigFromDirectory} from '../_helpers.js';

jasmine.DEFAULT_TIMEOUT_INTERVAL = 150000;

Expand Down Expand Up @@ -242,4 +242,33 @@ test.concurrent('install should ignore node_modules in workspaces when used with
});
});

test.concurrent('install should link binaries properly when run from child workspace', async () => {
await runInstall({binLinks: true}, 'workspaces-install-bin', async (config, reporter): Promise<void> => {
// initial install
expect(await fs.exists(`${config.cwd}/node_modules/.bin/rimraf`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/node_modules/.bin/touch`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/node_modules/.bin/workspace-1`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/packages/workspace-2/node_modules/.bin/rimraf`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/packages/workspace-2/node_modules/.bin/workspace-1`)).toEqual(true);

// reset package folders to simulate running 'install' from
// child workspace _before_ running it in the root (this is not
// possible to do without an initial install using the current
// testing infrastructure)
await fs.unlink(`${config.cwd}/node_modules`);
await fs.unlink(`${config.cwd}/packages/workspace-1/node_modules`);
await fs.unlink(`${config.cwd}/packages/workspace-2/node_modules`);

// run "install" in child package
const childConfig = await makeConfigFromDirectory(`${config.cwd}/packages/workspace-1`, reporter, {binLinks: true});
await install(childConfig, reporter, {}, []);

expect(await fs.exists(`${config.cwd}/node_modules/.bin/rimraf`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/node_modules/.bin/touch`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/node_modules/.bin/workspace-1`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/packages/workspace-2/node_modules/.bin/rimraf`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/packages/workspace-2/node_modules/.bin/workspace-1`)).toEqual(true);
});
});

// TODO need more thorough tests for all kinds of checks: integrity, verify-tree
28 changes: 28 additions & 0 deletions __tests__/commands/remove.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,31 @@ test.concurrent('removes from workspace packages', async () => {
expect(lockFileLines[0]).toEqual('left-pad@1.1.3:');
});
});

test.concurrent('preserves unaffected bin links after removing workspace packages', async () => {
await runInstall({binLinks: true}, 'workspaces-install-bin', async (config): Promise<void> => {
const reporter = new ConsoleReporter({});

expect(await fs.exists(`${config.cwd}/node_modules/.bin/rimraf`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/node_modules/.bin/touch`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/node_modules/.bin/workspace-1`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/packages/workspace-2/node_modules/.bin/rimraf`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/packages/workspace-2/node_modules/.bin/workspace-1`)).toEqual(true);

// remove package
const childConfig = await makeConfigFromDirectory(`${config.cwd}/packages/workspace-1`, reporter, {binLinks: true});
await remove(childConfig, reporter, {}, ['left-pad']);
await check(childConfig, reporter, {verifyTree: true}, []);

expect(
JSON.parse(await fs.readFile(path.join(config.cwd, 'packages/workspace-1/package.json'))).devDependencies,
).toEqual({});

// bin links should be preserved
expect(await fs.exists(`${config.cwd}/node_modules/.bin/rimraf`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/node_modules/.bin/touch`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/node_modules/.bin/workspace-1`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/packages/workspace-2/node_modules/.bin/rimraf`)).toEqual(true);
expect(await fs.exists(`${config.cwd}/packages/workspace-2/node_modules/.bin/workspace-1`)).toEqual(true);
});
});
Empty file.
12 changes: 12 additions & 0 deletions __tests__/fixtures/install/workspaces-install-bin/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "my-project",
"private": true,
"bin": {"file": "file.js"},
"dependencies": {},
"devDependencies": {
"touch": "^1.0.0"
},
"workspaces": [
"packages/*"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "workspace-1",
"version": "1.0.0",
"bin": "./bin.js",
"devDependencies": {
"left-pad": "1.0.0"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "workspace-2",
"version": "1.0.0",
"dependencies": {
"workspace-1": "^1.0.0"
},
"devDependencies": {
"rimraf": "^2.6.2"
}
}
90 changes: 90 additions & 0 deletions __tests__/fixtures/install/workspaces-install-bin/yarn.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


abbrev@1:
version "1.1.1"
resolved "https://registry.yarnpkg.com/abbrev/-/abbrev-1.1.1.tgz#f8f2c887ad10bf67f634f005b6987fed3179aac8"

balanced-match@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/balanced-match/-/balanced-match-1.0.0.tgz#89b4d199ab2bee49de164ea02b89ce462d71b767"

brace-expansion@^1.1.7:
version "1.1.8"
resolved "https://registry.yarnpkg.com/brace-expansion/-/brace-expansion-1.1.8.tgz#c07b211c7c952ec1f8efd51a77ef0d1d3990a292"
dependencies:
balanced-match "^1.0.0"
concat-map "0.0.1"

concat-map@0.0.1:
version "0.0.1"
resolved "https://registry.yarnpkg.com/concat-map/-/concat-map-0.0.1.tgz#d8a96bd77fd68df7793a73036a3ba0d5405d477b"

fs.realpath@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/fs.realpath/-/fs.realpath-1.0.0.tgz#1504ad2523158caa40db4a2787cb01411994ea4f"

glob@^7.0.5:
version "7.1.2"
resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.2.tgz#c19c9df9a028702d678612384a6552404c636d15"
dependencies:
fs.realpath "^1.0.0"
inflight "^1.0.4"
inherits "2"
minimatch "^3.0.4"
once "^1.3.0"
path-is-absolute "^1.0.0"

inflight@^1.0.4:
version "1.0.6"
resolved "https://registry.yarnpkg.com/inflight/-/inflight-1.0.6.tgz#49bd6331d7d02d0c09bc910a1075ba8165b56df9"
dependencies:
once "^1.3.0"
wrappy "1"

inherits@2:
version "2.0.3"
resolved "https://registry.yarnpkg.com/inherits/-/inherits-2.0.3.tgz#633c2c83e3da42a502f52466022480f4208261de"

left-pad@1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/left-pad/-/left-pad-1.0.0.tgz#c84e2417581bbb8eaf2b9e3d7a122e572ab1af37"

minimatch@^3.0.4:
version "3.0.4"
resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-3.0.4.tgz#5166e286457f03306064be5497e8dbb0c3d32083"
dependencies:
brace-expansion "^1.1.7"

nopt@~1.0.10:
version "1.0.10"
resolved "https://registry.yarnpkg.com/nopt/-/nopt-1.0.10.tgz#6ddd21bd2a31417b92727dd585f8a6f37608ebee"
dependencies:
abbrev "1"

once@^1.3.0:
version "1.4.0"
resolved "https://registry.yarnpkg.com/once/-/once-1.4.0.tgz#583b1aa775961d4b113ac17d9c50baef9dd76bd1"
dependencies:
wrappy "1"

path-is-absolute@^1.0.0:
version "1.0.1"
resolved "https://registry.yarnpkg.com/path-is-absolute/-/path-is-absolute-1.0.1.tgz#174b9268735534ffbc7ace6bf53a5a9e1b5c5f5f"

rimraf@^2.6.2:
version "2.6.2"
resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-2.6.2.tgz#2ed8150d24a16ea8651e6d6ef0f47c4158ce7a36"
dependencies:
glob "^7.0.5"

touch@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/touch/-/touch-1.0.0.tgz#449cbe2dbae5a8c8038e30d71fa0ff464947c4de"
dependencies:
nopt "~1.0.10"

wrappy@1:
version "1.0.2"
resolved "https://registry.yarnpkg.com/wrappy/-/wrappy-1.0.2.tgz#b5243d8f3ec1aa35f1364605bc0d1036e30ab69f"
Binary file not shown.
Binary file not shown.
2 changes: 1 addition & 1 deletion src/package-linker.js
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ export default class PackageLinker {
topLevelDependencies,
async ([dest, pkg]) => {
if (pkg._reference && pkg._reference.location && pkg.bin && Object.keys(pkg.bin).length) {
const binLoc = path.join(this.config.cwd, this.config.getFolder(pkg));
const binLoc = path.join(this.config.lockfileFolder, this.config.getFolder(pkg));
await this.linkSelfDependencies(pkg, dest, binLoc);
tickBin();
}
Expand Down