jonschlinkert / global-prefix

Get the npm global path prefix. Same code used internally by npm.
MIT License
28 stars 12 forks source link

Does not return the correct absolute path #20

Open jbouhier opened 6 years ago

jbouhier commented 6 years ago

Hello @jonschlinkert,

When I use npm-check --global --update, which depends on global-modules and finally global-prefix, I've got an incorrect path error which points to ${HOME}/.npm-packages instead of the actual absolute path.

I've done a little bit of digging inside global-prefix and ${HOME}/.npm-packages seems to be returned from tryConfigPath(configPath). Looks like the value of ${HOME} is not parsed with the actual value.

I'm not sure if I should fix this here or if the cause is coming from some other component (i.e: npm-check, node or npm version...).

Here's my config:

Let me know if I can do a PR to fix it 😃

jonschlinkert commented 6 years ago

Let me know if I can do a PR to fix it

That would be great, but first, let's get more info about your setup. What OS/version are you on? etc.

jbouhier commented 6 years ago

I'm on macOS Sierra 10.12.6. I already told you about the Node and npm version I'm using. The package npm-check which prompted the error is installed globally on my system.

I think I should take a look at the code of the ini package. What else can do you need to know ?

jonschlinkert commented 6 years ago

hmm, that answered my main question. I think your suggestion about looking into ini is good, since it seems to be a bug in a module that provides path information to this lib.

Was there any more information in the error message you could share? can you get a stack trace?

jbouhier commented 6 years ago

Of course. It's definitely coming from ini module. I'll dig into the bug this weekend. Should I open an issue on that other repo and ask guidance to the maintainers ? It's my first time trying to contribute to OSS.

➜ npm-check --global --update --debug

[npm-check] debug
cli.flags { update: true,
  u: true,
  global: true,
  g: true,
  skipUnused: false,
  s: false,
  production: false,
  p: false,
  devOnly: false,
  d: false,
  saveExact: false,
  E: false,
  color: false,
  emoji: true,
  spinner: true,
  debug: true,
  dir: '/Users/synxs' }
===============================
[npm-check] debug
cli.input []
===============================
[npm-check] debug
set key spinner to value true
===============================
[npm-check] debug
set key ignore to value undefined
===============================
Path "/Users/synxs/${HOME}/.npm-packages/lib/node_modules" does not exist. Please check the NODE_PATH environment variable.
   23 |                    modulesPath = process.env.NODE_PATH;
   24 |                }
   25 |            }
   26 |
   27 |            if (!fs.existsSync(modulesPath)) {
 > 28 |                throw new Error('Path "' + modulesPath + '" does not exist. Please check the NODE_PATH environment variable.');
   29 |            }
   30 |
   31 |            console.log(chalk.green('The global path you are searching is: ' + modulesPath));
   32 |
   33 |            currentState.set('cwd', globalModulesPath);

   at Promise (/Users/synxs/.npm-packages/lib/node_modules/npm-check/lib/state/init.js:28:23)
   at init (/Users/synxs/.npm-packages/lib/node_modules/npm-check/lib/state/init.js:12:12)
   at state (/Users/synxs/.npm-packages/lib/node_modules/npm-check/lib/state/state.js:75:12)
   at init (/Users/synxs/.npm-packages/lib/node_modules/npm-check/lib/index.js:7:12)
   at Object.<anonymous> (/Users/synxs/.npm-packages/lib/node_modules/npm-check/lib/cli.js:96:1)
   at Module._compile (module.js:635:30)
   at Object.Module._extensions..js (module.js:646:10)
   at Module.load (module.js:554:32)
   at tryModuleLoad (module.js:497:12)
doowb commented 6 years ago

@jbouhier that's stack trace is helpful, thanks.

Will you run the following commands from the command line?

node -e "console.log('NODE_PATH:', process.env.NODE_PATH)"
node -e "console.log('HOME:', process.env.HOME)"

I see in the code above that if process.env.NODE_PATH is set then that's used instead of the value from global-modules.

If it's not set, then we can dig deeper by looking at process.env.HOME.

It's my first time trying to contribute to OSS.

😀 maybe we'll track down a bug that you can submit a PR for!

jbouhier commented 6 years ago

Hey @doowb, here's the output:

➜  ~ node -e "console.log('NODE_PATH:', process.env.NODE_PATH)"
NODE_PATH: undefined
➜  ~ node -e "console.log('HOME:', process.env.HOME)"
HOME: /Users/synxs

Before creating this issue, I tried setting NODE_PATH in my shell .rc file and it worked fine. Nevertheless, the bug is still present and needs to be addressed in my opinion.

In my ~/.npmrc file I have: prefix = ${HOME}/.npm-packages ${HOME} should be parsed by npm/ini to /current/user/home/path/ as written on npmrc's doc.

I guess I'm on the right track, aren't I ? If not, what do you suggest I try ?

doowb commented 6 years ago

It looks like npm is replacing the environment variables after it reads in the config using ini.

I'm not sure if resolving environment variables belongs here or in consuming modules, but it seems similar to how ~ is expanded using expand-tilde in this module, so maybe it belongs here.

Also, does that syntax vary between operating systems?

jbouhier commented 6 years ago

I'll have to get back on you after spinning up a few VMs and try it out. Will follow up tomorrow.

jbouhier commented 6 years ago

Debian 9 / kernel 4.9.0-4-amd64

Same result exact error and stack trace with npm-check -gu --debug.
The error is thrown inside init.js 28:23 both on Debian and Mac.

Debugging init module now.

tunnckoCore commented 6 years ago

Ha. Is this still a bug? Probably. It sounds my like a source for the https://github.com/tunnckoCore/get-installed-path/issues/79 issue.

jbouhier commented 6 years ago

Hello @olstenlarck,

Yes, it might still be. I just commented out prefix = .npm-packages from my ~/.npmrc file as a workaround to get rid of the error messages.

jonschlinkert commented 5 years ago

If this is still a bug, does anyone have a suggested fix or solution I can try? Or a PR?

jbouhier commented 5 years ago

Haven't tried again. Gonna take a look again.