1277851 - [mozlint] Better flake8 configuration support

Status: RESOLVED WONTFIX

(Developer Infrastructure :: Lint and Formatting, defect, P5)

Description

[Andrew Halberstadt [:ahal]]

It turns out that just adding new .flake8 files to subdirectories doesn't work out as nicely as I said it did. I should have caught this earlier. Here's some extra context.

Flake8 currently defers all its configuration handling to the pep8 module, and pep8 handles configuration really stupidly. It will use the first configuration file that is common to *all specified paths*.

So if you run:
flake8 testing/marionette/client

Then it will use 'testing/marionette/client/.flake8'. But if you specify a second path, e.g:
flake8 testing/marionette/client tools/lint

Then it will search for a single common config file (which in our case is the global .flake8 file). This is incredibly dumb, I should have noticed this earlier. There is some bad news and good news.

The bad news is that pep8 as a project is stalled out, and the likelihood of them either implementing multiple configs or accepting multiple configs is pretty low. The good news is that flake8 is planning a version 3 which aims to stop using pep8 for things like configuration, and multiple config files is something they're at least interested in.

So until flake8 3 comes out with better config file support, we have two options:
1) Invoke a new flake8 subprocess for each .flake8 file we find, this will involve walking the tree of each file path we pass in.
2) Only use the global config (no subdirectory .flake8 files)

I'll take a quick look into 1) to see how feasible it is.

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1277851 - [mozlint] Run flake8 directories that contain a .flake8 file separately,

This is a crude workaround to get subdirectory .flake8 files working. Hopefully
this is temporary until flake8 3.0 is released with support for multiple config
files.

Note that .flake8 files that live outside of a directory that is explicitly
listed in the 'include' directive, will not be considered.

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

Comment 2

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8759686 [details]
Bug 1277851 - [mozlint] Run flake8 directories that contain a .flake8 file separately,

https://reviewboard.mozilla.org/r/57590/#review54392

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/91a825772c31
[mozlint] Run flake8 directories that contain a .flake8 file separately, r=maja_zf

Comment 4

[Andrew Halberstadt [:ahal]]

I consider this more of a quick hack than long term solution, so marking [leave-open] to tackle this properly in the future.

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/91a825772c31

Comment 6

[Andrew Halberstadt [:ahal]]

Flake8 no longer relies on pep8's configuration, but this is still a problem. For posterity, I filed an issue about this a year ago:
https://gitlab.com/pycqa/flake8/issues/143

It looks unlikely that feature request will be accepted though. Maybe a plugin could implement something like that.

Comment 7

[Andrew Halberstadt [:ahal]]

The yamllint linter has a similar problem to this, and a fix just landed to make it a bit better in bug 1392795. What it does is preprocess the paths by searching the ancestors of each path passed in for .yamllint configs. It then groups all paths that share the same configuration file and runs them together.

We should do this for flake8 as well. It still isn't ideal, but would be a big improvement on the status quo.

Comment 8

[Andrew Halberstadt [:ahal]]

Instead of this, I've decided to go the easier route of removing subdirectory .flake8 files. This is now possible because flake8 3.7 introduced a new feature that allows us to ignore specific errors in subdirectories in the root config file.

WONTFIXing in favour of bug 1515746.