Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

execa does not respect $PATH variable #153

Closed
lucasbraun opened this issue Nov 2, 2018 · 15 comments · Fixed by #377
Closed

execa does not respect $PATH variable #153

lucasbraun opened this issue Nov 2, 2018 · 15 comments · Fixed by #377
Milestone

Comments

@lucasbraun
Copy link

I have two installations of git and execa seems to pick the wrong one, not respecting $PATH correctly, while child-process does it correctly (even if I explicitly remove /usr/bin from $PTAH and add my desired path twice, once in the beginning and once in the end):

$ /usr/bin/git --version 
git version 1.8.3.1
$ /opt/rh/rh-git29/root/usr/bin/git --version
git version 2.9.3
$ echo $PATH
/opt/rh/rh-git29/root/usr/bin:/bin:/usr/local/bin:/opt/rh/rh-git29/root/usr/bin
$ cat ./index.js
const execa = require('execa');
(async () => {
    const {stdout} = await execa('git', ['--version']);
    console.log(stdout);
    //=> 'unicorns'
})();
const execFile = require('child_process').execFile;
const child = execFile('git', ['--version'], (error, stdout, stderr) => {
    if (error) {
        console.error('stderr', stderr);
        throw error;
    }
    console.log('stdout', stdout);

});
$ node ./index.js
git version 1.8.3.1
stdout git version 2.9.3

I am missing something here?

@ammarbinfaisal1
Copy link
Contributor

@lucasbraun This doesn't appear to be problem with execa. execa is an alternative to childProcess.spawn(actually better than using it) and you shouldn't compare execFile's output with execa. Try using childProcess.spawn instead of execa and it would show the same result. Also, have a look at this nodejs/node#12986

@ehmicky
Copy link
Collaborator

ehmicky commented Mar 13, 2019

@ammarbinfaisal childProcess.spawn() is the base for all other childProcess methods variants including execFile(). See the Node.js source.

@ehmicky
Copy link
Collaborator

ehmicky commented Mar 13, 2019

@lucasbraun There has been few changes to the code handling env lately, can you use the latest commit (not latest release) of execa and confirm this is still a bug then?

Also could you try the following code instead?

const execa = require('execa');
const childProcess = require('child_process');

console.log(execa.sync('echo $PATH', { shell: true }).stdout)
console.log(childProcess.spawnSync('echo $PATH', { shell: true, encoding: 'utf8' }).stdout)

If all goes fine:

  • the second call should give you your $PATH as is: /opt/rh/rh-git29/root/usr/bin:/bin:/usr/local/bin:/opt/rh/rh-git29/root/usr/bin
  • the first call should append a bunch of .bin directories in case you want to fire local binaries (see npm-run-path)

Also never forget to submit your environment information when submitting an issue as execa is highly environment-specific (but that's valid for other projects as well):

  • OS version
  • Node version
  • execa version or hash

@lucasbraun
Copy link
Author

Thanks, @ehmicky for your help with this. Unfortunately, the problem remains. I just checked with execa from master (7f8d911153c475d75da0925adf2db40cbe13a98a). While echoing the PATH does the right thing, it seems that the PATH is still not properly used:

$ cat test-lucas.js 
const execa = require('.');
const childProcess = require('child_process');

console.log("PATH with execa:", execa.sync('echo $PATH', { shell: true }).stdout);
console.log("git with execa:", execa.sync('git', ['--version'], { shell: true }).stdout);

console.log("PATH with child process:", childProcess.spawnSync('echo $PATH', { shell: true, encoding: 'utf8' }).stdout);
console.log("git with child process:", childProcess.spawnSync('git', ['--version'], { shell: true, encoding: 'utf8' }).stdout);
$ node test-lucas.js 
PATH with execa: /scratch/lbraun/execa/node_modules/.bin:/scratch/lbraun/node_modules/.bin:/scratch/node_modules/.bin:/node_modules/.bin:/usr/bin:/opt/rh/rh-git29/root/usr/bin:/scratch/opt/bin:/bin:/usr/bin:/usr/dev_infra/platform/bin:/usr/dev_infra/generic/bin:/usr/local/bin:/usr/X11R6/bin:/usr/local/ade/bin
git with execa: git version 1.8.3.1
PATH with child process: /opt/rh/rh-git29/root/usr/bin:/scratch/opt/bin:/bin:/usr/bin:/usr/dev_infra/platform/bin:/usr/dev_infra/generic/bin:/usr/local/bin:/usr/X11R6/bin:/usr/local/ade/bin

git with child process: git version 2.9.3

Here is my environment:

$ cat /etc/os-release
NAME="Oracle Linux Server"
VERSION="7.4"
ID="ol"
VERSION_ID="7.4"
PRETTY_NAME="Oracle Linux Server 7.4"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:oracle:linux:7:4:server"
HOME_URL="https://linux.oracle.com/"
BUG_REPORT_URL="https://bugzilla.oracle.com/"

ORACLE_BUGZILLA_PRODUCT="Oracle Linux 7"
ORACLE_BUGZILLA_PRODUCT_VERSION=7.4
ORACLE_SUPPORT_PRODUCT="Oracle Linux"
ORACLE_SUPPORT_PRODUCT_VERSION=7.4

$ node --version
v8.12.0

execa version: 7f8d911153c475d75da0925adf2db40cbe13a98a

I guess that it might be reproduce the problem on my OS, but I don't see why this should be an OS-specific problem. The problem should be reproducible on any OS where you install more than one version of git.

@ehmicky
Copy link
Collaborator

ehmicky commented Mar 13, 2019

You never know when a problem might be OS-specific unless you're very familiar with the implementation code, so it's better to be on the safe side :) Also the Node.js version is always a good idea to include in any GitHub issue.

