./mach lint -l eslint doesn't look for node in ~/.mozbuild

RESOLVED FIXED in Firefox 65

Status

enhancement
P1
normal
RESOLVED FIXED
9 months ago
8 months ago

People

(Reporter: kats, Assigned: standard8)

Tracking

unspecified
mozilla65
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(1 attachment)

I tried to run `./mach lint -l eslint`. It failed because it couldn't find node, and told me to run `./mach bootstrap --no-system-changes`. So I ran that, which downloaded a new clang, cbindgen, and node into my ~/.mozbuild.

Then I ran `./mach lint -l eslint` again, and again it failed with the same thing. After I ran `PATH=$PATH:$HOME/.mozbuild/node/bin` it worked. Seems to me like the mach command should look in .mozbuild for node.
(This was on Ubuntu 16.04, in case it matters)
Those bits seem to work ok. The problem is that found_exe ends up pointing to the npm binary at /home/kats/.mozbuild/node/bin/npm, and the very first line of that is "#!/usr/bin/env node". So when we run said npm binary to check the version, it fails to run because there's no `node` binary on the PATH and /usr/bin/env won't find it.
Flags: needinfo?(kats)
Here's what I see at the top of the output with a printout that I added:

$ ./mach lint -l eslint gfx/wr/
found_exe: nodejs /usr/bin/nodejs
found_exe: node /usr/bin/nodejs
found_exe: npm /home/kats/.mozbuild/node/bin/npm
/usr/bin/env: ‘node’: No such file or directory
Could not find npm executable later than 5.6.
Thank you for that, very helpful. Looks like we need to wrap the calls to npm. We're already doing this for eslint itself, but obviously need to do npm as well.

Could you try the patch I've just attached? I think that will fix it.

If you've already worked around it, then trying this should be enough (as long as node isn't in your path):

- Remove <topsrcdir>/node_modules
- ./mach eslint --setup
Assignee: nobody → standard8
Flags: needinfo?(kats)
Priority: -- → P1
The patch seems to work, thanks!
Flags: needinfo?(kats)
Great, thanks for checking, I'll get that reviewed :-)
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f851c3e82c2b
When setting up ESLint, call npm via node so that node doesn't need to be in the path. r=ahal
Oops:

    def check_executable_version(exe, wrap_call_with_node=True):

that default should have been False. That could quite easily cause a continuous loop.
Flags: needinfo?(standard8)
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a177ba2ca668
When setting up ESLint, call npm via node so that node doesn't need to be in the path. r=ahal
https://hg.mozilla.org/mozilla-central/rev/a177ba2ca668
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Target Milestone: Firefox 65 → mozilla65
You need to log in before you can comment on or make changes to this bug.