pw_protobuf_compiler: Remove pw_protobuf_GENERATORS build arg
This updates pw_protobuf_compiler to always create a target for each
supported protobuf generator, instead of having a build arg to select
which ones to use, simplifying the use of pw_proto_library.
Change-Id: I2699c9c5045e8671a061f8c8bc11a94244d008f1
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/22721
Reviewed-by: Armando Montanez <amontanez@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
Commit-Queue: Alexei Frolov <frolv@google.com>
diff --git a/docs/build_system.rst b/docs/build_system.rst
index 69f753e..6938d19 100644
--- a/docs/build_system.rst
+++ b/docs/build_system.rst
@@ -213,7 +213,6 @@
# Build arguments set across all Pigweed targets.
default_args = {
- pw_protobuf_GENERATORS = [ "nanopb_rpc" ]
dir_pw_third_party_nanopb = "//third_party/nanopb-0.4.2"
}
diff --git a/docs/targets.rst b/docs/targets.rst
index 8ada893..21f4ccc 100644
--- a/docs/targets.rst
+++ b/docs/targets.rst
@@ -98,7 +98,6 @@
# Extend with custom build arguments for the target.
pw_log_BACKEND = dir_pw_log_tokenized
- pw_protobuf_GENERATORS = [ "nanopb" ]
}
}
diff --git a/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py b/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
index 6396dc0..6638cad 100755
--- a/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
+++ b/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
@@ -102,8 +102,7 @@
build.gn_gen(ctx.root,
ctx.output_dir,
dir_pw_third_party_nanopb='"{}"'.format(ctx.package_root /
- 'nanopb'),
- pw_protobuf_GENERATORS='["nanopb", "nanopb_rpc", "pwpb"]')
+ 'nanopb'))
build.ninja(
ctx.output_dir,
*_at_all_optimization_levels('stm32f429i'),
diff --git a/pw_protobuf_compiler/BUILD.gn b/pw_protobuf_compiler/BUILD.gn
index 76549c3..58dce83 100644
--- a/pw_protobuf_compiler/BUILD.gn
+++ b/pw_protobuf_compiler/BUILD.gn
@@ -23,27 +23,16 @@
sources = [ "docs.rst" ]
}
-_compiling_for_nanopb = pw_protobuf_GENERATORS + [ "nanopb" ] - [ "nanopb" ] !=
- pw_protobuf_GENERATORS
-
pw_test_group("tests") {
- tests = []
- if (_compiling_for_nanopb) {
- tests += [ ":nanopb_test" ]
- }
+ tests = [ ":nanopb_test" ]
}
-if (_compiling_for_nanopb) {
- pw_test("nanopb_test") {
- deps = [ ":nanopb_test_protos.nanopb" ]
- sources = [ "nanopb_test.cc" ]
- }
+pw_test("nanopb_test") {
+ deps = [ ":nanopb_test_protos.nanopb" ]
+ sources = [ "nanopb_test.cc" ]
+ enable_if = dir_pw_third_party_nanopb != ""
+}
- pw_proto_library("nanopb_test_protos") {
- sources = [ "pw_protobuf_compiler_protos/nanopb_test.proto" ]
- }
-} else {
- pw_input_group("not_needed") {
- inputs = [ "nanopb_test.cc" ]
- }
+pw_proto_library("nanopb_test_protos") {
+ sources = [ "pw_protobuf_compiler_protos/nanopb_test.proto" ]
}
diff --git a/pw_protobuf_compiler/docs.rst b/pw_protobuf_compiler/docs.rst
index ae966ef..3e63881 100644
--- a/pw_protobuf_compiler/docs.rst
+++ b/pw_protobuf_compiler/docs.rst
@@ -3,13 +3,11 @@
--------------------
pw_protobuf_compiler
--------------------
-
The Protobuf compiler module provides build system integration and wrapper
scripts for generating source code for Protobuf definitions.
Generator support
=================
-
Protobuf code generation is currently supported for the following generators:
+-------------+----------------+-----------------------------------------------+
@@ -30,30 +28,29 @@
| Raw RPC | ``raw_rpc`` | Compiles raw binary pw_rpc service code. |
+-------------+----------------+-----------------------------------------------+
-The build variable ``pw_protobuf_GENERATORS`` tells the module the generators
-for which it should compile code. It is defined as a list of generator codes.
-
GN template
===========
-
The ``pw_proto_library`` GN template is provided by the module.
-It tells the build system to compile a set of source proto files to a library in
-each chosen generator. A different target is created for each generator, with
-the generator's code appended as a suffix to the template's target name.
+It defines a collection of protobuf files that should be compiled together. The
+template creates a sub-target for each supported generator, named
+``<target_name>.<generator>``. These sub-targets generate their respective
+protobuf code, and expose it to the build system appropriately (e.g. a
+``pw_source_set`` for C/C++).
-For example, given the definitions:
+For example, given the following target:
-.. code::
-
- pw_protobuf_GENERATORS = [ "pwpb", "go" ]
+.. code-block::
pw_proto_library("test_protos") {
sources = [ "test.proto" ]
}
-Two targets are created, named ``test_protos.pwpb`` and ``test_protos.go``,
-containing the generated code from their respective generators.
+``test_protos.pwpb`` compiles code for pw_protobuf, and ``test_protos.nanopb``
+compiles using Nanopb (if it's installed).
+
+Protobuf code is only generated when a generator sub-target is listed as a
+dependency of another GN target.
**Arguments**
diff --git a/pw_protobuf_compiler/proto.gni b/pw_protobuf_compiler/proto.gni
index 6878156..ad9f175 100644
--- a/pw_protobuf_compiler/proto.gni
+++ b/pw_protobuf_compiler/proto.gni
@@ -14,24 +14,12 @@
import("//build_overrides/pigweed.gni")
+import("$dir_pw_build/error.gni")
import("$dir_pw_build/input_group.gni")
import("$dir_pw_build/python_action.gni")
import("$dir_pw_build/target_types.gni")
import("$dir_pw_third_party/nanopb/nanopb.gni")
-declare_args() {
- # Generators with which to compile protobuf code. These are used by the
- # pw_proto_library template to determine which build targets to create.
- #
- # Supported generators:
- # "pwpb", "nanopb", "nanopb_rpc", "raw_rpc", "go"
- pw_protobuf_GENERATORS = [
- "pwpb",
- "raw_rpc",
- "go",
- ]
-}
-
# Python script that invokes protoc.
_gen_script_path =
"$dir_pw_protobuf_compiler/py/pw_protobuf_compiler/generate_protos.py"
@@ -81,8 +69,7 @@
}
# For C++ proto files, the generated proto directory is added as an include
- # path for the code. This requires using "all_dependent_configs" to force the
- # include on any code that transitively depends on the generated protos.
+ # path for the code.
_include_config_target = "${target_name}_includes"
config(_include_config_target) {
include_dirs = [ "$_proto_gen_dir" ]
@@ -90,7 +77,7 @@
# Create a library with the generated source files.
pw_source_set(target_name) {
- all_dependent_configs = [ ":$_include_config_target" ]
+ public_configs = [ ":$_include_config_target" ]
deps = [ ":$_gen_target" ]
public_deps = [ dir_pw_protobuf ] + invoker.gen_deps
sources = get_target_outputs(":$_gen_target")
@@ -106,9 +93,6 @@
# protos: List of input .proto files.
#
template("_pw_nanopb_rpc_proto_library") {
- assert(defined(dir_pw_third_party_nanopb) && dir_pw_third_party_nanopb != "",
- "\$dir_pw_third_party_nanopb must be set to compile nanopb protobufs")
-
_proto_gen_dir = "$root_gen_dir/protos"
_module_path = get_path_info(".", "abspath")
_relative_proto_paths = rebase_path(invoker.protos, _module_path)
@@ -147,8 +131,7 @@
}
# For C++ proto files, the generated proto directory is added as an include
- # path for the code. This requires using "all_dependent_configs" to force the
- # include on any code that transitively depends on the generated protos.
+ # path for the code.
_include_root = rebase_path(get_path_info(".", "abspath"), "//")
_include_config_target = "${target_name}_includes"
config(_include_config_target) {
@@ -160,7 +143,7 @@
# Create a library with the generated source files.
pw_source_set(target_name) {
- all_dependent_configs = [ ":$_include_config_target" ]
+ public_configs = [ ":$_include_config_target" ]
deps = [ ":$_gen_target" ]
public_deps = [
"$dir_pw_rpc:server",
@@ -178,9 +161,6 @@
# Args:
# protos: List of input .proto files.
template("_pw_nanopb_proto_library") {
- assert(defined(dir_pw_third_party_nanopb) && dir_pw_third_party_nanopb != "",
- "\$dir_pw_third_party_nanopb must be set to compile nanopb protobufs")
-
_proto_gen_dir = "$root_gen_dir/protos"
_module_path = get_path_info(".", "abspath")
_relative_proto_paths = rebase_path(invoker.protos, _module_path)
@@ -231,8 +211,7 @@
}
# For C++ proto files, the generated proto directory is added as an include
- # path for the code. This requires using "all_dependent_configs" to force the
- # include on any code that transitively depends on the generated protos.
+ # path for the code.
_include_root = rebase_path(get_path_info(".", "abspath"), "//")
_include_config_target = "${target_name}_includes"
config(_include_config_target) {
@@ -247,7 +226,7 @@
# Create a library with the generated source files.
pw_source_set(target_name) {
- all_dependent_configs = [ ":$_include_config_target" ]
+ public_configs = [ ":$_include_config_target" ]
deps = [ ":$_gen_target" ]
public_deps = [ "$dir_pw_third_party/nanopb" ] + invoker.gen_deps
sources = get_target_outputs(":$_gen_target")
@@ -358,29 +337,15 @@
}
# Generates protobuf code from .proto definitions for various languages.
-#
-# The generators to use are defined in the pw_protobuf_GENERATORS build
-# variable. Each listed generator creates a generated code target called
+# For each supported generator, creates a sub-target named:
#
# <target_name>.<generator>
#
-# For example, with the following definitions:
-#
-# pw_protobuf_GENERATORS = [ "pwpb", "py" ]
-#
-# pw_proto_library("my_protos") {
-# sources = [ "foo.proto" ]
-# }
-#
-# Two build targets will be created for the declared "my_protos" target.
-#
-# "my_protos.pwpb" <-- C++ source_set containing generated proto code
-# "my_protos.py" <-- Python module containing generated proto code
-#
# Args:
# sources: List of input .proto files.
# deps: List of other pw_proto_library dependencies.
# inputs: Other files on which the protos depend (e.g. nanopb .options files).
+#
template("pw_proto_library") {
assert(defined(invoker.sources) && invoker.sources != [],
"pw_proto_library requires .proto source files")
@@ -416,89 +381,72 @@
_deps += [ ":$_input_target_name" ]
}
- # If the nanopb_rpc generator is selected, make sure that nanopb is also
- # selected.
- has_nanopb_rpc = pw_protobuf_GENERATORS + [ "nanopb_rpc" ] -
- [ "nanopb_rpc" ] != pw_protobuf_GENERATORS
- if (has_nanopb_rpc) {
- _generators =
- pw_protobuf_GENERATORS + [ "nanopb" ] - [ "nanopb" ] + [ "nanopb" ]
+ _base_target = target_name
+
+ if (defined(invoker.deps)) {
+ _invoker_deps = invoker.deps
} else {
- _generators = pw_protobuf_GENERATORS
+ _invoker_deps = []
}
- foreach(_gen, _generators) {
- _lang_target = "${target_name}.${_gen}"
- _gen_deps = []
+ # Enumerate all of the protobuf generator targets.
- if (_gen == "nanopb_rpc") {
- # Generated RPC code depends on the library's core protos.
- _gen_deps += [ ":${target_name}.nanopb" ]
+ _pw_pwpb_proto_library("${_base_target}.pwpb") {
+ forward_variables_from(invoker, _forwarded_vars)
+ protos = invoker.sources
+ deps = _deps
+ include_file = _include_metadata_file
+ gen_deps = process_file_template(_invoker_deps, "{{source}}.pwpb")
+ protoc_deps = [ "$dir_pw_protobuf/py" ]
+ }
+
+ if (dir_pw_third_party_nanopb != "") {
+ _pw_nanopb_rpc_proto_library("${_base_target}.nanopb_rpc") {
+ forward_variables_from(invoker, _forwarded_vars)
+ protos = invoker.sources
+ deps = _deps
+ include_file = _include_metadata_file
+ gen_deps = process_file_template(_invoker_deps, "{{source}}.nanopb") +
+ [ ":${_base_target}.nanopb" ]
+ protoc_deps = [ "$dir_pw_rpc/py" ]
}
- if (defined(invoker.deps)) {
- _gen_deps += process_file_template(invoker.deps, "{{source}}.${_gen}")
-
- if (_gen == "nanopb_rpc") {
- # RPC dependencies also depend on their core generated protos.
- _gen_deps += process_file_template(invoker.deps, "{{source}}.nanopb")
- }
+ _pw_nanopb_proto_library("${target_name}.nanopb") {
+ forward_variables_from(invoker, _forwarded_vars)
+ protos = invoker.sources
+ deps = _deps
+ include_file = _include_metadata_file
+ gen_deps = process_file_template(_invoker_deps, "{{source}}.nanopb")
+ }
+ } else {
+ pw_error("${_base_target}.nanopb_rpc") {
+ message =
+ "\$dir_pw_third_party_nanopb must be set to generate nanopb RPC code."
}
- if (_gen == "pwpb") {
- _pw_pwpb_proto_library(_lang_target) {
- forward_variables_from(invoker, _forwarded_vars)
- protos = invoker.sources
- deps = _deps
- include_file = _include_metadata_file
- gen_deps = _gen_deps
- protoc_deps = [ "$dir_pw_protobuf/py" ]
- }
- } else if (_gen == "nanopb_rpc") {
- _pw_nanopb_rpc_proto_library(_lang_target) {
- forward_variables_from(invoker, _forwarded_vars)
- protos = invoker.sources
- deps = _deps
- include_file = _include_metadata_file
- gen_deps = _gen_deps
- protoc_deps = [ "$dir_pw_rpc/py" ]
- }
- } else if (_gen == "nanopb") {
- _pw_nanopb_proto_library(_lang_target) {
- forward_variables_from(invoker, _forwarded_vars)
- protos = invoker.sources
- deps = _deps
- include_file = _include_metadata_file
- gen_deps = _gen_deps
- }
- } else if (_gen == "raw_rpc") {
- _pw_raw_rpc_proto_library(_lang_target) {
- forward_variables_from(invoker, _forwarded_vars)
- protos = invoker.sources
- deps = _deps
- include_file = _include_metadata_file
- gen_deps = _gen_deps
- protoc_deps = [ "$dir_pw_rpc/py" ]
- }
- } else if (_gen == "go") {
- _pw_go_proto_library(_lang_target) {
- forward_variables_from(invoker, _forwarded_vars)
- protos = invoker.sources
- deps = _deps
- include_file = _include_metadata_file
- gen_deps = _gen_deps
- }
- } else {
- assert(false,
- string_join(
- " ",
- [
- "pw_proto_library doesn't know how to generate code for",
- "generator '$_gen'. Please add support if you require it.",
- ]))
+ pw_error("${_base_target}.nanopb") {
+ message =
+ "\$dir_pw_third_party_nanopb must be set to compile nanopb protobufs."
}
}
+ _pw_raw_rpc_proto_library("${target_name}.raw_rpc") {
+ forward_variables_from(invoker, _forwarded_vars)
+ protos = invoker.sources
+ deps = _deps
+ include_file = _include_metadata_file
+ gen_deps = []
+ protoc_deps = [ "$dir_pw_rpc/py" ]
+ }
+
+ _pw_go_proto_library("${target_name}.go") {
+ forward_variables_from(invoker, _forwarded_vars)
+ protos = invoker.sources
+ deps = _deps
+ include_file = _include_metadata_file
+ gen_deps = process_file_template(_invoker_deps, "{{source}}.go")
+ }
+
# All supported pw_protobuf generators.
_protobuf_generators = [
"pwpb",
@@ -508,28 +456,6 @@
"go",
]
- # Create stub versions of the proto library for other protobuf generators.
- foreach(_gen, _protobuf_generators - _generators) {
- pw_python_action("${target_name}.${_gen}") {
- forward_variables_from(invoker, _forwarded_vars)
- script = string_join("/",
- [
- dir_pw_protobuf_compiler,
- "py",
- "pw_protobuf_compiler",
- "generator_not_selected.py",
- ])
- args = [
- "--library",
- target_name,
- "--generator",
- _gen,
- ]
- inputs = invoker.sources
- stamp = true
- }
- }
-
# If the user attempts to use the target directly instead of one of the
# generator targets, run a script which prints a nice error message.
pw_python_action(target_name) {
@@ -547,7 +473,7 @@
get_path_info(".", "abspath"),
"--root",
"//",
- ] + pw_protobuf_GENERATORS
+ ] + _protobuf_generators
stamp = true
}
}
diff --git a/pw_protobuf_compiler/py/BUILD.gn b/pw_protobuf_compiler/py/BUILD.gn
index 6927bb1..3c5f663 100644
--- a/pw_protobuf_compiler/py/BUILD.gn
+++ b/pw_protobuf_compiler/py/BUILD.gn
@@ -21,7 +21,6 @@
sources = [
"pw_protobuf_compiler/__init__.py",
"pw_protobuf_compiler/generate_protos.py",
- "pw_protobuf_compiler/generator_not_selected.py",
"pw_protobuf_compiler/proto_target_invalid.py",
"pw_protobuf_compiler/python_protos.py",
]
diff --git a/pw_protobuf_compiler/py/pw_protobuf_compiler/generator_not_selected.py b/pw_protobuf_compiler/py/pw_protobuf_compiler/generator_not_selected.py
deleted file mode 100644
index e089cdb..0000000
--- a/pw_protobuf_compiler/py/pw_protobuf_compiler/generator_not_selected.py
+++ /dev/null
@@ -1,40 +0,0 @@
-# Copyright 2020 The Pigweed Authors
-#
-# Licensed under the Apache License, Version 2.0 (the "License"); you may not
-# use this file except in compliance with the License. You may obtain a copy of
-# the License at
-#
-# https://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-# License for the specific language governing permissions and limitations under
-# the License.
-"""Emits an error when using a protobuf library that is not generated"""
-
-import argparse
-import sys
-
-
-def parse_args():
- parser = argparse.ArgumentParser(description=__doc__)
- parser.add_argument('--library',
- required=True,
- help='The protobuf library being built')
- parser.add_argument('--generator',
- required=True,
- help='The protobuf generator requested')
- return parser.parse_args()
-
-
-def main(library: str, generator: str):
- print(f'ERROR: Attempting to build protobuf library {library}, but the '
- f'{generator} protobuf generator is not in use.')
- print(f'To use {generator} protobufs, list "{generator}" in '
- 'pw_protobuf_GENERATORS.')
- sys.exit(1)
-
-
-if __name__ == '__main__':
- main(**vars(parse_args()))
diff --git a/pw_rpc/nanopb/docs.rst b/pw_rpc/nanopb/docs.rst
index 702ae4a..5e3b0d2 100644
--- a/pw_rpc/nanopb/docs.rst
+++ b/pw_rpc/nanopb/docs.rst
@@ -8,23 +8,12 @@
Usage
=====
-To enable nanopb code generation, add ``nanopb_rpc`` as a generator to your
-Pigweed target's ``pw_protobuf_GENERATORS`` list. Refer to
-:ref:`module-pw_protobuf_compiler` for additional information.
-
-.. code::
-
- # my_target/target_toolchains.gni
-
- defaults = {
- pw_protobuf_GENERATORS = [
- "pwpb",
- "nanopb_rpc", # Enable RPC codegen
- ]
- }
+To enable nanopb code generation, the build argument
+``dir_pw_third_party_nanopb`` must be set to point to a local nanopb
+installation.
Define a ``pw_proto_library`` containing the .proto file defining your service
-(and optionally other related protos), then depend on the ``_nanopb_rpc``
+(and optionally other related protos), then depend on the ``nanopb_rpc``
version of that library in the code implementing the service.
.. code::
@@ -44,7 +33,7 @@
"chat_service.cc",
"chat_service.h",
]
- public_deps = [ ":chat_protos_nanopb_rpc" ]
+ public_deps = [ ":chat_protos.nanopb_rpc" ]
}
A C++ header file is generated for each input .proto file, with the ``.proto``
diff --git a/targets/docs/BUILD.gn b/targets/docs/BUILD.gn
index c82345f..73b1606 100644
--- a/targets/docs/BUILD.gn
+++ b/targets/docs/BUILD.gn
@@ -34,12 +34,6 @@
defaults = {
forward_variables_from(_base_toolchain.defaults, "*")
- _has_nanopb_rpc = pw_protobuf_GENERATORS + [ "nanopb_rpc" ] -
- [ "nanopb_rpc" ] != pw_protobuf_GENERATORS
- if (dir_pw_third_party_nanopb != "" && !_has_nanopb_rpc) {
- pw_protobuf_GENERATORS += [ "nanopb_rpc" ]
- }
-
# This is the docs target.
pw_docgen_BUILD_DOCS = true
}
diff --git a/targets/host/target_toolchains.gni b/targets/host/target_toolchains.gni
index 462cb02..51be296 100644
--- a/targets/host/target_toolchains.gni
+++ b/targets/host/target_toolchains.gni
@@ -38,13 +38,6 @@
# Tokenizer trace time.
pw_trace_tokenizer_time = "$dir_pw_trace_tokenized:host_trace_time"
- # Allow nanopb to be toggled via a build arg on host for easy testing.
- _has_nanopb = pw_protobuf_GENERATORS + [ "nanopb" ] - [ "nanopb" ] !=
- pw_protobuf_GENERATORS
- if (dir_pw_third_party_nanopb != "" && !_has_nanopb) {
- pw_protobuf_GENERATORS += [ "nanopb" ]
- }
-
# Specify builtin GN variables.
current_os = host_os
current_cpu = host_cpu