pw_build: Make GN Python installation more robust
- Since protobuf Python packages are generated and installed from the
build directory now, add dependencies on package installs in the
build. This ensures packages are properly installed before tests or
linting.
- Instantiate all Python actions in the default toolchain. This prevents
duplicate instantiations of Python actions for each toolchain, which
causes flakiness as packages are reinstalled for each toolchain.
- Add python_deps to pw_python_action that Python actions can depend on
Python package installations.
- Add various missing dependencies between Python packages.
Change-Id: I4ec34f06c04014b3552cc1b60aa506ac9a89b329
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/30000
Commit-Queue: Wyatt Hepler <hepler@google.com>
Reviewed-by: Alexei Frolov <frolv@google.com>
Reviewed-by: Rob Mohr <mohrr@google.com>
diff --git a/pw_bloat/bloat.gni b/pw_bloat/bloat.gni
index 9acfd05..a22f0b2 100644
--- a/pw_bloat/bloat.gni
+++ b/pw_bloat/bloat.gni
@@ -161,6 +161,7 @@
pw_doc_sources = rebase_path([ _doc_rst_output ], root_build_dir)
}
script = "$dir_pw_bloat/py/pw_bloat/no_bloaty.py"
+ python_deps = [ "$dir_pw_bloat/py" ]
args = [ rebase_path(_doc_rst_output) ]
outputs = [ _doc_rst_output ]
}
@@ -176,6 +177,7 @@
pw_doc_sources = rebase_path([ _doc_rst_output ], root_build_dir)
}
script = "$dir_pw_bloat/py/pw_bloat/bloat.py"
+ python_deps = [ "$dir_pw_bloat/py" ]
inputs = _bloaty_configs
outputs = [
"$target_gen_dir/${target_name}.txt",
@@ -321,6 +323,7 @@
pw_doc_sources = rebase_path([ _doc_rst_output ], root_build_dir)
}
script = "$dir_pw_bloat/py/pw_bloat/no_toolchains.py"
+ python_deps = [ "$dir_pw_bloat/py" ]
args = [ rebase_path(_doc_rst_output) ]
outputs = [ _doc_rst_output ]
}
diff --git a/pw_bloat/py/BUILD.gn b/pw_bloat/py/BUILD.gn
index 53198ae..c6d5ea3 100644
--- a/pw_bloat/py/BUILD.gn
+++ b/pw_bloat/py/BUILD.gn
@@ -27,4 +27,5 @@
"pw_bloat/no_toolchains.py",
]
pylintrc = "$dir_pigweed/.pylintrc"
+ python_deps = [ "$dir_pw_cli/py" ]
}
diff --git a/pw_bloat/py/setup.py b/pw_bloat/py/setup.py
index 73410da..73c2a7e 100644
--- a/pw_bloat/py/setup.py
+++ b/pw_bloat/py/setup.py
@@ -24,4 +24,5 @@
packages=setuptools.find_packages(),
package_data={'pw_bloat': ['py.typed']},
zip_safe=False,
+ install_requires=['pw_cli'],
)
diff --git a/pw_build/py/BUILD.gn b/pw_build/py/BUILD.gn
index fde1846..7f941d2 100644
--- a/pw_build/py/BUILD.gn
+++ b/pw_build/py/BUILD.gn
@@ -33,5 +33,9 @@
"pw_build/zip_test.py",
]
tests = [ "python_runner_test.py" ]
+ python_deps = [
+ "$dir_pw_cli/py",
+ "$dir_pw_presubmit/py",
+ ]
pylintrc = "$dir_pigweed/.pylintrc"
}
diff --git a/pw_build/py/setup.py b/pw_build/py/setup.py
index c8fc23e..2a5a746 100644
--- a/pw_build/py/setup.py
+++ b/pw_build/py/setup.py
@@ -26,5 +26,7 @@
zip_safe=False,
install_requires=[
'wheel',
+ 'pw_cli',
+ 'pw_presubmit',
],
)
diff --git a/pw_build/python.gni b/pw_build/python.gni
index 7c5a58a..5af65c7 100644
--- a/pw_build/python.gni
+++ b/pw_build/python.gni
@@ -17,6 +17,16 @@
import("$dir_pw_build/input_group.gni")
import("$dir_pw_build/python_action.gni")
+# Python packages provide the following targets as $target_name.$subtarget.
+_python_subtargets = [
+ "tests",
+ "lint",
+ "lint.mypy",
+ "lint.pylint",
+ "install",
+ "wheel",
+]
+
# Defines a Python package. GN Python packages contain several GN targets:
#
# - $name - Provides the Python files in the build, but does not take any
@@ -29,6 +39,9 @@
# - $name.install - Installs the package in a venv.
# - $name.wheel - Builds a Python wheel for the package. (Not implemented.)
#
+# All Python packages are instantiated with the default toolchain, regardless of
+# the current toolchain.
+#
# TODO(pwbug/239): Implement wheel building.
#
# Args:
@@ -115,10 +128,28 @@
}
}
+ _internal_target = "$target_name._internal"
+
+ # Create groups with the public target names ($target_name, $target_name.lint,
+ # $target_name.install, etc.). These are actually wrappers around internal
+ # Python actions instantiated with the default toolchain. This ensures there
+ # is only a single copy of each Python action in the build.
+ #
+ # The $target_name.tests group is created separately below.
+ foreach(subtarget, _python_subtargets - [ "tests" ]) {
+ group("$target_name.$subtarget") {
+ deps = [ ":$_internal_target.$subtarget($default_toolchain)" ]
+ }
+ }
+
+ group("$target_name") {
+ deps = [ ":$_internal_target($default_toolchain)" ]
+ }
+
# Declare the main Python package group. This represents the Python files, but
# does not take any actions. GN targets can depend on the package name to run
# when any files in the package change.
- pw_input_group(target_name) {
+ pw_input_group("$_internal_target") {
inputs = _all_py_files
if (defined(invoker.inputs)) {
inputs += invoker.inputs
@@ -131,12 +162,10 @@
}
}
- _package_target = ":$target_name"
-
if (_is_package) {
# Install this Python package and its dependencies in the current Python
# environment.
- pw_python_action("$target_name.install") {
+ pw_python_action("$_internal_target.install") {
module = "pip"
args = [ "install" ]
@@ -152,7 +181,7 @@
# Parallel pip installations don't work, so serialize pip invocations.
pool = "$dir_pw_build:pip_pool"
- deps = [ _package_target ]
+ deps = [ ":$_internal_target" ]
foreach(dep, _python_deps) {
deps += [ "$dep.install" ]
}
@@ -160,7 +189,7 @@
# TODO(pwbug/239): Add support for building groups of wheels. The code below
# is incomplete and untested.
- pw_python_action("$target_name.wheel") {
+ pw_python_action("$_internal_target.wheel") {
script = "$dir_pw_build/py/pw_build/python_wheels.py"
args = [
@@ -169,18 +198,18 @@
]
args += rebase_path(_all_py_files)
- deps = [ _package_target ]
+ deps = [ ":$_internal_target.install" ]
stamp = true
}
} else {
# If this is not a package, install or build wheels for its deps only.
- group("$target_name.install") {
+ group("$_internal_target.install") {
deps = []
foreach(dep, _python_deps) {
deps += [ "$dep.install" ]
}
}
- group("$target_name.wheel") {
+ group("$_internal_target.wheel") {
deps = []
foreach(dep, _python_deps) {
deps += [ "$dep.wheel" ]
@@ -189,10 +218,10 @@
}
# Define the static analysis targets for this package.
- group("$target_name.lint") {
+ group("$_internal_target.lint") {
deps = [
- "$_package_target.lint.mypy",
- "$_package_target.lint.pylint",
+ ":$_internal_target.lint.mypy",
+ ":$_internal_target.lint.pylint",
]
}
@@ -208,7 +237,7 @@
_lint_directory = rebase_path(".")
}
- pw_python_action("$target_name.lint.mypy") {
+ pw_python_action("$_internal_target.lint.mypy") {
module = "mypy"
args = [
"--pretty",
@@ -233,7 +262,7 @@
directory = _lint_directory
stamp = true
- deps = [ _package_target ]
+ deps = [ ":$_internal_target.install" ]
foreach(dep, _python_deps) {
deps += [ "$dep.lint.mypy" ]
}
@@ -241,7 +270,7 @@
# Create a target to run pylint on each of the Python files in this
# package and its dependencies.
- pw_python_action_foreach("$target_name.lint.pylint") {
+ pw_python_action_foreach("$_internal_target.lint.pylint") {
module = "pylint"
args = [
rebase_path(".") + "/{{source_target_relative}}",
@@ -264,18 +293,18 @@
directory = _lint_directory
stamp = "$target_gen_dir/{{source_target_relative}}.pylint.passed"
- deps = [ _package_target ]
+ deps = [ ":$_internal_target.install" ]
foreach(dep, _python_deps) {
deps += [ "$dep.lint.pylint" ]
}
}
} else {
- pw_input_group("$target_name.lint.mypy") {
+ pw_input_group("$_internal_target.lint.mypy") {
if (defined(invoker.pylintrc)) {
inputs = [ invoker.pylintrc ]
}
}
- pw_input_group("$target_name.lint.pylint") {
+ pw_input_group("$_internal_target.lint.pylint") {
if (defined(invoker.mypy_ini)) {
inputs = [ invoker.mypy_ini ]
}
@@ -287,19 +316,25 @@
foreach(test, _test_sources) {
_test_name = string_replace(test, "/", "_")
- _test_target = "$target_name.tests.$_test_name"
+ _internal_test_target = "$_internal_target.tests.$_test_name"
- pw_python_action(_test_target) {
+ pw_python_action(_internal_test_target) {
script = test
stamp = true
- deps = [ _package_target ]
+ deps = [ ":$_internal_target.install" ]
foreach(dep, _python_deps) {
deps += [ "$dep.tests" ]
}
}
- _test_targets += [ ":$_test_target" ]
+ # Create a public version of each test target, so tests can be executed as
+ # //path/to:package.tests.foo.py.
+ group("$target_name.tests.$_test_name") {
+ deps = [ ":$_internal_test_target" ]
+ }
+
+ _test_targets += [ ":$target_name.tests.$_test_name" ]
}
group("$target_name.tests") {
@@ -307,15 +342,6 @@
}
}
-_python_subtargets = [
- "tests",
- "lint",
- "lint.mypy",
- "lint.pylint",
- "install",
- "wheel",
-]
-
# Declares a group of Python packages or other Python groups. pw_python_groups
# expose the same set of subtargets as pw_python_package (e.g.
# "$group_name.lint" and "$group_name.tests"), but these apply to all packages
diff --git a/pw_build/python_action.gni b/pw_build/python_action.gni
index a420fce..87f4bc2 100644
--- a/pw_build/python_action.gni
+++ b/pw_build/python_action.gni
@@ -50,6 +50,8 @@
# <TARGET_OBJECTS(//some/label:here)> - expands to the
# object files produced by the provided GN target
#
+# python_deps Dependencies on pw_python_package or related Python targets.
+#
template("pw_python_action") {
_script_args = [
# GN root directory relative to the build directory (in which the runner
@@ -142,10 +144,23 @@
_action_type = "action"
}
+ if (defined(invoker.deps)) {
+ _deps = invoker.deps
+ } else {
+ _deps = []
+ }
+
+ if (defined(invoker.python_deps)) {
+ foreach(dep, invoker.python_deps) {
+ _deps += [ get_label_info(dep, "label_no_toolchain") + ".install" ]
+ }
+ }
+
target(_action_type, target_name) {
_ignore_vars = [
"script",
"args",
+ "deps",
"inputs",
"outputs",
]
@@ -155,6 +170,7 @@
args = _script_args
inputs = _inputs
outputs = _outputs
+ deps = _deps
}
}
diff --git a/pw_cpu_exception_cortex_m/py/BUILD.gn b/pw_cpu_exception_cortex_m/py/BUILD.gn
index 8af1eb2..df5dc96 100644
--- a/pw_cpu_exception_cortex_m/py/BUILD.gn
+++ b/pw_cpu_exception_cortex_m/py/BUILD.gn
@@ -24,5 +24,9 @@
"pw_cpu_exception_cortex_m/exception_analyzer.py",
]
tests = [ "exception_analyzer_test.py" ]
+ python_deps = [
+ "$dir_pw_cli/py",
+ "$dir_pw_protobuf_compiler/py",
+ ]
pylintrc = "$dir_pigweed/.pylintrc"
}
diff --git a/pw_cpu_exception_cortex_m/py/setup.py b/pw_cpu_exception_cortex_m/py/setup.py
index e476dee..9cc3e8e 100644
--- a/pw_cpu_exception_cortex_m/py/setup.py
+++ b/pw_cpu_exception_cortex_m/py/setup.py
@@ -24,5 +24,9 @@
packages=setuptools.find_packages(),
package_data={'pw_cpu_exception_cortex_m': ['py.typed']},
zip_safe=False,
- install_requires=['protobuf'],
+ install_requires=[
+ 'protobuf',
+ 'pw_cli',
+ 'pw_protobuf_compiler',
+ ],
)
diff --git a/pw_hdlc/py/BUILD.gn b/pw_hdlc/py/BUILD.gn
index 5532c3e..255c10b 100644
--- a/pw_hdlc/py/BUILD.gn
+++ b/pw_hdlc/py/BUILD.gn
@@ -31,6 +31,7 @@
"encode_test.py",
]
python_deps = [
+ "$dir_pw_build/py",
"$dir_pw_protobuf_compiler/py",
"$dir_pw_rpc/py",
]
diff --git a/pw_hdlc/py/setup.py b/pw_hdlc/py/setup.py
index a62ce75..88db976 100644
--- a/pw_hdlc/py/setup.py
+++ b/pw_hdlc/py/setup.py
@@ -24,6 +24,10 @@
packages=setuptools.find_packages(),
package_data={'pw_hdlc': ['py.typed']},
zip_safe=False,
- install_requires=['ipython'],
+ install_requires=[
+ 'ipython',
+ 'pw_protobuf_compiler',
+ 'pw_rpc',
+ ],
tests_require=['pw_build'],
)
diff --git a/pw_presubmit/py/setup.py b/pw_presubmit/py/setup.py
index a738a74..9832e9c 100644
--- a/pw_presubmit/py/setup.py
+++ b/pw_presubmit/py/setup.py
@@ -25,6 +25,7 @@
'mypy==0.790',
'pylint==2.6.0',
'yapf==0.30.0',
+ 'pw_cli',
'pw_package',
],
packages=setuptools.find_packages(),
diff --git a/pw_protobuf/py/BUILD.gn b/pw_protobuf/py/BUILD.gn
index e977f5d..6d42440 100644
--- a/pw_protobuf/py/BUILD.gn
+++ b/pw_protobuf/py/BUILD.gn
@@ -25,5 +25,9 @@
"pw_protobuf/plugin.py",
"pw_protobuf/proto_tree.py",
]
+ python_deps = [
+ "$dir_pw_cli/py",
+ "$dir_pw_protobuf_compiler/py",
+ ]
pylintrc = "$dir_pigweed/.pylintrc"
}
diff --git a/pw_protobuf/py/setup.py b/pw_protobuf/py/setup.py
index 96a8209..2c8c692 100644
--- a/pw_protobuf/py/setup.py
+++ b/pw_protobuf/py/setup.py
@@ -29,5 +29,7 @@
},
install_requires=[
'protobuf',
+ 'pw_cli',
],
+ tests_require=['pw_protobuf_compiler'],
)
diff --git a/pw_protobuf_compiler/proto.gni b/pw_protobuf_compiler/proto.gni
index 527e25b..94d0c6c 100644
--- a/pw_protobuf_compiler/proto.gni
+++ b/pw_protobuf_compiler/proto.gni
@@ -41,6 +41,11 @@
script =
"$dir_pw_protobuf_compiler/py/pw_protobuf_compiler/generate_protos.py"
+ python_deps = [ "$dir_pw_protobuf_compiler/py" ]
+ if (defined(invoker.python_deps)) {
+ python_deps += invoker.python_deps
+ }
+
deps = [
":${invoker.base_target}._metadata",
":${invoker.base_target}._inputs",
@@ -96,7 +101,7 @@
forward_variables_from(invoker, "*", _forwarded_vars)
language = "pwpb"
plugin = "$dir_pw_protobuf/py/pw_protobuf/plugin.py"
- deps += [ "$dir_pw_protobuf/py" ]
+ python_deps = [ "$dir_pw_protobuf/py" ]
output_extensions = [ ".pwpb.h" ]
}
@@ -121,7 +126,7 @@
forward_variables_from(invoker, "*", _forwarded_vars)
language = "nanopb_rpc"
plugin = "$dir_pw_rpc/py/pw_rpc/plugin_nanopb.py"
- deps += [ "$dir_pw_rpc/py" ]
+ python_deps = [ "$dir_pw_rpc/py" ]
include_paths = [ "$dir_pw_third_party_nanopb/generator/proto" ]
output_extensions = [ ".rpc.pb.h" ]
}
@@ -179,7 +184,7 @@
forward_variables_from(invoker, "*", _forwarded_vars)
language = "raw_rpc"
plugin = "$dir_pw_rpc/py/pw_rpc/plugin_raw.py"
- deps += [ "$dir_pw_rpc/py" ]
+ python_deps = [ "$dir_pw_rpc/py" ]
output_extensions = [ ".raw_rpc.pb.h" ]
}
diff --git a/pw_rpc/BUILD.gn b/pw_rpc/BUILD.gn
index 3569aeb..3e34b38 100644
--- a/pw_rpc/BUILD.gn
+++ b/pw_rpc/BUILD.gn
@@ -16,6 +16,7 @@
import("$dir_pw_bloat/bloat.gni")
import("$dir_pw_build/module_config.gni")
+import("$dir_pw_build/python_action.gni")
import("$dir_pw_build/target_types.gni")
import("$dir_pw_docgen/docs.gni")
import("$dir_pw_protobuf_compiler/proto.gni")
@@ -210,11 +211,14 @@
sources = [ "channel_test.cc" ]
}
-action("generate_ids_test") {
+pw_python_action("generate_ids_test") {
outputs = [ "$target_gen_dir/generated_ids_test.cc" ]
script = "py/ids_test.py"
args = [ "--generate-cc-test" ] + rebase_path(outputs)
- deps = [ "$dir_pw_build/py" ]
+ python_deps = [
+ "$dir_pw_build/py",
+ "py",
+ ]
}
pw_test("ids_test") {
diff --git a/pw_rpc/py/BUILD.gn b/pw_rpc/py/BUILD.gn
index 6f0baa6..802efd2 100644
--- a/pw_rpc/py/BUILD.gn
+++ b/pw_rpc/py/BUILD.gn
@@ -41,6 +41,7 @@
"packets_test.py",
]
python_deps = [
+ "$dir_pw_build/py",
"$dir_pw_protobuf_compiler/py",
"$dir_pw_status/py",
]
diff --git a/pw_trace_tokenized/py/BUILD.gn b/pw_trace_tokenized/py/BUILD.gn
index 2116cc5..00b85f8 100644
--- a/pw_trace_tokenized/py/BUILD.gn
+++ b/pw_trace_tokenized/py/BUILD.gn
@@ -22,5 +22,6 @@
"pw_trace_tokenized/__init__.py",
"pw_trace_tokenized/trace_tokenized.py",
]
+ python_deps = [ "$dir_pw_tokenizer/py" ]
pylintrc = "$dir_pigweed/.pylintrc"
}
diff --git a/pw_trace_tokenized/py/setup.py b/pw_trace_tokenized/py/setup.py
index cea2439..c20e9cf 100644
--- a/pw_trace_tokenized/py/setup.py
+++ b/pw_trace_tokenized/py/setup.py
@@ -24,4 +24,5 @@
packages=setuptools.find_packages(),
package_data={'pw_trace_tokenized': ['py.typed']},
zip_safe=False,
+ install_requires=['pw_tokenizer'],
)
diff --git a/pw_unit_test/py/setup.py b/pw_unit_test/py/setup.py
index ea467d5..0a7db0b 100644
--- a/pw_unit_test/py/setup.py
+++ b/pw_unit_test/py/setup.py
@@ -26,5 +26,6 @@
zip_safe=False,
install_requires=[
'pw_cli',
+ 'pw_rpc',
],
)
diff --git a/pw_unit_test/test.gni b/pw_unit_test/test.gni
index 50d107b..1fdb428 100644
--- a/pw_unit_test/test.gni
+++ b/pw_unit_test/test.gni
@@ -182,6 +182,7 @@
deps = [ ":$_test_target_name" ]
inputs = [ pw_unit_test_AUTOMATIC_RUNNER ]
script = "$dir_pw_unit_test/py/pw_unit_test/test_runner.py"
+ python_deps = [ "$dir_pw_cli/py" ]
args = [
"--runner",
rebase_path(pw_unit_test_AUTOMATIC_RUNNER),
diff --git a/pw_watch/py/BUILD.gn b/pw_watch/py/BUILD.gn
index b6c76f8..a4d4c0e 100644
--- a/pw_watch/py/BUILD.gn
+++ b/pw_watch/py/BUILD.gn
@@ -25,4 +25,5 @@
"pw_watch/watch_test.py",
]
pylintrc = "$dir_pigweed/.pylintrc"
+ python_deps = [ "$dir_pw_cli/py" ]
}
diff --git a/pw_watch/py/setup.py b/pw_watch/py/setup.py
index 7958eab..086747d 100644
--- a/pw_watch/py/setup.py
+++ b/pw_watch/py/setup.py
@@ -25,6 +25,7 @@
package_data={'pw_watch': ['py.typed']},
zip_safe=False,
install_requires=[
+ 'pw_cli',
'watchdog',
],
)
diff --git a/targets/stm32f429i-disc1/py/BUILD.gn b/targets/stm32f429i-disc1/py/BUILD.gn
index d4b229e..85dd255 100644
--- a/targets/stm32f429i-disc1/py/BUILD.gn
+++ b/targets/stm32f429i-disc1/py/BUILD.gn
@@ -26,4 +26,5 @@
"stm32f429i_disc1_utils/unit_test_server.py",
]
pylintrc = "$dir_pigweed/.pylintrc"
+ python_deps = [ "$dir_pw_cli/py" ]
}
diff --git a/targets/stm32f429i-disc1/py/setup.py b/targets/stm32f429i-disc1/py/setup.py
index 309ff7e..28372f2 100644
--- a/targets/stm32f429i-disc1/py/setup.py
+++ b/targets/stm32f429i-disc1/py/setup.py
@@ -37,5 +37,9 @@
' stm32f429i_disc1_utils.unit_test_client:main',
]
},
- install_requires=['pyserial', 'coloredlogs'],
+ install_requires=[
+ 'pyserial',
+ 'coloredlogs',
+ 'pw_cli',
+ ],
)
