commit 446f24c4074ba733b3cc9505a3dd59b51e67d417
author Wyatt Hepler <hepler@google.com> Wed Mar 24 09:29:35 2021 -0700
committer CQ Bot Account <pigweed-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Mar 25 22:29:55 2021 +0000
tree 65707b248b7ff71cac941d500db3a8a713527d8a
parent 7a3c6525a6f231b1984816cd1b37af58b2552424

pw_build: Dependency fixes

- Handle public_deps as well as deps in pw_mirror_tree.
- Have the Python .install target avoid unnecessary reinstalls, but
  have dependents re-run when any files change. This is done by
  splitting the installs into a separate target and having .install
  depend on it and the target that represents the source files.

Change-Id: Ie41de8cfdb84fff691e183a346465099d98c609f
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/39202
Commit-Queue: Wyatt Hepler <hepler@google.com>
Reviewed-by: Armando Montanez <amontanez@google.com>

pw_build/mirror_tree.gni

diff --git a/pw_build/mirror_tree.gni b/pw_build/mirror_tree.gni
index dc27b66..5e78de8 100644
--- a/pw_build/mirror_tree.gni
+++ b/pw_build/mirror_tree.gni
@@ -57,15 +57,20 @@
     _deps += invoker.deps
   }
 
-  if (defined(invoker.path_data_keys)) {
-    assert(_deps != [],
-           "'path_data_keys' requires at least one dependency in 'deps'")
+  _public_deps = []
+  if (defined(invoker.public_deps)) {
+    _public_deps += invoker.public_deps
+  }
 
+  if (defined(invoker.path_data_keys)) {
     generated_file("$target_name._path_list") {
       data_keys = invoker.path_data_keys
       rebase = root_build_dir
-      deps = _deps
       outputs = [ "$target_gen_dir/$target_name.txt" ]
+      deps = _deps + _public_deps
+
+      assert(deps != [],
+             "'path_data_keys' requires at least one dependency in 'deps'")
     }
 
     _deps += [ ":$target_name._path_list" ]
@@ -93,13 +98,15 @@
     }
 
     deps = _deps
+    public_deps = _public_deps
 
     _ignore_args = [
       "script",
       "args",
       "outputs",
-      "deps",
       "directory",
+      "deps",
+      "public_deps",
     ]
     forward_variables_from(invoker, "*", _ignore_args)
   }

pw_build/python.gni

diff --git a/pw_build/python.gni b/pw_build/python.gni
index bdde00f..550f592 100644
--- a/pw_build/python.gni
+++ b/pw_build/python.gni
@@ -26,6 +26,10 @@
   "lint.pylint",
   "install",
   "wheel",
+
+  # Internal targets that directly depend on one another.
+  "_run_pip_install",
+  "_build_wheel",
 ]
 
 # Internal template that runs Mypy.
@@ -398,8 +402,8 @@
 
     if (_is_package) {
       # Install this Python package and its dependencies in the current Python
-      # environment.
-      pw_python_action("$target_name.install") {
+      # environment using pip.
+      pw_python_action("$target_name._run_pip_install") {
         module = "pip"
         public_deps = []
 
@@ -428,13 +432,13 @@
           # formatted as "//path/to:target(toolchain)", so we can't just append
           # ".subtarget". Instead, we replace the opening parenthesis of the
           # toolchain with ".suffix(".
-          public_deps += [ string_replace(dep, "(", ".install(") ]
+          public_deps += [ string_replace(dep, "(", "._run_pip_install(") ]
         }
       }
 
       # Builds a Python wheel for this package. Records the output directory
       # in the pw_python_package_wheels metadata key.
-      pw_python_action("$target_name.wheel") {
+      pw_python_action("$target_name._build_wheel") {
         metadata = {
           pw_python_package_wheels = [ "$target_out_dir/$target_name" ]
         }
@@ -456,27 +460,49 @@
         stamp = true
       }
     } else {
-      # If this is not a package, install or build wheels for its deps only.
-      group("$target_name.install") {
-        deps = []
-        foreach(dep, _python_deps) {
-          deps += [ string_replace(dep, "(", ".install(") ]
-        }
+      # Stubs for non-package targets.
+      group("$target_name._run_pip_install") {
       }
-      group("$target_name.wheel") {
-        deps = []
-        foreach(dep, _python_deps) {
-          deps += [ string_replace(dep, "(", ".wheel(") ]
-        }
+      group("$target_name._build_wheel") {
+      }
+    }
+
+    # Create the .install and .wheel targets. To limit unnecessary pip
+    # executions, non-generated packages are only reinstalled when their
+    # setup.py changes. However, targets that depend on the .install subtarget
+    # re-run whenever any source files change.
+    #
+    # These targets just represent the source files if this isn't a package.
+    group("$target_name.install") {
+      public_deps = [ ":${invoker.target_name}" ]
+
+      if (_is_package) {
+        public_deps += [ ":${invoker.target_name}._run_pip_install" ]
+      }
+
+      foreach(dep, _python_deps) {
+        public_deps += [ string_replace(dep, "(", ".install(") ]
+      }
+    }
+
+    group("$target_name.wheel") {
+      public_deps = [ ":${invoker.target_name}.install" ]
+
+      if (_is_package) {
+        public_deps += [ ":${invoker.target_name}._build_wheel" ]
+      }
+
+      foreach(dep, _python_deps) {
+        public_deps += [ string_replace(dep, "(", ".wheel(") ]
       }
     }
 
     # Define the static analysis targets for this package.
     group("$target_name.lint") {
-      deps = [
-        ":${invoker.target_name}.lint.mypy",
-        ":${invoker.target_name}.lint.pylint",
-      ]
+      deps = []
+      foreach(_tool, _supported_static_analysis_tools) {
+        deps += [ ":${invoker.target_name}.lint.$_tool" ]
+      }
     }
 
     if (_static_analysis != [] || _test_sources != []) {

pw_build/python_action.gni

diff --git a/pw_build/python_action.gni b/pw_build/python_action.gni
index 87f4bc2..05b1e90 100644
--- a/pw_build/python_action.gni
+++ b/pw_build/python_action.gni
@@ -152,7 +152,8 @@
 
   if (defined(invoker.python_deps)) {
     foreach(dep, invoker.python_deps) {
-      _deps += [ get_label_info(dep, "label_no_toolchain") + ".install" ]
+      _deps += [ get_label_info(dep, "label_no_toolchain") + ".install(" +
+                 get_label_info(dep, "toolchain") + ")" ]
     }
   }