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

default prefix not resolved when using Brew to install node #7

Open
mhofman opened this issue Sep 27, 2016 · 8 comments
Open

default prefix not resolved when using Brew to install node #7

mhofman opened this issue Sep 27, 2016 · 8 comments

Comments

@mhofman
Copy link
Contributor

mhofman commented Sep 27, 2016

When node is installed using brew, the process.execPath isn't /usr/local/bin/node, which causes the default prefix to be invalidly resolved.

According to the npm config documentation, the config is resolved from the following places:

  1. per-project config file (/path/to/my/project/.npmrc)
  2. per-user config file (~/.npmrc)
  3. global config file ($PREFIX/etc/npmrc)
  4. npm builtin config file (/path/to/npm/npmrc)

The problem is that 3) & 4) are not possible to figure out without resolving the location of npm. The rc package will only find the configs in 1), 2) and sometimes 3) depending on the prefix.
In particular 4) caused issue #4 on Windows, which was solved by hard coding the content of the npm builtin prefix config.

Unless someone has a better suggestion, I suggest that we hard code the default prefix location on all platforms, similar to Windows.
This shouldn't have an impact since after #6, the resolve process first tries using prefix and then using the process.execPath anyway, which means that even if we hardcode the wrong default prefix, the execPath based resolution will still happen as it currently does.

@mhofman
Copy link
Contributor Author

mhofman commented Sep 27, 2016

I will open a PR with the proposed change.

@h2non
Copy link
Owner

h2non commented Sep 27, 2016

Great! Thank you!

@h2non
Copy link
Owner

h2non commented Oct 25, 2016

Fixed in: #6

Closing.

@h2non h2non closed this as completed Oct 25, 2016
@mhofman
Copy link
Contributor Author

mhofman commented Oct 25, 2016

Actually this wasn't fixed by #6 . I still need to create a PR. I started work on this but had to switch priorities and forgot about this.

@h2non
Copy link
Owner

h2non commented Oct 25, 2016

oh! fast reading... I was confused. Re-opening. Thanks!

@h2non h2non reopened this Oct 25, 2016
@pwang2
Copy link

pwang2 commented Nov 2, 2016

+1
when node was installed via homebrew. (lib/node_modules) resolver uses process.execPath instead of npm config get prefix.
image

using npm prefix seems more solid to me.

@pwang2
Copy link

pwang2 commented Nov 2, 2016

it seems the issue is rc('npm').prefix here. If the prefix is not explicitly set in npmrc. the builtin prefix will not be picked up. manual set in ~/.npmrc will fix.

@stevenzeck
Copy link

Yep, running npm config set prefix "/usr/local" fixes it if you installed Node via Homebrew.

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

No branches or pull requests

4 participants