kimmobrunfeldt / spawn-default-shell

Spawn shell command with platform default shell
https://www.npmjs.com/package/spawn-default-shell
MIT License
12 stars 4 forks source link

local project node paths not included #3

Open czeckd opened 8 years ago

czeckd commented 8 years ago

This is related to concurrently 3.0.0 and the change to use spawn-default-shell.

Previously, the path to the local node_modules/.bin were added to the path, so in my case /users/david/myproject/node_modules/.bin: was added to the path. This allowed concurrently to find dependencies not installed globally.

Since the paths npm/node added are not part of the shell created by spawn-default-shell, locally installed node_module dependencies are failing with Command not found.

kimmobrunfeldt commented 8 years ago

Could you provide more details about the issue, I can't reproduce it. Which platform are you on? Which shell? How are you verifying the bug?

I just did this:

mkdir tmp && cd tmp
npm init  # answer yes to everything
npm i --save-dev browserify concurrently
# I don't have browserify installed globally

Add "start": "concurrently 'browserify'",. After that running npm start works as expected and it finds browserify executable.

czeckd commented 8 years ago

I'm running a RedHat 7 Linux derivative (Oracle Linux Server release 7.2). npm 3.7.3 node v5.7.0 tcsh shell

Running your test, these are my results:

~/tmp>npm start

> spawn-shell-test@1.0.0 start /users/david/tmp
> concurrently 'browserify'

[0] browserify: Command not found.
[0] browserify exited with code 1

npm ERR! Linux 3.8.13-98.7.1.el7uek.x86_64
npm ERR! argv "/usr/local/bin/node" "/usr/local/bin/npm" "start"
npm ERR! node v5.7.0
npm ERR! npm  v3.7.3
npm ERR! code ELIFECYCLE
npm ERR! spawn-shell-test@1.0.0 start: `concurrently 'browserify'`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the spawn-shell-test@1.0.0 start script 'concurrently 'browserify''.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the spawn-shell-test package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     concurrently 'browserify'
npm ERR! You can get information on how to open an issue for this project with:
npm ERR!     npm bugs spawn-shell-test
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls spawn-shell-test
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     /users/david/tmp/npm-debug.log

Switching to bash shell, it works as expected. So, this seems to a problem with tcsh being invoked.

I was verifying the path by echoing the path on the start line before calling concurrently: "start": "echo $PATH && concurrently ..." I was running the angular2 quickstart project, which uses: "start": "tsc && concurrently \"tsc -w\" \"lite-server\" ",

And then in get-shell.js before the shell is returned, log out the env.PATH, by console.log(env.PATH);. The paths are different. What was missing from the spawned shell were /usr/lib/node_modules/npm/bin/node-gyp-bin:/users/david/quickstart/node_modules/.bin: in the path and concurrently wasn't then able to find the typescript tsc or lite-server that are symbolically like din the node_modules/.bin directory.

kimmobrunfeldt commented 8 years ago

Ok, let's see if fixing this: https://github.com/kimmobrunfeldt/concurrently/issues/61#issuecomment-250849768 fixes your issue too.

kimmobrunfeldt commented 8 years ago

I tried to install the angular2 quickstart project on my computer and it starts fine with concurrently. I tried running npm start inside zsh, bash, tcsh, sh shells and all of those seemed to work correctly.

Are you sure you have browserify installed locally (npm i browserify --save-dev)? npm should set the node_modules/.bin to $PATH, and concurrently uses child_process.spawn in the end which passes all environment variables to the spawned process. This means that also the new PATH should be correctly forwarded to concurrently.

You can test the PATH like this:

  1. Add new npm script: "path": "concurrently \"echo $PATH\"",
  2. Run npm run path

For me it outputs:

> angular-quickstart@1.0.0 path /Users/kbru/code/personal/my-proj
> concurrently "echo $PATH"

[0] /Users/kbru/.nvm/versions/node/v6.3.1/lib/node_modules/npm/bin/node-gyp-bin:/Users/kbru/code/personal/my-proj/node_modules/.bin ...

my-proj is a angular quickstart example project and you can see that node_modules/.bin is added to PATH by npm.

czeckd commented 8 years ago

Here's a new run on a different box, with a slightly older npm/node set-up. npm@3.7.2 node@v5.6.0

~/projects> mkdir tmp2 && cd tmp2

~/projects/tmp2> npm init
This utility will walk you through creating a package.json file.
It only covers the most common items, and tries to guess sensible defaults.

See `npm help json` for definitive documentation on these fields
and exactly what they do.

Use `npm install <pkg> --save` afterwards to install a package and
save it as a dependency in the package.json file.

Press ^C at any time to quit.
name: (tmp2) tmp2
version: (1.0.0) 
description: 
entry point: (index.js) 
test command: 
git repository: 
keywords: 
author: 
license: (ISC) 
About to write to /users/dczeck/projects/tmp2/package.json:

{
  "name": "tmp2",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC"
}

Is this ok? (yes) y
~/projects/tmp2> npm i --save-dev browserify concurrently
tmp2@1.0.0 /users/dczeck/projects/tmp2

[...]

npm WARN tmp2@1.0.0 No description
npm WARN tmp2@1.0.0 No repository field.

~/projects/tmp2>vi package.json
~/projects/tmp2> cat package.json
{
  "name": "tmp2",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "start": "concurrently 'browserify'",
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "devDependencies": {
    "browserify": "^13.1.0",
    "concurrently": "^3.0.0"
  }
}

Test as before.

