Closed Bug 1424921 Opened 3 years ago Closed 3 years ago

Bootstrap necessary linting (mozlint, eslint, etc) dependencies

Categories

(Firefox Build System :: Lint and Formatting, enhancement)

3 Branch
enhancement
Not set
normal

Tracking

(firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: ato, Assigned: standard8)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

Because linting is now an integral part of the Gecko development
process, we should ensure the necessary package dependencies are
installed during the normal "./mach bootstrap" process.

For example, macOS High Sierra does not ship with python3 or node
which are necessary for mozlint and eslint respectively.  Figuring
this out manually is not fun, and can be I suspect, easily automated
using Homebrew.
I'm not sure how the existing dependencies are managed for bootstrap, but I'm wondering if it makes sense to combine implementing this with bug 1295833 - I think we were thinking there about having a setup function for the linters.
I guess I don’t particularly care about the _where_ the setup happens
for as long as it is an automatable and repeatable process.  Figuring
out you additionally need python3 and nodejs packages to use "./mach
lint" is the annoying part here.

"./mach lint --setup" could work although I don’t see why the
dependencies cannot just be added to "./mach bootstrap" directly
to avoid the chance of any mutally incompatible packages and linkings.
Mozlint shouldn't require python3, are you getting some sort of error?

As for node, at the very least we should block on bug 1257478 (and potentially dupe depending on the scope we decide to take on that). But I think the other bug could be providing a mechanism to do setup, and then this could be for actually implementing the setup for eslint.
Depends on: 1257478
Blocks: 1427845
Taking, I have a patch that I think will do what we want. It is based on the instructions at https://nodejs.org/en/download/package-manager/ and https://fedoraproject.org/wiki/Features/NodeJS though I haven't tested it on all the linux distros.
Assignee: nobody → standard8
Let me know if you need help with debian testing.
(In reply to Sylvestre Ledru [:sylvestre] from comment #5)
> Let me know if you need help with debian testing.

If you can test the above part on debian (or any other Linux distro!), that would be useful.

When `./mach bootstrap` is run, it should install node along with the other dependencies. Doing `node --version` and `npm --version` should both work, as should something like `./mach eslint browser`
Flags: needinfo?(sledru)
Works for me on Debian:
Les NOUVEAUX paquets suivants seront installés :
   nodejs (4.8.4~dfsg-1)
Flags: needinfo?(sledru)
But npm isn't installed
Summary: Bootstrap necessary linting dependencies → Bootstrap necessary linting (mozlint, eslint, etc) dependencies
Attachment #8939649 - Flags: review?(gps)
(In reply to Sylvestre Ledru [:sylvestre] from comment #9)
> But npm isn't installed

Oh, missed that, I'll update the patch.
Comment on attachment 8939649 [details]
Bug 1424921 - Support Lint dependencies in bootstrap.

https://reviewboard.mozilla.org/r/209944/#review215850

::: python/mozboot/mozboot/debian.py
(Diff revisions 1 - 2)
>          'libdbus-1-dev',
>          'libdbus-glib-1-dev',
>          'libgconf2-dev',
>          'libgtk-3-dev',
>          'libgtk2.0-dev',
> -        'libiw-dev',

Why do you remove that in that patch?
Comment on attachment 8939649 [details]
Bug 1424921 - Support Lint dependencies in bootstrap.

https://reviewboard.mozilla.org/r/209944/#review215850

> Why do you remove that in that patch?

I didn't, that's the interdiff picking up a rebase across bug 1427450.
my bad, long day

the new version worked. It just installed only 69 new packages :p
Comment on attachment 8939649 [details]
Bug 1424921 - Support Lint dependencies in bootstrap.

https://reviewboard.mozilla.org/r/209944/#review215868

Thanks!
Attachment #8939649 - Flags: review+
Comment on attachment 8939649 [details]
Bug 1424921 - Support Lint dependencies in bootstrap.

Sigh, mozreview.  I requested autoland on this already.
Attachment #8939649 - Flags: review?(core-build-config-reviews) → review+
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/33860782891d
Support Lint dependencies in bootstrap. r=froydnj
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/42358e64b1cc
Support Lint dependencies in bootstrap. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/42358e64b1cc
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
See Also: → 1432892
Depends on: 1433036
Depends on: 1432892
See Also: 1432892
Product: Testing → Firefox Build System
Version: Version 3 → 3 Branch
Blocks: 1643234
You need to log in before you can comment on or make changes to this bug.