pw_build: Support specifying final link dependencies
- pw_build_LINK_DEPS is a list of build targets that are linked into
all build artifacts produced by pw_executable, pw_static_library, and
pw_shared_library.
- Add a ":deps" group to the pw_assert build interface. Require
$dir_pw_assert:deps to be listed in pw_build_LINK_DEPS if
pw_assert_BACKEND is set.
Change-Id: Icc01dbe86f95d971e76aa357c7b7174ce85546ba
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/43521
Pigweed-Auto-Submit: Wyatt Hepler <hepler@google.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed.google.com.iam.gserviceaccount.com>
Reviewed-by: Armando Montanez <amontanez@google.com>
diff --git a/pw_assert/BUILD.gn b/pw_assert/BUILD.gn
index 2b78c00..0362da6 100644
--- a/pw_assert/BUILD.gn
+++ b/pw_assert/BUILD.gn
@@ -83,6 +83,46 @@
public_deps = [ dir_pw_preprocessor ]
}
+# pw_assert is low-level and ubiquitous. Because of this, it can often cause
+# circular dependencies. This target collects dependencies from the backend that
+# cannot be used because they would cause circular deps.
+#
+# This group ("$dir_pw_assert:deps") must listed in pw_build_LINK_DEPS if
+# pw_assert_BACKEND is set.
+#
+# pw_assert backends must provide their own "deps" group that collects their
+# actual dependencies. The backend "deps" group may be empty.
+group("deps") {
+ public_deps = []
+
+ if (pw_assert_BACKEND != "") {
+ public_deps += [ get_label_info(pw_assert_BACKEND, "dir") + ":deps" ]
+
+ # Make sure this target is listed in pw_build_LINK_DEPS. This
+ # ensures these dependencies are available during linking, even if nothing
+ # else in the build depends on them.
+ _deps_label = get_label_info(":$target_name", "label_no_toolchain")
+ _deps_is_in_link_dependencies = false
+
+ foreach(label, pw_build_LINK_DEPS) {
+ if (get_label_info(label, "label_no_toolchain") == _deps_label) {
+ _deps_is_in_link_dependencies = true
+ }
+ }
+
+ # TODO(pwbug/372): Update projects with pw_assert to link the :deps target.
+ _disable_this_check_for_now = true
+ not_needed([
+ "_deps_is_in_link_dependencies",
+ "_deps_label",
+ ])
+
+ assert(_disable_this_check_for_now || _deps_is_in_link_dependencies,
+ "\$dir_pw_assert:$target_name must be listed in " +
+ "pw_build_LINK_DEPS when pw_assert_BACKEND is set")
+ }
+}
+
# Note: While this is technically a test, doesn't verify any of the output and
# is more of a compile test. The results can be visually verified if desired.
pw_test("assert_test") {
diff --git a/pw_assert/docs.rst b/pw_assert/docs.rst
index 6a6576e..a66c3f7 100644
--- a/pw_assert/docs.rst
+++ b/pw_assert/docs.rst
@@ -472,20 +472,26 @@
``pw_assert_HandleFailure()``, which must be provided by the ``pw_assert``
backend.
+.. _module-pw_assert-circular-deps:
+
Avoiding circular dependencies with ``PW_ASSERT``
-------------------------------------------------
Because asserts are so widely used, including in low-level libraries, it is
-common for the ``pw_assert`` backend to cause circular dependencies. These can
-be avoided by only using the ``PW_ASSERT`` macro and depending directly on the
-library that provides it, rather than the whole ``pw_assert`` module. The
-``pw_assert`` backend, which defines ``pw_assert_HandleFailure()``, will need to
-be linked in elsewhere in the build, so only depend directly on the
-``pw_assert`` build target when necessary.
+common for the ``pw_assert`` backend to cause circular dependencies. Because of
+this, assert backends may avoid declaring explicit dependencies, instead relying
+on include paths to access header files.
-In GN, depend on ``"$dir_pw_assert:assert"`` rather than on ``dir_pw_assert`` to
-access only ``PW_ASSERT`` without risking circular dependencies. The
-``pw_assert_HandleFailure()`` function is provided by the
-``$dir_pw_assert:pw_assert`` or ``"$dir_pw_assert:check"`` target.
+In GN, the ``pw_assert`` backend's true dependencies are made available through
+the ``$dir_pw_assert:deps`` group. When ``pw_assert_BACKEND`` is set,
+``$dir_pw_assert:deps`` must be listed in the ``pw_build_LINK_DEPS`` variable.
+See :ref:`module-pw_build-link-deps`.
+
+If necessary, ``pw_assert`` backends can access dependencies from include paths
+rather than GN ``deps``. In this case, the may disable GN header checking with
+``check_includes = false``. The true build dependencies must be listed in a
+``deps`` group, which the ``pw_assert`` facade depends on. The ``deps`` group
+may be empty if the backend can use its dependencies directly without causing
+circular dependencies.
.. _module-pw_assert-backend_api:
@@ -574,6 +580,14 @@
file, expression, or other rich assert information. Backends should do
something reasonable in this case; typically, capturing the stack is useful.
+Backend build targets
+---------------------
+In GN, the backend must provide a ``deps`` build target in the same directory as
+the backend target. The ``deps`` target contains the backend's dependencies that
+could result in a dependency cycle. In the simplest case, it can be an empty
+group. Circular dependencies are a common problem with ``pw_assert`` because it
+is so widely used. See :ref:`module-pw_assert-circular-deps`.
+
--------------------------
Frequently asked questions
--------------------------
diff --git a/pw_assert_basic/BUILD.gn b/pw_assert_basic/BUILD.gn
index a8682f1..c2e48d1 100644
--- a/pw_assert_basic/BUILD.gn
+++ b/pw_assert_basic/BUILD.gn
@@ -19,17 +19,29 @@
import("$dir_pw_docgen/docs.gni")
import("backend.gni")
-config("default_config") {
+config("public_include_path") {
include_dirs = [ "public" ]
+ visibility = [ ":*" ]
}
config("backend_config") {
include_dirs = [ "public_overrides" ]
+ visibility = [ ":*" ]
+}
+
+# These assert backend deps that might cause circular dependencies, since
+# pw_assert is so ubiquitous. These deps are kept separate so they can be
+# depended on from elsewhere.
+group("deps") {
+ public_deps = [
+ dir_pw_string,
+ dir_pw_sys_io,
+ ]
}
pw_facade("handler") {
backend = pw_assert_basic_HANDLER_BACKEND
- public_configs = [ ":default_config" ]
+ public_configs = [ ":public_include_path" ]
public_deps = [ "$dir_pw_preprocessor" ]
public = [ "public/pw_assert_basic/handler.h" ]
}
@@ -37,7 +49,7 @@
pw_source_set("pw_assert_basic") {
public_configs = [
":backend_config",
- ":default_config",
+ ":public_include_path",
]
deps = [
"$dir_pw_assert:facade",
@@ -52,14 +64,20 @@
sources = [ "assert_basic.cc" ]
}
-# A basic handler backend using sysio.
+# A basic handler backend using pw_sys_io.
pw_source_set("basic_handler") {
+ # Turn off GN check since this target intentionally leaves out deps to avoid
+ # circular dependencies.
+ check_includes = false
+
+ # Depend on the include path instead of the library to avoid circular deps.
+ configs = [ "$dir_pw_string:public_include_path" ]
deps = [
":handler.facade",
"$dir_pw_assert:facade",
"$dir_pw_preprocessor",
- "$dir_pw_string",
- "$dir_pw_sys_io",
+ "$dir_pw_sys_io:facade", # Only pull in the facade to avoid circular deps
+ dir_pw_status,
]
sources = [ "basic_handler.cc" ]
}
diff --git a/pw_assert_log/BUILD.gn b/pw_assert_log/BUILD.gn
index 083c5e8..f6726b0 100644
--- a/pw_assert_log/BUILD.gn
+++ b/pw_assert_log/BUILD.gn
@@ -46,6 +46,11 @@
sources = [ "assert_log.cc" ]
}
+# pw_assert_log doesn't have deps with potential circular dependencies, so
+# "deps" can be empty.
+group("deps") {
+}
+
pw_doc_group("docs") {
sources = [ "docs.rst" ]
}
diff --git a/pw_build/BUILD.gn b/pw_build/BUILD.gn
index 706b8d6..bb83617 100644
--- a/pw_build/BUILD.gn
+++ b/pw_build/BUILD.gn
@@ -16,6 +16,7 @@
import("$dir_pw_build/python.gni")
import("$dir_pw_docgen/docs.gni")
+import("target_types.gni")
# IMPORTANT: The compilation flags in this file must be kept in sync with
# the CMake flags pw_build/CMakeLists.txt.
@@ -121,6 +122,18 @@
]
}
+# This group is linked into all pw_executable, pw_static_library, and
+# pw_shared_library targets. This makes it possible to ensure symbols are
+# defined without a dependency on them.
+#
+# pw_build_LINK_DEPS should only be used when necessary. For example,
+# pw_assert relies on pw_build_LINK_DEPS to avoid circular dependencies
+# in GN. In almost all other cases, build targets should explicitly depend on
+# the other targets they use.
+group("link_deps") {
+ deps = pw_build_LINK_DEPS
+}
+
# This empty target is used as the default value for module configurations.
# Projects may set pw_build_DEFAULT_MODULE_CONFIG to a different GN target that
# overrides modules' configuration options via macro definitions or a header
diff --git a/pw_build/docs.rst b/pw_build/docs.rst
index 6135755..ad43bff 100644
--- a/pw_build/docs.rst
+++ b/pw_build/docs.rst
@@ -72,6 +72,21 @@
All of the ``pw_*`` target type overrides accept any arguments, as they simply
forward them through to the underlying target.
+.. _module-pw_build-link-deps:
+
+Link-only deps
+--------------
+It may be necessary to specify additional link-time dependencies that may not be
+explicitly depended on elsewhere in the build. One example of this is a
+``pw_assert`` backend, which may need to leave out dependencies to avoid
+circular dependencies. Its dependencies need to be linked for executables and
+libraries, even if they aren't pulled in elsewhere.
+
+The ``pw_build_LINK_DEPS`` build arg is a list of dependencies to add to all
+``pw_executable``, ``pw_static_library``, and ``pw_shared_library`` targets.
+This should only be used as a last resort when dependencies cannot be properly
+expressed in the build.
+
Python packages
---------------
GN templates for :ref:`Python build automation <docs-python-build>` are
diff --git a/pw_build/target_types.gni b/pw_build/target_types.gni
index 7590f2d..fb1709e 100644
--- a/pw_build/target_types.gni
+++ b/pw_build/target_types.gni
@@ -27,6 +27,14 @@
# If pw_build_EXECUTABLE_TARGET_TYPE is not the default of `executable`, this
# .gni file is imported to provide the template definition.
pw_build_EXECUTABLE_TARGET_TYPE_FILE = ""
+
+ # Additional build targets to add as dependencies for pw_executable,
+ # pw_static_library, and pw_shared_library targets. The
+ # $dir_pw_build:link_deps target pulls in these libraries.
+ #
+ # pw_build_LINK_DEPS can be used to break circular dependencies for low-level
+ # libraries such as pw_assert.
+ pw_build_LINK_DEPS = []
}
if (pw_build_EXECUTABLE_TARGET_TYPE != "executable" &&
@@ -112,10 +120,9 @@
configs += invoker.configs
}
+ public_deps = [ "$dir_pw_build:link_deps" ]
if (defined(pw_build_defaults.public_deps)) {
- public_deps = pw_build_defaults.public_deps
- } else {
- public_deps = []
+ public_deps += pw_build_defaults.public_deps
}
if (defined(remove_public_deps)) {
if (remove_public_deps != [] && remove_public_deps[0] == "*") {
@@ -161,10 +168,9 @@
configs += invoker.configs
}
+ public_deps = [ "$dir_pw_build:link_deps" ]
if (defined(pw_build_defaults.public_deps)) {
- public_deps = pw_build_defaults.public_deps
- } else {
- public_deps = []
+ public_deps += pw_build_defaults.public_deps
}
if (defined(remove_public_deps)) {
if (remove_public_deps != [] && remove_public_deps[0] == "*") {
@@ -213,10 +219,9 @@
configs += invoker.configs
}
+ public_deps = [ "$dir_pw_build:link_deps" ]
if (defined(pw_build_defaults.public_deps)) {
- public_deps = pw_build_defaults.public_deps
- } else {
- public_deps = []
+ public_deps += pw_build_defaults.public_deps
}
if (defined(remove_public_deps)) {
if (remove_public_deps != [] && remove_public_deps[0] == "*") {
diff --git a/pw_string/BUILD.gn b/pw_string/BUILD.gn
index bf59ff2..5aa0134 100644
--- a/pw_string/BUILD.gn
+++ b/pw_string/BUILD.gn
@@ -19,12 +19,12 @@
import("$dir_pw_docgen/docs.gni")
import("$dir_pw_unit_test/test.gni")
-config("default_config") {
+config("public_include_path") {
include_dirs = [ "public" ]
}
pw_source_set("pw_string") {
- public_configs = [ ":default_config" ]
+ public_configs = [ ":public_include_path" ]
public = [
"public/pw_string/format.h",
"public/pw_string/string_builder.h",
diff --git a/pw_sys_io/BUILD.gn b/pw_sys_io/BUILD.gn
index 47feb43..a15e999 100644
--- a/pw_sys_io/BUILD.gn
+++ b/pw_sys_io/BUILD.gn
@@ -19,13 +19,13 @@
import("$dir_pw_docgen/docs.gni")
import("backend.gni")
-config("default_config") {
+config("public_include_path") {
include_dirs = [ "public" ]
}
pw_facade("pw_sys_io") {
backend = pw_sys_io_BACKEND
- public_configs = [ ":default_config" ]
+ public_configs = [ ":public_include_path" ]
public_deps = [ dir_pw_status ]
public = [ "public/pw_sys_io/sys_io.h" ]
}
diff --git a/targets/arduino/target_toolchains.gni b/targets/arduino/target_toolchains.gni
index 469a95f..0ee8322 100644
--- a/targets/arduino/target_toolchains.gni
+++ b/targets/arduino/target_toolchains.gni
@@ -51,6 +51,8 @@
"$dir_pigweed/targets/arduino:system_rpc_server"
pw_arduino_build_INIT_BACKEND = "$dir_pigweed/targets/arduino:pre_init"
+ pw_build_LINK_DEPS = [ "$dir_pw_assert:deps" ]
+
current_cpu = "arm"
current_os = ""
}
diff --git a/targets/host/target_toolchains.gni b/targets/host/target_toolchains.gni
index 8980c45..f0fa57e 100644
--- a/targets/host/target_toolchains.gni
+++ b/targets/host/target_toolchains.gni
@@ -64,6 +64,9 @@
pw_thread_YIELD_BACKEND = "$dir_pw_thread_stl:yield"
pw_thread_THREAD_BACKEND = "$dir_pw_thread_stl:thread"
+ pw_build_LINK_DEPS = [] # Explicit list overwrite required by GN
+ pw_build_LINK_DEPS = [ "$dir_pw_assert:deps" ]
+
# Specify builtin GN variables.
current_os = host_os
current_cpu = host_cpu
diff --git a/targets/lm3s6965evb-qemu/target_toolchains.gni b/targets/lm3s6965evb-qemu/target_toolchains.gni
index c3fba37..dc18b3f 100644
--- a/targets/lm3s6965evb-qemu/target_toolchains.gni
+++ b/targets/lm3s6965evb-qemu/target_toolchains.gni
@@ -61,6 +61,8 @@
"PW_BOOT_VECTOR_TABLE_SIZE=512",
]
+ pw_build_LINK_DEPS = [ "$dir_pw_assert:deps" ]
+
current_cpu = "arm"
current_os = ""
}
diff --git a/targets/stm32f429i-disc1/target_toolchains.gni b/targets/stm32f429i-disc1/target_toolchains.gni
index e87245e..6c690b8 100644
--- a/targets/stm32f429i-disc1/target_toolchains.gni
+++ b/targets/stm32f429i-disc1/target_toolchains.gni
@@ -73,6 +73,8 @@
"PW_BOOT_VECTOR_TABLE_SIZE=512",
]
+ pw_build_LINK_DEPS = [ "$dir_pw_assert:deps" ]
+
current_cpu = "arm"
current_os = ""
}
