diff options
Diffstat (limited to 'pw_presubmit/docs.rst')
| -rw-r--r-- | pw_presubmit/docs.rst | 622 |
1 files changed, 334 insertions, 288 deletions
diff --git a/pw_presubmit/docs.rst b/pw_presubmit/docs.rst index 5be527bc9..f7f8a5b6b 100644 --- a/pw_presubmit/docs.rst +++ b/pw_presubmit/docs.rst @@ -28,6 +28,8 @@ system. If it's not Bazel formatting passes without checking.) The ``pw_presubmit`` package includes presubmit checks that can be used with any project. These checks include: +.. todo-check: disable + * Check code format of several languages including C, C++, and Python * Initialize a Python environment * Run all Python tests @@ -36,7 +38,15 @@ project. These checks include: * 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`` +* Ensure all header files contain ``#pragma once`` (or, that they have matching + ``#ifndef``/``#define`` lines) +* Ensure lists are kept in alphabetical order +* Forbid non-inclusive language +* Check format of TODO lines +* Apply various rules to ``.gitmodules`` or ``OWNERS`` files +* Ensure all source files are in the build + +.. todo-check: enable ------------- Compatibility @@ -60,11 +70,23 @@ 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 ----------------- +Common ``pw presubmit`` command line arguments +---------------------------------------------- +.. argparse:: + :module: pw_presubmit.cli + :func: _get_default_parser + :prog: pw presubmit + :nodefaultconst: + :nodescription: + :noepilog: + + +``pw_presubmit.cli`` Python API +------------------------------- .. automodule:: pw_presubmit.cli :members: add_arguments, run + Presubmit output directory -------------------------- The ``pw_presubmit`` command line interface includes an ``--output-directory`` @@ -73,6 +95,8 @@ path is ``out/presubmit``. A subdirectory is created for each presubmit step. This directory persists between presubmit runs and can be cleaned by deleting it or running ``pw presubmit --clean``. +.. _module-pw_presubmit-presubmit-checks: + Presubmit checks ================ A presubmit check is defined as a function or other callable. The function must @@ -86,16 +110,16 @@ Either of these functions could be used as presubmit checks: .. code-block:: python - @pw_presubmit.filter_paths(endswith='.py') - def file_contains_ni(ctx: PresubmitContext): - for path in ctx.paths: - with open(path) as file: - contents = file.read() - if 'ni' not in contents and 'nee' not in contents: - raise PresumitFailure('Files must say "ni"!', path=path) + @pw_presubmit.filter_paths(endswith='.py') + def file_contains_ni(ctx: PresubmitContext): + for path in ctx.paths: + with open(path) as file: + contents = file.read() + if 'ni' not in contents and 'nee' not in contents: + raise PresumitFailure('Files must say "ni"!', path=path) - def run_the_build(_): - subprocess.run(['make', 'release'], check=True) + def run_the_build(_): + subprocess.run(['make', 'release'], check=True) Presubmit checks functions are grouped into "programs" -- a named series of checks. Projects may find it helpful to have programs for different purposes, @@ -103,65 +127,33 @@ 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. -``PresubmitContext`` has the following members: - -* ``root``: Source checkout root directory -* ``repos``: Repositories (top-level and submodules) processed by - ``pw presubmit`` -* ``output_dir``: Output directory for this specific presubmit step -* ``failure_summary_log``: File path where steps should write a brief summary - of any failures -* ``paths``: Modified files for the presubmit step to check (often used in - formatting steps but ignored in compile steps) -* ``all_paths``: All files in the repository tree. -* ``package_root``: Root directory for ``pw package`` installations -* ``override_gn_args``: Additional GN args processed by ``build.gn_gen()`` -* ``luci``: Information about the LUCI build or None if not running in LUCI -* ``num_jobs``: Number of jobs to run in parallel -* ``continue_after_build_error``: For steps that compile, don't exit on the - first compilation error - -The ``luci`` member is of type ``LuciContext`` and has the following members: - -* ``buildbucket_id``: The globally-unique buildbucket id of the build -* ``build_number``: The builder-specific incrementing build number, if - configured for this builder -* ``project``: The LUCI project under which this build is running (often - ``pigweed`` or ``pigweed-internal``) -* ``bucket``: The LUCI bucket under which this build is running (often ends - with ``ci`` or ``try``) -* ``builder``: The builder being run -* ``swarming_server``: The swarming server on which this build is running -* ``swarming_task_id``: The swarming task id of this build -* ``cas_instance``: The CAS instance accessible from this build -* ``pipeline``: Information about the build pipeline, if applicable. -* ``triggers``: Information about triggering commits, if applicable. - -The ``pipeline`` member, if present, is of type ``LuciPipeline`` and has the -following members: - -* ``round``: The zero-indexed round number. -* ``builds_from_previous_iteration``: A list of the buildbucket ids from the - previous round, if any, encoded as strs. - -The ``triggers`` member is a sequence of ``LuciTrigger`` objects, which have the -following members: - -* ``number``: The number of the change in Gerrit. -* ``patchset``: The number of the patchset of the change. -* ``remote``: The full URL of the remote. -* ``branch``: The name of the branch on which this change is being/was - submitted. -* ``ref``: The ``refs/changes/..`` path that can be used to reference the - patch for unsubmitted changes and the hash for submitted changes. -* ``gerrit_name``: The name of the googlesource.com Gerrit host. -* ``submitted``: Whether the change has been submitted or is still pending. +By default, presubmit steps are only run on files changed since ``@{upstream}``. +If all such files are filtered out by ``filter_paths``, then that step will be +skipped. This can be overridden with the ``--base`` and ``--full`` arguments to +``pw presubmit``. In automated testing ``--full`` is recommended, except for +lint/format checks where ``--base HEAD~1`` is recommended. + +.. autoclass:: pw_presubmit.presubmit_context.PresubmitContext + :members: + :noindex: Additional members can be added by subclassing ``PresubmitContext`` and ``Presubmit``. Then override ``Presubmit._create_presubmit_context()`` to return the subclass of ``PresubmitContext``. Finally, add ``presubmit_class=PresubmitSubClass`` when calling ``cli.run()``. +.. autoclass:: pw_presubmit.presubmit_context.LuciContext + :members: + :noindex: + +.. autoclass:: pw_presubmit.presubmit_context.LuciPipeline + :members: + :noindex: + +.. autoclass:: pw_presubmit.presubmit_context.LuciTrigger + :members: + :noindex: + Substeps -------- Presubmit steps can define substeps that can run independently in other tooling. @@ -190,6 +182,13 @@ others. All of these checks can be included by adding ``pw_presubmit.format_code.presubmit_checks()`` to a presubmit program. These all use language-specific formatters like clang-format or black. +Example changes demonstrating how to add formatters: + +* `CSS <https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/178810>`_ +* `JSON <https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/171991>`_ +* `reStructuredText <https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/168541>`_ +* `TypeScript <https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/164825>`_ + These will suggest fixes using ``pw format --fix``. Options for code formatting can be specified in the ``pigweed.json`` file @@ -201,37 +200,41 @@ or fix code formatting. by Pigweed itself) and ``yapf`` (the default). * ``black_path``: If ``python_formatter`` is ``black``, use this as the executable instead of ``black``. - -.. TODO(b/264578594) Add exclude to pigweed.json file. -.. * ``exclude``: List of path regular expressions to ignore. +* ``black_config_file``: Set the config file for the black formatter. +* ``exclude``: List of path regular expressions to ignore. Will be evaluated + against paths relative to the checkout root using ``re.search``. Example section from a ``pigweed.json`` file: -.. code-block:: - - { - "pw": { - "pw_presubmit": { - "format": { - "python_formatter": "black", - "black_path": "black" - } - } - } - } +.. code-block:: json + + { + "pw": { + "pw_presubmit": { + "format": { + "python_formatter": "black", + "black_config_file": "$pw_env{PW_PROJECT_ROOT}/config/.black.toml" + "black_path": "black", + "exclude": [ + "\\bthird_party/foo/src" + ] + } + } + } + } Sorted Blocks ^^^^^^^^^^^^^ Blocks of code can be required to be kept in sorted order using comments like the following: -.. code-block:: +.. code-block:: python - # keep-sorted: start - bar - baz - foo - # keep-sorted: end + # keep-sorted: start + bar + baz + foo + # keep-sorted: end This can be included by adding ``pw_presubmit.keep_sorted.presubmit_check`` to a presubmit program. Adding ``ignore-case`` to the start line will use @@ -245,13 +248,13 @@ Prefixes can be ignored by adding ``ignore-prefix=`` followed by a comma-separated list of prefixes. The list below will be kept in this order. Neither commas nor whitespace are supported in prefixes. -.. code-block:: +.. code-block:: python - # keep-sorted: start ignore-prefix='," - 'bar', - "baz", - 'foo', - # keep-sorted: end + # keep-sorted: start ignore-prefix='," + 'bar', + "baz", + 'foo', + # keep-sorted: end Inline comments are assumed to be associated with the following line. For example, the following is already sorted. This can be disabled with @@ -259,14 +262,14 @@ example, the following is already sorted. This can be disabled with .. todo-check: disable -.. code-block:: +.. code-block:: python - # keep-sorted: start - # TODO(b/1234) Fix this. - bar, - # TODO(b/5678) Also fix this. - foo, - # keep-sorted: end + # keep-sorted: start + # TODO: b/1234 - Fix this. + bar, + # TODO: b/5678 - Also fix this. + foo, + # keep-sorted: end .. todo-check: enable @@ -280,12 +283,12 @@ nested, so there's no ability to add a keep-sorted block for the sub-items. .. code-block:: - # keep-sorted: start - * abc - * xyz - * uvw - * def - # keep-sorted: end + # keep-sorted: start + * abc + * xyz + * uvw + * def + # keep-sorted: end The presubmit check will suggest fixes using ``pw keep-sorted --fix``. @@ -298,7 +301,8 @@ by adding ``pw_presubmit.gitmodules.create()`` to a presubmit program. This function takes an optional argument of type ``pw_presubmit.gitmodules.Config``. ``Config`` objects have several properties. -* ``allow_non_googlesource_hosts: bool = False`` — If false, all submodules URLs +* ``allow_submodules: bool = True`` — If false, don't allow any submodules. +* ``allow_non_googlesource_hosts: bool = False`` — If false, all submodule URLs must be on a Google-managed Gerrit server. * ``allowed_googlesource_hosts: Sequence[str] = ()`` — If set, any Google-managed Gerrit URLs for submodules most be in this list. Entries @@ -309,7 +313,7 @@ function takes an optional argument of type ``pw_presubmit.gitmodules.Config``. URLs are prohibited. * ``allow_git_corp_google_com: bool = True`` — If false, ``git.corp.google.com`` submodule URLs are prohibited. -* ``require_branch: bool = False`` — If True, all submodules must reference a +* ``require_branch: bool = False`` — If true, all submodules must reference a branch. * ``validator: Callable[[PresubmitContext, Path, str, Dict[str, str]], None] = None`` — A function that can be used for arbitrary submodule validation. It's called @@ -322,14 +326,40 @@ There's a ``pragma_once`` check that confirms the first non-comment line of C/C++ headers is ``#pragma once``. This is enabled by adding ``pw_presubmit.cpp_checks.pragma_once`` to a presubmit program. +#ifndef/#define +^^^^^^^^^^^^^^^ +There's an ``ifndef_guard`` check that confirms the first two non-comment lines +of C/C++ headers are ``#ifndef HEADER_H`` and ``#define HEADER_H``. This is +enabled by adding ``pw_presubmit.cpp_checks.include_guard_check()`` to a +presubmit program. ``include_guard_check()`` has options for specifying what the +header guard should be based on the path. + +This check is not used in Pigweed itself but is available to projects using +Pigweed. + .. todo-check: disable TODO(b/###) Formatting ^^^^^^^^^^^^^^^^^^^^^^^^^ There's a check that confirms ``TODO`` lines match a given format. Upstream -Pigweed expects these to look like ``TODO(b/###): Explanation``, but makes it +Pigweed expects these to look like ``TODO: b/### - Explanation``, but makes it easy for projects to define their own pattern instead. +Some older forms are still allowed but discouraged. In order of preference we +allow the following formats by default. + +.. todo-check: disable + +.. code-block:: + + # TODO: b/1234 - Explanation. + # TODO: username@ - Explanation. + # TODO: username@example.com - Explanation. + # TODO: b/1234 - Explanation. + # TODO(username) Explanation. + +.. todo-check: enable + To use this check add ``todo_check.create(todo_check.BUGS_OR_USERNAMES)`` to a presubmit program. @@ -370,6 +400,11 @@ a callable as an argument that indicates, for a given file, where a controlling Formatting of ``OWNERS`` files is handled similary to formatting of other source files and is discussed in `Code Formatting`. +JSON +^^^^ +The JSON check requires all ``*.json`` files to be valid JSON files. It can be +included by adding ``json_check.presubmit_check()`` to a presubmit program. + Source in Build ^^^^^^^^^^^^^^^ Pigweed provides checks that source files are configured as part of the build @@ -412,163 +447,174 @@ See ``pigweed_presubmit.py`` for a more complex presubmit check script example. .. code-block:: python - """Example presubmit check script.""" - - import argparse - import logging - import os - from pathlib import Path - import re - import sys - from typing import List, Optional, Pattern - - try: - import pw_cli.log - except ImportError: - print('ERROR: Activate the environment before running presubmits!', - file=sys.stderr) - sys.exit(2) - - import pw_presubmit - from pw_presubmit import ( - build, - cli, - cpp_checks, - environment, - format_code, - git_repo, - inclusive_language, - filter_paths, - python_checks, - 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' - - # 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(ctx, build_type='release') - build.ninja(ctx) - - - def host_tests(ctx: PresubmitContext): - build.gn_gen(ctx, run_host_tests='true') - build.ninja(ctx) - - - # 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): - cpp_checks.pragma_once(ctx) - - - # - # Presubmit check programs - # - OTHER = ( - # Checks not ran by default but that should be available. These might - # include tests that are expensive to run or that don't yet pass. - build.gn_quick_check, - ) - - QUICK = ( - # 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), - # Include the upstream inclusive language check. - inclusive_language.presubmit_check, - # Include just the lint-related Python checks. - python_checks.gn_pylint.with_filter(exclude=PATH_EXCLUSIONS), - ) - - FULL = ( - QUICK, # Add all checks from the 'quick' program - release_build, - # Use the upstream Python checks, with custom path filters applied. - # Checks listed multiple times are only run once. - python_checks.gn_python_check.with_filter(exclude=PATH_EXCLUSIONS), - ) - - PROGRAMS = pw_presubmit.Programs(other=OTHER, quick=QUICK, full=FULL) - - - # - # Allowlist of remote refs for presubmit. If the remote ref being pushed to - # matches any of these values (with regex matching), then the presubmits - # checks will be run before pushing. - # - PRE_PUSH_REMOTE_REF_ALLOWLIST = ( - 'refs/for/main', - ) - - - def run(install: bool, remote_ref: Optional[str], **presubmit_args) -> int: - """Process the --install argument then invoke pw_presubmit.""" - - # Install the presubmit Git pre-push hook, if requested. - if install: - # '$remote_ref' will be replaced by the actual value of the remote ref - # at runtime. - install_git_hook('pre-push', [ - 'python', '-m', 'tools.presubmit_check', '--base', 'HEAD~', - '--remote-ref', '$remote_ref' - ]) - return 0 - - # Run the checks if either no remote_ref was passed, or if the remote ref - # matches anything in the allowlist. - if remote_ref is None or any( - re.search(pattern, remote_ref) - for pattern in PRE_PUSH_REMOTE_REF_ALLOWLIST): - 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.') - - # Define an optional flag to pass the remote ref into this script, if it - # is run as a pre-push hook. The destination variable in the parsed args - # will be `remote_ref`, as dashes are replaced with underscores to make - # valid variable names. - parser.add_argument( - '--remote-ref', - default=None, - nargs='?', # Make optional. - help='Remote ref of the push command, for use by the pre-push hook.') - - return run(**vars(parser.parse_args())) - - if __name__ == '__main__': - pw_cli.log.install(logging.INFO) - sys.exit(main()) + """Example presubmit check script.""" + + import argparse + import logging + import os + from pathlib import Path + import re + import sys + from typing import Optional + + try: + import pw_cli.log + except ImportError: + print("ERROR: Activate the environment before running presubmits!", file=sys.stderr) + sys.exit(2) + + import pw_presubmit + from pw_presubmit import ( + build, + cli, + cpp_checks, + format_code, + inclusive_language, + python_checks, + ) + from pw_presubmit.presubmit import filter_paths + from pw_presubmit.presubmit_context import PresubmitContext + from pw_presubmit.install_hook import install_git_hook + + # Set up variables for key project paths. + PROJECT_ROOT = Path(os.environ["MY_PROJECT_ROOT"]) + PIGWEED_ROOT = PROJECT_ROOT / "pigweed" + + # 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(ctx, build_type="release") + build.ninja(ctx) + build.gn_check(ctx) # Run after building to check generated files. + + + def host_tests(ctx: PresubmitContext): + build.gn_gen(ctx, run_host_tests="true") + build.ninja(ctx) + build.gn_check(ctx) + + + # 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): + cpp_checks.pragma_once(ctx) + + + # + # Presubmit check programs + # + OTHER = ( + # Checks not ran by default but that should be available. These might + # include tests that are expensive to run or that don't yet pass. + build.gn_gen_check, + ) + + QUICK = ( + # 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), + # Include the upstream inclusive language check. + inclusive_language.presubmit_check, + # Include just the lint-related Python checks. + python_checks.gn_python_lint.with_filter(exclude=PATH_EXCLUSIONS), + ) + + FULL = ( + QUICK, # Add all checks from the 'quick' program + release_build, + # Use the upstream Python checks, with custom path filters applied. + # Checks listed multiple times are only run once. + python_checks.gn_python_check.with_filter(exclude=PATH_EXCLUSIONS), + ) + + PROGRAMS = pw_presubmit.Programs(other=OTHER, quick=QUICK, full=FULL) + + + # + # Allowlist of remote refs for presubmit. If the remote ref being pushed to + # matches any of these values (with regex matching), then the presubmits + # checks will be run before pushing. + # + PRE_PUSH_REMOTE_REF_ALLOWLIST = ("refs/for/main",) + + + def run(install: bool, remote_ref: Optional[str], **presubmit_args) -> int: + """Process the --install argument then invoke pw_presubmit.""" + + # Install the presubmit Git pre-push hook, if requested. + if install: + # '$remote_ref' will be replaced by the actual value of the remote ref + # at runtime. + install_git_hook( + "pre-push", + [ + "python", + "-m", + "tools.presubmit_check", + "--base", + "HEAD~", + "--remote-ref", + "$remote_ref", + ], + ) + return 0 + + # Run the checks if either no remote_ref was passed, or if the remote ref + # matches anything in the allowlist. + if remote_ref is None or any( + re.search(pattern, remote_ref) + for pattern in PRE_PUSH_REMOTE_REF_ALLOWLIST + ): + return cli.run(root=PROJECT_ROOT, **presubmit_args) + return 0 + + + 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.", + ) + + # Define an optional flag to pass the remote ref into this script, if it + # is run as a pre-push hook. The destination variable in the parsed args + # will be `remote_ref`, as dashes are replaced with underscores to make + # valid variable names. + parser.add_argument( + "--remote-ref", + default=None, + nargs="?", # Make optional. + help="Remote ref of the push command, for use by the pre-push hook.", + ) + + return run(**vars(parser.parse_args())) + + + if __name__ == "__main__": + pw_cli.log.install(logging.INFO) + sys.exit(main()) --------------------- Code formatting tools @@ -585,35 +631,35 @@ a formatter or remove/disable a PigWeed supplied one. .. code-block:: python - #!/usr/bin/env python - """Formats files in repository. """ + #!/usr/bin/env python + """Formats files in repository. """ - import logging - import sys + import logging + import sys - import pw_cli.log - from pw_presubmit import format_code - from your_project import presubmit_checks - from your_project import your_check + import pw_cli.log + from pw_presubmit import format_code + from your_project import presubmit_checks + from your_project import your_check - YOUR_CODE_FORMAT = CodeFormat('YourFormat', - filter=FileFilter(suffix=('.your', )), - check=your_check.check, - fix=your_check.fix) + YOUR_CODE_FORMAT = CodeFormat('YourFormat', + filter=FileFilter(suffix=('.your', )), + check=your_check.check, + fix=your_check.fix) - CODE_FORMATS = (*format_code.CODE_FORMATS, YOUR_CODE_FORMAT) + CODE_FORMATS = (*format_code.CODE_FORMATS, YOUR_CODE_FORMAT) - def _run(exclude, **kwargs) -> int: - """Check and fix formatting for source files in the repo.""" - return format_code.format_paths_in_repo(exclude=exclude, - code_formats=CODE_FORMATS, - **kwargs) + def _run(exclude, **kwargs) -> int: + """Check and fix formatting for source files in the repo.""" + return format_code.format_paths_in_repo(exclude=exclude, + code_formats=CODE_FORMATS, + **kwargs) - def main(): - return _run(**vars(format_code.arguments(git_paths=True).parse_args())) + def main(): + return _run(**vars(format_code.arguments(git_paths=True).parse_args())) - if __name__ == '__main__': - pw_cli.log.install(logging.INFO) - sys.exit(main()) + if __name__ == '__main__': + pw_cli.log.install(logging.INFO) + sys.exit(main()) |