From 743c145d881e52afaf7bc086762feb16fa2dd2f7 Mon Sep 17 00:00:00 2001 From: Ryan Burrows Date: Tue, 9 Oct 2018 06:33:31 -0700 Subject: [PATCH] Fix: handle empty bin strings (#6515) * 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 * Add an entry to the changelog * Add a test to make sure empty binlinks aren't created Test was suggested/provided by @rally25rs * Fix empty bin string test on Windows --- CHANGELOG.md | 4 ++++ __tests__/__snapshots__/normalize-manifest.js.snap | 6 ++++++ __tests__/commands/install/bin-links.js | 10 ++++++++++ .../install/install-empty-bin/depA/package.json | 5 +++++ .../fixtures/install/install-empty-bin/depB/depb.js | 2 ++ .../install/install-empty-bin/depB/package.json | 5 +++++ .../fixtures/install/install-empty-bin/package.json | 9 +++++++++ .../normalize-manifest/empty bin string/actual.json | 4 ++++ .../normalize-manifest/empty bin string/expected.json | 5 +++++ src/util/normalize-manifest/fix.js | 2 +- 10 files changed, 51 insertions(+), 1 deletion(-) 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 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/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) 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__/commands/install/bin-links.js b/__tests__/commands/install/bin-links.js index 851b72f37d..420013dc66 100644 --- a/__tests__/commands/install/bin-links.js +++ b/__tests__/commands/install/bin-links.js @@ -175,6 +175,16 @@ 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')); + const linkCount = process.platform === 'win32' ? 2 : 1; + expect(binScripts).toHaveLength(linkCount); + + 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" + } +} 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};