presubmit: Improve venv setup; expand checks
- Use env_setup/virtualenv/init.py to setup the Python venv.
- Improve diff colorization.
- Add function for finding Python packages from .py file changes.
- Add check that runs Python tests.
- Enable full pylint and yapf as warnings.
- Delete unused code in watch.py that used an undefined variable.
- Enable required python -E check.
Change-Id: I5ba9882ea9ec93f006260d5636031dd0b5005a85
diff --git a/presubmit.py b/presubmit.py
index 163450b..2d9efd3 100755
--- a/presubmit.py
+++ b/presubmit.py
@@ -25,7 +25,18 @@
from pw_presubmit import format_cc, presubmit_tools
-def _init_cipd():
+def presubmit_dir(*paths):
+ return os.path.join('.presubmit', *paths)
+
+
+def run_python_module(*args, **kwargs):
+ return call('python', '-m', *args, **kwargs)
+
+
+#
+# Initialization
+#
+def init_cipd():
cipd = os.path.abspath('.presubmit/cipd')
call(sys.executable, 'env_setup/cipd/update.py', '--install-dir', cipd)
os.environ['PATH'] = os.pathsep.join((
@@ -36,28 +47,27 @@
print('PATH', os.environ['PATH'])
-def _init_virtualenv():
+@filter_paths(endswith='.py') # Only run if there are .py files.
+def init_virtualenv(unused_paths):
"""Set up virtualenv, assumes recent Python 3 is already installed."""
venv = os.path.abspath('.presubmit/venv')
+
+ # For speed, don't build the venv if it exists. Use --clean to recreate it.
if not os.path.isdir(venv):
- call('python', '-m', 'venv', venv)
+ call('python3',
+ 'env_setup/virtualenv/init.py', f'--venv_path={venv}',
+ '--requirements=env_setup/virtualenv/requirements.txt')
+
os.environ['PATH'] = os.pathsep.join((
os.path.join(venv, 'bin'),
os.environ['PATH'],
))
- call('python', '-m', 'pip', 'install', '--upgrade', 'pip')
- call('python', '-m', 'pip', 'install',
- '--log', os.path.join(venv, 'pip.log'),
- '-r', 'env_setup/virtualenv/requirements.txt') # yapf: disable
-
-def init():
- _init_cipd()
- _init_virtualenv()
-
-
-presubmit_dir = lambda *paths: os.path.join('.presubmit', *paths)
+INIT = (
+ init_cipd,
+ init_virtualenv,
+)
#
@@ -83,7 +93,8 @@
call('ninja', '-C', presubmit_dir('clang'))
-def gn_gcc_build():
+@filter_paths(endswith=format_cc.SOURCE_EXTENSIONS)
+def gn_gcc_build(unused_paths):
call(
*GN_GEN, presubmit_dir('gcc'),
gn_args(pw_target_config='"//targets/host/host.gni"',
@@ -91,7 +102,8 @@
call('ninja', '-C', presubmit_dir('gcc'))
-def gn_arm_build():
+@filter_paths(endswith=format_cc.SOURCE_EXTENSIONS)
+def gn_arm_build(unused_paths):
call(
*GN_GEN, presubmit_dir('arm'),
gn_args(
@@ -137,43 +149,58 @@
# Python presubmit checks
#
@filter_paths(endswith='.py')
-def pylint_errors(paths):
- call(sys.executable, '-m', 'pylint', '-E', *paths)
+def test_python_packages(paths):
+ packages = presubmit_tools.find_python_packages(paths)
+
+ if not packages:
+ print('No Python packages were found.')
+ return
+
+ for package in packages:
+ call('python', os.path.join(package, 'setup.py'), 'test')
+
+
+@filter_paths(endswith='.py')
+def pylint_errors_only(paths):
+ run_python_module('pylint', '-E', '-j', '0', *paths)
+
+
+@filter_paths(endswith='.py')
+def pylint(paths):
+ try:
+ run_python_module('pylint', '-j', '0', *paths)
+ except PresubmitFailure:
+ # TODO(hepler): Enforce pylint when it passes.
+ print('--> pylint checks FAILED!')
+ print(' Treating this as a warning... for now.')
@filter_paths(endswith='.py')
def yapf(paths):
- from yapf.yapflib.yapf_api import FormatFile
-
- errors = []
-
- for path in paths:
- diff, _, changed = FormatFile(path, print_diff=True, in_place=False)
- if changed:
- errors.append(path)
- print(format_cc.colorize_diff(diff))
-
- if errors:
- print(f'--> Files with formatting errors: {len(errors)}')
- print(' ', '\n '.join(errors))
- raise PresubmitFailure
+ try:
+ run_python_module('yapf',
+ '--diff',
+ '--parallel',
+ *paths,
+ print_output=False)
+ except PresubmitFailure as e:
+ # TODO(hepler): Enforce yapf when it passes.
+ print(format_cc.colorize_diff(str(e)))
+ print('--> Python formatting checks FAILED!')
+ print(' Treating this as a warning... for now.')
@filter_paths(endswith='.py', exclude=r'(?:.+/)?setup\.py')
def mypy(paths):
- import mypy.api as mypy_api
-
- report, errors, exit_status = mypy_api.run(paths)
- if exit_status:
- print(errors)
- print(report)
- raise PresubmitFailure
+ run_python_module('mypy', *paths)
PYTHON = (
- # TODO(hepler): Enable yapf, mypy, and pylint when they pass.
- # pylint_errors,
- # yapf,
+ test_python_packages,
+ pylint_errors_only, # TODO(hepler): Remove this when pylint is passing.
+ pylint,
+ yapf,
+ # TODO(hepler): Enable mypy when it passes.
# mypy,
)
@@ -260,16 +287,17 @@
# Presubmit check programs
#
QUICK_PRESUBMIT = (
- *GENERAL,
+ *INIT,
*PYTHON,
gn_format,
gn_clang_build,
presubmit_tools.pragma_once,
clang_format,
+ *GENERAL,
)
PROGRAMS = {
- 'full': GN + CC + PYTHON + BAZEL + GENERAL,
+ 'full': INIT + GN + CC + PYTHON + BAZEL + GENERAL,
'quick': QUICK_PRESUBMIT,
}
@@ -280,10 +308,6 @@
'--clean',
action='store_true',
help='Deletes the .presubmit directory before starting')
- parser.add_argument(
- '--skip_init',
- action='store_true',
- help='Clone the buildtools to prior to running the checks')
parser.add_argument('-p',
'--program',
choices=PROGRAMS,
@@ -297,11 +321,10 @@
if args.clean and os.path.exists(presubmit_dir()):
shutil.rmtree(presubmit_dir())
- init_step = () if args.skip_init else (init,)
- program = init_step + PROGRAMS[args.program]
+ program = PROGRAMS[args.program]
# Remove custom arguments so we can use args to call run_presubmit.
- del args.clean, args.program, args.skip_init
+ del args.clean, args.program
return 0 if presubmit_tools.run_presubmit(program, **vars(args)) else 1
diff --git a/pw_cli/py/pw_cli/watch.py b/pw_cli/py/pw_cli/watch.py
index 08acdc3..9fc5618 100755
--- a/pw_cli/py/pw_cli/watch.py
+++ b/pw_cli/py/pw_cli/watch.py
@@ -176,14 +176,6 @@
self.state = _State.WAITING_FOR_FILE_CHANGE_EVENT
self.on_any_event() # Retrigger.
- def on_success(self):
- _LOG.debug('Build and tests passed')
- print(_Color.green(PASS_MESSAGE))
-
- def on_fail(self):
- _LOG.debug('Build and tests failed')
- print(_Color.red(_FAIL_MESSAGE))
-
_WATCH_PATTERN_DELIMITER = ','
_WATCH_PATTERNS = (
diff --git a/pw_presubmit/format_cc.py b/pw_presubmit/format_cc.py
index 625a2c6..9ef63ae 100755
--- a/pw_presubmit/format_cc.py
+++ b/pw_presubmit/format_cc.py
@@ -31,8 +31,10 @@
return f'{start}{{}}\033[0m'.format if os.name == 'posix' else str
-color_green = _make_color(32)
color_red = _make_color(31)
+color_green = _make_color(32)
+color_aqua = _make_color(36)
+color_bold_white = _make_color(37, 1)
def _find_extensions(directory, extensions) -> Iterable[str]:
@@ -63,10 +65,14 @@
def _colorize_diff_line(line: str) -> str:
- if line.startswith('-') and not line.startswith('--- '):
+ if line.startswith('--- ') or line.startswith('+++ '):
+ return color_bold_white(line)
+ if line.startswith('-'):
return color_red(line)
- if line.startswith('+') and not line.startswith('+++ '):
+ if line.startswith('+'):
return color_green(line)
+ if line.startswith('@@ '):
+ return color_aqua(line)
return line
diff --git a/pw_presubmit/presubmit_tools.py b/pw_presubmit/presubmit_tools.py
index 9cb56e3..c3ad296 100755
--- a/pw_presubmit/presubmit_tools.py
+++ b/pw_presubmit/presubmit_tools.py
@@ -50,7 +50,7 @@
import subprocess
import sys
import time
-from typing import Callable, Iterable, List, Optional, Sequence
+from typing import Callable, Dict, Iterable, List, Optional, Sequence
from inspect import signature
@@ -327,6 +327,26 @@
return run_presubmit(program, **vars(arg_parser.parse_args()))
+def find_python_packages(python_paths) -> Dict[str, List[str]]:
+ """Returns Python package directories for the files in python_paths."""
+ setup_pys = [
+ os.path.dirname(file)
+ for file in git_list_files('setup.py', '*/setup.py')
+ ]
+
+ package_dirs = collections.defaultdict(list)
+
+ for path in python_paths:
+ try:
+ setup_dir = max(setup for setup in setup_pys
+ if path.startswith(setup))
+ package_dirs[os.path.abspath(setup_dir)].append(path)
+ except ValueError:
+ continue
+
+ return package_dirs
+
+
def _wrap_if_str(value: Iterable[str]) -> List[str]:
return [value] if isinstance(value, str) else value
@@ -362,7 +382,7 @@
return filter_paths_for_function
-def call(*args, **kwargs) -> None:
+def call(*args, print_output=True, **kwargs) -> None:
"""Optional subprocess wrapper with helpful output."""
print('[COMMAND]',
', '.join(f'{k}={v}' for k, v in sorted(kwargs.items())))
@@ -374,9 +394,14 @@
**kwargs)
if process.returncode:
print(f'[RESULT] Failed with return code {process.returncode}.')
- print('[OUTPUT]')
- print(process.stdout.decode(errors='backslashreplace'))
- raise PresubmitFailure
+ output = process.stdout.decode(errors='backslashreplace')
+
+ if print_output:
+ print('[OUTPUT]')
+ print(output)
+ raise PresubmitFailure
+
+ raise PresubmitFailure(output)
@filter_paths(endswith='.h')