Just to be 100% sure, could you please paste your code above but with 'git --version' instead of git, ['--version']? The shell option allows you to do that, and it might give different results (that's not likely but just in case).

@ehmicky
Copy link
Collaborator

ehmicky commented Mar 13, 2019

Also could you:

  • add a third command output for both execa() and child_process.spawn(): which git?
  • add a console.log(process.execPath) in your example?

If my intuition is correct, I think this bug might be related to sindresorhus/npm-run-path#3

@lucasbraun
Copy link
Author

Yes, I can :-)

$ cat test-lucas.js 
const execa = require('.');
const childProcess = require('child_process');
console.log("PATH with execa:", execa.sync('echo $PATH', { shell: true }).stdout);
console.log("git with execa:", execa.sync('git', ['--version'], { shell: true }).stdout);
console.log("git --version with execa:", execa.sync('git --version', { shell: true }).stdout);
console.log("which git with execa:", execa.sync('which git', { shell: true }).stdout);

console.log("PATH with child process:", childProcess.spawnSync('echo $PATH', { shell: true, encoding: 'utf8' }).stdout);
console.log("git with child process:", childProcess.spawnSync('git', ['--version'], { shell: true, encoding: 'utf8' }).stdout);
console.log("git --version with child process:", childProcess.spawnSync('git --version', { shell: true, encoding: 'utf8' }).stdout);
console.log("which git with child process:", childProcess.spawnSync('which git', { shell: true, encoding: 'utf8' }).stdout);


$ node test-lucas.js                                                                                                                    
PATH with execa: /scratch/lbraun/execa/node_modules/.bin:/scratch/lbraun/node_modules/.bin:/scratch/node_modules/.bin:/node_modules/.bin:/usr/bin:/opt/rh/rh-git29/root/usr/bin:/scratch/opt/bin:/bin:/usr/bin:/usr/dev_infra/platform/bin:/usr/dev_infra/generic/bin:/usr/local/bin:/usr/X11R6/bin:/usr/local/ade/bin
git with execa: git version 1.8.3.1
git --version with execa: git version 1.8.3.1
which git with execa: /usr/bin/git
PATH with child process: /opt/rh/rh-git29/root/usr/bin:/scratch/opt/bin:/bin:/usr/bin:/usr/dev_infra/platform/bin:/usr/dev_infra/generic/bin:/usr/local/bin:/usr/X11R6/bin:/usr/local/ade/bin

git with child process: git version 2.9.3

git --version with child process: git version 2.9.3

which git with child process: /opt/rh/rh-git29/root/usr/bin/git

@ehmicky
Copy link
Collaborator

ehmicky commented Mar 13, 2019

I think the PR I just opened here: sindresorhus/npm-run-path#5 should solve your problem. npm-run-path is the module used by execa that probably is creating this bug.

This needs to be reviewed by @sindresorhus first though.

@lucasbraun
Copy link
Author

OK, thanks a lot! Looking forward to see whether it does fix the problem. That would be awesome!

@ehmicky
Copy link
Collaborator

ehmicky commented Mar 13, 2019

