pw_build: Facade and configuration tweaks and docs
- Add documentation for facades and the pw_facade template.
- Document the module config pattern.
- Remove the need for the facade_name argument to pw_facade.
Change-Id: I77529583967cfdb4f47ee87313982b1259ca036e
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/22045
Reviewed-by: Alexei Frolov <frolv@google.com>
Commit-Queue: Wyatt Hepler <hepler@google.com>
diff --git a/docs/build_system.rst b/docs/build_system.rst
index 9c0b2e6..2bf65e7 100644
--- a/docs/build_system.rst
+++ b/docs/build_system.rst
@@ -394,8 +394,10 @@
.. code::
- # This must be imported before .gni files from any other Pigweed modules.
- # To prevent gn format from reordering this import, a comment is added above.
+ # This must be imported before .gni files from any other Pigweed modules. To
+ # prevent gn format from reordering this import, it must be separated by a
+ # blank line from other imports.
+
import("//build_overrides/pigweed.gni")
GN target type wrappers
diff --git a/docs/module_structure.rst b/docs/module_structure.rst
index 89ec692..c10106a 100644
--- a/docs/module_structure.rst
+++ b/docs/module_structure.rst
@@ -27,7 +27,7 @@
public/pw_foo/foo.h
public/pw_foo/baz.h
- # Exposed public headers go under internal/
+ # Exposed private headers go under internal/
public/pw_foo/internal/bar.h
public/pw_foo/internal/qux.h
@@ -186,6 +186,169 @@
BUILD.gn
README.md
+Compile-time configuration
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+Pigweed modules are intended to be used in a wide variety of environments.
+In support of this, some modules expose compile-time configuration options.
+Pigweed has an established pattern for declaring and overriding module
+configuration.
+
+.. tip::
+
+ Compile-time configuration provides flexibility, but also imposes
+ restrictions. A module can only have one configuration in a given build.
+ Compile-time configuration also makes testing more difficult. Where
+ appropriate, consider alternatives such as C++ templates or runtime
+ configuration.
+
+Declaring configuration
+^^^^^^^^^^^^^^^^^^^^^^^
+Configuration values are declared in a header file with macros. If the macro
+value is not already defined, a default definition is provided. Otherwise,
+nothing is done. Configuration headers may include ``static_assert`` statements
+to validate configuration values.
+
+.. code-block:: c++
+
+ // Example configuration header
+
+ #ifndef PW_FOO_INPUT_BUFFER_SIZE_BYTES
+ #define PW_FOO_INPUT_BUFFER_SIZE_BYTES 128
+ #endif // PW_FOO_INPUT_BUFFER_SIZE_BYTES
+
+ static_assert(PW_FOO_INPUT_BUFFER_SIZE_BYTES >= 64);
+
+The configuration header may go in one of three places in the module, depending
+on whether the header should be exposed by the module or not.
+
+.. code-block::
+
+ pw_foo/...
+
+ # Publicly accessible configuration header
+ public/pw_foo/config.h
+
+ # Internal configuration header that is included by other module headers
+ public/pw_foo/internal/config.h
+
+ # Internal configuration header
+ pw_foo_private/config.h
+
+The configuration header is provided by a build system library. This library
+acts as a :ref:`facade<docs-module-structure-facades>`. The facade uses a
+variable such as ``pw_foo_CONFIG``. In upstream Pigweed, all config facades
+default to the ``pw_build_DEFAULT_MODULE_CONFIG`` backend. In the GN build
+system, the config facade is declared as follows:
+
+.. code-block::
+
+ declare_args() {
+ # The build target that overrides the default configuration options for this
+ # module. This should point to a source set that provides defines through a
+ # public config (which may -include a file or add defines directly).
+ pw_foo_CONFIG = pw_build_DEFAULT_MODULE_CONFIG
+ }
+
+ # Publicly accessible configuration header (most common)
+ pw_source_set("config") {
+ public = [ "public/pw_foo/config.h" ]
+ public_configs = [ ":public_include_path" ]
+ public_deps = [ pw_foo_CONFIG ]
+ }
+
+ # Internal configuration header that is included by other module headers
+ pw_source_set("config") {
+ sources = [ "public/pw_foo/internal/config.h" ]
+ public_configs = [ ":public_include_path" ]
+ public_deps = [ pw_foo_CONFIG ]
+ visibility = [":*"] # Only allow this module to depend on ":config"
+ friend = [":*"] # Allow this module to access the config.h header.
+ }
+
+ # Internal configuration header
+ pw_source_set("config") {
+ public = [ "pw_foo_private/config.h" ]
+ public_deps = [ pw_foo_CONFIG ]
+ visibility = [":*"] # Only allow this module to depend on ":config"
+ }
+
+Overriding configuration
+^^^^^^^^^^^^^^^^^^^^^^^^
+As noted above, all module configuration facades default to the same backend
+(``pw_build_DEFAULT_MODULE_CONFIG``). This allows projects to override
+configuration values for multiple modules from a single configuration backend,
+if desired. The configuration values may also be overridden individually by
+setting backends for the individual module configurations (e.g. in GN,
+``pw_foo_CONFIG = "//configuration:my_foo_config"``).
+
+Configurations are overridden by setting compilation options in the config
+backend. These options could be set through macro definitions, such as
+``-DPW_FOO_INPUT_BUFFER_SIZE_BYTES=256``, or in a header file included with the
+``-include`` option.
+
+This example shows how two ways to configure a module in the GN build system.
+
+.. code-block::
+
+ # In the toolchain, set either pw_build_DEFAULT_MODULE_CONFIG or pw_foo_CONFIG
+ pw_build_DEFAULT_MODULE_CONFIG = get_path_info(":define_overrides", "abspath")
+
+ # This configuration sets PW_FOO_INPUT_BUFFER_SIZE_BYTES using the -D macro.
+ pw_source_set("define_overrides") {
+ public_configs = [ ":define_options" ]
+ }
+
+ config("define_options") {
+ defines = [ "-DPW_FOO_INPUT_BUFFER_SIZE_BYTES=256" ]
+ }
+
+ # This configuration sets PW_FOO_INPUT_BUFFER_SIZE_BYTES with a header file.
+ pw_source_set("include_overrides") {
+ public_configs = [ ":header_options" ]
+
+ # Header file with #define PW_FOO_INPUT_BUFFER_SIZE_BYTES 256
+ sources = [ "my_config_overrides.h" ]
+ }
+
+ config("header_options") {
+ cflags = [
+ "-include",
+ "my_config_overrides.h",
+ ]
+ }
+
+.. _docs-module-structure-facades:
+
+Facades
+-------
+In Pigweed, facades represent a dependency that can be swapped at compile time.
+Facades are similar in concept to a virtual interface, but the implementation is
+set by the build system. Runtime polymorphism with facades is not
+possible, and each facade may only have one implementation (backend) per
+toolchain compilation.
+
+In the simplest sense, a facade is just a dependency represented by a variable.
+For example, the ``pw_log`` facade is represented by the ``pw_log_BACKEND``
+build variable. Facades typically are bundled with a build system library that
+depends on the backend.
+
+Modules should only use facades when necessary. Since they are fixed at compile
+time, runtime dependency injection is not possible. Where appropriate, modules
+should use other mechanisms, such as virtual interfaces or callbacks, in place
+of facades.
+
+Facades are essential in some circumstances:
+
+* Low-level, platform-specific features (:ref:`module-pw_cpu_exception`).
+* Features that require a macro or non-virtual function interface
+ (:ref:`module-pw_tokenizer`),
+* Highly leveraged code where a virtual interface or callback is too costly or
+ cumbersome (:ref:`module-pw_log`, :ref:`module-pw_assert`).
+
+The GN build system provides the
+:ref:`pw_facade template<module-pw_build-facade>` as a convenient way to declare
+facades.
+
Documentation
-------------
Documentation should go in the root module folder, typically in the
diff --git a/pw_arduino_build/BUILD.gn b/pw_arduino_build/BUILD.gn
index 5556dc5..db72ea8 100644
--- a/pw_arduino_build/BUILD.gn
+++ b/pw_arduino_build/BUILD.gn
@@ -28,7 +28,6 @@
pw_facade("arduino_init") {
backend = pw_arduino_build_INIT_BACKEND
public = [ "public/pw_arduino_build/init.h" ]
- facade_name = "arduino_init_facade"
public_configs = [ ":default_config" ]
}
config("default_config") {
diff --git a/pw_build/docs.rst b/pw_build/docs.rst
index 64c67f9..8fe235b 100644
--- a/pw_build/docs.rst
+++ b/pw_build/docs.rst
@@ -42,7 +42,7 @@
Target types
^^^^^^^^^^^^
-.. code::
+.. code-block::
import("$dir_pw_build/target_types.gni")
@@ -75,6 +75,34 @@
All of the ``pw_*`` target type overrides accept any arguments, as they simply
forward them through to the underlying target.
+.. _module-pw_build-facade:
+
+pw_facade
+^^^^^^^^^
+In their simplest form, a :ref:`facade<docs-module-structure-facades>` is a GN
+build arg used to change a dependency at compile time. Pigweed targets configure
+these facades as needed.
+
+The ``pw_facade`` template bundles a ``pw_source_set`` with a facade build arg.
+This allows the facade to provide header files, compilation options or anything
+else a GN ``source_set`` provides.
+
+The ``pw_facade`` template declares two targets:
+
+* ``$target_name``: the public-facing ``pw_source_set``, with a ``public_dep``
+ on the backend
+* ``$target_name.facade``: target used by the backend to avoid circular
+ dependencies
+
+.. code-block::
+
+ # Declares ":foo" and ":foo.facade" GN targets
+ pw_facade("foo") {
+ backend = pw_log_BACKEND
+ public_configs = [ ":public_include_path" ]
+ public = [ "public/pw_foo/foo.h" ]
+ }
+
.. _module-pw_build-python-script:
pw_python_action
@@ -127,13 +155,13 @@
Evaluates to the output file of the provided GN target. For example, the
expression
- .. code::
+ .. code-block::
"<TARGET_FILE(//foo/bar:static_lib)>"
might expand to
- .. code::
+ .. code-block::
"/home/User/project_root/out/obj/foo/bar/static_lib.a"
@@ -158,14 +186,14 @@
For example, consider this expression:
- .. code::
+ .. code-block::
"--database=<TARGET_FILE_IF_EXISTS(//alpha/bravo)>"
If the ``//alpha/bravo`` target file exists, this might expand to the
following:
- .. code::
+ .. code-block::
"--database=/home/User/project/out/obj/alpha/bravo/bravo.elf"
@@ -181,13 +209,13 @@
For example, the expression
- .. code::
+ .. code-block::
"<TARGET_OBJECTS(//foo/bar:a_source_set)>"
might expand to multiple separate arguments:
- .. code::
+ .. code-block::
"/home/User/project_root/out/obj/foo/bar/a_source_set.file_a.cc.o"
"/home/User/project_root/out/obj/foo/bar/a_source_set.file_b.cc.o"
@@ -195,7 +223,7 @@
**Example**
-.. code::
+.. code-block::
import("$dir_pw_build/python_action.gni")
@@ -235,7 +263,7 @@
**Example**
-.. code::
+.. code-block::
import("$dir_pw_build/input_group.gni")
@@ -254,7 +282,7 @@
files are modified.
pw_zip
-^^^^^^^^^^^^^^
+^^^^^^
``pw_zip`` is a target that allows users to zip up a set of input files and
directories into a single output ``.zip`` file—a simple automation of a
potentially repetitive task.
@@ -286,7 +314,7 @@
Let's say we have the following structure for a ``//source/`` directory:
-.. code::
+.. code-block::
source/
├── file1.txt
@@ -299,7 +327,7 @@
And we create the following build target:
-.. code::
+.. code-block::
import("$dir_pw_build/zip.gni")
@@ -324,7 +352,7 @@
This will result in a ``.zip`` file called ``foo.zip`` stored in
``//$target_out_dir`` with the following structure:
-.. code::
+.. code-block::
foo.zip
├── bar/
@@ -345,7 +373,7 @@
The following command generates Ninja build files in the out/cmake directory.
-.. code:: sh
+.. code-block:: sh
cmake -B out/cmake -S /path/to/pigweed -G Ninja
@@ -373,7 +401,7 @@
To use Pigweed libraries form a CMake-based project, simply include the Pigweed
repository from a ``CMakeLists.txt``.
-.. code:: cmake
+.. code-block:: cmake
add_subdirectory(path/to/pigweed pigweed)
@@ -382,7 +410,7 @@
If desired, modules can be included individually.
-.. code:: cmake
+.. code-block:: cmake
include(path/to/pigweed/pw_build/pigweed.cmake)
diff --git a/pw_build/facade.gni b/pw_build/facade.gni
index 684a4f7..4bda84f 100644
--- a/pw_build/facade.gni
+++ b/pw_build/facade.gni
@@ -23,8 +23,19 @@
# link against. Typically this will be done by pointing a build arg like
# `pw_[module]_BACKEND` at a backend implementation for that module.
#
+# To avoid circular dependencies, pw_facade creates two targets:
+#
+# - $target_name: the public-facing pw_source_set
+# - $target_name.facade: target used by the backend to avoid circular
+# dependencies
+#
+# If the target name matches the directory name (e.g. //foo:foo), a ":facade"
+# alias of the facade target (e.g. //foo:facade) is also provided. This avoids
+# the need to repeat the directory name, for consistency with the main target.
+#
# Example facade:
#
+# # Creates ":module_name" and ":module_name.facade" GN targets.
# pw_facade("module_name") {
# backend = dir_module_name_backend
# public_deps = [
@@ -32,20 +43,32 @@
# ]
# }
#
-# Args:
-# - backend: the dependency that implements this facade
-# - facade_name: (optional) The name to use for the facade target on which the
-# backend depends. Only required when a module defines multiple facades.
-# Defaults to "facade".
+# Accepts the standard pw_source_set args with the following additions:
+#
+# - backend: the dependency that implements this facade (a GN variable)
#
template("pw_facade") {
assert(defined(invoker.backend),
"pw_facade requires a reference to a backend variable for the facade")
+ _facade_name = "$target_name.facade"
+
if (defined(invoker.facade_name)) {
- _facade_name = invoker.facade_name
- } else {
- _facade_name = "facade"
+ not_needed([ invoker.facade_name ])
+ print("pw_facade(\"$target_name\") uses the facade_name arg, which is",
+ "deprecated. Access the facade target as $target_name.facade")
+ }
+
+ if (get_path_info(get_label_info(":$target_name", "dir"), "name") ==
+ get_label_info(":$target_name", "name")) {
+ group("facade") {
+ public_deps = [ ":$_facade_name" ]
+ }
+ }
+
+ # For backwards compatibility, provide a _facade version of the name.
+ group(target_name + "_facade") {
+ public_deps = [ ":$_facade_name" ]
}
# A facade's headers are split into a separate target to avoid a circular
@@ -90,7 +113,7 @@
# error message.
_main_target_name = target_name
- pw_python_action(_main_target_name + "_NO_BACKEND_SET") {
+ pw_python_action(_main_target_name + ".NO_BACKEND_SET") {
stamp = true
script = "$dir_pw_build/py/pw_build/null_backend.py"
args = [ _main_target_name ]
@@ -112,8 +135,8 @@
if (invoker.backend != "") {
public_deps += [ invoker.backend ]
} else {
- # If the backend is not set, depend on the *_NO_BACKEND_SET target.
- public_deps += [ ":$_main_target_name" + "_NO_BACKEND_SET" ]
+ # If the backend is not set, depend on the *.NO_BACKEND_SET target.
+ public_deps += [ ":$_main_target_name" + ".NO_BACKEND_SET" ]
}
}
}
diff --git a/pw_cpu_exception/BUILD.gn b/pw_cpu_exception/BUILD.gn
index 58879fc..f2562e2 100644
--- a/pw_cpu_exception/BUILD.gn
+++ b/pw_cpu_exception/BUILD.gn
@@ -52,16 +52,14 @@
pw_facade("entry") {
backend = pw_cpu_exception_ENTRY_BACKEND
- facade_name = "entry_facade"
public_configs = [ ":default_config" ]
public_deps = [ "$dir_pw_preprocessor" ]
- deps = [ ":handler_facade" ]
+ deps = [ ":handler.facade" ]
public = [ "public/pw_cpu_exception/entry.h" ]
}
pw_facade("handler") {
backend = pw_cpu_exception_HANDLER_BACKEND
- facade_name = "handler_facade"
public_configs = [ ":default_config" ]
public_deps = [
"$dir_pw_preprocessor",
@@ -77,7 +75,6 @@
# pw_CpuExceptionState members.
pw_facade("support") {
backend = pw_cpu_exception_SUPPORT_BACKEND
- facade_name = "support_facade"
public_configs = [ ":default_config" ]
public_deps = [ "$dir_pw_span" ]
public = [ "public/pw_cpu_exception/support.h" ]
@@ -85,7 +82,7 @@
pw_source_set("basic_handler") {
deps = [
- ":handler_facade",
+ ":handler.facade",
dir_pw_log,
]
sources = [ "basic_handler.cc" ]
diff --git a/pw_cpu_exception_armv7m/BUILD.gn b/pw_cpu_exception_armv7m/BUILD.gn
index 5a97e57..a3a4b35 100644
--- a/pw_cpu_exception_armv7m/BUILD.gn
+++ b/pw_cpu_exception_armv7m/BUILD.gn
@@ -26,7 +26,7 @@
pw_source_set("support") {
public_configs = [ ":default_config" ]
public_deps = [
- "$dir_pw_cpu_exception:support_facade",
+ "$dir_pw_cpu_exception:support.facade",
dir_pw_preprocessor,
dir_pw_string,
]
@@ -56,7 +56,7 @@
public_deps = [
":proto_dump",
":support",
- "$dir_pw_cpu_exception:entry_facade",
+ "$dir_pw_cpu_exception:entry.facade",
"$dir_pw_cpu_exception:handler",
"$dir_pw_preprocessor",
]
diff --git a/pw_log_tokenized/BUILD.gn b/pw_log_tokenized/BUILD.gn
index 3de893c..94db9c1 100644
--- a/pw_log_tokenized/BUILD.gn
+++ b/pw_log_tokenized/BUILD.gn
@@ -33,7 +33,7 @@
# backend.
#
# This target depends on the pw_tokenizer facade target
-# (dir_pw_tokenizer:global_handler_with_payload_facade) to avoid circular
+# (dir_pw_tokenizer:global_handler_with_payload.facade) to avoid circular
# dependencies. The dependency graph for pw_log_tokenized is the following:
#
# pw_log:facade <--- :pw_log_tokenized
@@ -45,7 +45,7 @@
public_configs = [ ":default_config" ]
public_deps = [
"$dir_pw_log:facade",
- "$dir_pw_tokenizer:global_handler_with_payload_facade",
+ "$dir_pw_tokenizer:global_handler_with_payload.facade",
dir_pw_preprocessor,
]
public = [ "public/pw_log_tokenized/log_tokenized.h" ]
diff --git a/pw_log_tokenized/docs.rst b/pw_log_tokenized/docs.rst
index 323522f..1c30ee7 100644
--- a/pw_log_tokenized/docs.rst
+++ b/pw_log_tokenized/docs.rst
@@ -38,7 +38,7 @@
``log_backend``. The ``pw_log_tokenized`` target provides the
``pw_log_tokenized/log_tokenized.h`` header. The ``log_backend`` target
implements the backend for the ``pw_log`` facade. ``pw_log_tokenized`` invokes
-the ``pw_tokenizer:global_handler_with_facade`` facade, which must be
+the ``pw_tokenizer:global_handler_with_payload`` facade, which must be
implemented by the user of ``pw_log_tokenized``.
.. note::
diff --git a/pw_tokenizer/BUILD.gn b/pw_tokenizer/BUILD.gn
index 1f7e442..8d245a2 100644
--- a/pw_tokenizer/BUILD.gn
+++ b/pw_tokenizer/BUILD.gn
@@ -88,7 +88,6 @@
}
pw_facade("global_handler") {
- facade_name = "global_handler_facade"
backend = pw_tokenizer_GLOBAL_HANDLER_BACKEND
public_configs = [ ":public_include_path" ]
@@ -98,7 +97,6 @@
}
pw_facade("global_handler_with_payload") {
- facade_name = "global_handler_with_payload_facade"
backend = pw_tokenizer_GLOBAL_HANDLER_WITH_PAYLOAD_BACKEND
public_configs = [ ":public_include_path" ]
diff --git a/targets/arduino/BUILD.gn b/targets/arduino/BUILD.gn
index 06f8318..07d84a6 100644
--- a/targets/arduino/BUILD.gn
+++ b/targets/arduino/BUILD.gn
@@ -84,7 +84,7 @@
"$dir_pw_third_party_arduino:arduino_core_sources",
]
deps = [
- "$dir_pw_arduino_build:arduino_init_facade",
+ "$dir_pw_arduino_build:arduino_init.facade",
"$dir_pw_preprocessor",
]
}
