From 88759f171a17ec1bed9c98517e9c876259fc656b Mon Sep 17 00:00:00 2001 From: Ryan Burrows Date: Sun, 7 Oct 2018 16:52:41 -0700 Subject: [PATCH 1/4] Fix: handle empty bin strings When the package.json of a dependency sets `bin` to an empty string, do not normalize it. This prevents creating a symlink to the dependency's root directory within the `.bin` directory if bin-links are enabled --- __tests__/__snapshots__/normalize-manifest.js.snap | 6 ++++++ .../normalize-manifest/empty bin string/actual.json | 4 ++++ .../normalize-manifest/empty bin string/expected.json | 5 +++++ src/util/normalize-manifest/fix.js | 2 +- 4 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 __tests__/fixtures/normalize-manifest/empty bin string/actual.json create mode 100644 __tests__/fixtures/normalize-manifest/empty bin string/expected.json diff --git a/__tests__/__snapshots__/normalize-manifest.js.snap b/__tests__/__snapshots__/normalize-manifest.js.snap index 6485c0b6fa..c5a78707da 100644 --- a/__tests__/__snapshots__/normalize-manifest.js.snap +++ b/__tests__/__snapshots__/normalize-manifest.js.snap @@ -90,6 +90,12 @@ Array [ ] `; +exports[`empty bin string: empty bin string 1`] = ` +Array [ + "foo: No license field", +] +`; + exports[`engines array: engines array 1`] = ` Array [ "No license field", diff --git a/__tests__/fixtures/normalize-manifest/empty bin string/actual.json b/__tests__/fixtures/normalize-manifest/empty bin string/actual.json new file mode 100644 index 0000000000..b065b9ea1e --- /dev/null +++ b/__tests__/fixtures/normalize-manifest/empty bin string/actual.json @@ -0,0 +1,4 @@ +{ + "name": "foo", + "bin": "" +} diff --git a/__tests__/fixtures/normalize-manifest/empty bin string/expected.json b/__tests__/fixtures/normalize-manifest/empty bin string/expected.json new file mode 100644 index 0000000000..7c894d5fd8 --- /dev/null +++ b/__tests__/fixtures/normalize-manifest/empty bin string/expected.json @@ -0,0 +1,5 @@ +{ + "name": "foo", + "version": "", + "bin": "" +} diff --git a/src/util/normalize-manifest/fix.js b/src/util/normalize-manifest/fix.js index 53845efead..95d19262a1 100644 --- a/src/util/normalize-manifest/fix.js +++ b/src/util/normalize-manifest/fix.js @@ -153,7 +153,7 @@ export default (async function( // if the `bin` field is as string then expand it to an object with a single property // based on the original `bin` field and `name field` // { name: "foo", bin: "cli.js" } -> { name: "foo", bin: { foo: "cli.js" } } - if (typeof info.name === 'string' && typeof info.bin === 'string') { + if (typeof info.name === 'string' && typeof info.bin === 'string' && info.bin.length > 0) { // Remove scoped package name for consistency with NPM's bin field fixing behaviour const name = info.name.replace(/^@[^\/]+\//, ''); info.bin = {[name]: info.bin}; From 610daba973ae4a8dbe0352a5c751b5a1d7066efc Mon Sep 17 00:00:00 2001 From: Ryan Burrows Date: Sun, 7 Oct 2018 23:31:16 -0700 Subject: [PATCH 2/4] Add an entry to the changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b357ceb707..c4b72dd357 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ Please add one entry in this file for each change in Yarn's behavior. Use the sa ## Master +- Fixes handling of empty string entries for bin in package.json + + [#6515](https://github.com/yarnpkg/yarn/pull/6515) - [**Ryan Burrows**](https://github.com/rhburrows) + - Adds support for basic auth for registries with paths, such as artifactory [#5322](https://github.com/yarnpkg/yarn/pull/5322) - [**Karolis Narkevicius**](https://twitter.com/KidkArolis) From 3571127af7a06ccbe2abda5f70eccc1cbe998480 Mon Sep 17 00:00:00 2001 From: Ryan Burrows Date: Mon, 8 Oct 2018 06:57:59 -0700 Subject: [PATCH 3/4] Add a test to make sure empty binlinks aren't created Test was suggested/provided by @rally25rs --- __tests__/commands/install/bin-links.js | 9 +++++++++ .../fixtures/install/install-empty-bin/depA/package.json | 5 +++++ .../fixtures/install/install-empty-bin/depB/depb.js | 2 ++ .../fixtures/install/install-empty-bin/depB/package.json | 5 +++++ .../fixtures/install/install-empty-bin/package.json | 9 +++++++++ 5 files changed, 30 insertions(+) create mode 100644 __tests__/fixtures/install/install-empty-bin/depA/package.json create mode 100644 __tests__/fixtures/install/install-empty-bin/depB/depb.js create mode 100644 __tests__/fixtures/install/install-empty-bin/depB/package.json create mode 100644 __tests__/fixtures/install/install-empty-bin/package.json diff --git a/__tests__/commands/install/bin-links.js b/__tests__/commands/install/bin-links.js index 851b72f37d..7fe74d2fd4 100644 --- a/__tests__/commands/install/bin-links.js +++ b/__tests__/commands/install/bin-links.js @@ -175,6 +175,15 @@ test('can use link protocol to install a package that would not be found via nod }); }); +test('empty bin string does not create a link', (): Promise => { + return runInstall({binLinks: true}, 'install-empty-bin', async config => { + const binScripts = await fs.walk(path.join(config.cwd, 'node_modules', '.bin')); + expect(binScripts).toHaveLength(1); + + expect(await linkAt(config, 'node_modules', '.bin', 'depB')).toEqual('../depB/depb.js'); + }); +}); + describe('with nohoist', () => { // address https://github.com/yarnpkg/yarn/issues/5487 test('nohoist bin should be linked to its own local module', (): Promise => { diff --git a/__tests__/fixtures/install/install-empty-bin/depA/package.json b/__tests__/fixtures/install/install-empty-bin/depA/package.json new file mode 100644 index 0000000000..f333589ee3 --- /dev/null +++ b/__tests__/fixtures/install/install-empty-bin/depA/package.json @@ -0,0 +1,5 @@ +{ + "name": "depA", + "version": "0.0.0", + "bin": "" +} diff --git a/__tests__/fixtures/install/install-empty-bin/depB/depb.js b/__tests__/fixtures/install/install-empty-bin/depB/depb.js new file mode 100644 index 0000000000..95230d6179 --- /dev/null +++ b/__tests__/fixtures/install/install-empty-bin/depB/depb.js @@ -0,0 +1,2 @@ +#!/usr/bin/env node +console.log('depB'); diff --git a/__tests__/fixtures/install/install-empty-bin/depB/package.json b/__tests__/fixtures/install/install-empty-bin/depB/package.json new file mode 100644 index 0000000000..c2832b9228 --- /dev/null +++ b/__tests__/fixtures/install/install-empty-bin/depB/package.json @@ -0,0 +1,5 @@ +{ + "name": "depB", + "version": "0.0.0", + "bin": "./depb.js" +} diff --git a/__tests__/fixtures/install/install-empty-bin/package.json b/__tests__/fixtures/install/install-empty-bin/package.json new file mode 100644 index 0000000000..7e59f4f342 --- /dev/null +++ b/__tests__/fixtures/install/install-empty-bin/package.json @@ -0,0 +1,9 @@ +{ + "name": "test", + "version": "0.0.0", + "bin": "", + "dependencies": { + "depA": "file:depA", + "depB": "file:depB" + } +} From ad9420f4e65a4d371c26340bf3d4255453e31457 Mon Sep 17 00:00:00 2001 From: Ryan Burrows Date: Mon, 8 Oct 2018 17:31:14 -0700 Subject: [PATCH 4/4] Fix empty bin string test on Windows --- __tests__/commands/install/bin-links.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/__tests__/commands/install/bin-links.js b/__tests__/commands/install/bin-links.js index 7fe74d2fd4..420013dc66 100644 --- a/__tests__/commands/install/bin-links.js +++ b/__tests__/commands/install/bin-links.js @@ -178,7 +178,8 @@ test('can use link protocol to install a package that would not be found via nod test('empty bin string does not create a link', (): Promise => { return runInstall({binLinks: true}, 'install-empty-bin', async config => { const binScripts = await fs.walk(path.join(config.cwd, 'node_modules', '.bin')); - expect(binScripts).toHaveLength(1); + const linkCount = process.platform === 'win32' ? 2 : 1; + expect(binScripts).toHaveLength(linkCount); expect(await linkAt(config, 'node_modules', '.bin', 'depB')).toEqual('../depB/depb.js'); });