~/projects/tmp2> echo $SHELL
/bin/tcsh

~/projects/tmp2> npm start

> tmp2@1.0.0 start /users/dczeck/projects/tmp2
> concurrently 'browserify'

[0] browserify: Command not found.
[0] browserify exited with code 1

npm ERR! Linux 3.10.0-229.el7.x86_64
npm ERR! argv "/usr/bin/node" "/bin/npm" "start"
npm ERR! node v5.6.0
npm ERR! npm  v3.7.2
npm ERR! code ELIFECYCLE
npm ERR! tmp2@1.0.0 start: `concurrently 'browserify'`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the tmp2@1.0.0 start script 'concurrently 'browserify''.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the tmp2 package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     concurrently 'browserify'
npm ERR! You can get information on how to open an issue for this project with:
npm ERR!     npm bugs tmp2
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls tmp2
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     /users/dczeck/projects/tmp2/npm-debug.log

~/projects/tmp2> npm bin
/users/dczeck/projects/tmp2/node_modules/.bin
~/projects/tmp2> ls /users/dczeck/projects/tmp2/node_modules/.bin
acorn@         concurrent@    has-ansi@               miller-rabin@  strip-ansi@      umd@
browserify@    concurrently@  insert-module-globals@  module-deps@   supports-color@
browser-pack@  deps-sort@     JSONStream@             sha.js@        tree-kill@

Edit to add path script.

~/projects/tmp2> vi package.json
~/projects/tmp2> cat package.json
{
  "name": "tmp2",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "start": "concurrently 'browserify'",
    "path": "concurrently \"echo $PATH\"",
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "devDependencies": {
    "browserify": "^13.1.0",
    "concurrently": "^3.0.0"
  }
}

~/projects/tmp2> npm run path

> tmp2@1.0.0 path /users/dczeck/projects/tmp2
> concurrently "echo $PATH"

[0] /usr/lib/node_modules/npm/bin/node-gyp-bin:/users/dczeck/projects/tmp2/node_modules/.bin:/usr/bin:/users/dczeck/bin:/users/dczeck/bin:/bin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/bin/X11:/xswe/bin
[0] echo /usr/lib/node_modules/npm/bin/node-gyp-bin:/users/dczeck/projects/tmp2/node_modules/.bin:/usr/bin:/users/dczeck/bin:/users/dczeck/bin:/bin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/bin/X11:/xswe/bin exited with code 0

Paths match, but the node_modules part of the path isn't propagating to the spawned shell for me.

Edit to add "nstart" script to explicitly path node_modules/.bin.

~/projects/tmp2> vi package.json
~/projects/tmp2> cat package.json
{
  "name": "tmp2",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "start": "concurrently 'browserify'",
    "nstart": "concurrently \"`npm bin`/browserify\" ",
    "path": "concurrently \"echo $PATH\"",
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "devDependencies": {
    "browserify": "^13.1.0",
    "concurrently": "^3.0.0"
  }
}
~/projects/tmp2> npm run nstart

> tmp2@1.0.0 nstart /users/dczeck/projects/tmp2
> concurrently "`npm bin`/browserify" 

[0] (function e(t,n,r){function s(o,u){if(!n[o]){if(!t[o]){var a=typeof require=="function"&&require;if(!u&&a)return a(o,!0);if(i)return i(o,!0);var f=new Error("Cannot find module '"+o+"'");throw f.code="MODULE_NOT_FOUND",f}var l=n[o]={exports:{}};t[o][0].call(l.exports,function(e){var n=t[o][1][e];return s(n?n:e)},l,l.exports,e,t,n,r)}return n[o].exports}var i=typeof require=="function"&&require;for(var o=0;o<r.length;o++)s(r[o]);return s})({
[0] },{},[]);
[0] /users/dczeck/projects/tmp2/node_modules/.bin/browserify exited with code 0

I tried the original test on OS X 10.11 with tcsh and it found browserify without issue. So, maybe this is an issue with Oracle Linux? This seems like a corner-case. I can work around the problem by installing globally.

kimmobrunfeldt commented 8 years ago

Thanks for the detailed report. This indeed seems like a strange behaviour. Node docs state this about child_process.spawn env option:

Use env to specify environment variables that will be visible to the new process, the default is process.env.

In spawn-default-shell we're not passing any parameters for the spawn, so the spawned environment should inherit the environment variables. https://github.com/kimmobrunfeldt/spawn-default-shell/blob/1.1.0/src/index.js#L7

The command it ends up spawning will be e.g. with bash: /bin/bash -l -c "browserify". Maybe that extra layer of shell somehow loses the PATH?

czeckd commented 8 years ago

It does seem to be losing the PATH. I did a little more experimentation with tcsh and found a solution of sorts.

Since you have getShell() checking for a SHELL_EXECUTE_FLAG environmental variable, I set mine to -cf:

setenv SHELL_EXECUTE_FLAG -cf

With that flag set, spawn-default-shell (and therefore concurrently) works fine. The man page for tcsh states:

-f The shell does not load any resource or startup files, or perform any command hashing, and thus starts faster.

So that may be helpful to someone, if they get stuck on this particular issue. Since my issue appears to be an odd corner-case, I'm not sure you'd want to modify code for to resolve it. But if you did, then this is how I fixed in my local get-shell.js source:

   return {
    shell: env.SHELL || '/bin/sh',
    executeFlag: env.SHELL_EXECUTE_FLAG || (env.SHELL.includes('tcsh') ? '-cf' : '-c'),
  };