Closed Bug 1229194 Opened 9 years ago Closed 8 years ago

"mach eslint --setup" ends up telling me to run mach as root, which is probably not a good idea

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox45 affected)

RESOLVED FIXED
Tracking Status
firefox45 --- affected

People

(Reporter: dholbert, Unassigned)

References

Details

(Whiteboard: [fixed by bug 1265082])

STR:
 (I'm assuming you don't have node.js or NPM configured at all, to start.)

 1. On Ubuntu (15.10 in my case), run the following to install nodejs & npm:
   sudo apt-get install nodejs npm

 2. In a mozilla-central clone, run:
    mach eslint --setup

  (If you hit bug 1228761, apply its hackaround & repeat step 2.)

ACTUAL RESULTS:
{
$ ./mach eslint --setup 

Installing eslint using "/usr/bin/npm install eslint -g"...
npm ERR! tar.unpack untar error /home/dholbert/.npm/eslint/1.10.2/package.tgz
npm ERR! Error: EACCES, mkdir '/usr/local/lib/node_modules'
npm ERR!  { [Error: EACCES, mkdir '/usr/local/lib/node_modules']
npm ERR!   errno: 3,
npm ERR!   code: 'EACCES',
npm ERR!   path: '/usr/local/lib/node_modules',
npm ERR!   fstream_type: 'Directory',
npm ERR!   fstream_path: '/usr/local/lib/node_modules/eslint',
npm ERR!   fstream_class: 'DirWriter',
npm ERR!   fstream_stack: 
npm ERR!    [ '/usr/lib/nodejs/fstream/lib/writer.js:171:23',
npm ERR!      '/usr/lib/nodejs/mkdirp/index.js:46:53',
npm ERR!      'Object.oncomplete (fs.js:107:15)' ] }
npm ERR! 
npm ERR! Please try running this command again as root/Administrator.

npm ERR! System Linux 4.2.0-18-generic
npm ERR! command "/usr/bin/nodejs" "/usr/bin/npm" "install" "eslint" "-g"
npm ERR! cwd /scratch/work/builds/mozilla-inbound/mozilla
npm ERR! node -v v0.10.25
npm ERR! npm -v 1.4.21
npm ERR! path /usr/local/lib/node_modules
npm ERR! fstream_path /usr/local/lib/node_modules/eslint
npm ERR! fstream_type Directory
npm ERR! fstream_class DirWriter
npm ERR! code EACCES
npm ERR! errno 3
npm ERR! stack Error: EACCES, mkdir '/usr/local/lib/node_modules'
npm ERR! fstream_stack /usr/lib/nodejs/fstream/lib/writer.js:171:23
npm ERR! fstream_stack /usr/lib/nodejs/mkdirp/index.js:46:53
npm ERR! fstream_stack Object.oncomplete (fs.js:107:15)
npm ERR! 
npm ERR! Additional logging details can be found in:
npm ERR!     /scratch/work/builds/mozilla-inbound/mozilla/npm-debug.log
npm ERR! not ok code 0

Error installing eslint, aborting.
}

Looks like npm is using a folder outside of my home directory, by default, and it's failing due to lacking write access.

So, it says "Please try running this command again as root/Administrator" in the middle there.

I don't know how confident we are in our mach hardening, but we probably do *not* want to be encouraging users to run mach as root...  If there's anything we can do to autoconfigure npm in the way that we expect it to be configured (or to print out some additional error output to help the user in a non-"run-as-root" way here), that would be good...
(Incidentally, I did actually try running "./mach eslint --setup" as root (call me foolhardy), and I ended up failing with a *** MERCURIAL NOT CONFIGURED *** error, telling me that I need to run "./mach mercurial-setup" as root before I run this other thing as root. I didn't go any further than that. Anyway, point is, we shouldn't be prompting npm to suggest that users run mach as root.)
A possible work around would be to get the user to fix npm permissions:

https://docs.npmjs.com/getting-started/fixing-npm-permissions

The only other options are:

- we change to some sort of setup where the modules are installed locally in either the src tree or the objdir
-- This would likely break editor integration (as editors will typically need eslint installed globally)
- we adjust mach to do sudo just for the npm install process. Which I'm not sure would be great either.
When I installed Node.js from source (because the Debian packages for stable are not modern enough for Gaia) the other day, I noticed that the `sudo make install` step set up the directory group permissions for /usr/local/lib/node_modules incorrectly.  There’s as far as I can tell no benefit to having this directory owned by root only unless you run a public, multi-user system.

As I also pointed out in bug 1228761, we could make the fetching of eslint from npm part of the mach bootstrapping process, which anyway is run as root, and consequently guaranteed to succeed on all systems.
Is there a reason that we have to install these modules globally? The usual practice with node is to provide a top-level package.json, and install the required modules in a top-level node_modules directory. Or at least install them locally by default, and provide a -g flag for people who need them installed globally?

I only managed to make this work by reading the mach_commands.py source, and manually running the npm commands as root. I actually (inadvisably) tried running `mach eslint --setup` as root, but that required me to also run `mach mercurial-setup` as root, which I wasn't willing to do.
I agree we should remove the "-g" flags in https://dxr.mozilla.org/mozilla-central/source/python/mach_commands.py

- We shouldn't impose our special eslint version + plugins onto the user's whole system
- These can live within our sourcedir, installed by npm into a local ./node_modules/ folder.
- We should let the user (or their editor) decide about having a global eslint

However, that wouldn't entirely solve the sudo-requirement, because we `npm link` the local plugin "./testing/eslint-plugin-mozilla/", which also requires write access to "/usr/local/lib/".

But given that it's also published to npm, maybe we can simply `npm install` it, like the other plugins?
(In reply to Jan Keromnes [:janx] from comment #5)
> I agree we should remove the "-g" flags in
> https://dxr.mozilla.org/mozilla-central/source/python/mach_commands.py
> 
> - We shouldn't impose our special eslint version + plugins onto the user's
> whole system
> - These can live within our sourcedir, installed by npm into a local
> ./node_modules/ folder.
> - We should let the user (or their editor) decide about having a global
> eslint
> 
> However, that wouldn't entirely solve the sudo-requirement, because we `npm
> link` the local plugin "./testing/eslint-plugin-mozilla/", which also
> requires write access to "/usr/local/lib/".
> 
> But given that it's also published to npm, maybe we can simply `npm install`
> it, like the other plugins?

If you npm install it you will almost certainly turn out with lots of incompatibilities between the plugin and the in tree .eslintrc files. That is why we link it instead.
This was fixed a while ago by bug 1265082.
Status: NEW → RESOLVED
Closed: 8 years ago
Depends on: 1265082
Resolution: --- → FIXED
Whiteboard: [fixed by bug 1265082]
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.