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

fix: Fix npm install --prefix on windows #7725

Open
wants to merge 1 commit into
base: latest
Choose a base branch
from

Conversation

nipunn1313
Copy link

I found that globalDir/localDir and globalPrefix/localPrefix end up being the same value when --prefix is set. This was surprising to me, but it does appear that globalPrefix is set to match prefix at the end of loading.

This means that the globalTop logic is actually incorrect here. It accidentally worked on non-windows machines.

For example if running npm i --prefix scripts

On non-windows
this.npm.globalDir = $CWD/scripts/lib/node_modules

On windows
this.npm.globalDir = $CWD/scripts/node_modules

IMHO, the logic inside of npmconfig is also incorrect here, since globalDir is being set to this bogus $CWD/scripts/lib/node_modules

However, this fix should work regardless - since it stops relying on the specific values of globalDir.

I manually tested npm install and npm install --prefix on both windows and mac. Everything works now.

References

Fixes #7722

I found that globalDir/localDir and globalPrefix/localPrefix
end up being the same value when `--prefix` is set. This was
surprising to me, but it does appear that `globalPrefix` is set
to match `prefix` at the end of loading.

This means that the globalTop logic is actually incorrect here.
It accidentally worked on non-windows machines.

For example if running `npm i --prefix scripts`

On non-windows
this.npm.globalDir = $CWD/scripts/lib/node_modules

On windows
this.npm.globalDir = $CWD/scripts/node_modules

IMHO, the logic inside of npmconfig is also incorrect here, since
globalDir is being set to this bogus `$CWD/scripts/lib/node_modules`

However, this fix should work regardless - since it stops relying
on the specific values of globalDir.

Fixes npm#7722

I manually tested `npm install` and `npm install --prefix` on both
windows and mac. Everything works now.
@wraithgar
Copy link
Member

This needs tests

@nipunn1313
Copy link
Author

good idea. I'll see when I can make some time for it. install.test.js has some examples which I had taken a quick pass over. It was a little hard to figure out how to get the equivalent of --prefix to happen since the tests don't take command line args directly. I'll have to dive a bit to figure out how to get coverage.

If anyone out there reading this has time to offer help, I'd take it.

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

Successfully merging this pull request may close these issues.

[BUG] Windows npm install --prefix errors and looks for the package.json in the cwd
2 participants