Bootstrap necessary linting (mozlint, eslint, etc) dependencies

RESOLVED FIXED in Firefox 59

Status

enhancement
RESOLVED FIXED
2 years ago
6 months ago

People

(Reporter: ato, Assigned: standard8)

Tracking

(Blocks 1 bug)

3 Branch
mozilla59
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

2 years ago
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.
Assignee

Comment 1

2 years ago
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.
Reporter

Comment 2

2 years ago
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
Assignee

Updated

2 years ago
Blocks: 1427845
Assignee

Comment 4

2 years ago
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.
Comment hidden (mozreview-request)
Assignee

Comment 7

2 years ago
(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`
Assignee

Updated

2 years ago
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
Assignee

Updated

2 years ago
Attachment #8939649 - Flags: review?(gps)
Assignee

Comment 10

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

Oh, missed that, I'll update the patch.
Comment hidden (mozreview-request)

Comment 12

2 years ago
mozreview-review
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?
Assignee

Comment 13

2 years ago
mozreview-review-reply
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 hidden (mozreview-request)

Comment 16

2 years ago
mozreview-review
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+

Comment 18

2 years ago
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/33860782891d
Support Lint dependencies in bootstrap. r=froydnj

Comment 19

2 years ago
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/42358e64b1cc
Support Lint dependencies in bootstrap. r=froydnj

Comment 20

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/42358e64b1cc
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59

Updated

Last year
See Also: → 1432892

Updated

Last year
Depends on: 1433036
Depends on: 1432892
See Also: 1432892

Updated

Last year
Product: Testing → Firefox Build System
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.