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

Fixing global add in linux root folder #1029 #1344

Merged
merged 6 commits into from
Oct 31, 2016
Merged

Fixing global add in linux root folder #1029 #1344

merged 6 commits into from
Oct 31, 2016

Conversation

lucaskatayama
Copy link
Contributor

Summary
Fixing yarn global add being installed in root folder described in issue #1029
.
Users can't access /root/.yarn-* .
Using /usr/local/libinstead like npm modules (/usr/local/lib/node_modules)
eg:

yarn global add react-native-cli
/usr/local/bin/react-native -> ../lib/.yarn-config/global/node_modules/.bin/react-native*

Reference

@lucaskatayama
Copy link
Contributor Author

Getting error fetching modules

@bestander
Copy link
Member

@Daniel15, assigning to you

@kentnek
Copy link

kentnek commented Oct 29, 2016

Please merge this asap, since I'm trying to use yarn to install pm2 globally :)

const path = require('path');
let userHome = require('user-home');
if (process.platform === 'linux' && process.env.USER === 'root') {
userHome = path.resolve(process.execPath, '..', '..', 'lib');
Copy link
Member

@Daniel15 Daniel15 Oct 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes an assumption about where Node.js is installed, which could vary based on the environment (eg. /usr/bin/node if the user installed it via a package manager, /usr/local/bin/node or /opt/nodejs/ or really anything else if they compiled it themselves or used something like nvm).

If you want to use /usr/local/lib/ then it's probably best to just hard-code that here. Don't use /usr/lib as that's purely for libraries managed by the system.

I think /usr/local/share/ is actually the right place for global Node.js modules as they're not native code and are thus platform-independent. Either /usr/local/share or /usr/local/lib is fine though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to understand...
When you say that I made an assumption (e.g. /usr/bin/node) you're talking about this variable process.execPath which contains the path to node bin, right?
So, if I have a custom node location, it will contain another path and this path.resolve(process.execPath, '..', '..', 'lib'); creates a bug.
That's it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes :) What I mean is that for example, what if Node.js is installed at /opt/node/versions/6.91/? Then ../../lib will resolve to /opt/node/lib which is probably not what you want.

Copy link
Contributor Author

@lucaskatayama lucaskatayama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing to a hardcoded path /usr/local/share

const path = require('path');
let userHome = require('user-home');
if (process.platform === 'linux' && process.env.USER === 'root') {
userHome = path.resolve(process.execPath, '..', '..', 'lib');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to understand...
When you say that I made an assumption (e.g. /usr/bin/node) you're talking about this variable process.execPath which contains the path to node bin, right?
So, if I have a custom node location, it will contain another path and this path.resolve(process.execPath, '..', '..', 'lib'); creates a bug.
That's it?

@Daniel15
Copy link
Member

Cool, let's try this out and see how it goes!

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.

4 participants