From b9119c5b50e40f4ec7277e92bdfd0f54f2a388a5 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 21 Mar 2021 00:04:20 +0200 Subject: [PATCH] fix: installing packages that have bin dirs with subdirs (#3265) close #3263 close #2332 --- .changeset/four-wombats-clap.md | 5 +++ .changeset/khaki-humans-jump.md | 5 +++ .changeset/kind-cats-love.md | 5 +++ .changeset/tricky-carrots-sip.md | 5 +++ packages/lifecycle/src/index.ts | 22 ++++++---- .../link-bins/test/{fixtures => }/.gitignore | 1 + .../fixtures/bin-dir/pkg/bin/subdir/index.js | 1 + .../test/fixtures/bin-dir/pkg/package.json | 7 +++ packages/link-bins/test/index.ts | 8 ++++ packages/package-bins/jest.config.js | 6 ++- packages/package-bins/package.json | 4 +- packages/package-bins/src/index.ts | 20 ++++----- .../test/fixtures/bin-dir/rootBin.js | 1 + .../test/fixtures/bin-dir/subdir/subBin.js | 1 + packages/package-bins/test/index.ts | 22 ++++++++++ packages/read-package-json/package.json | 3 +- packages/read-package-json/src/index.ts | 10 ++--- packages/read-project-manifest/src/index.ts | 7 +-- pnpm-lock.yaml | 43 +++---------------- 19 files changed, 106 insertions(+), 70 deletions(-) create mode 100644 .changeset/four-wombats-clap.md create mode 100644 .changeset/khaki-humans-jump.md create mode 100644 .changeset/kind-cats-love.md create mode 100644 .changeset/tricky-carrots-sip.md rename packages/link-bins/test/{fixtures => }/.gitignore (61%) create mode 100644 packages/link-bins/test/fixtures/bin-dir/pkg/bin/subdir/index.js create mode 100644 packages/link-bins/test/fixtures/bin-dir/pkg/package.json create mode 100644 packages/package-bins/test/fixtures/bin-dir/rootBin.js create mode 100644 packages/package-bins/test/fixtures/bin-dir/subdir/subBin.js diff --git a/.changeset/four-wombats-clap.md b/.changeset/four-wombats-clap.md new file mode 100644 index 0000000000..01227847ea --- /dev/null +++ b/.changeset/four-wombats-clap.md @@ -0,0 +1,5 @@ +--- +"@pnpm/package-bins": minor +--- + +Find all files inside the bin directory. diff --git a/.changeset/khaki-humans-jump.md b/.changeset/khaki-humans-jump.md new file mode 100644 index 0000000000..a9df678112 --- /dev/null +++ b/.changeset/khaki-humans-jump.md @@ -0,0 +1,5 @@ +--- +"@pnpm/lifecycle": patch +--- + +Run `node-gyp` when `binding.gyp` is present, even if an install lifecycle script is not present in the scripts field. diff --git a/.changeset/kind-cats-love.md b/.changeset/kind-cats-love.md new file mode 100644 index 0000000000..a257ff6671 --- /dev/null +++ b/.changeset/kind-cats-love.md @@ -0,0 +1,5 @@ +--- +"@pnpm/link-bins": patch +--- + +Don't fail when linking bins of a package that uses the `directories.bin` and points to a directory that has subdirectories. diff --git a/.changeset/tricky-carrots-sip.md b/.changeset/tricky-carrots-sip.md new file mode 100644 index 0000000000..13c9b8aaa6 --- /dev/null +++ b/.changeset/tricky-carrots-sip.md @@ -0,0 +1,5 @@ +--- +"@pnpm/read-package-json": major +--- + +Don't normalize the `bin` field of `package.json`. diff --git a/packages/lifecycle/src/index.ts b/packages/lifecycle/src/index.ts index d18d180ce8..c0c310ff66 100644 --- a/packages/lifecycle/src/index.ts +++ b/packages/lifecycle/src/index.ts @@ -23,27 +23,31 @@ export async function runPostinstallHooks ( } ): Promise { const pkg = await readPackageJsonFromDir(opts.pkgRoot) - const scripts = pkg?.scripts ?? {} - - if (!scripts.install) { - await checkBindingGyp(opts.pkgRoot, scripts) + if (pkg.scripts == null) { + pkg.scripts = {} } - if (scripts.preinstall) { + if (!pkg.scripts.install) { + await checkBindingGyp(opts.pkgRoot, pkg.scripts) + } + + if (pkg.scripts.preinstall) { await runLifecycleHook('preinstall', pkg, opts) } - if (scripts.install) { + if (pkg.scripts.install) { await runLifecycleHook('install', pkg, opts) } - if (scripts.postinstall) { + if (pkg.scripts.postinstall) { await runLifecycleHook('postinstall', pkg, opts) } - if (opts.prepare && scripts.prepare) { + if (opts.prepare && pkg.scripts.prepare) { await runLifecycleHook('prepare', pkg, opts) } - return !!scripts.preinstall || !!scripts.install || !!scripts.postinstall + return pkg.scripts.preinstall != null || + pkg.scripts.install != null || + pkg.scripts.postinstall != null } /** diff --git a/packages/link-bins/test/fixtures/.gitignore b/packages/link-bins/test/.gitignore similarity index 61% rename from packages/link-bins/test/fixtures/.gitignore rename to packages/link-bins/test/.gitignore index fc97489343..c5770b472a 100644 --- a/packages/link-bins/test/fixtures/.gitignore +++ b/packages/link-bins/test/.gitignore @@ -1,2 +1,3 @@ !node_modules node_modules/.bin +fixtures_for_testing \ No newline at end of file diff --git a/packages/link-bins/test/fixtures/bin-dir/pkg/bin/subdir/index.js b/packages/link-bins/test/fixtures/bin-dir/pkg/bin/subdir/index.js new file mode 100644 index 0000000000..908ba8417a --- /dev/null +++ b/packages/link-bins/test/fixtures/bin-dir/pkg/bin/subdir/index.js @@ -0,0 +1 @@ +#!/usr/bin/env node diff --git a/packages/link-bins/test/fixtures/bin-dir/pkg/package.json b/packages/link-bins/test/fixtures/bin-dir/pkg/package.json new file mode 100644 index 0000000000..38f5876552 --- /dev/null +++ b/packages/link-bins/test/fixtures/bin-dir/pkg/package.json @@ -0,0 +1,7 @@ +{ + "name": "bin-dir", + "version": "0.0.0", + "directories": { + "bin": "bin" + } +} \ No newline at end of file diff --git a/packages/link-bins/test/index.ts b/packages/link-bins/test/index.ts index 8bacfa35d2..cd89519866 100644 --- a/packages/link-bins/test/index.ts +++ b/packages/link-bins/test/index.ts @@ -260,3 +260,11 @@ test('linkBins() would not give warning if package has no bin field but inside n expect(warn).not.toHaveBeenCalled() }) + +test('linkBins() links commands from bin directory with a subdirectory', async () => { + const binTarget = tempy.directory() + + await linkBins(path.join(fixtures, 'bin-dir'), binTarget, { warn: () => {} }) + + expect(await fs.readdir(binTarget)).toEqual(getExpectedBins(['index.js'])) +}) \ No newline at end of file diff --git a/packages/package-bins/jest.config.js b/packages/package-bins/jest.config.js index f697d83169..9f9f8710fd 100644 --- a/packages/package-bins/jest.config.js +++ b/packages/package-bins/jest.config.js @@ -1 +1,5 @@ -module.exports = require('../../jest.config.js') +const config = require('../../jest.config.js') +module.exports = Object.assign({}, config, { + // Shallow so fixtures aren't matched + testMatch: ["**/test/*.[jt]s?(x)"] +}) diff --git a/packages/package-bins/package.json b/packages/package-bins/package.json index ea1489a445..6ff32a007e 100644 --- a/packages/package-bins/package.json +++ b/packages/package-bins/package.json @@ -32,8 +32,8 @@ "homepage": "https://github.com/pnpm/pnpm/blob/master/packages/package-bins#readme", "dependencies": { "@pnpm/types": "workspace:6.4.0", - "is-subdir": "^1.1.1", - "p-filter": "^2.1.0" + "fast-glob": "^3.2.4", + "is-subdir": "^1.1.1" }, "devDependencies": { "@types/node": "^14.14.33" diff --git a/packages/package-bins/src/index.ts b/packages/package-bins/src/index.ts index 6d91256a66..f2f2c98c07 100644 --- a/packages/package-bins/src/index.ts +++ b/packages/package-bins/src/index.ts @@ -1,8 +1,7 @@ -import { promises as fs } from 'fs' import path from 'path' import { DependencyManifest, PackageBin } from '@pnpm/types' +import fastGlob from 'fast-glob' import isSubdir from 'is-subdir' -import pFilter from 'p-filter' export interface Command { name: string @@ -16,20 +15,21 @@ export default async function binify (manifest: DependencyManifest, pkgPath: str if (manifest.directories?.bin) { const binDir = path.join(pkgPath, manifest.directories.bin) const files = await findFiles(binDir) - return pFilter( - files.map((file) => ({ - name: file, - path: path.join(binDir, file), - })), - async (cmd: Command) => (await fs.stat(cmd.path)).isFile() - ) + return files.map((file) => ({ + name: path.basename(file), + path: path.join(binDir, file), + })) } return [] } async function findFiles (dir: string): Promise { try { - return await fs.readdir(dir) + return await fastGlob('**', { + cwd: dir, + onlyFiles: true, + followSymbolicLinks: false, + }) } catch (err) { if ((err as NodeJS.ErrnoException).code !== 'ENOENT') { throw err diff --git a/packages/package-bins/test/fixtures/bin-dir/rootBin.js b/packages/package-bins/test/fixtures/bin-dir/rootBin.js new file mode 100644 index 0000000000..090ff6ecf9 --- /dev/null +++ b/packages/package-bins/test/fixtures/bin-dir/rootBin.js @@ -0,0 +1 @@ +#!/usr/bin/env node \ No newline at end of file diff --git a/packages/package-bins/test/fixtures/bin-dir/subdir/subBin.js b/packages/package-bins/test/fixtures/bin-dir/subdir/subBin.js new file mode 100644 index 0000000000..090ff6ecf9 --- /dev/null +++ b/packages/package-bins/test/fixtures/bin-dir/subdir/subBin.js @@ -0,0 +1 @@ +#!/usr/bin/env node \ No newline at end of file diff --git a/packages/package-bins/test/index.ts b/packages/package-bins/test/index.ts index 0ef65794f2..33515a6a1f 100644 --- a/packages/package-bins/test/index.ts +++ b/packages/package-bins/test/index.ts @@ -15,6 +15,28 @@ test('getBinsFromPkg()', async () => { ) }) +test('find all the bin files from a bin directory', async () => { + const fixtures = path.join(__dirname, 'fixtures') + expect( + await getBinsFromPkg({ + name: 'bin-dir', + version: '1.0.0', + + directories: { bin: 'bin-dir' }, + }, fixtures)).toStrictEqual( + [ + { + name: 'rootBin.js', + path: path.join(fixtures, 'bin-dir/rootBin.js'), + }, + { + name: 'subBin.js', + path: path.join(fixtures, 'bin-dir/subdir/subBin.js'), + }, + ] + ) +}) + test('get bin of scoped package', async () => { expect( await getBinsFromPkg({ diff --git a/packages/read-package-json/package.json b/packages/read-package-json/package.json index 634524756f..f30c68c334 100644 --- a/packages/read-package-json/package.json +++ b/packages/read-package-json/package.json @@ -32,7 +32,8 @@ "dependencies": { "@pnpm/error": "workspace:1.4.0", "@pnpm/types": "workspace:6.4.0", - "read-package-json": "^3.0.0" + "load-json-file": "^6.2.0", + "normalize-package-data": "^3.0.2" }, "funding": "https://opencollective.com/pnpm" } diff --git a/packages/read-package-json/src/index.ts b/packages/read-package-json/src/index.ts index d28da7d2f9..2ad2030835 100644 --- a/packages/read-package-json/src/index.ts +++ b/packages/read-package-json/src/index.ts @@ -1,14 +1,14 @@ -import { promisify } from 'util' import path from 'path' import PnpmError from '@pnpm/error' import { PackageManifest } from '@pnpm/types' -import readPackageManifestCB from 'read-package-json' - -const readPackageManifest = promisify(readPackageManifestCB) +import loadJsonFile from 'load-json-file' +import normalizePackageData from 'normalize-package-data' export default async function readPkg (pkgPath: string): Promise { try { - return await readPackageManifest(pkgPath) + const manifest = await loadJsonFile(pkgPath) + normalizePackageData(manifest) + return manifest } catch (err: any) { // eslint-disable-line if (err.code) throw err throw new PnpmError('BAD_PACKAGE_JSON', `${pkgPath}: ${err.message as string}`) diff --git a/packages/read-project-manifest/src/index.ts b/packages/read-project-manifest/src/index.ts index 96deb74f76..c9a5396057 100644 --- a/packages/read-project-manifest/src/index.ts +++ b/packages/read-project-manifest/src/index.ts @@ -1,5 +1,4 @@ -import fs, { Stats } from 'fs' -import { promisify } from 'util' +import { promises as fs, Stats } from 'fs' import path from 'path' import PnpmError from '@pnpm/error' import { ProjectManifest } from '@pnpm/types' @@ -15,8 +14,6 @@ import { readJsonFile, } from './readFile' -const stat = promisify(fs.stat) - type WriteProjectManifest = (manifest: ProjectManifest, force?: boolean) => Promise export async function safeReadProjectManifestOnly (projectDir: string) { @@ -102,7 +99,7 @@ export async function tryReadProjectManifest (projectDir: string): Promise<{ // ENOTDIR isn't used on Windows, but pnpm expects it. let s: Stats | undefined try { - s = await stat(projectDir) + s = await fs.stat(projectDir) } catch (err) { // Ignore } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3ce439d360..403d47d24b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1426,12 +1426,12 @@ importers: '@pnpm/package-bins': 'link:' '@pnpm/types': workspace:6.4.0 '@types/node': ^14.14.33 + fast-glob: ^3.2.4 is-subdir: ^1.1.1 - p-filter: ^2.1.0 dependencies: '@pnpm/types': link:../types + fast-glob: 3.2.5 is-subdir: 1.2.0 - p-filter: 2.1.0 devDependencies: '@pnpm/package-bins': 'link:' '@types/node': 14.14.35 @@ -2501,11 +2501,13 @@ importers: '@pnpm/error': workspace:1.4.0 '@pnpm/read-package-json': 'link:' '@pnpm/types': workspace:6.4.0 - read-package-json: ^3.0.0 + load-json-file: ^6.2.0 + normalize-package-data: ^3.0.2 dependencies: '@pnpm/error': link:../error '@pnpm/types': link:../types - read-package-json: 3.0.1 + load-json-file: 6.2.0 + normalize-package-data: 3.0.2 devDependencies: '@pnpm/read-package-json': 'link:' @@ -8469,19 +8471,11 @@ packages: lru-cache: 6.0.0 dev: false - /hosted-git-info/4.0.0: - resolution: {integrity: sha512-fqhGdjk4av7mT9fU/B01dUtZ+WZSc/XEXMoLXDVZukiQRXxeHSSz3AqbeWRJHtF8EQYHlAgB1NSAHU0Cm7aqZA==} - engines: {node: '>=10'} - dependencies: - lru-cache: 6.0.0 - dev: false - /hosted-git-info/4.0.1: resolution: {integrity: sha512-eT7NrxAsppPRQEBSwKSosReE+v8OzABwEScQYk5d4uxaEPlzxTIku7LINXtBGalthkLhJnq5lBI89PfK43zAKg==} engines: {node: '>=10'} dependencies: lru-cache: 6.0.0 - dev: true /html-encoding-sniffer/1.0.2: resolution: {integrity: sha512-71lZziiDnsuabfdYiUeWdCVyKuqwWi23L8YeIgV9jSSZHCtb6wB1BKWooH7L3tn4/FuZJMVWyNaIDr4RGmaSYw==} @@ -10802,16 +10796,6 @@ packages: semver: 5.7.1 validate-npm-package-license: 3.0.4 - /normalize-package-data/3.0.1: - resolution: {integrity: sha512-D/ttLdxo71msR4FF3VgSwK4blHfE3/vGByz1NCeE7/Dh8reQOKNJJjk5L10mLq9jxa+ZHzT1/HLgxljzbXE7Fw==} - engines: {node: '>=10'} - dependencies: - hosted-git-info: 4.0.0 - resolve: 1.20.0 - semver: 7.3.4 - validate-npm-package-license: 3.0.4 - dev: false - /normalize-package-data/3.0.2: resolution: {integrity: sha512-6CdZocmfGaKnIHPVFhJJZ3GuR8SsLKvDANFp47Jmy51aKIr8akjAWTSxtpI+MBgBFdSMRyo4hMpDlT6dTffgZg==} engines: {node: '>=10'} @@ -10820,7 +10804,6 @@ packages: resolve: 1.20.0 semver: 7.3.4 validate-npm-package-license: 3.0.4 - dev: true /normalize-path/2.1.1: resolution: {integrity: sha1-GrKLVW4Zg2Oowab35vogE3/mrtk=} @@ -10850,10 +10833,6 @@ packages: once: 1.4.0 dev: true - /npm-normalize-package-bin/1.0.1: - resolution: {integrity: sha512-EPfafl6JL5/rU+ot6P3gRSCpPDW5VmIzX959Ob1+ySFUuuYHWHekXpwdUZcKP5C+DS4GEtdJluwBjnsNDl+fSA==} - dev: false - /npm-package-arg/4.2.1: resolution: {integrity: sha1-WTMD/eqF98Qid18X+et2cPaA4+w=} dependencies: @@ -11832,16 +11811,6 @@ packages: strip-bom: 4.0.0 dev: false - /read-package-json/3.0.1: - resolution: {integrity: sha512-aLcPqxovhJTVJcsnROuuzQvv6oziQx4zd3JvG0vGCL5MjTONUc4uJ90zCBC6R7W7oUKBNoR/F8pkyfVwlbxqng==} - engines: {node: '>=10'} - dependencies: - glob: 7.1.6 - json-parse-even-better-errors: 2.3.1 - normalize-package-data: 3.0.1 - npm-normalize-package-bin: 1.0.1 - dev: false - /read-pkg-up/1.0.1: resolution: {integrity: sha1-nWPBMnbAZZGNV/ACpX9AobZD+wI=} engines: {node: '>=0.10.0'}