Ensure "code generation" has a fixed target of "*_generate" that prov…

…ides the files in the build output directory. (project-chip#32346) * Define some source sets for data_model * Update dependencies, this seems to work for non-gen for now (with odd naming) * Rename the generate commands and hopefully this makes pregen work * Start using privilege-constants * Add missing file * Fix lint error * Fix android build ... a bit ugly as this dynamic server business is too coupled * Restyle * fix dependency * use file copy and consistent configs for code generation * Fixed dependencies and better generated file logic * Some cleanup * Add a comment about generation guarantees * Better comments * Stronger enforcement of "do not allow code generation" * Ensure we match supaths: use star globbing for file names * Use slashes for clarity ... I don't think \b is needed here * Update to previous version ... ignore odd merges --------- Co-authored-by: Andrei Litvin <[email protected]>
pgregorr-amazon · Mar 4, 2024 · 0c4ecc7 · 0c4ecc7
1 parent fb786aa
commit 0c4ecc7
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 94 deletions.
diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
@@ -223,6 +223,7 @@ jobs:
               run: |
                   ./scripts/run_in_build_env.sh "./scripts/codepregen.py ./zzz_pregenerated"
                   mv scripts/codegen.py scripts/codegen.py.renamed
+                  mv scripts/tools/zap/generate.py scripts/tools/zap/generate.py.renamed
             - name: Clean output
               run: rm -rf ./out
             - name: Build using build_examples.py (pregen)
@@ -234,18 +235,11 @@ jobs:
                        --pregen-dir ./zzz_pregenerated \
                        build \
                     "
-            - name: Check no code generation in output
-              run: |
-                  CNT=$(find out -name "CHIPClusters.h" | wc -l)
-                  if [ "${CNT}" != "0" ]; then
-                     echo "ERROR: found unexpected generated files:"
-                     find out -name "CHIPClusters.h"
-                     exit 1
-                  fi
             - name: Undo code pre-generation changes (make compile time codegen work again)
               run: |
                   rm -rf ./zzz_pregenerated
                   mv scripts/codegen.py.renamed scripts/codegen.py
+                  mv scripts/tools/zap/generate.py.renamed scripts/tools/zap/generate.py 
             - name: Run fake linux tests with build_examples
               run: |
                   ./scripts/run_in_build_env.sh \

diff --git a/.github/workflows/examples-esp32.yaml b/.github/workflows/examples-esp32.yaml
@@ -71,6 +71,7 @@ jobs:
               run: |
                   ./scripts/run_in_build_env.sh "./scripts/codepregen.py ./zzz_pregenerated"
                   mv scripts/codegen.py scripts/codegen.py.renamed
+                  mv scripts/tools/zap/generate.py scripts/tools/zap/generate.py.renamed
             - name: Clean output
               run: rm -rf ./out
             - name: Build some M5Stack variations with pregen
@@ -84,18 +85,11 @@ jobs:
                         build \
                         --copy-artifacts-to out/artifacts \
                      "
-            - name: Check no code generation in output
-              run: |
-                  CNT=$(find out -name "CHIPClusters.h" | wc -l)
-                  if [ "${CNT}" != "0" ]; then
-                     echo "ERROR: found unexpected generated files:"
-                     find out -name "CHIPClusters.h"
-                     exit 1
-                  fi
             - name: Undo code pregeneration changes
               run: |
                   rm -rf ./zzz_pregenerated
                   mv scripts/codegen.py.renamed scripts/codegen.py
+                  mv scripts/tools/zap/generate.py.renamed scripts/tools/zap/generate.py 
             - name: Build example All Clusters App C3
               run: scripts/examples/esp_example.sh all-clusters-app sdkconfig_c3devkit.defaults
             - name: Copy aside build products

diff --git a/build/chip/chip_codegen.gni b/build/chip/chip_codegen.gni
@@ -31,10 +31,6 @@ template("_chip_build_time_codegen") {
   _name = target_name
   _generator = invoker.generator
 
-  config("${_name}_config") {
-    include_dirs = [ target_gen_dir ]
-  }
-
   pw_python_action("${_name}_generate") {
     script = "${chip_root}/scripts/codegen.py"
 
@@ -124,10 +120,6 @@ template("_chip_build_time_zapgen") {
   _name = target_name
   _generator = invoker.generator
 
-  config("${_name}_config") {
-    include_dirs = [ "${target_gen_dir}/zapgen/" ]
-  }
-
   assert(_generator == "app-templates")
 
   if (_generator == "app-templates") {
@@ -225,6 +217,9 @@ template("_chip_build_time_zapgen") {
 #   generator
 #     Name of the generator to use (e.g. java-jni, java-class, cpp-app)
 #
+#     outputs MUST share the same directory prefix (e.g. 'app/' or 'tlv/meta'
+#     or 'jni')
+#
 #   outputs
 #     Explicit names of the expected outputs. Enforced to validate that
 #     expected outputs are generated when processing input files.
@@ -265,7 +260,17 @@ template("_chip_build_time_zapgen") {
 #    ]
 #  }
 #
+# Guarantees a target named "${target_name}_generate" exists and contains all
+# generated files (this works even in the case of using a pre-generated directory
+# by using a copy target to import pre-generated data)
+#
 template("chip_codegen") {
+  _name = target_name
+
+  config("${_name}_config") {
+    include_dirs = [ target_gen_dir ]
+  }
+
   if (chip_code_pre_generated_directory == "") {
     _chip_build_time_codegen(target_name) {
       forward_variables_from(invoker,
@@ -279,8 +284,6 @@ template("chip_codegen") {
                              ])
     }
   } else {
-    _name = target_name
-
     not_needed(invoker, [ "options" ])
 
     # This constructs a path like:
@@ -292,8 +295,21 @@ template("chip_codegen") {
         string_replace(rebase_path(invoker.input, chip_root), ".matter", "") +
         "/codegen/" + invoker.generator
 
-    config("${_name}_config") {
-      include_dirs = [ "${_generation_dir}" ]
+    # Generation in this case just involves some files copying
+    copy("${_name}_generate") {
+      sources = []
+
+      foreach(name, invoker.outputs) {
+        sources += [ "${_generation_dir}/${name}" ]
+      }
+
+      # NOTE: we assume ALL outputs have a common subdir. This is generally the case with
+      #       paths like "app/callback-stub.cpp" and "app/PluginApplicationCallbacks.h"
+      _outputs = invoker.outputs
+      _dir_name = get_path_info(_outputs[0], "dir")
+      outputs = [ "${target_gen_dir}/${_dir_name}/{{source_file_part}}" ]
+
+      public_configs = [ ":${_name}_config" ]
     }
 
     source_set(_name) {
@@ -302,13 +318,13 @@ template("chip_codegen") {
       if (defined(invoker.public_configs)) {
         public_configs += invoker.public_configs
       }
+      sources = get_target_outputs(":${_name}_generate")
 
       forward_variables_from(invoker, [ "deps" ])
-
-      sources = []
-      foreach(name, invoker.outputs) {
-        sources += [ "${_generation_dir}/${name}" ]
+      if (!defined(deps)) {
+        deps = []
       }
+      deps += [ ":${_name}_generate" ]
     }
   }
 }
@@ -327,6 +343,9 @@ template("chip_codegen") {
 #     Explicit names of the expected outputs. Enforced to validate that
 #     expected outputs are generated when processing input files.
 #
+#     outputs MUST share the same directory prefix (e.g. 'app/' or 'tlv/meta'
+#     or 'jni')
+#
 #   deps, public_configs
 #     Forwarded to the resulting source set
 #
@@ -352,18 +371,28 @@ template("chip_codegen") {
 #
 # Example usage:
 #
-#  chip_codegen("java-jni-generate") {
-#    input = "controller-clusters.matter"
-#    generator = "java-jni"
+# chip_zapgen("controller-clusters-zap") {
+#     input = "controller-clusters.zap"
+#     generator = "app-templates"
 #
-#    outputs = [
-#       "jni/IdentifyClient-ReadImpl.cpp",
-#       "jni/IdentifyClient-InvokeSubscribeImpl.cpp",
-#       # ... more to follow
-#    ]
-#  }
+#     outputs = [
+#       "zap-generated/access.h",
+#       "zap-generated/gen_config.h",
+#       "zap-generated/endpoint_config.h",
+#     ]
+# }
+#
+# Guarantees a target named "${target_name}_generate" exists and contains all
+# generated files (this works even in the case of using a pre-generated directory
+# by using a copy target to import pre-generated data)
 #
 template("chip_zapgen") {
+  _name = target_name
+
+  config("${_name}_config") {
+    include_dirs = [ "${target_gen_dir}/zapgen/" ]
+  }
+
   if (chip_code_pre_generated_directory == "") {
     _chip_build_time_zapgen(target_name) {
       forward_variables_from(invoker,
@@ -376,8 +405,6 @@ template("chip_zapgen") {
                              ])
     }
   } else {
-    _name = target_name
-
     # This contstructs a path like:
     #  FROM all-clusters-app.zap (inside examples/all-clusters-app/all-clusters-common/)
     #  USING "cpp-app" for generator:
@@ -387,10 +414,6 @@ template("chip_zapgen") {
         string_replace(rebase_path(invoker.input, chip_root), ".zap", "") +
         "/zap/" + invoker.generator
 
-    config("${_name}_config") {
-      include_dirs = [ "${_generation_dir}" ]
-    }
-
     # Pick up only the headers and mark them available to use
     # Specifically controller seems to require header files but NOT cpp (does)
     # not want to include cpp compilation of IM command handler data
@@ -429,6 +452,23 @@ template("chip_zapgen") {
       public_deps = [ ":${_name}_headers" ]
     }
 
+    # Generation in this case just involves some files copying
+    copy("${_name}_generate") {
+      sources = []
+
+      foreach(name, invoker.outputs) {
+        sources += [ "${_generation_dir}/${name}" ]
+      }
+
+      # NOTE: we assume ALL outputs have a common subdir. This is generally the case with
+      #       paths like "app/callback-stub.cpp" and "app/PluginApplicationCallbacks.h"
+      _outputs = invoker.outputs
+      _dir_name = get_path_info(_outputs[0], "dir")
+      outputs = [ "${target_gen_dir}/zapgen/${_dir_name}/{{source_file_part}}" ]
+
+      public_configs = [ ":${_name}_config" ]
+    }
+
     source_set(_name) {
       forward_variables_from(invoker,
                              [
@@ -440,10 +480,12 @@ template("chip_zapgen") {
       }
       public_configs += [ ":${_name}_config" ]
 
-      sources = []
-      foreach(name, invoker.outputs) {
-        sources += [ "${_generation_dir}/${name}" ]
+      if (!defined(public_deps)) {
+        public_deps = []
       }
+      public_deps += [ ":${_name}_generate" ]
+
+      sources = get_target_outputs(":${_name}_generate")
 
       # Ugly, but references WILL reference back into main code.
       check_includes = false

diff --git a/src/app/chip_data_model.gni b/src/app/chip_data_model.gni
@@ -107,64 +107,50 @@ template("chip_data_model") {
     ]
   }
 
-  # where generated files reside
-  # TODO: where can this reside?
-  if (chip_code_pre_generated_directory == "") {
-    _zapgen_gen_dir = "${target_gen_dir}/zapgen"
-    _codegen_gen_dir = "${target_gen_dir}"
-  } else {
-    # NOTE: if zap and matter file will reside in different locations
-    #       this path will change between zapgen and codegen
-    _pregen_dir =
-        chip_code_pre_generated_directory + "/" +
-        string_replace(rebase_path(invoker.zap_file, chip_root), ".zap", "")
-
-    _zapgen_gen_dir = "${_pregen_dir}/zap/app-templates"
-    _codegen_gen_dir = "${_pregen_dir}/codegen/cpp-app"
-  }
-
   # Fixed source sets for allowing reasonable dependencies on things:
   source_set("${_data_model_name}-endpoint-metadata") {
-    sources = [
-      "${_codegen_gen_dir}/app/PluginApplicationCallbacks.h",
-      "${_zapgen_gen_dir}/zap-generated/access.h",
-      "${_zapgen_gen_dir}/zap-generated/endpoint_config.h",
-      "${_zapgen_gen_dir}/zap-generated/gen_config.h",
-    ]
+    sources = filter_include(
+            get_target_outputs(":${_data_model_name}_codegen_generate"),
+            [ "*/PluginApplicationCallbacks.h" ])
+    sources += filter_include(
+            get_target_outputs(":${_data_model_name}_zapgen_generate"),
+            [
+              "*/access.h",
+              "*/endpoint_config.h",
+              "*/gen_config.h",
+            ])
 
-    deps = [ "${chip_root}/src/lib/core:chip_config_header" ]
-
-    if (chip_code_pre_generated_directory == "") {
-      deps += [
-        ":${_data_model_name}_codegen_generate",
-        ":${_data_model_name}_zapgen_generate",
-      ]
-    }
+    deps = [
+      ":${_data_model_name}_codegen_generate",
+      ":${_data_model_name}_zapgen_generate",
+      "${chip_root}/src/lib/core:chip_config_header",
+    ]
   }
 
   source_set("${_data_model_name}-callbacks") {
-    sources = [
-      "${_codegen_gen_dir}/app/callback-stub.cpp",
-      "${_codegen_gen_dir}/app/cluster-init-callback.cpp",
-    ]
+    sources = filter_include(
+            get_target_outputs(":${_data_model_name}_codegen_generate"),
+            [
+              "*/callback-stup.cpp",
+              "*/cluster-init-callback.cpp",
+            ])
 
     deps = [
+      ":${_data_model_name}_codegen_generate",
       "${chip_root}/src/app/common:ids",
       "${chip_root}/src/lib/support:span",
       "${chip_root}/src/protocols/interaction_model",
     ]
-
-    if (chip_code_pre_generated_directory == "") {
-      deps += [ ":${_data_model_name}_codegen_generate" ]
-    }
   }
 
   if (!chip_build_controller_dynamic_server) {
     source_set("${_data_model_name}-command-dispatch") {
-      sources =
-          [ "${_zapgen_gen_dir}/zap-generated/IMClusterCommandHandler.cpp" ]
+      sources = filter_include(
+              get_target_outputs(":${_data_model_name}_zapgen_generate"),
+              [ "*/IMClusterCommandHandler.cpp" ])
 
       deps = [
+        ":${_data_model_name}_zapgen_generate",
         "${chip_root}/src/app",
         "${chip_root}/src/app:interaction-model",
         "${chip_root}/src/app/common:cluster-objects",
@@ -173,10 +159,6 @@ template("chip_data_model") {
         "${chip_root}/src/lib/core",
         "${chip_root}/src/lib/support",
       ]
-
-      if (chip_code_pre_generated_directory == "") {
-        deps += [ ":${_data_model_name}_zapgen_generate" ]
-      }
     }
   }