.. _docs-pw-style-bazel: =========== Bazel style =========== In general, Pigweed follows the `Bazel Style Guide `. This document provides some additional guidance. ------ Naming ------ General ======= All names in build and Starlark files with the exception of `Providers `__ should use snake_case naming style. Providers use CamelCase and always end with the ``Info`` suffix. Constants ========= Constants exported by ``.bzl`` files should be all uppercase and prefixed with ``PW_``. Private constants must be prefixed with an underscore ``_``, but do not need any ``PW_`` prefix. .. _docs-pw-style-bazel-naming-rules: Rules ===== All rules should begin with a ``pw_`` prefix. When possible, name rules as nouns describing the resulting artifact rather than verbs. .. admonition:: Bad names :class: error * ``pw_sign_binary``: Named like a verb. * ``generated_header``: Missing ``pw_`` prefix. .. admonition:: Good names :class: checkmark * ``pw_signed_binary``: Clearly intended to produce a new artifact rather than suggesting in-place modification. * ``pw_generated_header``: Properly ``pw_`` prefixed to reduce name collisions. Macros ====== Macros that wrap rules should follow :ref:`docs-pw-style-bazel-naming-rules`. Macros that are effectively Starlark helper methods may be named as verbs when appropriate. Sub-target naming ================= If a macro generates multiple targets, it should follow the following rules: * Public sub-targets should be documented in the macro docstring and use a dot ``.`` to separate the base rule name from the sub-target name. Example: ``my_firmware_bundle.metadata`` * Private sub-targets should NOT be documented, and should be named with a dot and underscore ``._`` to separate the base rule name from the sub-target name. Example: ``my_firmware_bundle._hidden_metadata`` Starlark files (.bzl) ===================== Starlark files should be named exactly after the primary rule exported by the file. Example: ``pw_flipped_table.bzl`` that exports a rule named ``pw_flipped_table``. ----------- Build files ----------- Do not reference ``@pigweed`` from Pigweed ========================================== All labels and ``load()`` statements within Pigweed that reference other things in Pigweed should **not** use ``@pigweed`` in the label name. Instead, just use the project-absolute path to the ``.bzl`` file to load. Example: ``load("//pw_build:pw_cc_binary.bzl", "pw_cc_binary")`` Reasoning: Referencing things in the current repository via the repository name is an antipattern, and can introduce subtle bugs in ``WORKSPACE`` projects. Avoid ``glob`` when possible ============================ Pigweed only permits ``glob()`` for cases where the collection of inputs is truly variable, or infeasible to exhaustively express. The biggest example of this is ``BUILD`` files for external dependencies (which may have different source file paths/names for different versions) or binary packages (e.g. toolchains, which have many supporting files). Reasoning: The ``glob()`` function in Bazel is quite expensive at scale, so it's best to avoid it. More background can be found in the Bazelcon talk `How Bazel handles globs `__. Do not use ``$(location :foo)`` =============================== The ``location`` keyword is error-prone. Use ``execpath`` for actions that are only relevant to the build (e.g. ``run_binary``). Use ``rlocationpath`` when exposing paths to tools that will be loaded via runfiles libraries. ``rootpath`` should be used when exposing a file path to a binary via arguments or environment variable (particularly for ``native_binary`` and ``*_test`` rules). Do not ``load`` from dev dependencies in public Build files =========================================================== If a ``BUILD.bazel`` file contains any target with ``visibility = ["//visibility:public"]``, it may not contain ``load`` statements referencing dev dependencies. Instead, do one of the following, as appropriate: #. Mark all all build targets in the ``BUILD.bazel`` file as ``visibility = "//:__subpackages__"`` (to make is clear that they cannot be used by downstream projects). #. Move the targets requiring the ``load`` to a different ``BUILD.bazel`` file. #. Remove the ``dev_dependency`` attribute from the dependency. Do not take this step lightly: it will require all Pigweed downstream projects to provide this dependency, which may require work if they vendor their dependencies. Reasoning: Bzlmod allows `bazel_dep `__ targets to be registered with ``dev_dependency=True``. These dependencies are ignored by downstream projects. This is good because it trims their dependency graph. But it means downstream projects can't use any functionality that the dev dependencies were used for in upstream. In particular, a downstream project cannot reference *any target* in an upstream ``BUILD.bazel`` file that loads a macro or a rule from a dev dependency. To make this restriction clear to anyone working in upstream Pigweed, explicitly express it through the ``visibility`` attribute. --------------------- Starlark files (.bzl) --------------------- Avoid monolithic Starlark files =============================== In general, Starlark files should export a single rule or macro and be named identically to the exported rule. A Starlark file may export multiple symbols if they are closely related helper functions, or in rare cases adjacent rules that are *very* closely related. Avoid creating or extending "grab bag" Starlark files that contain collections of vaguely similar rules and macros. Reasoning: Monolithic Starlark files can quickly pull in many external dependencies accidentally, which prevents Bazel from efficiently fetching only the external repositories that are actually used by build rules that need to be evaluated. Rule-wrapper macro signatures ============================= * Macros that wrap other rules should always begin with an asterisk ``*`` as the first argument to prevent rules from being declared with positional arguments. * The second argument should always be ``name``. * Nearly all rule-wrapper macros should capture and forward ``**kwargs`` to the underlying rules to properly support `Attributes common to all build rules `__. Example: .. code-block:: py def pw_generated_header(*, name, src, dest, **kwargs): # Implementation... --------------------- C++ specific patterns --------------------- Use ``strip_include_prefix`` rather than ``includes`` ===================================================== Nearly all ``cc_*`` libraries should introduce include paths via ``strip_include_prefix``. Typically, uses of ``includes`` should be considered bugs. Note that ``strip_include_prefix`` doesn't work with ``textual_hdrs`` (see `bazelbuild/bazel#12424 `__), so ``textual_hdrs`` may use ``includes`` in cases where ``textual_hdrs`` is strictly necessary. Reasoning: The unfortunately-named ``includes`` attribute of ``cc_*`` rules is always intended to resolve to ``-isystem`` include directories which does two things: 1. It affects include ordering in unintended ways. 2. It masks any warnings that originate in headers covered by the ``-isystem`` include. To get a include directory to resolve to ``-I``, ``strip_include_prefix`` must be used. This has an added benefit of creating a virtual include directory that provides stronger correctness guarantees. More information can be found at https://pwbug.dev/378564135. Use ``pw_cc_binary`` and ``pw_cc_test`` ======================================= Use ``pw_cc_binary`` instead of ``cc_binary`` and ``pw_cc_test`` instead of ``cc_test``. Reasoning: While these wrappers are very similar to their native counterparts, Pigweed has some requirements that must be applied to every single one of these, so copying those requirements across Pigweed is not scalable. Downstream projects may choose whether or not to use these wrappers, there is no strict requirement that the wrappers are used by any downstream user of Pigweed. Always load symbols from ``@rules_cc`` before using them ======================================================== When using any native ``cc_*`` rule, always load the symbol from ``rules_cc`` first. Reasoning: Bazel's C/C++ rules are migrating from native Java implementations to Starlark definitions. Loading from ``rules_cc`` may be absolutely necessary in the future, so it's good practice to start doing it now. ------------------------ Python specific patterns ------------------------ Use ``pw_py_binary`` and ``pw_py_test`` ======================================= Use ``pw_py_binary`` instead of ``py_binary`` and ``pw_py_test`` instead of ``py_test``. Reasoning: While these wrappers are very similar to their native counterparts, Pigweed has some requirements that must be applied to every single one of these, so copying those requirements across Pigweed is not scalable. Downstream projects may choose whether or not to use these wrappers, there is no strict requirement that the wrappers are used by any downstream user of Pigweed. Always load symbols from ``@rules_python`` before using them ============================================================ When using any native ``py_*`` rule, always load the symbol from ``rules_python`` first. Reasoning: The native Python Bazel rules are subtly different from the rules loaded from ``@rules_python``. Forgetting to load from ``@rules_python`` can result in subtle but confusing breakages at best, and things silently working at worst.