From 2e418d1415fd5e655c96922fde438c73019ac95f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Sat, 24 Jun 2017 20:31:09 +0100 Subject: [PATCH] feat(local): improve the behavior when calling ./local paths (#48) Fixes: #49 BREAKING CHANGE: `npx ./something` will now execute `./something` as a binary or script instead of trying to install it as npm would. Other behavior related to local path deps has likewise been changed. See [#49](https://github.com/zkat/npx/issues/49) for a detailed explanation of all the various cases and how each of them is handled. --- child.js | 2 ++ index.js | 56 +++++++++++++++++++++++++++----------- parse-args.js | 19 +++++++++---- test/guess-command-name.js | 13 +++++---- test/index.js | 12 +++++++- test/parse-args.js | 16 +++++++++++ 6 files changed, 91 insertions(+), 27 deletions(-) diff --git a/child.js b/child.js index 75ecb64..c2aa53e 100644 --- a/child.js +++ b/child.js @@ -18,6 +18,8 @@ function runCommand (command, opts) { }` ) err.exitCode = 127 + } else { + err.message = require('./y.js')`Command failed: ${cmd} ${err.message}` } throw err }) diff --git a/index.js b/index.js index 5612a96..0e7c9eb 100755 --- a/index.js +++ b/index.js @@ -123,7 +123,11 @@ function ensurePackages (specs, opts) { module.exports._getExistingPath = getExistingPath function getExistingPath (command, opts) { - if (opts.cmdHadVersion || opts.packageRequested || opts.ignoreExisting) { + if (opts.isLocal) { + return Promise.resolve(command) + } else if ( + opts.cmdHadVersion || opts.packageRequested || opts.ignoreExisting + ) { return Promise.resolve(false) } else { return which(command).catch(err => { @@ -182,9 +186,9 @@ function installPackages (specs, prefix, opts) { module.exports._execCommand = execCommand function execCommand (_existing, argv) { - return findNodeScript(_existing).then(existing => { + return findNodeScript(_existing, argv).then(existing => { const Module = require('module') - if (existing && Module.runMain && !argv.shell) { + if (existing && Module.runMain && !argv.shell && existing !== __filename) { // let it take over the process. This means we can skip node startup! if (!argv.noYargs) { // blow away built-up yargs crud @@ -211,22 +215,42 @@ function execCommand (_existing, argv) { } module.exports._findNodeScript = findNodeScript -function findNodeScript (existing) { +function findNodeScript (existing, opts) { if (!existing || process.platform === 'win32') { return Promise.resolve(false) } else { - // NOTE: only *nix is supported for process-replacement juggling - const line = '#!/usr/bin/env node\n' - const bytecount = line.length - const buf = Buffer.alloc(bytecount) - return promisify(fs.open)(existing, 'r').then(fd => { - return promisify(fs.read)(fd, buf, 0, bytecount, 0).then(() => { - return promisify(fs.close)(fd) - }, err => { - return promisify(fs.close)(fd).then(() => { throw err }) - }) - }).then(() => { - return buf.toString('utf8') === line && existing + return promisify(fs.stat)(existing).then(stat => { + if (opts && opts.isLocal && path.extname(existing) === '.js') { + return existing + } else if (opts && opts.isLocal && stat.isDirectory()) { + // npx will execute the directory itself + try { + const pkg = require(path.resolve(existing, 'package.json')) + const target = path.resolve(existing, pkg.bin || pkg.main || 'index.js') + return findNodeScript(target, opts).then(script => { + if (script) { + return script + } else { + throw new Error(Y()`command not found: ${target}`) + } + }) + } catch (e) { + throw new Error(Y()`command not found: ${existing}`) + } + } else { + const line = '#!/usr/bin/env node\n' + const bytecount = line.length + const buf = Buffer.alloc(bytecount) + return promisify(fs.open)(existing, 'r').then(fd => { + return promisify(fs.read)(fd, buf, 0, bytecount, 0).then(() => { + return promisify(fs.close)(fd) + }, err => { + return promisify(fs.close)(fd).then(() => { throw err }) + }) + }).then(() => { + return buf.toString('utf8') === line && existing + }) + } }) } } diff --git a/parse-args.js b/parse-args.js index 165ec80..c7c7c50 100644 --- a/parse-args.js +++ b/parse-args.js @@ -39,15 +39,16 @@ function parseArgs (argv) { if (cmdIndex) { const parsed = parser.parse(argv.slice(0, cmdIndex)) const parsedCmd = npa(argv[cmdIndex]) - parsed.command = parsed.package + parsed.command = parsed.package && parsedCmd.type !== 'directory' ? argv[cmdIndex] : guessCmdName(parsedCmd) + parsed.isLocal = parsedCmd.type === 'directory' parsed.cmdOpts = argv.slice(cmdIndex + 1) if (typeof parsed.package === 'string') { parsed.package = [parsed.package] } parsed.packageRequested = !!parsed.package - parsed.cmdHadVersion = parsed.package + parsed.cmdHadVersion = parsed.package || parsedCmd.type === 'directory' ? false : parsedCmd.name !== parsedCmd.raw const pkg = parsed.package || [argv[cmdIndex]] @@ -95,13 +96,21 @@ function fastPathArgs (argv) { } else { npa = require('npm-package-arg') parsedCmd = npa(argv[2]) - pkg = [parsedCmd.toString()] + if (parsedCmd.type === 'directory') { + pkg = [] + } else { + pkg = [parsedCmd.toString()] + } } return { command: guessCmdName(parsedCmd), cmdOpts: argv.slice(3), packageRequested: false, - cmdHadVersion: parsedCmd.name !== parsedCmd.raw, + isLocal: parsedCmd.type === 'directory', + cmdHadVersion: ( + parsedCmd.name !== parsedCmd.raw && + parsedCmd.type !== 'directory' + ), package: pkg, p: pkg, shell: false, @@ -129,7 +138,7 @@ function guessCmdName (spec) { const match = spec.fetchSpec.match(/([a-z0-9-]+)(?:\.git)?$/i) return match[1] } else if (spec.type === 'directory') { - return path.basename(spec.fetchSpec) + return spec.raw } else if (spec.type === 'file' || spec.type === 'remote') { let ext = path.extname(spec.fetchSpec) if (ext === '.gz') { diff --git a/test/guess-command-name.js b/test/guess-command-name.js index 454df8c..80598b5 100644 --- a/test/guess-command-name.js +++ b/test/guess-command-name.js @@ -26,11 +26,14 @@ test('guesses git binaries', t => { t.done() }) -test('guesses local directory binaries', t => { - t.equal(guessCmdName('./foo'), 'foo') - t.equal(guessCmdName('./dir/foo'), 'foo') - t.equal(guessCmdName('../../../dir/foo'), 'foo') - t.equal(guessCmdName('C:\\Program Files\\node\\foo'), 'foo') +test('leaves local directory/file commands intact', t => { + t.equal(guessCmdName('./foo'), './foo') + t.equal(guessCmdName('./dir/foo'), './dir/foo') + t.equal(guessCmdName('../../../dir/foo'), '../../../dir/foo') + t.equal( + guessCmdName('C:\\Program Files\\node\\foo'), + 'C:\\Program Files\\node\\foo' + ) t.done() }) diff --git a/test/index.js b/test/index.js index e1d40d3..02961f0 100644 --- a/test/index.js +++ b/test/index.js @@ -164,7 +164,8 @@ test('getNpmCache', t => { }) test('findNodeScript', t => { - const scriptPath = path.resolve(__dirname, '..', 'index.js') + const scriptDir = path.resolve(__dirname, '..') + const scriptPath = path.join(scriptDir, 'index.js') return main._findNodeScript(scriptPath).then(script => { if (process.platform === 'win32') { t.notOk(script, 'win32 never detects Node scripts like this') @@ -178,9 +179,18 @@ test('findNodeScript', t => { return main._findNodeScript(null).then(bool => { t.notOk(bool, 'no node script found if existing is null') }) + }).then(() => { + return main._findNodeScript(scriptDir, {isLocal: true}).then(script => { + t.equal(script, scriptPath, 'resolved dir dep to index.js') + }) }).then(() => { const findScript = requireInject('../index.js', { fs: { + stat (file, cb) { + cb(null, { + isDirectory () { return !file.indexOf('./') } + }) + }, open (file, perm, cb) { cb(null, file) }, diff --git a/test/parse-args.js b/test/parse-args.js index de6bf13..33a77a6 100644 --- a/test/parse-args.js +++ b/test/parse-args.js @@ -140,3 +140,19 @@ test('allows configuration of npm binary', t => { t.equal(parsed.npm, './mynpm') t.done() }) + +test('treats directory-type commands specially', t => { + let parsed = parseArgs(['/node', '/npx', './foo']) + t.equal(parsed.command, './foo') + t.deepEqual(parsed.package, []) + t.equal(parsed.packageRequested, false) + t.equal(parsed.cmdHadVersion, false) + t.ok(parsed.isLocal) + parsed = parseArgs(['/node', '/npx', '-p', 'x', '../foo/bar.sh']) + t.equal(parsed.command, '../foo/bar.sh') + t.ok(parsed.isLocal) + t.deepEqual(parsed.package, ['x@latest']) + t.equal(parsed.packageRequested, true) + t.equal(parsed.cmdHadVersion, false) + t.done() +})