Move codegen.py invoke logic into a gni template (#23154)

* Move codegen logic into a standalone gni template

* Remove extra hadcoded path in codegen.gni

* Remove extra hadcoded path in codegen.gni

* Use common codegen instruction for bridge as well

* Make sure that the codegen include is a public target, so that includes work for generated code

* Restyle

* Enforce that outputs in the codegen are known rather than dynamic

* Apply some code review comments

* rephrased a comment based on code review

* Restyle

* Stop using exec_script in gn (faster gn)

* Fix path rebasing to get valid paths

* Restyle

* make sure that generated expected outputs is one of the expected input files for the codegen script

* Add documentation on how to get the list of expected files

* Add missing init call in the test storage impl

build/chip/chip_codegen.gni

@@ -0,0 +1,95 @@
+# Copyright (c) 2022 Project CHIP 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
+#
+# http://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.
+
+import("//build_overrides/build.gni")
+import("//build_overrides/chip.gni")
+import("//build_overrides/pigweed.gni")
+
+import("$dir_pw_build/python.gni")
+
+# Defines a target that runs code generation based on
+# scripts/codegen.py
+#
+# Arguments:
+#   input
+#     The ".matter" file to use to start the code generation
+#
+#   generator
+#     Name of the generator to use (e.g. java, cpp-app)
+#
+#   outputs
+#     Explicit names of the expected outputs. Enforced to validate that
+#     expected outputs are generated when processing input files.
+#
+# NOTE: content of "outputs" is verified to match the output of codegen.py
+#       exactly. It is not inferred on purpose, to make build-rules explicit
+#       and verifiable (even though codege.py can at runtime report its outputs)
+#
+#       To find the list of generated files, you can run codegen.py with the
+#       "--name-only" argument
+#
+# Example usage:
+#
+#  chip_codegen("java-jni-generate") {
+#    input = "controller-clusters.matter"
+#    generator = "java"
+#
+#    outputs = [
+#       "jni/IdentifyClient-ReadImpl.cpp",
+#       "jni/IdentifyClient-InvokeSubscribeImpl.cpp",
+#       # ... more to follow
+#    ]
+#  }
+#
+template("chip_codegen") {
+  _name = target_name
+  _generator = invoker.generator
+
+  config("${_name}_config") {
+    include_dirs = [ target_gen_dir ]
+  }
+
+  pw_python_action(_name) {
+    script = "${chip_root}/scripts/codegen.py"
+
+    _idl_file = invoker.input
+    _expected_outputs = "${target_gen_dir}/${_name}.expected.outputs"
+
+    write_file(_expected_outputs, invoker.outputs, "list lines")
+
+    args = [
+      "--generator",
+      _generator,
+      "--output-dir",
+      rebase_path(target_gen_dir, root_build_dir),
+      "--expected-outputs",
+      rebase_path(_expected_outputs, root_build_dir),
+      rebase_path(_idl_file, root_build_dir),
+    ]
+
+    deps = [ "${chip_root}/scripts/idl" ]
+    public_configs = [ ":${_name}_config" ]
+
+    inputs = [
+      _idl_file,
+      _expected_outputs,
+    ]
+    sources = [ _idl_file ]
+
+    outputs = []
+    foreach(name, invoker.outputs) {
+      outputs += [ "${target_gen_dir}/${name}" ]
+    }
+  }
+}

examples/dynamic-bridge-app/linux/BUILD.gn

@@ -15,6 +15,7 @@
 import("//build_overrides/build.gni")
 import("//build_overrides/chip.gni")
 
+import("${chip_root}/build/chip/chip_codegen.gni")
 import("${chip_root}/build/chip/tools.gni")
 import("${chip_root}/src/app/common_flags.gni")
 
@@ -27,24 +28,19 @@ if (chip_enable_pw_rpc) {
 
 assert(chip_build_tools)
 
-action("chip-bridge-codegen") {
-  script = "${chip_root}/scripts/codegen.py"
-  sources =
-      [ "${chip_root}/examples/bridge-app/bridge-common/bridge-app.matter" ]
-
-  # Also several other files, but this is sufficient for dependency purposes.
-  outputs = [ "$target_gen_dir/bridge/BridgeClustersImpl.h" ]
-
-  args = [
-    "--generator",
-    "bridge",
-    "--output-dir",
-    rebase_path(target_gen_dir, root_build_dir),
-    rebase_path(
-        "${chip_root}/examples/dynamic-bridge-app/bridge-common/bridge-app.matter",
-        root_build_dir),
+chip_codegen("chip-bridge-codegen") {
+  input = "${chip_root}/examples/bridge-app/bridge-common/bridge-app.matter"
+  generator = "bridge"
+
+  outputs = [
+    "bridge/OnOff.h",
+    "bridge/LevelControl.h",
+    "bridge/Descriptor.h",
+    "bridge/Switch.h",
+    "bridge/TemperatureMeasurement.h",
+    "bridge/BridgeClustersImpl.h",
+    "bridge/BridgeGlobalStructs.h",
   ]
-  deps = [ "${chip_root}/scripts/idl" ]
 }
 
 executable("dynamic-chip-bridge-app") {

scripts/codegen.py

@@ -17,12 +17,12 @@
 import logging
 import coloredlogs
 import enum
+import sys
 
 try:
     from idl.matter_idl_parser import CreateParser
 except:
     import os
-    import sys
     sys.path.append(os.path.abspath(os.path.dirname(__file__)))
     from idl.matter_idl_parser import CreateParser
 
@@ -54,6 +54,9 @@ class ListGeneratedFilesStorage(GeneratorStorage):
     A storage that prints out file names that would have content in them.
     """
 
+    def __init__(self):
+        super().__init__()
+
     def get_existing_data(self, relative_path: str):
         return None  # stdout has no pre-existing data
 
@@ -102,10 +105,15 @@ def write_new_data(self, relative_path: str, content: str):
     default=False,
     is_flag=True,
     help='Output just a list of file names that would be generated')
+@click.option(
+    '--expected-outputs',
+    type=click.Path(exists=True),
+    default=None,
+    help='A file containing all expected outputs. Script will fail if outputs do not match')
 @click.argument(
     'idl_path',
     type=click.Path(exists=True))
-def main(log_level, generator, output_dir, dry_run, name_only, idl_path):
+def main(log_level, generator, output_dir, dry_run, name_only, expected_outputs, idl_path):
     """
     Parses MATTER IDL files (.matter) and performs SDK code generation
     as set up by the program arguments.
@@ -124,6 +132,29 @@ def main(log_level, generator, output_dir, dry_run, name_only, idl_path):
     generator = __GENERATORS__[
         generator].CreateGenerator(storage, idl=idl_tree)
     generator.render(dry_run)
+
+    if expected_outputs:
+        with open(expected_outputs, 'rt') as fin:
+            expected = set()
+            for l in fin.readlines():
+                l = l.strip()
+                if l:
+                    expected.add(l)
+
+            if expected != storage.generated_paths:
+                logging.fatal("expected and generated files do not match.")
+
+                extra = storage.generated_paths - expected
+                missing = expected - storage.generated_paths
+
+                for name in extra:
+                    logging.fatal("   '%s' was generated but not expected" % name)
+
+                for name in missing:
+                    logging.fatal("   '%s' was expected but not generated" % name)
+
+                sys.exit(1)
+
     logging.info("Done")
 
 

scripts/idl/generators/__init__.py

@@ -25,12 +25,22 @@
 
 class GeneratorStorage:
     """
-    Handles file operations for generator output. Specificall can create
+    Handles file operations for generator output. Specifically can create
     required files for output.
 
     Is overriden for unit tests.
     """
 
+    def __init__(self):
+        self._generated_paths = set()
+
+    @property
+    def generated_paths(self):
+        return self._generated_paths
+
+    def report_output_file(self, relative_path: str):
+        self._generated_paths.add(relative_path)
+
     def get_existing_data(self, relative_path: str):
         """Gets the existing data at the given path.
         If such data does not exist, will return None.
@@ -49,6 +59,7 @@ class FileSystemGeneratorStorage(GeneratorStorage):
     """
 
     def __init__(self, output_dir: str):
+        super().__init__()
         self.output_dir = output_dir
 
     def get_existing_data(self, relative_path: str):
@@ -148,6 +159,11 @@ def internal_render_one_output(self, template_path: str, output_file_name: str,
 
         rendered = self.jinja_env.get_template(template_path).render(vars)
 
+        # Report regardless if it has changed or not. This is because even if
+        # files are unchanged, validation of what the correct output is should
+        # still be done.
+        self.storage.report_output_file(output_file_name)
+
         if rendered == self.storage.get_existing_data(output_file_name):
             logging.info("File content not changed")
         else:

scripts/idl/test_generators.py

@@ -58,6 +58,7 @@ def add_outputs(self, yaml_outputs_dict):
 
 class TestCaseStorage(GeneratorStorage):
     def __init__(self, test_case: GeneratorTestCase, checker: unittest.TestCase):
+        super().__init__()
         self.test_case = test_case
         self.checker = checker
         self.checked_files = set()