46 bytes, text/x-phabricator-request
|Details | Review|
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
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.
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).
Summary: audit and update any in-tree uses of Node (eg eslint, speedy tests, etc.) to use version found by configure → Update ESLint to use node finding routines from configure
Sounds good to me!
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.
Assignee: nobody → standard8
Status: NEW → ASSIGNED
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.
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.
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.
I've now requested review, I think this is all working ok on the different platforms & for different cases.
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.
Attachment #9010756 - Flags: review+
Pushed by email@example.com: 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
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
status-firefox64: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.