Skip to content

Commit

Permalink
many: Clean up Python proto imports
Browse files Browse the repository at this point in the history
Fixes Python import issues in the Bazel build caused by import namespace
overlaps between native Python libraries and generated Python protos.

Bug: b/241456982
Change-Id: Ia6310b66aea219226d7eb78ac6571f545d65a6c2
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/193641
Pigweed-Auto-Submit: Armando Montanez <[email protected]>
Reviewed-by: Ted Pudlik <[email protected]>
Commit-Queue: Auto-Submit <[email protected]>
Presubmit-Verified: CQ Bot Account <[email protected]>
  • Loading branch information
armandomontanez authored and CQ Bot Account committed Feb 23, 2024
1 parent 9c1540d commit 510845c
Show file tree
Hide file tree
Showing 12 changed files with 44 additions and 31 deletions.
8 changes: 4 additions & 4 deletions pw_log/py/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
# License for the specific language governing permissions and limitations under
# the License.

load("@rules_python//python:defs.bzl", "py_library", "py_test")

package(default_visibility = ["//visibility:public"])

py_library(
Expand All @@ -22,22 +24,20 @@ py_library(
],
imports = ["."],
deps = [
# TODO: b/241456982 - Add this dep back in
# "//pw_log:log_proto_py_pb2",
"//pw_log:log_proto_py_pb2",
"//pw_log_tokenized/py:pw_log_tokenized",
"//pw_rpc/py:pw_rpc",
"//pw_status/py:pw_status",
"//pw_tokenizer:tokenizer_proto_py_pb2",
"//pw_tokenizer/py:pw_tokenizer",
],
)

