From 10391885c3d202d868794bb11c0ced6554a0dad4 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 31 Jan 2023 11:39:22 -0500 Subject: [PATCH] Switch default generation target for generate.py to matter idl files (#24756) * 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 --- docs/code_generation.md | 20 +++++++++++-- scripts/tools/zap/generate.py | 31 ++++++++++++++------ scripts/tools/zap_regen_all.py | 53 ++++++++++++++++++++++++++-------- 3 files changed, 81 insertions(+), 23 deletions(-) diff --git a/docs/code_generation.md b/docs/code_generation.md index 58c3474881fad2..2479f69107979a 100644 --- 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 `.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 index d3c4283aca0f94..a4d36a7cbc730e 100755 --- 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 index 9b2a78f3489f1d..b5bfae13730278 100755 --- 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 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/", "/../")