Bug 1395126 - Support cascading configuration for flake8, r?bc
This allows .flake8 files to override one another, and fixes a pretty bad known
bug with our flake8 implementation. For example, say we have a .flake8 file at:
/foo/.flake8
Before this patch, if we ran |mach lint foo/bar|, the configuration defined in
that .flake8 file wouldn't get picked up. It would only work if running the
specific directory that contains it, e.g |mach lint foo|.
This change additionally allows multiple .flake8 files to be used. So if
there's one defined at both:
/.flake8
/foo/.flake8
Then running |mach lint foo/bar| will first apply the root .flake8, then the
one under /foo (overriding earlier configuration).
This bug still doesn't make flake8 configuration perfect though. Any directory
containing a .flake8 file still needs to be explicitly listed in the "include"
section of /tools/lint/flake8.yml. Otherwise in the example above, if running
|mach lint /|, it wouldn't be able to find /foo/.flake8. This is a hard problem
and is likely best solved by fixing flake8's upstream configuration handling.
Unfortunately this means we still can't switch from a whitelist to a blacklist.
MozReview-Commit-ID: 3DZAi1QHYYo
--- a/.flake8
+++ b/.flake8
@@ -1,4 +1,6 @@
[flake8]
# See http://pep8.readthedocs.io/en/latest/intro.html#configuration
ignore = E121, E123, E126, E129, E133, E226, E241, E242, E704, W503, E402
max-line-length = 99
+exclude =
+ testing/mochitest/pywebsocket,
--- a/python/mozlint/mozlint/pathutils.py
+++ b/python/mozlint/mozlint/pathutils.py
@@ -171,8 +171,31 @@ def findobject(path):
modulepath, objectpath = path.split(':')
obj = __import__(modulepath)
for a in modulepath.split('.')[1:]:
obj = getattr(obj, a)
for a in objectpath.split('.'):
obj = getattr(obj, a)
return obj
+
+
+def ancestors(path):
+ while path:
+ yield path
+ (path, child) = os.path.split(path)
+ if child == "":
+ break
+
+
+def get_ancestors_by_name(name, path, root):
+ """Returns a list of files called `name` in `path`'s ancestors,
+ sorted from closest->furthest. This can be useful for finding
+ relevant configuration files.
+ """
+ configs = []
+ for path in ancestors(path):
+ config = os.path.join(path, name)
+ if os.path.isfile(config):
+ configs.append(config)
+ if path == root:
+ break
+ return configs
--- a/testing/firefox-ui/.flake8
+++ b/testing/firefox-ui/.flake8
@@ -1,3 +1,3 @@
[flake8]
-max-line-length = 99
-exclude = __init__.py,
+exclude =
+ __init__.py,
--- a/testing/marionette/client/.flake8
+++ b/testing/marionette/client/.flake8
@@ -1,3 +1,5 @@
[flake8]
-max-line-length = 99
-exclude = __init__.py,disti/*,build/*,
+exclude =
+ __init__.py,
+ disti/*,
+ build/*,
--- a/testing/marionette/harness/.flake8
+++ b/testing/marionette/harness/.flake8
@@ -1,3 +1,7 @@
[flake8]
-max-line-length = 99
-exclude = __init__.py,disti/*,build/*,marionette_harness/runner/mixins/*, marionette_harness/tests/*
+exclude =
+ __init__.py,
+ disti/*,
+ build/*,
+ marionette_harness/runner/mixins/*,
+ marionette_harness/tests/*,
--- a/testing/marionette/puppeteer/.flake8
+++ b/testing/marionette/puppeteer/.flake8
@@ -1,3 +1,3 @@
[flake8]
-max-line-length = 99
-exclude = __init__.py,
+exclude =
+ __init__.py,
deleted file mode 100644
--- a/toolkit/components/telemetry/.flake8
+++ /dev/null
@@ -1,4 +0,0 @@
-[flake8]
-# See http://pep8.readthedocs.io/en/latest/intro.html#configuration
-max-line-length = 99
-filename = *.py, +.lint
--- a/tools/lint/flake8.yml
+++ b/tools/lint/flake8.yml
@@ -14,13 +14,14 @@ flake8:
- testing/mozbase
- testing/mochitest
- testing/talos/
- tools/git
- tools/lint
- tools/mercurial
- tools/tryselect
- toolkit/components/telemetry
- exclude:
- - testing/mochitest/pywebsocket
+ # Excludes should be added to topsrcdir/.flake8 due to a bug in flake8 where
+ # specifying --exclude causes custom configuration files to be ignored.
+ exclude: []
extensions: ['py']
type: external
payload: flake8_:lint
--- a/tools/lint/flake8_/__init__.py
+++ b/tools/lint/flake8_/__init__.py
@@ -1,21 +1,23 @@
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
import json
import os
import signal
import subprocess
+from collections import defaultdict
import which
from mozprocess import ProcessHandlerMixin
from mozlint import result
+from mozlint.pathutils import get_ancestors_by_name
here = os.path.abspath(os.path.dirname(__file__))
FLAKE8_REQUIREMENTS_PATH = os.path.join(here, 'flake8_requirements.txt')
FLAKE8_NOT_FOUND = """
Could not find flake8! Install flake8 and try again.
@@ -133,17 +135,17 @@ def run_process(config, cmd):
proc = Flake8Process(config, cmd)
proc.run()
try:
proc.wait()
except KeyboardInterrupt:
proc.kill()
-def lint(files, config, **lintargs):
+def lint(paths, config, **lintargs):
if not reinstall_flake8():
print(FLAKE8_INSTALL_ERROR)
return 1
binary = get_flake8_binary()
cmdargs = [
@@ -151,26 +153,24 @@ def lint(files, config, **lintargs):
'--format', '{"path":"%(path)s","lineno":%(row)s,'
'"column":%(col)s,"rule":"%(code)s","message":"%(text)s"}',
]
# Run any paths with a .flake8 file in the directory separately so
# it gets picked up. This means only .flake8 files that live in
# directories that are explicitly included will be considered.
# See bug 1277851
- no_config = []
- for f in files:
- if not os.path.isfile(os.path.join(f, '.flake8')):
- no_config.append(f)
- continue
- run_process(config, cmdargs+[f])
+ paths_by_config = defaultdict(list)
+ for path in paths:
+ configs = get_ancestors_by_name('.flake8', path, lintargs['root'])
+ paths_by_config[os.pathsep.join(configs) if configs else 'default'].append(path)
- # XXX For some reason passing in --exclude results in flake8 not using
- # the local .flake8 file. So for now only pass in --exclude if there
- # is no local config.
- exclude = lintargs.get('exclude')
- if exclude:
- cmdargs += ['--exclude', ','.join(lintargs['exclude'])]
+ for configs, paths in paths_by_config.items():
+ cmd = cmdargs[:]
- if no_config:
- run_process(config, cmdargs+no_config)
+ if configs != 'default':
+ configs = reversed(configs.split(os.pathsep))
+ cmd.extend(['--append-config={}'.format(c) for c in configs])
+
+ cmd.extend(paths)
+ run_process(config, cmd)
return results
--- a/tools/lint/yamllint_/__init__.py
+++ b/tools/lint/yamllint_/__init__.py
@@ -7,16 +7,17 @@ import os
import signal
import subprocess
from collections import defaultdict
import which
from mozprocess import ProcessHandlerMixin
from mozlint import result
+from mozlint.pathutils import get_ancestors_by_name
here = os.path.abspath(os.path.dirname(__file__))
YAMLLINT_REQUIREMENTS_PATH = os.path.join(here, 'yamllint_requirements.txt')
YAMLLINT_INSTALL_ERROR = """
Unable to install correct version of yamllint
@@ -115,39 +116,16 @@ def gen_yamllint_args(cmdargs, paths=Non
args = cmdargs[:]
if isinstance(paths, basestring):
paths = [paths]
if conf_file and conf_file != 'default':
return args + ['-c', conf_file] + paths
return args + paths
-def ancestors(path):
- while path:
- yield path
- (path, child) = os.path.split(path)
- if child == "":
- break
-
-
-def get_relevant_configs(name, path, root):
- """Returns a list of configuration files that exist in `path`'s ancestors,
- sorted from closest->furthest.
- """
- configs = []
- for path in ancestors(path):
- if path == root:
- break
-
- config = os.path.join(path, name)
- if os.path.isfile(config):
- configs.append(config)
- return configs
-
-
def lint(files, config, **lintargs):
if not reinstall_yamllint():
print(YAMLLINT_INSTALL_ERROR)
return 1
binary = get_yamllint_binary()
cmdargs = [
@@ -158,15 +136,15 @@ def lint(files, config, **lintargs):
config = config.copy()
config['root'] = lintargs['root']
# Run any paths with a .yamllint file in the directory separately so
# it gets picked up. This means only .yamllint files that live in
# directories that are explicitly included will be considered.
paths_by_config = defaultdict(list)
for f in files:
- conf_files = get_relevant_configs('.yamllint', f, config['root'])
+ conf_files = get_ancestors_by_name('.yamllint', f, config['root'])
paths_by_config[conf_files[0] if conf_files else 'default'].append(f)
for conf_file, paths in paths_by_config.items():
run_process(config, gen_yamllint_args(cmdargs, conf_file=conf_file, paths=paths))
return results