1424921 - Bootstrap necessary linting (mozlint, eslint, etc) dependencies

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, enhancement)

Description

[Andreas Tolfsen ❲:ato❳]

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.

Comment 1

[Mark Banner (:standard8)]

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.

Comment 2

[Andreas Tolfsen ❲:ato❳]

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.

Comment 3

[Andrew Halberstadt [:ahal] (PTO Until Apr 3rd)]

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.

Comment 4

[Mark Banner (:standard8)]

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.

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Let me know if you need help with debian testing.

Comment 6

[Mark Banner (:standard8)]

Attachment: Bug 1424921 - Support Lint dependencies in bootstrap.

Review commit: https://reviewboard.mozilla.org/r/209944/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/209944/

Comment 7

[Mark Banner (:standard8)]

(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`

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Works for me on Debian:
Les NOUVEAUX paquets suivants seront installés :
   nodejs (4.8.4~dfsg-1)

Comment 9

[Sylvestre Ledru [:Sylvestre]]

But npm isn't installed

Comment 10

[Mark Banner (:standard8)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #9)
> But npm isn't installed

Oh, missed that, I'll update the patch.

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 8939649 [details]
Bug 1424921 - Support Lint dependencies in bootstrap.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/209944/diff/1-2/

Comment 12

[Sylvestre Ledru [:Sylvestre]]

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 13

[Mark Banner (:standard8)]

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.

Comment 14

[Sylvestre Ledru [:Sylvestre]]

my bad, long day

the new version worked. It just installed only 69 new packages :p

Comment 15

[Mark Banner (:standard8)]

Comment on attachment 8939649 [details]
Bug 1424921 - Support Lint dependencies in bootstrap.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/209944/diff/2-3/

Comment 16

[Nathan Froyd [:froydnj]]

Comment on attachment 8939649 [details]
Bug 1424921 - Support Lint dependencies in bootstrap.

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

Thanks!

Comment 17

[Nathan Froyd [:froydnj]]

Comment on attachment 8939649 [details]
Bug 1424921 - Support Lint dependencies in bootstrap.

Sigh, mozreview.  I requested autoland on this already.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33860782891d
Support Lint dependencies in bootstrap. r=froydnj

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/42358e64b1cc
Support Lint dependencies in bootstrap. r=froydnj

Comment 20

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/42358e64b1cc

Comment 21

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/33860782891d