Switch default generation target for generate.py to matter idl files (#24756)

andy31415 · andreilitvin · pull[bot] · commit 3472839d1cf0 · 2023-11-15T08:20:45.000Z
* Documentation update

* Fix paths

* No need for an output directory for matter idl generation

* Restyle

* Make the output directory for generate.py really not needed (use a default that always exists for now)

* Simplify the code

* Restyle

* Update regen_all to be able to figure out distinct commands using dry-run

* Use a proper temporary directory, so matter idl naming does not clash

* Regen

* Fix usage of None for output directory in chef

* Update template name for statistics output

* Updated template text again to make it clearer what we generate

* Fix chef codegen

* Use shutil move since on CI the temporary directory is separate from the checkout filesystem

---------

Co-authored-by: Andrei Litvin &lt;andreilitvin@google.com&gt;
diff --git a/docs/code_generation.md b/docs/code_generation.md
@@ -172,8 +172,24 @@ Additionally, individual code regeneration can be done using
 
 ```bash
 /scripts/tools/zap/generate.py                       \
-    examples/bridge-app/bridge-common/bridge-app.zap \
-    -o zzz_generated/bridge-app/zap-generated
+    examples/bridge-app/bridge-common/bridge-app.zap
+```
+
+The above will just generate a `<app>.matter` file along side the `.zap` file,
+as this is the only file that requires updates for applications. You can code
+generate other things by passing in the `-t/--templates` argument to
+generate.py. In those cases, you may also need to specify an output directory
+via `-o/--output-dir`.
+
+#### Flow for updating an application zap file:
+
+```
+# use zap UI to edit the file (or edit zap file in any other way)
+./scripts/tools/zap/run_zaptool.sh $PATH_TO_ZAP_FILE
+
+# re-generate .matter file. Note that for .matter file generation, output
+# directory is NOT used
+./scripts/tools/zap/generate.py $PATH_TO_ZAP_FILE
 ```
 
 ### Compile-time code generation / pre-generated code
diff --git a/scripts/tools/zap/generate.py b/scripts/tools/zap/generate.py
@@ -19,6 +19,7 @@
 import fcntl
 import json
 import os
+import shutil
 import subprocess
 import sys
 import tempfile
@@ -41,6 +42,7 @@ class CmdLineArgs:
     prettify_output: bool = True
     version_check: bool = True
     lock_file: Optional[str] = None
+    delete_output_dir: bool = False
 
 
 CHIP_ROOT_DIR = os.path.realpath(
@@ -106,8 +108,12 @@ def detectZclFile(zapFile):
 
 
 def runArgumentsParser() -> CmdLineArgs:
-    default_templates = 'src/app/zap-templates/app-templates.json'
-    default_output_dir = 'zap-generated/'
+    # By default generate the idl file only. This will get moved from the
+    # output directory into the zap file directory automatically.
+    #
+    # All the rest of the files (app-templates.json) are generally built at
+    # compile time.
+    default_templates = 'src/app/zap-templates/matter-idl.json'
 
     parser = argparse.ArgumentParser(
         description='Generate artifacts from .zapt templates')
@@ -117,7 +123,7 @@ def runArgumentsParser() -> CmdLineArgs:
     parser.add_argument('-z', '--zcl',
                         help='Path to the zcl templates records to use for generating artifacts (default: autodetect read from zap file)')
     parser.add_argument('-o', '--output-dir', default=None,
-                        help='Output directory for the generated files (default: automatically selected)')
+                        help='Output directory for the generated files (default: a temporary directory in out)')
     parser.add_argument('--run-bootstrap', default=None, action='store_true',
                         help='Automatically run ZAP bootstrap. By default the bootstrap is not triggered')
     parser.add_argument('--parallel', action='store_true')
@@ -129,20 +135,21 @@ def runArgumentsParser() -> CmdLineArgs:
     parser.add_argument('--version-check', action='store_true')
     parser.add_argument('--no-version-check',
                         action='store_false', dest='version_check')
+    parser.add_argument('--keep-output-dir', action='store_true',
+                        help='Keep any created output directory. Useful for temporary directories.')
     parser.set_defaults(parallel=True)
     parser.set_defaults(prettify_output=True)
     parser.set_defaults(version_check=True)
     parser.set_defaults(lock_file=None)
+    parser.set_defaults(keep_output_dir=False)
     args = parser.parse_args()
 
-    # By default, this script assumes that the global CHIP template is used with
-    # a default 'zap-generated/' output folder relative to APP_ROOT_DIR.
-    # If needed, the user may specify a specific template as a second argument. In
-    # this case the output folder is relative to CHIP_ROOT_DIR.
+    delete_output_dir = False
     if args.output_dir:
         output_dir = args.output_dir
     elif args.templates == default_templates:
-        output_dir = os.path.join(Path(args.zap).parent, default_output_dir)
+        output_dir = tempfile.mkdtemp(prefix='zapgen')
+        delete_output_dir = not args.keep_output_dir
     else:
         output_dir = ''
 
@@ -162,6 +169,7 @@ def runArgumentsParser() -> CmdLineArgs:
         prettify_output=args.prettify_output,
         version_check=args.version_check,
         lock_file=args.lock_file,
+        delete_output_dir=delete_output_dir,
     )
 
 
@@ -181,7 +189,7 @@ def extractGeneratedIdl(output_dir, zap_config_path):
         # multiple extensions. This is to work with existing codebase only
         raise Error("Unexpected input zap file  %s" % self.zap_config)
 
-    os.rename(idl_path, target_path)
+    shutil.move(idl_path, target_path)
 
 
 def runGeneration(cmdLineArgs):
@@ -327,6 +335,11 @@ def main():
         for prettifier in prettifiers:
             prettifier(cmdLineArgs.templateFile, cmdLineArgs.outputDir)
 
+    if cmdLineArgs.delete_output_dir:
+        shutil.rmtree(cmdLineArgs.outputDir)
+    else:
+        print("Files generated in: %s" % cmdLineArgs.outputDir)
+
 
 if __name__ == '__main__':
     main()
diff --git a/scripts/tools/zap_regen_all.py b/scripts/tools/zap_regen_all.py
@@ -81,6 +81,13 @@ class ZapDistinctOutput:
 
 
 class ZAPGenerateTarget:
+
+    @staticmethod
+    def MatterIdlTarget(zap_config):
+        # NOTE: this assumes `src/app/zap-templates/matter-idl.json` is the
+        #       DEFAULT generation target and it needs no output_dir
+        return ZAPGenerateTarget(zap_config, template=None, output_dir=None)
+
     def __init__(self, zap_config, template, output_dir=None):
         self.script = './scripts/tools/zap/generate.py'
         self.zap_config = str(zap_config)
@@ -93,7 +100,16 @@ def __init__(self, zap_config, template, output_dir=None):
             self.output_dir = None
 
     def distinct_output(self):
-        return ZapDistinctOutput(input_template=self.template, output_directory=self.output_dir)
+        if not self.template and not self.output_dir:
+            # Matter IDL templates have no template/output dir as they go with the
+            # default.
+            #
+            # output_directory is MIS-USED here because zap files may reside in the same
+            # directory (e.g. chef) so we claim the zap config is an output directory
+            # for uniqueness
+            return ZapDistinctOutput(input_template=None, output_directory=self.zap_config)
+        else:
+            return ZapDistinctOutput(input_template=self.template, output_directory=self.output_dir)
 
     def log_command(self):
         """Log the command that will get run for this target
@@ -128,7 +144,23 @@ def generate(self) -> TargetRunStats:
         generate_end = time.time()
 
         if "chef" in self.zap_config:
-            af_gen_event = os.path.join(self.output_dir, "af-gen-event.h")
+            if self.output_dir:
+                af_gen_event = os.path.join(self.output_dir, "af-gen-event.h")
+            else:
+                # location of file is based on zap file, we update a default here.
+                # This is because matter idl codegen does NOT require an output directory.
+                #
+                # a file of:
+                #    examples/chef/devices/rootnode_heatingcoolingunit_ncdGai1E5a.zap
+                # would get:
+                #    zzz_generated/chef-rootnode_heatingcoolingunit_ncdGai1E5a/zap-generated/af-gen-event.h
+                #
+                af_gen_event = os.path.join(
+                    CHIP_ROOT_DIR, 'zzz_generated',
+                    'chef-' + os.path.splitext(os.path.basename(self.zap_config))[0],
+                    'zap-generated', 'af-gen-event.h'
+                )
+
             with open(af_gen_event, "w+"):  # Empty file needed for linux
                 pass
             idl_path = self.zap_config.replace(".zap", ".matter")
@@ -246,10 +278,8 @@ def getGlobalTemplatesTargets():
                 'zzz_generated', 'placeholder', example_name, 'zap-generated')
             template = 'examples/placeholder/templates/templates.json'
 
-            targets.append(ZAPGenerateTarget(
-                filepath, output_dir=output_dir, template="src/app/zap-templates/matter-idl.json"))
-            targets.append(
-                ZAPGenerateTarget(filepath, output_dir=output_dir, template=template))
+            targets.append(ZAPGenerateTarget.MatterIdlTarget(filepath))
+            targets.append(ZAPGenerateTarget(filepath, output_dir=output_dir, template=template))
             continue
 
         if example_name == "chef":
@@ -271,13 +301,9 @@ def getGlobalTemplatesTargets():
         # a name like <zap-generated/foo.h>
         output_dir = os.path.join(
             'zzz_generated', generate_subdir, 'zap-generated')
-        targets.append(ZAPGenerateTarget(filepath, output_dir=output_dir,
-                       template="src/app/zap-templates/matter-idl.json"))
+        targets.append(ZAPGenerateTarget.MatterIdlTarget(filepath))
 
-    targets.append(ZAPGenerateTarget(
-        'src/controller/data_model/controller-clusters.zap',
-        template="src/app/zap-templates/matter-idl.json",
-        output_dir=os.path.join('zzz_generated/controller-clusters/zap-generated')))
+    targets.append(ZAPGenerateTarget.MatterIdlTarget('src/controller/data_model/controller-clusters.zap'))
 
     # This generates app headers for darwin only, for easier/clearer include
     # in .pbxproj files.
@@ -425,6 +451,9 @@ def main():
     for timing in timings:
         tmpl = timing.template
 
+        if tmpl is None:
+            tmpl = '[NONE (matter idl generation)]'
+
         if len(tmpl) > 50:
             # easier to distinguish paths ... shorten common in-fixes
             tmpl = tmpl.replace("/zap-templates/", "/../")