I think the issue here is that execa is prepending the directory of process.execPath to $PATH. This is to allow users firing global binaries. In your case that directory is /usr/bin, which leads /usr/bin being added to your $PATH.

Until the PR is merged, using nvm would probably be a workaround for your issue as process.execPath would then be modified by nvm.

@ehmicky
Copy link
Collaborator

ehmicky commented May 24, 2019

I thought of a way to move forward with this: see sindresorhus/npm-run-path#5 (comment)

@ehmicky
Copy link
Collaborator

ehmicky commented Jun 24, 2019

@lucasbraun If I understood your problem correctly, using the preferLocal: false option should solve it, could you confirm this?

We are considering changing its default value from true to false (#314).

@lucasbraun
Copy link
Author

Yes, it does. Thanks a lot for fixing!

@ehmicky
Copy link
Collaborator

ehmicky commented Jun 24, 2019

Closing this thanks to #314.

@ehmicky ehmicky closed this as completed Jun 24, 2019
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 3, 2019
…g `./mach browsertime` r=nalexander

If `./mach browsertime` runs browsertime with a globally-installed node, due to
an existing bug in [execa][1], the wrong Python will be executed. We now
specify the full path of the Python binary we wish to use (via the `PYTHON`
environment variable that our fork of browsertime supports) and avoid this
issue altogether.

[1]: sindresorhus/execa#153

Differential Revision: https://phabricator.services.mozilla.com/D35374

--HG--
extra : moz-landing-system : lando
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Jul 3, 2019
…g `./mach browsertime` r=nalexander

If `./mach browsertime` runs browsertime with a globally-installed node, due to
an existing bug in [execa][1], the wrong Python will be executed. We now
specify the full path of the Python binary we wish to use (via the `PYTHON`
environment variable that our fork of browsertime supports) and avoid this
issue altogether.

[1]: sindresorhus/execa#153

Differential Revision: https://phabricator.services.mozilla.com/D35374
@Glandos
Copy link

Glandos commented Jul 12, 2019

My solution:

export PATH

I see that the original poster uses Software Collections. I've run into the very same issue with fish and set -g -p PATH /custom/bin, until I found that in a bare node interpreter, process.env.PATH is undefined.
When using preferLocal to true, the environment is set by npm-run-path which does the right thing: it appends the current path to some local path.

It seems to be a trick of fish shell:

> python3 -c 'import os; print(os.environ["PATH"])'
/home/user/.local/bin/:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games
> set -g -p PATH /truc
> python3 -c 'import os; print(os.environ["PATH"])'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib/python3.7/os.py", line 678, in __getitem__
    raise KeyError(key) from None
KeyError: 'PATH'
> set -x -p PATH /truc
> python3 -c 'import os; print(os.environ["PATH"])'
/truc:/truc:/home/user/.local/bin/:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games

Node has (fortunately) the same behavior.

And if for some reason, PATH is not exported, you'll run into the very same issue.

gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
…g `./mach browsertime` r=nalexander

If `./mach browsertime` runs browsertime with a globally-installed node, due to
an existing bug in [execa][1], the wrong Python will be executed. We now
specify the full path of the Python binary we wish to use (via the `PYTHON`
environment variable that our fork of browsertime supports) and avoid this
issue altogether.

[1]: sindresorhus/execa#153

Differential Revision: https://phabricator.services.mozilla.com/D35374

UltraBlame original commit: d5f3bcfcd6bdf0245264e94e5050aa3dde799938
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 4, 2019
…g `./mach browsertime` r=nalexander

If `./mach browsertime` runs browsertime with a globally-installed node, due to
an existing bug in [execa][1], the wrong Python will be executed. We now
specify the full path of the Python binary we wish to use (via the `PYTHON`
environment variable that our fork of browsertime supports) and avoid this
issue altogether.

[1]: sindresorhus/execa#153

Differential Revision: https://phabricator.services.mozilla.com/D35374

UltraBlame original commit: d5f3bcfcd6bdf0245264e94e5050aa3dde799938
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
…g `./mach browsertime` r=nalexander

If `./mach browsertime` runs browsertime with a globally-installed node, due to
an existing bug in [execa][1], the wrong Python will be executed. We now
specify the full path of the Python binary we wish to use (via the `PYTHON`
environment variable that our fork of browsertime supports) and avoid this
issue altogether.

[1]: sindresorhus/execa#153

Differential Revision: https://phabricator.services.mozilla.com/D35374

UltraBlame original commit: d5f3bcfcd6bdf0245264e94e5050aa3dde799938
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants