1482435 - Update ESLint to use node finding routines from configure

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Comment 1

[Ryan VanderMeulen [:RyanVM]]

ESLint on Windows definitely doesn't work if node isn't in the MozillaBuild path.

$ mach lint -l eslint
nodejs is either not installed or is installed to a non-standard path.
Please install nodejs from https://nodejs.org and try again.

Valid installation paths:
  - C:\Program Files\nodejs
  - C:\nodejs
Node Package Manager (npm) is either not installed or installed to a
non-standard path. Please install npm from https://nodejs.org (it comes as an
option in the node installation) and try again.

Valid installation paths:
  - C:\Program Files\nodejs
  - C:\nodejs

Comment 2

[Gregory Szorc [:gps]]

IIRC `mach lint` has its own bespoke logic for finding Node. We will want it to look in self.substs (assuming the presence of a MozbuildObject class) to resolve the path to Node.

Or we should extract the Python code for finding Node to a Python module usable outside of configure and have both configure and random mach commands use that to find Node. This is the approach we use for locating a Python 3 executable.

Comment 3

[Mark Banner (:standard8)]

The speedy tests no longer exist, so as far as we know node is only used by ESLint at the moment.

I'm currently thinking that extracting the code to a python module might be the easiest option and avoids ESLint depending on configure (for the time being at least).

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Sounds good to me!

Comment 5

[Mark Banner (:standard8)]

I have a partial WIP for this that works for ESLint, just got to hook up the configure changes, and then do some tidy-up.

Comment 6

[Mark Banner (:standard8)]

Attachment: Bug 1482435 - Separate out nodejs finding logic from configure and use it for ESLint.

This extracts the current logic for finding nodejs into its own module in mozbuild. Configure and ESLint then use it.

For ESLint, this will change the first location it looks for nodejs to be the .mozbuild directory.

Comment 7

[Mark Banner (:standard8)]

The initial patch here isn't quite working on try, I need to handle the NODEJS env variable properly. I'll look at that tomorrow.

Comment 8

[Mark Banner (:standard8)]

I've tried getting ESLint to use the linux64-node toolchain in taskcluster, but it appears there's more than just a few configuration changes - somewhere along the line we need to get something to download the toolchain.

That seems more complicated than I want to cover in this bug, so for now, I've just gone with updating the ESLint builder to the matching version of node, and it'll find that from the PATH.

We can file a follow-up to make that nicer I think.

Comment 9

[Mark Banner (:standard8)]

I've now requested review, I think this is all working ok on the different platforms & for different cases.

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 9010756 [details]
Bug 1482435 - Separate out nodejs finding logic from configure and use it for ESLint.

Gregory Szorc [:gps] has approved the revision.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/951f04d1bb51
Separate out nodejs finding logic from configure and use it for ESLint. r=firefox-build-system-reviewers,gps

Comment 12

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/951f04d1bb51