# TODO: b/241456982 - Not expected to build yet.
py_test(
name = "log_decoder_test",
srcs = [
"log_decoder_test.py",
],
tags = ["manual"],
deps = [
":pw_log",
],
Expand Down
7 changes: 7 additions & 0 deletions pw_log/py/pw_log/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,10 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations under
# the License.
"""pw_log Python libraries."""

# The generated protos for this module overlap this `__init__.py` file's import
# namespace, so we need to use extend_path() for them to be discoverable.
# Note: this needs to be done in every nested `__init__.py` file as well (if
# any exist).
__path__ = __import__('pkgutil').extend_path(__path__, __name__)
7 changes: 3 additions & 4 deletions pw_log_rpc/py/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
# License for the specific language governing permissions and limitations under
# the License.

load("@rules_python//python:defs.bzl", "py_library", "py_test")

package(default_visibility = ["//visibility:public"])

py_library(
Expand All @@ -22,21 +24,18 @@ py_library(
],
imports = ["."],
deps = [
# TODO: b/241456982 - Add this dep back in
# "//pw_log:log_proto_py_pb2",
"//pw_log:log_proto_py_pb2",
"//pw_log/py:pw_log",
"//pw_rpc/py:pw_rpc",
"//pw_status/py:pw_status",
],
)

# TODO: b/241456982 - Not expected to build yet.
py_test(
name = "rpc_log_stream_test",
srcs = [
"rpc_log_stream_test.py",
],
tags = ["manual"],
deps = [
":pw_log_rpc",
],
Expand Down
2 changes: 2 additions & 0 deletions pw_metric/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ pw_proto_filegroup(
proto_library(
name = "metric_proto",
srcs = [":metric_proto_and_options"],
# TODO: b/326636665 - Stripping the import prefix breaks nanopb compilation.
# strip_import_prefix = "/pw_metric",
)

py_proto_library(
Expand Down
6 changes: 3 additions & 3 deletions pw_metric/py/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ py_library(
],
imports = ["."],
deps = [
"//pw_metric:metric_proto_py_pb2",
"//pw_rpc/py:pw_rpc",
"//pw_tokenizer/py:pw_tokenizer",
# TODO: b/241456982 - Add this dep back in
# "//pw_metric:metric_proto_py_pb2",
],
)

# TODO: b/241456982 - Not expected to build yet.
# TODO: b/326636665 - This test works, but requires strip_import_prefix on
# the proto library, which breaks the nanopb libraries.
py_test(
name = "metric_parser_test",
size = "small",
Expand Down
21 changes: 9 additions & 12 deletions pw_system/py/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,6 @@ load("@rules_python//python:defs.bzl", "py_binary", "py_library")

package(default_visibility = ["//visibility:public"])

# TODO: b/241456982 - The following deps are required to build :pw_system_lib
# deps = [
# "//pw_thread/py:pw_thread",
# "//pw_log:log_proto_py_pb2",
# "//pw_metric:metric_proto_py_pb2",
# "//pw_thread:thread_proto_py_pb2",
# "//pw_thread:thread_snapshot_service_py_pb2",
# "//pw_tokenizer:tokenizer_proto_py_pb2",
# "//pw_unit_test:unit_test_py_pb2",
# "//pw_unit_test/py:pw_unit_test_lib",
# ],
py_library(
name = "pw_system_lib",
srcs = [
Expand All @@ -42,15 +31,23 @@ py_library(
"//pw_console/py:pw_console",
"//pw_file:file_proto_py_pb2",
"//pw_hdlc/py:pw_hdlc",
"//pw_log:log_proto_py_pb2",
"//pw_log/py:pw_log",
"//pw_log_rpc/py:pw_log_rpc",
"//pw_metric:metric_proto_py_pb2", # TODO: b/326636665 - Needs stripped import prefix.
"//pw_metric/py:pw_metric",
"//pw_rpc/py:pw_rpc",
"//pw_thread:thread_proto_py_pb2",
"//pw_thread:thread_snapshot_service_py_pb2",
"//pw_thread/py:pw_thread",
"//pw_tokenizer:tokenizer_proto_py_pb2",
"//pw_tokenizer/py:pw_tokenizer",
"//pw_trace/py:pw_trace",
"//pw_trace_tokenized:proto_py",
"//pw_trace_tokenized:proto_py", # TODO: b/326636665 - Needs stripped import prefix.
"//pw_trace_tokenized/py:pw_trace_tokenized",
"//pw_transfer/py:pw_transfer",
"//pw_unit_test:unit_test_py_pb2",
"//pw_unit_test/py:pw_unit_test",
],
)

Expand Down
11 changes: 4 additions & 7 deletions pw_thread/py/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,25 @@
# License for the specific language governing permissions and limitations under
# the License.

load("@rules_python//python:defs.bzl", "py_library", "py_test")

package(default_visibility = ["//visibility:public"])

# TODO: b/241456982 - Not expected to build. We need a dependency on a
# py_proto_library built from thread_proto, but that in turn depends on
# creating a py_proto_library for tokenizer_proto.
py_library(
name = "pw_thread",
srcs = [
"pw_thread/__init__.py",
"pw_thread/thread_analyzer.py",
],
tags = ["manual"],
imports = ["."],
deps = [
"//pw_symbolizer/py:pw_symbolizer",
"//pw_thread:thread_proto_py_pb2",
"//pw_tokenizer/py:pw_tokenizer",
],
)

# TODO: b/241456982 - Not expected to build. We need a dependency on a
# py_proto_library built from thread_proto, but that in turn depends on
# creating a py_proto_library for tokenizer_proto.
# TODO: b/241307309 - Requires injection of a `llvm-symbolizer` tool to pass.
py_test(
name = "thread_analyzer_test",
srcs = [
Expand Down
1 change: 1 addition & 0 deletions pw_trace/py/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ py_library(
"pw_trace/__init__.py",
"pw_trace/trace.py",
],
imports = ["."],
)
2 changes: 2 additions & 0 deletions pw_trace_tokenized/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ cc_library(
],
)

# TODO: b/326636665 - Needs stripped import prefix, but prefix modification
# doesn't work with nanopb.
proto_library(
name = "protos",
srcs = [
Expand Down
1 change: 1 addition & 0 deletions pw_trace_tokenized/py/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ py_library(
"pw_trace_tokenized/get_trace.py",
"pw_trace_tokenized/trace_tokenized.py",
],
imports = ["."],
deps = [
"//pw_hdlc/py:pw_hdlc",
"//pw_tokenizer/py:pw_tokenizer",
Expand Down
6 changes: 6 additions & 0 deletions pw_transfer/py/pw_transfer/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
# the License.
"""Provides a simple interface for transferring bulk data over pw_rpc."""

# The generated protos for this module overlap this `__init__.py` file's import
# namespace, so we need to use extend_path() for them to be discoverable.
# Note: this needs to be done in every nested `__init__.py` file as well (if
# any exist).
__path__ = __import__('pkgutil').extend_path(__path__, __name__)

from pw_transfer.transfer import (
ProgressCallback,
ProgressStats,
Expand Down
3 changes: 2 additions & 1 deletion pw_unit_test/py/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@ load("@rules_python//python:defs.bzl", "py_library")
package(default_visibility = ["//visibility:public"])

py_library(
name = "pw_unit_test_lib",
name = "pw_unit_test",
srcs = [
"pw_unit_test/__init__.py",
"pw_unit_test/rpc.py",
"pw_unit_test/test_runner.py",
],
imports = ["."],
deps = [
"//pw_cli/py:pw_cli",
"//pw_rpc/py:pw_rpc",
Expand Down

0 comments on commit 510845c

Please sign in to comment.