pw_protobuf_compiler: Support standalone external protos
Projects may want to use externally defined proto files that are not
organized for Python packaging as required by pw_proto_library. This
change makes it possible to support standalone, externally defined
protobufs, such as nanopb.proto.
Change-Id: I4ae053a950c664878150d911cea9e9de031bb20d
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/31800
Reviewed-by: Alexei Frolov <frolv@google.com>
diff --git a/pw_protobuf_compiler/BUILD.gn b/pw_protobuf_compiler/BUILD.gn
index e974422..c611c6b 100644
--- a/pw_protobuf_compiler/BUILD.gn
+++ b/pw_protobuf_compiler/BUILD.gn
@@ -17,6 +17,7 @@
import("$dir_pw_build/python.gni")
import("$dir_pw_docgen/docs.gni")
import("$dir_pw_protobuf_compiler/proto.gni")
+import("$dir_pw_third_party/nanopb/nanopb.gni")
import("$dir_pw_unit_test/test.gni")
pw_doc_group("docs") {
@@ -35,6 +36,10 @@
pw_proto_library("nanopb_test_protos") {
sources = [ "pw_protobuf_compiler_nanopb_protos/nanopb_test.proto" ]
+
+ if (dir_pw_third_party_nanopb != "") {
+ deps = [ "$dir_pw_third_party/nanopb:proto" ]
+ }
}
pw_proto_library("test_protos") {
diff --git a/pw_protobuf_compiler/docs.rst b/pw_protobuf_compiler/docs.rst
index 1c4c1d7..30bb37a 100644
--- a/pw_protobuf_compiler/docs.rst
+++ b/pw_protobuf_compiler/docs.rst
@@ -46,7 +46,7 @@
.. code-block::
pw_proto_library("test_protos") {
- sources = [ "test.proto" ]
+ sources = [ "my_test_protos/test.proto" ]
}
``test_protos.pwpb`` compiles code for pw_protobuf, and ``test_protos.nanopb``
@@ -68,15 +68,13 @@
pw_proto_library("my_protos") {
sources = [
- "foo.proto",
- "bar.proto",
+ "my_protos/foo.proto",
+ "my_protos/bar.proto",
]
}
pw_proto_library("my_other_protos") {
- sources = [
- "baz.proto", # imports foo.proto
- ]
+ sources = [ "my_other_protos/baz.proto" ] # imports foo.proto
# Proto libraries depend on other proto libraries directly.
deps = [ ":my_protos" ]
@@ -92,3 +90,22 @@
# When depending on protos in a source_set, specify the generator suffix.
deps = [ ":my_other_protos.pwpb" ]
}
+
+Proto file structure
+--------------------
+Protobuf source files must be nested under another directory when they are
+listed in sources. This ensures that they can be packaged properly in Python.
+The first directory is used as the Python package name.
+
+The requirements for proto file structure in the source tree will be relaxed in
+future updates.
+
+Working with externally defined protos
+--------------------------------------
+``pw_proto_library`` targets may be used to build ``.proto`` sources from
+existing projects. In these cases, it may be necessary to supply the
+``include_path`` argument, which specifies the protobuf include path to use for
+``protoc``. If only a single external protobuf is being compiled, the
+requirement that the protobuf be nested under a directory is waived. This
+exception should only be used when absolutely necessary -- for example, to
+support proto files that includes ``import "nanopb.proto"`` in them.
diff --git a/pw_protobuf_compiler/proto.cmake b/pw_protobuf_compiler/proto.cmake
index bd8584e..73b06b6 100644
--- a/pw_protobuf_compiler/proto.cmake
+++ b/pw_protobuf_compiler/proto.cmake
@@ -90,7 +90,7 @@
"${script}"
--language "${LANGUAGE}"
--plugin-path "${PLUGIN}"
- --module-path "${CMAKE_CURRENT_SOURCE_DIR}"
+ --include-path "${CMAKE_CURRENT_SOURCE_DIR}"
--include-file "${INCLUDE_FILE}"
--out-dir "${OUT_DIR}"
${ARGN}
@@ -168,7 +168,6 @@
"${OUT_DIR}"
"${SOURCES}"
"${DEPS}"
- --include-paths "${nanopb_dir}/generator/proto"
)
# Create the library with the generated source files.
diff --git a/pw_protobuf_compiler/proto.gni b/pw_protobuf_compiler/proto.gni
index 1478a64..ce297c6 100644
--- a/pw_protobuf_compiler/proto.gni
+++ b/pw_protobuf_compiler/proto.gni
@@ -54,8 +54,8 @@
args = [
"--language",
invoker.language,
- "--module-path",
- rebase_path("."),
+ "--include-path",
+ rebase_path(invoker.include_path),
"--include-file",
_output[0],
"--out-dir",
@@ -69,17 +69,11 @@
args += [ "--plugin-path=" + rebase_path(invoker.plugin) ]
}
- if (defined(invoker.include_paths)) {
- args += [
- "--include-paths",
- string_join(";", rebase_path(invoker.include_paths)),
- ]
- }
-
outputs = []
foreach(extension, invoker.output_extensions) {
foreach(proto,
- rebase_path(invoker.sources, get_path_info(".", "abspath"))) {
+ rebase_path(invoker.sources,
+ get_path_info(invoker.include_path, "abspath"))) {
_output = string_replace(proto, ".proto", extension)
outputs += [ "${invoker.gen_dir}/$_output" ]
}
@@ -127,7 +121,6 @@
language = "nanopb_rpc"
plugin = "$dir_pw_rpc/py/pw_rpc/plugin_nanopb.py"
python_deps = [ "$dir_pw_rpc/py" ]
- include_paths = [ "$dir_pw_third_party_nanopb/generator/proto" ]
output_extensions = [ ".rpc.pb.h" ]
}
@@ -156,7 +149,6 @@
forward_variables_from(invoker, "*", _forwarded_vars)
language = "nanopb"
plugin = "$dir_pw_third_party_nanopb/generator/protoc-gen-nanopb"
- include_paths = [ "$dir_pw_third_party_nanopb/generator/proto" ]
output_extensions = [
".pb.h",
".pb.c",
@@ -230,12 +222,24 @@
# file. Use pw_proto_library instead.
template("_pw_python_proto_library") {
_target = target_name
- _package_dir = invoker.package_dir
+
+ # For standalone protos (e.g. `import "nanopb.proto"`), nest the proto file in
+ # a directory with the same name for Python packaging purposes.
+ if (invoker.standalone_proto) {
+ _source_name = get_path_info(invoker.sources, "name")
+ _proto_gen_dir = "${invoker.gen_dir}/${_source_name[0]}_pb2"
+ } else {
+ _proto_gen_dir = invoker.gen_dir
+ }
_pw_invoke_protoc(target_name) {
forward_variables_from(invoker, "*", _forwarded_vars)
+ gen_dir = _proto_gen_dir
language = "python"
- output_extensions = [ "_pb2.py" ]
+ output_extensions = [
+ "_pb2.py",
+ "_pb2.pyi",
+ ]
deps += [ "$dir_pw_protobuf_compiler:protobuf_requirements.install" ]
}
@@ -249,9 +253,13 @@
"--setup",
rebase_path(_setup_py),
"--package",
- _package_dir,
+ invoker._package_dir,
] + rebase_path(_generated_files, invoker.gen_dir)
+ if (invoker.standalone_proto) {
+ args += [ "--standalone" ]
+ }
+
public_deps = [ ":$_target._gen" ]
outputs = [ _setup_py ]
}
@@ -276,18 +284,52 @@
# 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).
+# include_path: Sets the proto include path. The default is ".". It is not
+# recommended to set this, unless pulling in an externally defined proto.
#
template("pw_proto_library") {
assert(defined(invoker.sources) && invoker.sources != [],
"pw_proto_library requires .proto source files")
+ _common = {
+ base_target = target_name
+ gen_dir = "$target_gen_dir/$target_name"
+ sources = invoker.sources
+
+ if (defined(invoker.include_path)) {
+ include_path = invoker.include_path
+ } else {
+ include_path = "."
+ }
+ }
+
+ _rebased_sources = rebase_path(invoker.sources, _common.include_path)
+
+ # The pw_proto_library GN target requires protos to be nested under the
+ # include directory unless three conditions are met:
+ #
+ # 1. There is only one .proto file.
+ # 2. The file is in a different directory (an externally defined proto).
+ # 3. The include path is the .proto's include directory. Since there are no
+ # nested directories, the proto cannot be packaged properly in Python.
+ #
+ # When these conditions are met, the proto library is allowed, even though the
+ # proto file is not nested. The Python package for it uses the Python module's
+ # name. This is a special exception to the typical pattern to allow for
+ # working with single, external, standalone protobuf not set up for Python
+ # packaging (such as nanopb.proto).
+ _standalone_proto =
+ _rebased_sources == [ _rebased_sources[0] ] &&
+ _common.include_path != "." &&
+ string_split(_rebased_sources[0], "/") == [ _rebased_sources[0] ]
+
_package_dir = ""
- foreach(_rebased_proto_path, rebase_path(invoker.sources, ".")) {
+ foreach(_rebased_source, _rebased_sources) {
_path_components = []
- _path_components = string_split(_rebased_proto_path, "/")
+ _path_components = string_split(_rebased_source, "/")
- assert(_path_components != [ _rebased_proto_path ] &&
+ assert((_standalone_proto || _path_components != [ _rebased_source ]) &&
_path_components[0] != "..",
"Sources in a pw_proto_library must live in subdirectories " +
"of where it is defined")
@@ -308,12 +350,6 @@
visibility = []
}
- _common = {
- base_target = target_name
- gen_dir = "$target_gen_dir/$target_name"
- sources = invoker.sources
- }
-
if (defined(invoker.deps)) {
_deps = invoker.deps
} else {
@@ -331,7 +367,7 @@
# Indicate this library's base directory for its dependents.
metadata = {
- protoc_includes = [ rebase_path(".") ]
+ protoc_includes = [ rebase_path(_common.include_path) ]
}
}
@@ -368,10 +404,19 @@
deps = process_file_template(_deps, "{{source}}.nanopb_rpc")
}
- _pw_nanopb_proto_library("$target_name.nanopb") {
- forward_variables_from(invoker, _forwarded_vars)
- forward_variables_from(_common, "*")
- deps = process_file_template(_deps, "{{source}}.nanopb")
+ # When compiling with the Nanopb plugin, the nanopb.proto file is already
+ # compiled internally, so skip recompiling it here.
+ if (invoker.sources ==
+ [ "$dir_pw_third_party_nanopb/generator/proto/nanopb.proto" ]) {
+ pw_input_group("$target_name.nanopb") {
+ sources = invoker.sources
+ }
+ } else {
+ _pw_nanopb_proto_library("$target_name.nanopb") {
+ forward_variables_from(invoker, _forwarded_vars)
+ forward_variables_from(_common, "*")
+ deps = process_file_template(_deps, "{{source}}.nanopb")
+ }
}
} else {
pw_error("$target_name.nanopb_rpc") {
@@ -395,6 +440,7 @@
sources = invoker.sources
deps = process_file_template(_deps, "{{source}}.go")
base_target = _common.base_target
+ include_path = _common.include_path
}
_pw_python_proto_library("$target_name.python") {
@@ -402,7 +448,7 @@
forward_variables_from(_common, "*")
deps = process_file_template(_deps, "{{source}}.python")
base_target = _common.base_target
- package_dir = _package_dir
+ standalone_proto = _standalone_proto
}
# All supported pw_protobuf generators.
diff --git a/pw_protobuf_compiler/py/BUILD.gn b/pw_protobuf_compiler/py/BUILD.gn
index f2231e0..1d5a10b 100644
--- a/pw_protobuf_compiler/py/BUILD.gn
+++ b/pw_protobuf_compiler/py/BUILD.gn
@@ -15,6 +15,7 @@
import("//build_overrides/pigweed.gni")
import("$dir_pw_build/python.gni")
+import("$dir_pw_third_party/nanopb/nanopb.gni")
pw_python_package("py") {
setup = [ "setup.py" ]
@@ -32,4 +33,10 @@
python_deps = [ "$dir_pw_cli/py" ]
python_test_deps = [ "..:test_protos.python" ]
pylintrc = "$dir_pigweed/.pylintrc"
+
+ # If Nanopb is available, test protos that import nanopb.proto.
+ if (dir_pw_third_party_nanopb != "") {
+ python_test_deps += [ "..:nanopb_test_protos.python" ]
+ tests += [ "compiled_nanopb_protos_test.py" ]
+ }
}
diff --git a/pw_protobuf_compiler/py/compiled_nanopb_protos_test.py b/pw_protobuf_compiler/py/compiled_nanopb_protos_test.py
new file mode 100755
index 0000000..d384063
--- /dev/null
+++ b/pw_protobuf_compiler/py/compiled_nanopb_protos_test.py
@@ -0,0 +1,29 @@
+#!/usr/bin/env python3
+# 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.
+"""Tests compiling and importing Python protos that import Nanopb."""
+
+import unittest
+
+from pw_protobuf_compiler_nanopb_protos import nanopb_test_pb2
+
+
+class TestCompileAndImport(unittest.TestCase):
+ def test_access_compiled_protobufs(self):
+ message = nanopb_test_pb2.Point(name='hello world')
+ self.assertEqual(message.name, 'hello world')
+
+
+if __name__ == '__main__':
+ unittest.main()
diff --git a/pw_protobuf_compiler/py/pw_protobuf_compiler/generate_protos.py b/pw_protobuf_compiler/py/pw_protobuf_compiler/generate_protos.py
index 3d2932b..5db2f2c 100644
--- a/pw_protobuf_compiler/py/pw_protobuf_compiler/generate_protos.py
+++ b/pw_protobuf_compiler/py/pw_protobuf_compiler/generate_protos.py
@@ -50,13 +50,9 @@
parser.add_argument('--plugin-path',
type=Path,
help='Path to the protoc plugin')
- parser.add_argument('--module-path',
+ parser.add_argument('--include-path',
required=True,
- help='Path to the module containing the .proto files')
- parser.add_argument('--include-paths',
- default=[],
- type=lambda arg: arg.split(';'),
- help='protoc include paths')
+ help='Include path for proto compilation')
parser.add_argument('--include-file',
type=argparse.FileType('r'),
help='File containing additional protoc include paths')
@@ -94,7 +90,7 @@
f'protoc-gen-nanopb={args.plugin_path}',
# nanopb_opt provides the flags to use for nanopb_out. Windows doesn't
# like when you merge the two using the `flag,...:out` syntax.
- f'--nanopb_opt=-I{args.module_path}',
+ f'--nanopb_opt=-I{args.include_path}',
f'--nanopb_out={args.out_dir}',
)
@@ -155,8 +151,7 @@
os.makedirs(args.out_dir, exist_ok=True)
- include_paths = [f'-I{path}' for path in args.include_paths]
- include_paths += [f'-I{line.strip()}' for line in args.include_file]
+ include_paths = [f'-I{line.strip()}' for line in args.include_file]
wrapper_script: Optional[Path] = None
@@ -178,7 +173,7 @@
process = subprocess.run(
[
'protoc',
- f'-I{args.module_path}',
+ f'-I{args.include_path}',
*include_paths,
*DEFAULT_PROTOC_ARGS[args.language](args),
*args.protos,
diff --git a/pw_protobuf_compiler/py/pw_protobuf_compiler/generate_python_package.py b/pw_protobuf_compiler/py/pw_protobuf_compiler/generate_python_package.py
index 4c3e865..b79d770 100644
--- a/pw_protobuf_compiler/py/pw_protobuf_compiler/generate_python_package.py
+++ b/pw_protobuf_compiler/py/pw_protobuf_compiler/generate_python_package.py
@@ -54,15 +54,21 @@
required=True,
type=Path,
help='Path to setup.py file')
+ parser.add_argument('--standalone',
+ action='store_true',
+ help='The package is a standalone external proto')
parser.add_argument('sources',
type=Path,
nargs='+',
- help='Relative paths to sources in the package')
+ help='Relative paths to the .py and .pyi files')
return parser.parse_args()
-def main(package: str, setup: Path, sources: List[Path]) -> int:
+def main(package: str, setup: Path, standalone: bool,
+ sources: List[Path]) -> int:
"""Generates __init__.py and py.typed files and a setup.py."""
+ assert not standalone or len(sources) == 2
+
base = setup.parent.resolve()
base.mkdir(exist_ok=True)
@@ -77,24 +83,27 @@
# Create __init__.py and py.typed files for each subdirectory.
for pkg in subpackages:
pkg.mkdir(exist_ok=True, parents=True)
- pkg.joinpath('__init__.py').touch()
+ pkg.joinpath('__init__.py').write_text('')
- package_name = '.'.join(pkg.relative_to(base).as_posix().split('/'))
+ package_name = pkg.relative_to(base).as_posix().replace('/', '.')
pkg.joinpath('py.typed').touch()
pkg_data[package_name].append('py.typed')
- # Add the .pyi for each source file.
- for source in sources:
- pkg = base / source.parent
- package_name = '.'.join(pkg.relative_to(base).as_posix().split('/'))
+ # Add the Mypy stub (.pyi) for each source file.
+ for mypy_stub in (s for s in sources if s.suffix == '.pyi'):
+ pkg = base / mypy_stub.parent
+ package_name = pkg.relative_to(base).as_posix().replace('/', '.')
+ pkg_data[package_name].append(mypy_stub.name)
- path = base.joinpath(source).relative_to(pkg).with_suffix('.pyi')
- pkg_data[package_name].append(str(path))
+ if standalone:
+ pkg.joinpath('__init__.py').write_text(
+ f'from {mypy_stub.stem}.{mypy_stub.stem} import *\n')
setup.write_text(
_SETUP_TEMPLATE.format(name=package,
packages=list(pkg_data),
package_data=dict(pkg_data)))
+
return 0
diff --git a/third_party/nanopb/BUILD.gn b/third_party/nanopb/BUILD.gn
index 1927ab1..695ce18 100644
--- a/third_party/nanopb/BUILD.gn
+++ b/third_party/nanopb/BUILD.gn
@@ -15,6 +15,7 @@
import("//build_overrides/pigweed.gni")
import("$dir_pw_build/target_types.gni")
+import("$dir_pw_protobuf_compiler/proto.gni")
import("nanopb.gni")
# This file defines a GN source_set for an external installation of nanopb.
@@ -40,6 +41,11 @@
"$dir_pw_third_party_nanopb/pb_encode.c",
]
}
+
+ pw_proto_library("proto") {
+ include_path = "$dir_pw_third_party_nanopb/generator/proto"
+ sources = [ "$dir_pw_third_party_nanopb/generator/proto/nanopb.proto" ]
+ }
} else {
group("nanopb") {
}