pw_presubmit: Support multiple repositories
- Support checking files in multiple repositories.
- Support specifying the root path to use, which does not have to be in
a Git repo.
- Remove the --repository argument, which is redundant with -C and
conflicts with having multiple repos.
- Update the semantics for specifying pathspecs to work naturally across
multiple repositories.
- Update the Pigweed presubmit with the new arguments.
- Expand docs.rst and docstrings.
Change-Id: Ibaa494bdc645af3e8f8cc3b736f89098de0d1634
diff --git a/pw_cli/docs.rst b/pw_cli/docs.rst
index 99a22fa..fa5da59 100644
--- a/pw_cli/docs.rst
+++ b/pw_cli/docs.rst
@@ -58,7 +58,6 @@
defined in Python packages that are installed in the Pigweed virtual
environment.
-
Plugin registrations in a ``PW_PLUGINS`` file apply to the their directory and
all subdirectories, similarly to configuration files like ``.clang-format``.
Registered plugins appear as commands in the ``pw`` tool when ``pw`` is run from
diff --git a/pw_presubmit/docs.rst b/pw_presubmit/docs.rst
index 3bbfd6f..427f15c 100644
--- a/pw_presubmit/docs.rst
+++ b/pw_presubmit/docs.rst
@@ -4,9 +4,9 @@
.. highlight:: sh
-------------
+============
pw_presubmit
-------------
+============
The presubmit module provides Python tools for running presubmit checks and
checking and fixing code format. It also includes the presubmit check script for
the Pigweed repository, ``pigweed_presubmit.py``.
@@ -27,20 +27,56 @@
:alt: ``pw format`` demo
:align: left
+The ``pw_presubmit`` package includes presubmit checks that can be used with any
+project. These checks include:
+
+* Check code format of several languages including C, C++, and Python
+* Initialize a Python environment
+* Run all Python tests
+* Run pylint
+* Run mypy
+* Ensure source files are included in the GN and Bazel builds
+* Build and run all tests with GN
+* Build and run all tests with Bazel
+* Ensure all header files contain ``#pragma once``
+
+-------------
Compatibility
-=============
+-------------
Python 3
-Presubmit tools
-===============
+-------------------------------------------
+Creating a presubmit check for your project
+-------------------------------------------
+Creating a presubmit check for a project using ``pw_presubmit`` is simple, but
+requires some customization. Projects must define their own presubmit check
+Python script that uses the ``pw_presubmit`` package.
-Defining presubmit checks
--------------------------
+A project's presubmit script can be registered as a
+:ref:`pw_cli <chapter-pw-cli>` plugin, so that it can be run as ``pw
+presubmit``.
+
+Setting up the command-line interface
+-------------------------------------
+The ``pw_presubmit.cli`` module sets up the command-line interface for a
+presubmit script. This defines a standard set of arguments for invoking
+presubmit checks. Its use is optional, but recommended.
+
+pw_presubmit.cli
+~~~~~~~~~~~~~~~~
+.. automodule:: pw_presubmit.cli
+ :members: add_arguments, run
+
+Presubmit checks
+----------------
A presubmit check is defined as a function or other callable. The function must
accept one argument: a ``PresubmitContext``, which provides the paths on which
-to run. Presubmit checks communicate failure by raising any exception.
+to run. Presubmit checks communicate failure by raising an exception.
-For example, either of these functions may be used as presubmit checks:
+Presubmit checks may use the ``filter_paths`` decorator to automatically filter
+the paths list for file types they care about.
+
+Either of these functions could be used as presubmit checks:
.. code-block:: python
@@ -55,42 +91,155 @@
def run_the_build(_):
subprocess.run(['make', 'release'], check=True)
-Presubmit checks may use the ``filter_paths`` decorator to automatically filter
-the paths list for file types they care about.
+Presubmit checks functions are grouped into "programs" -- a named series of
+checks. Projects may find it helpful to have programs for different purposes,
+such as a quick program for local use and a full program for automated use. The
+:ref:`example script <example-script>` uses ``pw_presubmit.Programs`` to define
+``quick`` and ``full`` programs.
-See ``pigweed_presubmit.py`` for an example of how to define presubmit checks.
+pw_presubmit
+~~~~~~~~~~~~
+.. automodule:: pw_presubmit
+ :members: filter_paths, call, PresubmitFailure, Programs
-Key members
-^^^^^^^^^^^
-.. autofunction:: pw_presubmit.cli.add_arguments
+.. _example-script:
-.. autofunction:: pw_presubmit.cli.run
+Example
+-------
+A simple example presubmit check script follows. This can be copied-and-pasted
+to serve as a starting point for a project's presubmit check script.
-.. autodecorator:: pw_presubmit.filter_paths
+See ``pigweed_presubmit.py`` for a more complex presubmit check script example.
-.. autofunction:: pw_presubmit.call
+.. code-block:: python
-.. autoexception:: pw_presubmit.PresubmitFailure
+ """Example presubmit check script."""
-.. autoexception:: pw_presubmit.Programs
+ import argparse
+ import logging
+ import os
+ from pathlib import Path
+ import re
+ import sys
+ from typing import List, Pattern
-Included presubmit checks
--------------------------
-The ``pw_presubmit`` package includes presubmit checks that can be used with any
-project. These checks include:
+ try:
+ import pw_cli.log
+ except ImportError:
+ print('ERROR: Activate the environment before running presubmits!',
+ file=sys.stderr)
+ sys.exit(2)
-* Check code format of several languages including C, C++, and Python
-* Initialize a Python environment
-* Run all Python tests
-* Run pylint
-* Run mypy
-* Ensure source files are included in the GN and Bazel builds
-* Build and run all tests with GN
-* Build and run all tests with Bazel
-* Ensure all header files contain ``#pragma once``
+ import pw_presubmit
+ from pw_presubmit import build, cli, environment, format_code, git_repo
+ from pw_presubmit import python_checks, filter_paths, PresubmitContext
+ from pw_presubmit.install_hook import install_hook
+ # Set up variables for key project paths.
+ PROJECT_ROOT = Path(os.environ['MY_PROJECT_ROOT'])
+ PIGWEED_ROOT = PROJECT_ROOT / 'pigweed'
+
+ #
+ # Initialization
+ #
+ def init_cipd(ctx: PresubmitContext):
+ environment.init_cipd(PIGWEED_ROOT, ctx.output_dir)
+
+
+ def init_virtualenv(ctx: PresubmitContext):
+ environment.init_virtualenv(PIGWEED_ROOT,
+ ctx.output_dir,
+ setup_py_roots=[PROJECT_ROOT])
+
+
+ # Rerun the build if files with these extensions change.
+ _BUILD_EXTENSIONS = frozenset(
+ ['.rst', '.gn', '.gni', *format_code.C_FORMAT.extensions])
+
+
+ #
+ # Presubmit checks
+ #
+ def release_build(ctx: PresubmitContext):
+ build.gn_gen(PROJECT_ROOT, ctx.output_dir, build_type='release')
+ build.ninja(ctx.output_dir)
+
+
+ def host_tests(ctx: PresubmitContext):
+ build.gn_gen(PROJECT_ROOT, ctx.output_dir, run_host_tests='true')
+ build.ninja(ctx.output_dir)
+
+
+ # Avoid running some checks on certain paths.
+ PATH_EXCLUSIONS = (
+ re.compile(r'^external/'),
+ re.compile(r'^vendor/'),
+ )
+
+
+ # Use the upstream pragma_once check, but apply a different set of path
+ # filters with @filter_paths.
+ @filter_paths(endswith='.h', exclude=PATH_EXCLUSIONS)
+ def pragma_once(ctx: PresubmitContext):
+ pw_presubmit.pragma_once(ctx)
+
+
+ #
+ # Presubmit check programs
+ #
+ QUICK = (
+ # Initialize an environment for running presubmit checks.
+ init_cipd,
+ init_virtualenv,
+ # List some presubmit checks to run
+ pragma_once,
+ host_tests,
+ # Use the upstream formatting checks, with custom path filters applied.
+ format_code.presubmit_checks(exclude=PATH_EXCLUSIONS),
+ )
+
+ FULL = (
+ QUICK, # Add all checks from the 'quick' program
+ release_build,
+ # Use the upstream Python checks, with custom path filters applied.
+ python_checks.all_checks(exclude=PATH_EXCLUSIONS),
+ )
+
+ PROGRAMS = pw_presubmit.Programs(quick=QUICK, full=FULL)
+
+
+ def run(install: bool, **presubmit_args) -> int:
+ """Process the --install argument then invoke pw_presubmit."""
+
+ # Install the presubmit Git pre-push hook, if requested.
+ if install:
+ install_hook(__file__, 'pre-push', ['--base', 'HEAD~'],
+ git_repo.root())
+ return 0
+
+ return cli.run(root=PROJECT_ROOT, **presubmit_args)
+
+
+ def main() -> int:
+ """Run the presubmit checks for this repository."""
+ parser = argparse.ArgumentParser(description=__doc__)
+ cli.add_arguments(parser, PROGRAMS, 'quick')
+
+ # Define an option for installing a Git pre-push hook for this script.
+ parser.add_argument(
+ '--install',
+ action='store_true',
+ help='Install the presubmit as a Git pre-push hook and exit.')
+
+ return run(**vars(parser.parse_args()))
+
+ if __name__ == '__main__':
+ pw_cli.log.install(logging.INFO)
+ sys.exit(main())
+
+---------------------
Code formatting tools
-=====================
+---------------------
The ``pw_presubmit.format_code`` module formats supported source files using
external code format tools. The file ``format_code.py`` can be invoked directly
from the command line or from ``pw`` as ``pw format``.
diff --git a/pw_presubmit/py/pw_presubmit/cli.py b/pw_presubmit/py/pw_presubmit/cli.py
index 2c46b10..37d1a57 100644
--- a/pw_presubmit/py/pw_presubmit/cli.py
+++ b/pw_presubmit/py/pw_presubmit/cli.py
@@ -18,7 +18,7 @@
from pathlib import Path
import re
import shutil
-from typing import Callable, Optional, Sequence
+from typing import Callable, Collection, Optional, Sequence
from pw_presubmit import git_repo, presubmit
@@ -32,24 +32,24 @@
'paths',
metavar='pathspec',
nargs='*',
- type=Path,
- help=('Paths to which to restrict the checks. These are interpreted '
- 'as Git pathspecs. If --base is provided, only paths changed '
- 'since that commit are checked.'))
+ help=('Paths or patterns to which to restrict the checks. These are '
+ 'interpreted as Git pathspecs. If --base is provided, only '
+ 'paths changed since that commit are checked.'))
parser.add_argument(
'-b',
'--base',
- metavar='COMMIT',
+ metavar='commit',
help=('Git revision against which to diff for changed files. '
'If none is provided, the entire repository is used.'))
parser.add_argument(
'-e',
'--exclude',
- metavar='REGULAR_EXPRESSION',
+ metavar='regular_expression',
default=[],
action='append',
type=re.compile,
- help='Exclude paths matching any of these regular expressions.')
+ help=('Exclude paths matching any of these regular expressions, '
+ "which are interpreted relative to each Git repository's root."))
def _add_programs_arguments(exclusive: argparse.ArgumentParser,
@@ -99,13 +99,6 @@
"""Adds common presubmit check options to an argument parser."""
add_path_arguments(parser)
- parser.add_argument(
- '--repository',
- default=Path.cwd(),
- type=Path,
- help=(
- 'Change to this directory before resolving paths or running the '
- 'presubmit. Presubmit checks must be run from a Git repository.'))
parser.add_argument('-k',
'--keep-going',
action='store_true',
@@ -132,16 +125,36 @@
def run(
- program: Sequence[Callable],
- clear: bool,
- repository: Path,
- output_directory: Path,
- **other_args,
+ program: Sequence[Callable],
+ output_directory: Path,
+ clear: bool,
+ root: Path = None,
+ repositories: Collection[Path] = (),
+ **other_args,
) -> int:
- """Processes all arguments from add_arguments and runs the presubmit."""
+ """Processes arguments from add_arguments and runs the presubmit.
+
+ Args:
+ root: base path from which to run presubmit checks; defaults to the root
+ of the current directory's repository
+ repositories: roots of Git repositories on which to run presubmit checks;
+ defaults to the root of the current directory's repository
+ program: from the --program option
+ output_directory: from --output-directory option
+ clear: from the --clear option
+ **other_args: remaining arguments defined by by add_arguments
+
+ Returns:
+ exit code for sys.exit; 0 if succesful, 1 if an error occurred
+ """
+ if root is None:
+ root = git_repo.root()
+
+ if not repositories:
+ repositories = [root]
if not output_directory:
- output_directory = git_repo.path('.presubmit', repo=repository)
+ output_directory = root / '.presubmit'
_LOG.debug('Using environment at %s', output_directory)
@@ -155,7 +168,8 @@
return 0
if presubmit.run(program,
- repo_path=repository,
+ root,
+ repositories,
output_directory=output_directory,
**other_args):
return 0
diff --git a/pw_presubmit/py/pw_presubmit/format_code.py b/pw_presubmit/py/pw_presubmit/format_code.py
index dc017cf..4f84408 100755
--- a/pw_presubmit/py/pw_presubmit/format_code.py
+++ b/pw_presubmit/py/pw_presubmit/format_code.py
@@ -29,7 +29,7 @@
import subprocess
import sys
from typing import Callable, Collection, Dict, Iterable, List, NamedTuple
-from typing import Optional, Pattern, Tuple
+from typing import Optional, Pattern, Tuple, Union
try:
import pw_presubmit
@@ -41,7 +41,7 @@
import pw_presubmit
from pw_presubmit import cli, git_repo
-from pw_presubmit.tools import file_summary, log_run, plural
+from pw_presubmit.tools import exclude_paths, file_summary, log_run, plural
_LOG: logging.Logger = logging.getLogger(__name__)
@@ -323,13 +323,14 @@
class CodeFormatter:
"""Checks or fixes the formatting of a set of files."""
- def __init__(self, files: Collection[Path]):
+ def __init__(self, files: Iterable[Path]):
self.paths = list(files)
self._formats: Dict[CodeFormat, List] = collections.defaultdict(list)
- for path in files:
+ for path in self.paths:
for code_format in CODE_FORMATS:
- if any(str(path).endswith(e) for e in code_format.extensions):
+ if any(path.as_posix().endswith(e)
+ for e in code_format.extensions):
self._formats[code_format].append(path)
def check(self) -> Dict[Path, str]:
@@ -350,19 +351,19 @@
plural(files, code_format.language + ' file'))
-def _file_summary(files: Iterable[Path], base: Path) -> List[str]:
+def _file_summary(files: Iterable[Union[Path, str]], base: Path) -> List[str]:
try:
- return file_summary(f.resolve().relative_to(base.resolve())
- for f in files)
+ return file_summary(
+ Path(f).resolve().relative_to(base.resolve()) for f in files)
except ValueError:
return []
-def format_paths_in_repo(paths: Collection[Path],
+def format_paths_in_repo(paths: Collection[Union[Path, str]],
exclude: Collection[Pattern[str]], fix: bool,
base: str) -> int:
"""Checks or fixes formatting for files in a Git repo."""
- files = [path.resolve() for path in paths if path.is_file()]
+ files = [Path(path).resolve() for path in paths if os.path.isfile(path)]
repo = git_repo.root() if git_repo.is_repo() else None
# If this is a Git repo, list the original paths with git ls-files or diff.
@@ -373,7 +374,8 @@
# Add files from Git and remove duplicates.
files = sorted(
- set(git_repo.list_files(base, paths, exclude)) | set(files))
+ set(exclude_paths(exclude, git_repo.list_files(base, paths)))
+ | set(files))
elif base:
_LOG.critical(
'A base commit may only be provided if running from a Git repo')
@@ -382,11 +384,11 @@
return format_files(files, fix, repo=repo)
-def format_files(paths: Collection[Path],
+def format_files(paths: Collection[Union[Path, str]],
fix: bool,
repo: Optional[Path] = None) -> int:
"""Checks or fixes formatting for the specified files."""
- formatter = CodeFormatter(paths)
+ formatter = CodeFormatter(Path(p) for p in paths)
_LOG.info('Checking formatting for %s', plural(formatter.paths, 'file'))
diff --git a/pw_presubmit/py/pw_presubmit/git_repo.py b/pw_presubmit/py/pw_presubmit/git_repo.py
index cb2ebc7..5f29ae1 100644
--- a/pw_presubmit/py/pw_presubmit/git_repo.py
+++ b/pw_presubmit/py/pw_presubmit/git_repo.py
@@ -16,7 +16,7 @@
import collections
from pathlib import Path
import subprocess
-from typing import Collection, Dict, Iterable, Iterator, List, Optional
+from typing import Collection, Dict, Iterable, List, Optional
from typing import Pattern, Union
from pw_presubmit.tools import log_run, plural
@@ -24,59 +24,56 @@
PathOrStr = Union[Path, str]
-def git_stdout(*args: PathOrStr, repo: PathOrStr = '.') -> str:
+def git_stdout(*args: PathOrStr,
+ show_stderr=False,
+ repo: PathOrStr = '.') -> str:
return log_run(['git', '-C', repo, *args],
stdout=subprocess.PIPE,
+ stderr=None if show_stderr else subprocess.DEVNULL,
check=True).stdout.decode().strip()
-def _ls_files(args: Collection[PathOrStr], repo: Path) -> List[Path]:
- repo = repo.resolve()
- return [
- repo / path for path in git_stdout('ls-files', '--', *args,
- repo=repo).splitlines()
- ]
+def _ls_files(args: Collection[PathOrStr], repo: Path) -> Iterable[Path]:
+ """Returns results of git ls-files as absolute paths."""
+ git_root = repo.resolve()
+ for file in git_stdout('ls-files', '--', *args, repo=repo).splitlines():
+ yield git_root / file
def _diff_names(commit: str, pathspecs: Collection[PathOrStr],
- repo: Path) -> List[Path]:
+ repo: Path) -> Iterable[Path]:
"""Returns absolute paths of files changed since the specified commit."""
git_root = root(repo)
- return [
- git_root.joinpath(path).resolve()
- for path in git_stdout('diff',
- '--name-only',
- '--diff-filter=d',
- commit,
- '--',
- *pathspecs,
- repo=repo).splitlines()
- ]
+ for file in git_stdout('diff',
+ '--name-only',
+ '--diff-filter=d',
+ commit,
+ '--',
+ *pathspecs,
+ repo=repo).splitlines():
+ yield git_root / file
def list_files(commit: Optional[str] = None,
pathspecs: Collection[PathOrStr] = (),
- exclude: Collection[Pattern[str]] = (),
- repo: Optional[Path] = None) -> List[Path]:
+ repo_path: Optional[Path] = None) -> List[Path]:
"""Lists files with git ls-files or git diff --name-only.
Args:
commit: commit to use as a base for git diff
pathspecs: Git pathspecs to use in git ls-files or diff
- exclude: regular expressions for Posix-style paths to exclude
- repo: repository path from which to run commands; defaults to Path.cwd()
+ repo_path: repo path from which to run commands; defaults to Path.cwd()
+
+ Returns:
+ A sorted list of absolute paths
"""
- if repo is None:
- repo = Path.cwd()
+ if repo_path is None:
+ repo_path = Path.cwd()
if commit:
- files = _diff_names(commit, pathspecs, repo)
- else:
- files = _ls_files(pathspecs, repo)
+ return sorted(_diff_names(commit, pathspecs, repo_path))
- git_root = root(repo=repo).resolve()
- return sorted(file for file in files if not any(
- e.search(file.relative_to(git_root).as_posix()) for e in exclude))
+ return sorted(_ls_files(pathspecs, repo_path))
def has_uncommitted_changes(repo: Optional[Path] = None) -> bool:
@@ -98,7 +95,7 @@
def _describe_constraints(git_root: Path, repo_path: Path,
commit: Optional[str],
pathspecs: Collection[PathOrStr],
- exclude: Collection[Pattern]) -> Iterator[str]:
+ exclude: Collection[Pattern[str]]) -> Iterable[str]:
if not git_root.samefile(repo_path):
yield (
f'under the {repo_path.resolve().relative_to(git_root.resolve())} '
@@ -132,14 +129,35 @@
return msg + ''.join(f'\n - {line}' for line in constraints)
+def root(repo_path: PathOrStr = '.', *, show_stderr: bool = True) -> Path:
+ """Returns the repository root as an absolute path.
+
+ Raises:
+ FileNotFoundError: the path does not exist
+ subprocess.CalledProcessError: the path is not in a Git repo
+ """
+ repo_path = Path(repo_path)
+ if not repo_path.exists():
+ raise FileNotFoundError(f'{repo_path} does not exist')
+
+ return Path(
+ git_stdout('rev-parse',
+ '--show-toplevel',
+ repo=repo_path,
+ show_stderr=show_stderr))
+
+
+def within_repo(repo_path: PathOrStr = '.') -> Optional[Path]:
+ """Similar to root(repo_path), returns None if the path is not in a repo."""
+ try:
+ return root(repo_path, show_stderr=False)
+ except subprocess.CalledProcessError:
+ return None
+
+
def is_repo(repo_path: PathOrStr = '.') -> bool:
- return not subprocess.run(['git', '-C', repo_path, 'rev-parse'],
- stderr=subprocess.DEVNULL).returncode
-
-
-def root(repo: PathOrStr = '.') -> Path:
- """Returns the repository root as an absolute path."""
- return Path(git_stdout('rev-parse', '--show-toplevel', repo=repo))
+ """True if the path is tracked by a Git repo."""
+ return within_repo(repo_path) is not None
def path(repo_path: PathOrStr,
diff --git a/pw_presubmit/py/pw_presubmit/install_hook.py b/pw_presubmit/py/pw_presubmit/install_hook.py
index ac16e47..1df7011 100755
--- a/pw_presubmit/py/pw_presubmit/install_hook.py
+++ b/pw_presubmit/py/pw_presubmit/install_hook.py
@@ -20,12 +20,12 @@
from pathlib import Path
import shlex
import subprocess
-from typing import Sequence
+from typing import Sequence, Union
_LOG: logging.Logger = logging.getLogger(__name__)
-def git_repo_root(path) -> Path:
+def git_repo_root(path: Union[Path, str]) -> Path:
return Path(
subprocess.run(['git', '-C', path, 'rev-parse', '--show-toplevel'],
check=True,
@@ -35,7 +35,7 @@
def install_hook(script,
hook: str,
args: Sequence[str] = (),
- repository='.') -> None:
+ repository: Union[Path, str] = '.') -> None:
"""Installs a simple Git hook that calls a script with arguments."""
root = git_repo_root(repository).resolve()
script = os.path.relpath(script, root)
diff --git a/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py b/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
index 154f400..b46df18 100755
--- a/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
+++ b/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
@@ -44,11 +44,11 @@
# Initialization
#
def init_cipd(ctx: PresubmitContext):
- environment.init_cipd(ctx.repo_root, ctx.output_dir)
+ environment.init_cipd(ctx.root, ctx.output_dir)
def init_virtualenv(ctx: PresubmitContext):
- environment.init_cipd(ctx.repo_root, ctx.output_dir)
+ environment.init_cipd(ctx.root, ctx.output_dir)
#
@@ -63,14 +63,14 @@
def gn_clang_build(ctx: PresubmitContext):
- build.gn_gen(ctx.repo_root, ctx.output_dir, _CLANG_GEN_ARGS)
+ build.gn_gen(ctx.root, ctx.output_dir, _CLANG_GEN_ARGS)
build.ninja(ctx.output_dir)
@filter_paths(endswith=format_code.C_FORMAT.extensions)
def gn_gcc_build(ctx: PresubmitContext):
build.gn_gen(
- ctx.repo_root, ctx.output_dir,
+ ctx.root, ctx.output_dir,
build.gn_args(pw_target_config='"//targets/host/target_config.gni"',
pw_target_toolchain='"//pw_toolchain:host_gcc_os"'))
build.ninja(ctx.output_dir)
@@ -82,7 +82,7 @@
@filter_paths(endswith=format_code.C_FORMAT.extensions)
def gn_arm_build(ctx: PresubmitContext):
- build.gn_gen(ctx.repo_root, ctx.output_dir, _ARM_GEN_ARGS)
+ build.gn_gen(ctx.root, ctx.output_dir, _ARM_GEN_ARGS)
build.ninja(ctx.output_dir)
@@ -92,17 +92,17 @@
@filter_paths(endswith=format_code.C_FORMAT.extensions)
def gn_qemu_build(ctx: PresubmitContext):
- build.gn_gen(ctx.repo_root, ctx.output_dir, _QEMU_GEN_ARGS)
+ build.gn_gen(ctx.root, ctx.output_dir, _QEMU_GEN_ARGS)
build.ninja(ctx.output_dir)
def gn_docs_build(ctx: PresubmitContext):
- build.gn_gen(ctx.repo_root, ctx.output_dir, _DOCS_GEN_ARGS)
+ build.gn_gen(ctx.root, ctx.output_dir, _DOCS_GEN_ARGS)
build.ninja(ctx.output_dir, 'docs:docs')
def gn_host_tools(ctx: PresubmitContext):
- build.gn_gen(ctx.repo_root,
+ build.gn_gen(ctx.root,
ctx.output_dir,
pw_target_config='"//targets/host/target_config.gni"',
pw_target_toolchain='"//pw_toolchain:host_clang_os"',
@@ -112,7 +112,7 @@
@filter_paths(endswith=format_code.C_FORMAT.extensions)
def oss_fuzz_build(ctx: PresubmitContext):
- build.gn_gen(ctx.repo_root,
+ build.gn_gen(ctx.root,
ctx.output_dir,
oss_fuzz_enabled='true',
pw_target_toolchain='"//pw_toolchain:host_clang_og"',
@@ -123,7 +123,7 @@
@filter_paths(endswith=(*format_code.C_FORMAT.extensions, '.cmake',
'CMakeLists.txt'))
def cmake_tests(ctx: PresubmitContext):
- build.cmake(ctx.repo_root, ctx.output_dir, env=build.env_with_clang_vars())
+ build.cmake(ctx.root, ctx.output_dir, env=build.env_with_clang_vars())
build.ninja(ctx.output_dir, 'pw_run_tests.modules')
@@ -138,7 +138,7 @@
'--worker_verbose',
'--symlink_prefix',
ctx.output_dir.joinpath('bazel-'),
- cwd=ctx.repo_root,
+ cwd=ctx.root,
env=build.env_with_clang_vars())
except:
_LOG.info('If the Bazel build inexplicably fails while the '
@@ -157,7 +157,7 @@
@filter_paths(endswith=format_code.C_FORMAT.extensions)
def clang_tidy(ctx: PresubmitContext):
- build.gn_gen(ctx.repo_root, ctx.output_dir, '--export-compile-commands',
+ build.gn_gen(ctx.root, ctx.output_dir, '--export-compile-commands',
_CLANG_GEN_ARGS)
build.ninja(ctx.output_dir)
build.ninja(ctx.output_dir, '-t', 'compdb', 'objcxx', 'cxx')
@@ -284,13 +284,13 @@
)
for directory, args in gn_gens_to_run:
- build.gn_gen(ctx.repo_root, directory, args)
+ build.gn_gen(ctx.root, directory, args)
missing = build.check_builds_for_files(_SOURCES_IN_BUILD,
ctx.paths,
- bazel_dirs=[ctx.repo_root],
+ bazel_dirs=[ctx.root],
gn_dirs=[
- (ctx.repo_root, path)
+ (ctx.root, path)
for path, _ in gn_gens_to_run
])
@@ -306,11 +306,11 @@
'Skipping build_env_setup since PW_CARGO_SETUP is not set')
return
- tmpl = ctx.repo_root.joinpath('pw_env_setup', 'py', 'pyoxidizer.bzl.tmpl')
+ tmpl = ctx.root.joinpath('pw_env_setup', 'py', 'pyoxidizer.bzl.tmpl')
out = ctx.output_dir.joinpath('pyoxidizer.bzl')
with open(tmpl, 'r') as ins:
- cfg = ins.read().replace('${PW_ROOT}', str(ctx.repo_root))
+ cfg = ins.read().replace('${PW_ROOT}', str(ctx.root))
with open(out, 'w') as outs:
outs.write(cfg)
@@ -385,16 +385,16 @@
return parser.parse_args()
-def run(install: bool, repository: Path, **presubmit_args) -> int:
+def run(install: bool, **presubmit_args) -> int:
"""Entry point for presubmit."""
if install:
install_hook(__file__, 'pre-push',
['--base', 'origin/master..HEAD', '--program', 'quick'],
- repository)
+ Path.cwd())
return 0
- return cli.run(repository=repository, **presubmit_args)
+ return cli.run(**presubmit_args)
def main() -> int:
diff --git a/pw_presubmit/py/pw_presubmit/presubmit.py b/pw_presubmit/py/pw_presubmit/presubmit.py
index 5e5b115..dd65aee 100644
--- a/pw_presubmit/py/pw_presubmit/presubmit.py
+++ b/pw_presubmit/py/pw_presubmit/presubmit.py
@@ -13,7 +13,6 @@
# the License.
"""Tools for running presubmit checks in a Git repository.
-
Presubmit checks are defined as a function or other callable. The function may
take either no arguments or a list of the paths on which to run. Presubmit
checks communicate failure by raising any exception.
@@ -50,8 +49,8 @@
import re
import subprocess
import time
-from typing import Callable, Dict, Iterable, Iterator, List, NamedTuple
-from typing import Optional, Pattern, Sequence, Tuple
+from typing import Callable, Collection, Dict, Iterable, Iterator, List
+from typing import NamedTuple, Optional, Pattern, Sequence, Set, Tuple
from pw_presubmit import git_repo, tools
from pw_presubmit.tools import plural
@@ -138,6 +137,9 @@
def __str__(self):
return self.name
+ def title(self):
+ return f'{self.name if self.name else ""} presubmit checks'.strip()
+
class Programs(collections.abc.Mapping):
"""A mapping of presubmit check programs.
@@ -171,38 +173,54 @@
@dataclasses.dataclass(frozen=True)
class PresubmitContext:
"""Context passed into presubmit checks."""
- repo_root: Path
+ root: Path
+ repos: Tuple[Path, ...]
output_dir: Path
- paths: Sequence[Path]
+ paths: Tuple[Path, ...]
+
+ def relative_paths(self, start: Optional[Path] = None):
+ return tuple(
+ tools.relative_paths(self.paths, start if start else self.root))
+
+
+class _Filter(NamedTuple):
+ endswith: Tuple[str, ...] = ('', )
+ exclude: Tuple[Pattern[str], ...] = ()
+
+ def matches(self, path: str) -> bool:
+ return (any(path.endswith(end) for end in self.endswith)
+ and not any(exp.search(path) for exp in self.exclude))
class Presubmit:
"""Runs a series of presubmit checks on a list of files."""
- def __init__(self, repository_root: Path, output_directory: Path,
- paths: Sequence[Path]):
- self._repository_root = repository_root
- self._output_directory = output_directory
- self._paths = paths
+ def __init__(self, root: Path, repos: Sequence[Path],
+ output_directory: Path, paths: Sequence[Path]):
+ self._root = root.resolve()
+ self._repos = tuple(repos)
+ self._output_directory = output_directory.resolve()
+ self._paths = tuple(paths)
+ self._relative_paths = tuple(
+ tools.relative_paths(self._paths, self._root))
def run(self, program: Program, keep_going: bool = False) -> bool:
"""Executes a series of presubmit checks on the paths."""
- title = f'{program.name if program.name else ""} presubmit checks'
- checks = _apply_filters(program, self._paths)
+ checks = self._apply_filters(program)
- _LOG.debug('Running %s for %s', title, self._repository_root.name)
- print(_title(f'{self._repository_root.name}: {title}'))
+ _LOG.debug('Running %s for %s', program.title(), self._root.name)
+ print(_title(f'{self._root.name}: {program.title()}'))
_LOG.info('%d of %d checks apply to %s in %s', len(checks),
- len(program), plural(self._paths,
- 'file'), self._repository_root)
+ len(program), plural(self._paths, 'file'), self._root)
print()
- for line in tools.file_summary(self._paths):
+ for line in tools.file_summary(self._relative_paths):
print(line)
print()
- _LOG.debug('Paths:\n%s', '\n'.join(str(path) for path in self._paths))
+ _LOG.debug('Paths, relative to %s:\n%s', self._root,
+ '\n'.join(p.as_posix() for p in self._relative_paths))
if not self._paths:
print(color_yellow('No files are being checked!'))
@@ -214,6 +232,40 @@
return not failed and not skipped
+ def _apply_filters(
+ self, program: Sequence[Callable]
+ ) -> List[Tuple['_Check', Sequence[Path]]]:
+ """Returns list of (check, paths) for checks that should run."""
+ checks = [c if isinstance(c, _Check) else _Check(c) for c in program]
+ filter_to_checks: Dict[_Filter,
+ List[_Check]] = collections.defaultdict(list)
+
+ for check in checks:
+ filter_to_checks[check.filter].append(check)
+
+ check_to_paths = self._map_checks_to_paths(filter_to_checks)
+ return [(c, check_to_paths[c]) for c in checks if c in check_to_paths]
+
+ def _map_checks_to_paths(
+ self, filter_to_checks: Dict[_Filter, List['_Check']]
+ ) -> Dict['_Check', Sequence[Path]]:
+ checks_to_paths: Dict[_Check, Sequence[Path]] = {}
+
+ posix_paths = tuple(p.as_posix() for p in self._relative_paths)
+
+ for filt, checks in filter_to_checks.items():
+ filtered_paths = tuple(
+ path for path, filter_path in zip(self._paths, posix_paths)
+ if filt.matches(filter_path))
+
+ for check in checks:
+ if filtered_paths or check.always_run:
+ checks_to_paths[check] = filtered_paths
+ else:
+ _LOG.debug('Skipping "%s": no relevant files', check.name)
+
+ return checks_to_paths
+
def _log_summary(self, time_s: float, passed: int, failed: int,
skipped: int) -> None:
summary_items = []
@@ -239,7 +291,7 @@
_format_time(time_s)))
@contextlib.contextmanager
- def _context(self, name: str, paths: Sequence[Path]):
+ def _context(self, name: str, paths: Tuple[Path, ...]):
# There are many characters banned from filenames on Windows. To
# simplify things, just strip everything that's not a letter, digit,
# or underscore.
@@ -255,8 +307,9 @@
_LOG.addHandler(handler)
yield PresubmitContext(
- repo_root=self._repository_root.absolute(),
- output_dir=output_directory.absolute(),
+ root=self._root,
+ repos=self._repos,
+ output_dir=output_directory,
paths=paths,
)
@@ -269,7 +322,6 @@
passed = failed = 0
for i, (check, paths) in enumerate(program, 1):
- paths = [self._repository_root.joinpath(p) for p in paths]
with self._context(check.name, paths) as ctx:
result = check.run(ctx, i, len(program))
@@ -285,55 +337,43 @@
return passed, failed, len(program) - passed - failed
-class _Filter(NamedTuple):
- endswith: Tuple[str, ...] = ('', )
- exclude: Tuple[str, ...] = ()
+def _process_pathspecs(repos: Iterable[Path],
+ pathspecs: Iterable[str]) -> Dict[Path, List[str]]:
+ pathspecs_by_repo: Dict[Path, List[str]] = {repo: [] for repo in repos}
+ repos_with_paths: Set[Path] = set()
+ for pathspec in pathspecs:
+ # If the pathspec is a path to an existing file, only use it for the
+ # repo it is in.
+ if os.path.exists(pathspec):
+ # Raise an exception if the path exists but is not in a known repo.
+ repo = git_repo.within_repo(pathspec)
+ if repo not in pathspecs_by_repo:
+ raise ValueError(
+ f'{pathspec} is not in a Git repository in this presubmit')
-def _apply_filters(
- program: Sequence[Callable],
- paths: Sequence[Path]) -> List[Tuple['_Check', Sequence[Path]]]:
- """Returns a list of (check, paths_to_check) for checks that should run."""
- checks = [c if isinstance(c, _Check) else _Check(c) for c in program]
- filter_to_checks: Dict[_Filter,
- List[_Check]] = collections.defaultdict(list)
+ # Make the path relative to the repo's root.
+ pathspecs_by_repo[repo].append(os.path.relpath(pathspec, repo))
+ repos_with_paths.add(repo)
+ else:
+ # Pathspecs that are not paths (e.g. '*.h') are used for all repos.
+ for patterns in pathspecs_by_repo.values():
+ patterns.append(pathspec)
- for check in checks:
- filter_to_checks[check.filter].append(check)
+ # If any paths were specified, only search for paths in those repos.
+ if repos_with_paths:
+ for repo in set(pathspecs_by_repo) - repos_with_paths:
+ del pathspecs_by_repo[repo]
- check_to_paths = _map_checks_to_paths(filter_to_checks, paths)
- return [(c, check_to_paths[c]) for c in checks if c in check_to_paths]
-
-
-def _map_checks_to_paths(
- filter_to_checks: Dict[_Filter, List['_Check']],
- paths: Sequence[Path]) -> Dict['_Check', Sequence[Path]]:
- checks_to_paths: Dict[_Check, Sequence[Path]] = {}
-
- str_paths = [path.as_posix() for path in paths]
-
- for filt, checks in filter_to_checks.items():
- exclude = [re.compile(exp) for exp in filt.exclude]
-
- filtered_paths = tuple(
- Path(path) for path in str_paths
- if any(path.endswith(end) for end in filt.endswith) and not any(
- exp.search(path) for exp in exclude))
-
- for check in checks:
- if filtered_paths or check.always_run:
- checks_to_paths[check] = filtered_paths
- else:
- _LOG.debug('Skipping "%s": no relevant files', check.name)
-
- return checks_to_paths
+ return pathspecs_by_repo
def run(program: Sequence[Callable],
+ root: Path,
+ repos: Collection[Path] = (),
base: Optional[str] = None,
- paths: Sequence[Path] = (),
+ paths: Sequence[str] = (),
exclude: Sequence[Pattern] = (),
- repo_path: Path = Optional[None],
output_directory: Optional[Path] = None,
keep_going: bool = False) -> bool:
"""Lists files in the current Git repo and runs a Presubmit with them.
@@ -341,42 +381,50 @@
This changes the directory to the root of the Git repository after listing
paths, so all presubmit checks can assume they run from there.
+ The paths argument contains Git pathspecs. If no pathspecs are provided, all
+ paths in all repos are included. If paths to files or directories are
+ provided, only files within those repositories are searched. Patterns are
+ searched across all repositories. For example, if the pathspecs "my_module/"
+ and "*.h", paths under "my_module/" in the containing repo and paths in all
+ repos matching "*.h" will be included in the presubmit.
+
Args:
program: list of presubmit check functions to run
+ root: root path of the project
+ repos: paths to the roots of Git repositories to check
name: name to use to refer to this presubmit check run
base: optional base Git commit to list files against
- paths: optional list of paths to run the presubmit checks against
- exclude: regular expressions of paths to exclude from checks
- repo_path: path in the git repository to check
+ paths: optional list of Git pathspecs to run the checks against
+ exclude: regular expressions for Posix-style paths to exclude
output_directory: where to place output files
keep_going: whether to continue running checks if an error occurs
Returns:
True if all presubmit checks succeeded
"""
- if repo_path is None:
- repo_path = Path.cwd()
+ repos = [repo.resolve() for repo in repos]
- if not git_repo.is_repo(repo_path):
- _LOG.critical('Presubmit checks must be run from a Git repo')
- return False
+ for repo in repos:
+ if git_repo.root(repo) != repo:
+ raise ValueError(f'{repo} is not the root of a Git repo; '
+ 'presubmit checks must be run from a Git repo')
- files = git_repo.list_files(base, paths, exclude, repo_path)
- root = git_repo.root(repo=repo_path)
+ pathspecs_by_repo = _process_pathspecs(repos, paths)
- _LOG.info('Checking %s',
- git_repo.describe_files(root, repo_path, base, paths, exclude))
+ files: List[Path] = []
- files = [path.relative_to(root) for path in files]
+ for repo, pathspecs in pathspecs_by_repo.items():
+ files += tools.exclude_paths(
+ exclude, git_repo.list_files(base, pathspecs, repo), root)
+
+ _LOG.info(
+ 'Checking %s',
+ git_repo.describe_files(repo, repo, base, pathspecs, exclude))
if output_directory is None:
- output_directory = root.joinpath('.presubmit')
+ output_directory = root / '.presubmit'
- presubmit = Presubmit(
- repository_root=root,
- output_directory=Path(output_directory),
- paths=files,
- )
+ presubmit = Presubmit(root, repos, output_directory, files)
if not isinstance(program, Program):
program = Program('', program)
@@ -478,6 +526,10 @@
if required_args else ''))
+def _make_str_tuple(value: Iterable[str]) -> Tuple[str, ...]:
+ return tuple([value] if isinstance(value, str) else value)
+
+
def filter_paths(endswith: Iterable[str] = (''),
exclude: Iterable[str] = (),
always_run: bool = False) -> Callable[[Callable], _Check]:
@@ -497,8 +549,8 @@
"""
def filter_paths_for_function(function: Callable):
return _Check(function,
- _Filter(tools.make_tuple(endswith),
- tools.make_tuple(exclude)),
+ _Filter(_make_str_tuple(endswith),
+ tuple(re.compile(e) for e in exclude)),
always_run=always_run)
return filter_paths_for_function
diff --git a/pw_presubmit/py/pw_presubmit/python_checks.py b/pw_presubmit/py/pw_presubmit/python_checks.py
index 6b08fb2..9b294ee 100644
--- a/pw_presubmit/py/pw_presubmit/python_checks.py
+++ b/pw_presubmit/py/pw_presubmit/python_checks.py
@@ -16,10 +16,12 @@
These checks assume that they are running in a preconfigured Python environment.
"""
+import logging
import os
+from pathlib import Path
import re
import sys
-import logging
+from typing import List
try:
import pw_presubmit
@@ -41,14 +43,16 @@
@filter_paths(endswith='.py')
def test_python_packages(ctx: pw_presubmit.PresubmitContext):
- packages = git_repo.find_python_packages(ctx.paths, repo=ctx.repo_root)
+ packages: List[Path] = []
+ for repo in ctx.repos:
+ packages += git_repo.find_python_packages(ctx.paths, repo=repo)
if not packages:
_LOG.info('No Python packages were found.')
return
for package in packages:
- call('python', os.path.join(package, 'setup.py'), 'test', cwd=package)
+ call('python', package / 'setup.py', 'test', cwd=package)
@filter_paths(endswith='.py')
@@ -67,7 +71,7 @@
'--jobs=0',
f'--disable={",".join(disable_checkers)}',
*ctx.paths,
- cwd=ctx.repo_root,
+ cwd=ctx.root,
)
diff --git a/pw_presubmit/py/pw_presubmit/tools.py b/pw_presubmit/py/pw_presubmit/tools.py
index 02d873d..c7f89e3 100644
--- a/pw_presubmit/py/pw_presubmit/tools.py
+++ b/pw_presubmit/py/pw_presubmit/tools.py
@@ -20,7 +20,7 @@
from pathlib import Path
import shlex
import subprocess
-from typing import Any, Dict, Iterable, Iterator, List, Sequence, Tuple
+from typing import Any, Dict, Iterable, Iterator, List, Sequence, Pattern, Tuple
_LOG: logging.Logger = logging.getLogger(__name__)
@@ -121,8 +121,24 @@
return output
-def make_tuple(value: Iterable[str]) -> Tuple[str, ...]:
- return tuple([value] if isinstance(value, str) else value)
+def relative_paths(paths: Iterable[Path], start: Path) -> Iterable[Path]:
+ """Returns relative Paths calculated with os.path.relpath."""
+ for path in paths:
+ yield Path(os.path.relpath(path, start))
+
+
+def exclude_paths(exclusions: Iterable[Pattern[str]],
+ paths: Iterable[Path],
+ relative_to: Path = None) -> Iterable[Path]:
+ """Excludes paths based on a series of regular expressions."""
+ if relative_to:
+ relpath = lambda path: Path(os.path.relpath(path, relative_to))
+ else:
+ relpath = lambda path: path
+
+ for path in paths:
+ if not any(e.search(relpath(path).as_posix()) for e in exclusions):
+ yield path
def _truncate(value, length: int = 60) -> str:
