From 0e92a395226613a0004fd13b7f315307a4740928 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Tue, 14 May 2024 01:31:24 +0200 Subject: [PATCH 01/13] add ExtraCode declarationFile and implementationFile directives --- .pre-commit-config.yaml | 4 ++-- doc/datamodel_syntax.md | 8 ++++---- python/podio_gen/generator_base.py | 9 ++++++++- python/podio_gen/podio_config_reader.py | 14 ++++++++++---- python/templates/MutableObject.cc.jinja2 | 3 +++ python/templates/MutableObject.h.jinja2 | 2 ++ python/templates/Object.cc.jinja2 | 1 + python/templates/Object.h.jinja2 | 1 + python/templates/macros/utils.jinja2 | 15 +++++++++++++++ tests/datalayout.yaml | 20 ++++++++++++++++++++ tests/extra_code/declarations.cc | 4 ++++ tests/extra_code/implementations.cc | 1 + tests/extra_code/mutable_declarations.cc | 0 tests/extra_code/mutable_implementations.cc | 0 tests/unittests/unittest.cpp | 16 ++++++++++++++++ 15 files changed, 87 insertions(+), 11 deletions(-) create mode 100644 tests/extra_code/declarations.cc create mode 100644 tests/extra_code/implementations.cc create mode 100644 tests/extra_code/mutable_declarations.cc create mode 100644 tests/extra_code/mutable_implementations.cc diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 00ca2cf97..da229b489 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,12 +11,12 @@ repos: name: clang-tidy entry: clang-tidy -warnings-as-errors='*,-clang-diagnostic-deprecated-declarations' -p compile_commands.json types: [c++] - exclude: (tests/(datamodel|src)/.*(h|cc)|podioVersion.in.h|SIOFrame.*h) + exclude: (tests/(datamodel|src)/.*(h|cc)|podioVersion.in.h|SIOFrame.*h|tests/extra_code/*) language: system - id: clang-format name: clang-format entry: .github/scripts/clang-format-hook - exclude: (tests/(datamodel|src)/.*(h|cc)$|podioVersion.in.h) + exclude: (tests/(datamodel|src)/.*(h|cc)$|podioVersion.in.h|tests/extra_code/*) types: [c++] language: system - id: pylint diff --git a/doc/datamodel_syntax.md b/doc/datamodel_syntax.md index b14c85f18..68ecab748 100644 --- a/doc/datamodel_syntax.md +++ b/doc/datamodel_syntax.md @@ -119,14 +119,14 @@ The `includes` will be add to the header files of the generated classes. includes: declaration: implementation : - declarationFile: (to be implemented!) - implementationFile: (to be implemented!) + declarationFile: + implementationFile: MutableExtraCode: includes: declaration: implementation : - declarationFile: (to be implemented!) - implementationFile: (to be implemented!) + declarationFile: + implementationFile: ``` The code being provided has to use the macro `{name}` in place of the concrete name of the class. diff --git a/python/podio_gen/generator_base.py b/python/podio_gen/generator_base.py index e772d1c98..5df0f62fe 100644 --- a/python/podio_gen/generator_base.py +++ b/python/podio_gen/generator_base.py @@ -127,7 +127,7 @@ def __init__(self, yamlfile, install_dir, package_name, verbose, dryrun, upstrea lstrip_blocks=True, trim_blocks=True, ) - + self.env.globals["embed_file"] = self._embed_file self.get_syntax = self.datamodel.options["getSyntax"] self.incfolder = self.datamodel.options["includeSubfolder"] self.expose_pod_members = self.datamodel.options["exposePODMembers"] @@ -137,6 +137,13 @@ def __init__(self, yamlfile, install_dir, package_name, verbose, dryrun, upstrea self.generated_files = [] self.any_changes = False + def _embed_file(self, filename): + """Get file contents for embedding in jinja generated code""" + if not os.path.isabs(filename): + parent_path = os.path.dirname(self.yamlfile) + filename = os.path.join(parent_path, filename) + return open(filename, encoding="utf-8").read() + def process(self): """Run the actual generation""" datamodel = self.pre_process() diff --git a/python/podio_gen/podio_config_reader.py b/python/podio_gen/podio_config_reader.py index 65f569f56..36fc86774 100644 --- a/python/podio_gen/podio_config_reader.py +++ b/python/podio_gen/podio_config_reader.py @@ -171,9 +171,15 @@ class ClassDefinitionValidator: # it applies and also which accessor functions to generate required_interface_keys = required_datatype_keys + ("Members", "Types") - valid_extra_code_keys = ("declaration", "implementation", "includes") + valid_extra_code_keys = ( + "declaration", + "implementation", + "includes", + "declarationFile", + "implementationFile", + ) # documented but not yet implemented - not_yet_implemented_extra_code = ("declarationFile", "implementationFile") + not_yet_implemented_extra_code = () @classmethod def validate(cls, datamodel, upstream_edm=None): @@ -205,10 +211,10 @@ def _check_components(cls, datamodel, upstream_edm): if "ExtraCode" in component: for key in component["ExtraCode"]: - if key not in ("declaration", "includes"): + if key not in ("declaration", "declarationFile", "includes"): raise DefinitionError( f"'{key}' field found in 'ExtraCode' of component '{name}'." - " Only 'declaration' and 'includes' are allowed here" + "Only 'declaration', 'declarationFile' and 'includes' are allowed here" ) for member in component["Members"]: diff --git a/python/templates/MutableObject.cc.jinja2 b/python/templates/MutableObject.cc.jinja2 index eaec2f716..accfc4808 100644 --- a/python/templates/MutableObject.cc.jinja2 +++ b/python/templates/MutableObject.cc.jinja2 @@ -25,7 +25,10 @@ {{ macros.multi_relation_handling(class, OneToManyRelations + VectorMembers, use_get_syntax, with_adder=True, prefix='Mutable') }} {{ utils.if_present_with_replacement(ExtraCode, "implementation", '{name}', 'Mutable' + class.bare_type) }} +{{ utils.if_present_embed_with_replacement(ExtraCode, "implementationFile",'{name}', 'Mutable' + class.bare_type) }} {{ utils.if_present_with_replacement(MutableExtraCode, "implementation", '{name}', 'Mutable' + class.bare_type) }} +{{ utils.if_present_embed_with_replacement(MutableExtraCode, "implementationFile",'{name}', 'Mutable' + class.bare_type) }} + {{ macros.common_object_funcs(class, prefix='Mutable') }} diff --git a/python/templates/MutableObject.h.jinja2 b/python/templates/MutableObject.h.jinja2 index 19af5e3a3..030ec36e0 100644 --- a/python/templates/MutableObject.h.jinja2 +++ b/python/templates/MutableObject.h.jinja2 @@ -47,7 +47,9 @@ public: {{ macros.single_relation_setters(OneToOneRelations, use_get_syntax) }} {{ macros.multi_relation_handling(OneToManyRelations + VectorMembers, use_get_syntax, with_adder=True) }} {{ utils.if_present(ExtraCode, "declaration") }} +{{ utils.if_present_embed(ExtraCode, "declarationFile") }} {{ utils.if_present(MutableExtraCode, "declaration") }} +{{ utils.if_present_embed(MutableExtraCode, "declarationFile") }} {{ macros.common_object_funcs(class.bare_type, prefix='Mutable') }} private: diff --git a/python/templates/Object.cc.jinja2 b/python/templates/Object.cc.jinja2 index 62707c79a..f5a5b199e 100644 --- a/python/templates/Object.cc.jinja2 +++ b/python/templates/Object.cc.jinja2 @@ -31,6 +31,7 @@ {{ macros.multi_relation_handling(class, OneToManyRelations + VectorMembers, use_get_syntax) }} {{ utils.if_present_with_replacement(ExtraCode, "implementation", '{name}', class.bare_type) }} +{{ utils.if_present_embed_with_replacement(ExtraCode, "implementationFile",'{name}', class.bare_type) }} {{ macros.common_object_funcs(class) }} diff --git a/python/templates/Object.h.jinja2 b/python/templates/Object.h.jinja2 index acee909be..b14b003e0 100644 --- a/python/templates/Object.h.jinja2 +++ b/python/templates/Object.h.jinja2 @@ -54,6 +54,7 @@ public: {{ macros.single_relation_getters(OneToOneRelations, use_get_syntax) }} {{ macros.multi_relation_handling(OneToManyRelations + VectorMembers, use_get_syntax) }} {{ utils.if_present(ExtraCode, "declaration") }} +{{ utils.if_present_embed(ExtraCode, "declarationFile") }} {{ macros.common_object_funcs(class.bare_type) }} private: diff --git a/python/templates/macros/utils.jinja2 b/python/templates/macros/utils.jinja2 index 56511a6fe..da4dbc1d7 100644 --- a/python/templates/macros/utils.jinja2 +++ b/python/templates/macros/utils.jinja2 @@ -17,6 +17,21 @@ namespace {{ nsp }} { {% endif %} {%- endmacro %} +{% macro if_present_embed(content, field) %} +{% if content and content[field] %} +// embedded file {{ content[field] }} +{{ embed_file(content[field]) }} +// end of embedded file +{% endif %} +{%- endmacro %} + +{% macro if_present_embed_with_replacement(content, field, old, new) %} +{% if content and content[field] %} +// embedded file {{ content[field] }} +{{ embed_file(content[field]) | replace(old, new) }} +// end of embedded file +{% endif %} +{%- endmacro %} {% macro if_present_with_replacement(content, field, old, new) %} {% if content and content[field] %} diff --git a/tests/datalayout.yaml b/tests/datalayout.yaml index 4c5dd515c..37052043f 100644 --- a/tests/datalayout.yaml +++ b/tests/datalayout.yaml @@ -209,6 +209,26 @@ datatypes : - TypeWithEnergy manyEnergies // multiple relations - ex42::AnotherTypeWithEnergy moreEnergies // multiple namespace relations + ExampleWithExternalExtraCode: + Description: "Type showing usage of 'declaration', 'implementation', 'declarationFile' and 'implementationFile' directives'" + Author: "Mateusz Jakub Fila" + Members: + - int number // a number + ExtraCode: + declaration: + "int add(int i) const;" + declarationFile: "extra_code/declarations.cc" + implementation: + "int {name}::add(int i) const { return number() + i; }" + implementationFile: "extra_code/implementations.cc" + MutableExtraCode: + declaration: + int add_inplace(int i); + declarationFile: "extra_code/mutable_declarations.cc" + implementation: + int {name}::add_inplace(int i) { return number() += i; } + implementationFile: "extra_code/mutable_implementations.cc" + nsp::EnergyInNamespace: Description: "A type with energy in a namespace" Author: "Thomas Madlener" diff --git a/tests/extra_code/declarations.cc b/tests/extra_code/declarations.cc new file mode 100644 index 000000000..7fba31182 --- /dev/null +++ b/tests/extra_code/declarations.cc @@ -0,0 +1,4 @@ +bool gt(int i) const { + return number() > i; +} +bool lt(int i) const; \ No newline at end of file diff --git a/tests/extra_code/implementations.cc b/tests/extra_code/implementations.cc new file mode 100644 index 000000000..dc342e205 --- /dev/null +++ b/tests/extra_code/implementations.cc @@ -0,0 +1 @@ +bool {name}::lt(int i) const { return number() < i; } \ No newline at end of file diff --git a/tests/extra_code/mutable_declarations.cc b/tests/extra_code/mutable_declarations.cc new file mode 100644 index 000000000..e69de29bb diff --git a/tests/extra_code/mutable_implementations.cc b/tests/extra_code/mutable_implementations.cc new file mode 100644 index 000000000..e69de29bb diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp index 5f5345427..50f80b03e 100644 --- a/tests/unittests/unittest.cpp +++ b/tests/unittests/unittest.cpp @@ -43,6 +43,7 @@ #include "datamodel/ExampleWithArray.h" #include "datamodel/ExampleWithArrayComponent.h" #include "datamodel/ExampleWithComponent.h" +#include "datamodel/ExampleWithExternalExtraCode.h" #include "datamodel/ExampleWithFixedWidthIntegers.h" #include "datamodel/ExampleWithOneRelationCollection.h" #include "datamodel/ExampleWithUserInitCollection.h" @@ -50,6 +51,8 @@ #include "datamodel/MutableExampleCluster.h" #include "datamodel/MutableExampleWithArray.h" #include "datamodel/MutableExampleWithComponent.h" +#include "datamodel/MutableExampleWithExternalExtraCode.h" + #include "podio/UserDataCollection.h" TEST_CASE("AutoDelete", "[basics][memory-management]") { @@ -398,6 +401,19 @@ TEST_CASE("Extracode", "[basics][code-gen]") { REQUIRE(simple.z == 3); } +TEST_CASE("ExtraCode declarationFile and implementationFile", "[basics][code-gen]") { + MutableExampleWithExternalExtraCode mutable_example; + mutable_example.number(0); + REQUIRE(mutable_example.add(2) == 2); + REQUIRE(mutable_example.add_inplace(1) == 1); + REQUIRE(mutable_example.gt(-1)); + REQUIRE(mutable_example.lt(100)); + auto example = static_cast(mutable_example); + REQUIRE(example.add(1) == 2); + REQUIRE(example.gt(0)); + REQUIRE(example.lt(100)); +} + TEST_CASE("AssociativeContainer", "[basics]") { auto clu1 = MutableExampleCluster(); auto clu2 = MutableExampleCluster(); From 22aa6574077db8d4b1ab1e2290f837768c814172 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 15 May 2024 15:03:38 +0200 Subject: [PATCH 02/13] add declarationFile extracode for components --- python/templates/Component.h.jinja2 | 1 + tests/datalayout.yaml | 9 +++++++ tests/extra_code/component_declarations.cc | 4 ++++ tests/unittests/unittest.cpp | 28 ++++++++++++++-------- 4 files changed, 32 insertions(+), 10 deletions(-) create mode 100644 tests/extra_code/component_declarations.cc diff --git a/python/templates/Component.h.jinja2 b/python/templates/Component.h.jinja2 index b410c5c8b..1ebe3b5a7 100644 --- a/python/templates/Component.h.jinja2 +++ b/python/templates/Component.h.jinja2 @@ -23,6 +23,7 @@ public: {% endfor %} {{ utils.if_present(ExtraCode, "declaration") }} +{{ utils.if_present_embed(ExtraCode, "declarationFile") }} }; std::ostream& operator<<(std::ostream& o, const {{class.full_type}}& value); diff --git a/tests/datalayout.yaml b/tests/datalayout.yaml index 37052043f..eb3e40c82 100644 --- a/tests/datalayout.yaml +++ b/tests/datalayout.yaml @@ -53,6 +53,15 @@ components : - int i{42} // is there even another value to initialize ints to? - std::array arr {1.2, 3.4} // half initialized double array + StructWithExtraCode: + Members: + - int x + ExtraCode: + declaration: " + int negate() { x = -x; return x; }\n + " + declarationFile: "extra_code/component_declarations.cc" + datatypes : EventInfo: diff --git a/tests/extra_code/component_declarations.cc b/tests/extra_code/component_declarations.cc new file mode 100644 index 000000000..fe0ce26fe --- /dev/null +++ b/tests/extra_code/component_declarations.cc @@ -0,0 +1,4 @@ +int reset() { + x = 0; + return x; +} \ No newline at end of file diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp index 50f80b03e..6508f1c4f 100644 --- a/tests/unittests/unittest.cpp +++ b/tests/unittests/unittest.cpp @@ -52,6 +52,7 @@ #include "datamodel/MutableExampleWithArray.h" #include "datamodel/MutableExampleWithComponent.h" #include "datamodel/MutableExampleWithExternalExtraCode.h" +#include "datamodel/StructWithExtraCode.h" #include "podio/UserDataCollection.h" @@ -402,16 +403,23 @@ TEST_CASE("Extracode", "[basics][code-gen]") { } TEST_CASE("ExtraCode declarationFile and implementationFile", "[basics][code-gen]") { - MutableExampleWithExternalExtraCode mutable_example; - mutable_example.number(0); - REQUIRE(mutable_example.add(2) == 2); - REQUIRE(mutable_example.add_inplace(1) == 1); - REQUIRE(mutable_example.gt(-1)); - REQUIRE(mutable_example.lt(100)); - auto example = static_cast(mutable_example); - REQUIRE(example.add(1) == 2); - REQUIRE(example.gt(0)); - REQUIRE(example.lt(100)); + auto mutable_number = MutableExampleWithExternalExtraCode(); + mutable_number.number(0); + REQUIRE(mutable_number.add(2) == 2); + REQUIRE(mutable_number.add_inplace(1) == 1); + REQUIRE(mutable_number.gt(-1)); + REQUIRE(mutable_number.lt(100)); + ExampleWithExternalExtraCode number = mutable_number; + REQUIRE(number.add(1) == 2); + REQUIRE(number.gt(0)); + REQUIRE(number.lt(100)); +} + +TEST_CASE("ExtraCode declarationFile in component", "[basics][code-gen]") { + auto value = StructWithExtraCode(); + value.x = 1; + REQUIRE(value.negate() == -1); + REQUIRE(value.reset() == 0 ); } TEST_CASE("AssociativeContainer", "[basics]") { From 6a6a972692c0abdd475867b6ab3e0103343723ef Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 15 May 2024 15:10:32 +0200 Subject: [PATCH 03/13] mend --- tests/unittests/unittest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp index 6508f1c4f..36e0d8c8d 100644 --- a/tests/unittests/unittest.cpp +++ b/tests/unittests/unittest.cpp @@ -419,7 +419,7 @@ TEST_CASE("ExtraCode declarationFile in component", "[basics][code-gen]") { auto value = StructWithExtraCode(); value.x = 1; REQUIRE(value.negate() == -1); - REQUIRE(value.reset() == 0 ); + REQUIRE(value.reset() == 0); } TEST_CASE("AssociativeContainer", "[basics]") { From b8e2743d239968e5aca104cebf2db7d962786bb9 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 15 May 2024 15:57:01 +0200 Subject: [PATCH 04/13] fix pre-commit exclude --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index da229b489..fbb879039 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,12 +11,12 @@ repos: name: clang-tidy entry: clang-tidy -warnings-as-errors='*,-clang-diagnostic-deprecated-declarations' -p compile_commands.json types: [c++] - exclude: (tests/(datamodel|src)/.*(h|cc)|podioVersion.in.h|SIOFrame.*h|tests/extra_code/*) + exclude: (tests/(datamodel|src)/.*(h|cc)|podioVersion.in.h|SIOFrame.*h|tests/extra_code/.*) language: system - id: clang-format name: clang-format entry: .github/scripts/clang-format-hook - exclude: (tests/(datamodel|src)/.*(h|cc)$|podioVersion.in.h|tests/extra_code/*) + exclude: (tests/(datamodel|src)/.*(h|cc)$|podioVersion.in.h|tests/extra_code/.*) types: [c++] language: system - id: pylint From 19892741cbfeb1a213bd0bd0d290cfd298aec120 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 15 May 2024 17:35:33 +0200 Subject: [PATCH 05/13] add test for mutable declarationFile and implementationFile --- tests/extra_code/mutable_declarations.cc | 1 + tests/extra_code/mutable_implementations.cc | 4 ++++ tests/unittests/unittest.cpp | 4 ++-- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/extra_code/mutable_declarations.cc b/tests/extra_code/mutable_declarations.cc index e69de29bb..21b94d1de 100644 --- a/tests/extra_code/mutable_declarations.cc +++ b/tests/extra_code/mutable_declarations.cc @@ -0,0 +1 @@ +int reset(); \ No newline at end of file diff --git a/tests/extra_code/mutable_implementations.cc b/tests/extra_code/mutable_implementations.cc index e69de29bb..49c9f3def 100644 --- a/tests/extra_code/mutable_implementations.cc +++ b/tests/extra_code/mutable_implementations.cc @@ -0,0 +1,4 @@ +int {name}::reset() { + number() = 0; + return number(); +} \ No newline at end of file diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp index 36e0d8c8d..934577bb3 100644 --- a/tests/unittests/unittest.cpp +++ b/tests/unittests/unittest.cpp @@ -404,14 +404,14 @@ TEST_CASE("Extracode", "[basics][code-gen]") { TEST_CASE("ExtraCode declarationFile and implementationFile", "[basics][code-gen]") { auto mutable_number = MutableExampleWithExternalExtraCode(); - mutable_number.number(0); + REQUIRE(mutable_number.reset() == 0); REQUIRE(mutable_number.add(2) == 2); REQUIRE(mutable_number.add_inplace(1) == 1); REQUIRE(mutable_number.gt(-1)); REQUIRE(mutable_number.lt(100)); ExampleWithExternalExtraCode number = mutable_number; REQUIRE(number.add(1) == 2); - REQUIRE(number.gt(0)); + REQUIRE(number.gt(-1)); REQUIRE(number.lt(100)); } From 8dbbe06f83868d98eddd97b3691a9ab2127c1715 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 15 May 2024 23:37:16 +0200 Subject: [PATCH 06/13] copy extra code files to root_io and sio_io for dumpmodel tests --- tests/dumpmodel/CMakeLists.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/dumpmodel/CMakeLists.txt b/tests/dumpmodel/CMakeLists.txt index 6e9dfef1e..737c6a987 100644 --- a/tests/dumpmodel/CMakeLists.txt +++ b/tests/dumpmodel/CMakeLists.txt @@ -1,6 +1,10 @@ # Copy the main .clang-format to here so that the tests can always find it configure_file(${PROJECT_SOURCE_DIR}/.clang-format ${CMAKE_CURRENT_BINARY_DIR}/.clang-format COPYONLY) +# copy extra code declarations and implementations files since their paths are defined relative to the yaml file +file(CREATE_LINK ${CMAKE_CURRENT_SOURCE_DIR}/../extra_code + ${CMAKE_CURRENT_BINARY_DIR}/../root_io/extra_code COPY_ON_ERROR SYMBOLIC) + # Add tests for storing and retrieving the EDM definitions into the produced # files add_test(NAME datamodel_def_store_roundtrip_root COMMAND ${PROJECT_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh @@ -30,6 +34,10 @@ set_tests_properties( set(sio_roundtrip_tests "") if (ENABLE_SIO) + # copy extra code declarations and implementations files since their paths are defined relative to the yaml file + file(CREATE_LINK ${CMAKE_CURRENT_SOURCE_DIR}/../extra_code + ${CMAKE_CURRENT_BINARY_DIR}/../sio_io/extra_code COPY_ON_ERROR SYMBOLIC) + add_test(NAME datamodel_def_store_roundtrip_sio COMMAND ${PROJECT_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh ${PROJECT_BINARY_DIR}/tests/sio_io/example_frame.sio datamodel From 5e497a35d03a7268a5d25ce8743e4226f7d9f853 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Thu, 16 May 2024 22:32:26 +0200 Subject: [PATCH 07/13] revert embedding files with jinja --- python/podio_gen/generator_base.py | 9 +-------- python/templates/Component.h.jinja2 | 1 - python/templates/MutableObject.cc.jinja2 | 3 --- python/templates/MutableObject.h.jinja2 | 2 -- python/templates/Object.cc.jinja2 | 1 - python/templates/Object.h.jinja2 | 1 - python/templates/macros/utils.jinja2 | 15 --------------- tests/dumpmodel/CMakeLists.txt | 8 -------- 8 files changed, 1 insertion(+), 39 deletions(-) diff --git a/python/podio_gen/generator_base.py b/python/podio_gen/generator_base.py index 5df0f62fe..e772d1c98 100644 --- a/python/podio_gen/generator_base.py +++ b/python/podio_gen/generator_base.py @@ -127,7 +127,7 @@ def __init__(self, yamlfile, install_dir, package_name, verbose, dryrun, upstrea lstrip_blocks=True, trim_blocks=True, ) - self.env.globals["embed_file"] = self._embed_file + self.get_syntax = self.datamodel.options["getSyntax"] self.incfolder = self.datamodel.options["includeSubfolder"] self.expose_pod_members = self.datamodel.options["exposePODMembers"] @@ -137,13 +137,6 @@ def __init__(self, yamlfile, install_dir, package_name, verbose, dryrun, upstrea self.generated_files = [] self.any_changes = False - def _embed_file(self, filename): - """Get file contents for embedding in jinja generated code""" - if not os.path.isabs(filename): - parent_path = os.path.dirname(self.yamlfile) - filename = os.path.join(parent_path, filename) - return open(filename, encoding="utf-8").read() - def process(self): """Run the actual generation""" datamodel = self.pre_process() diff --git a/python/templates/Component.h.jinja2 b/python/templates/Component.h.jinja2 index 1ebe3b5a7..b410c5c8b 100644 --- a/python/templates/Component.h.jinja2 +++ b/python/templates/Component.h.jinja2 @@ -23,7 +23,6 @@ public: {% endfor %} {{ utils.if_present(ExtraCode, "declaration") }} -{{ utils.if_present_embed(ExtraCode, "declarationFile") }} }; std::ostream& operator<<(std::ostream& o, const {{class.full_type}}& value); diff --git a/python/templates/MutableObject.cc.jinja2 b/python/templates/MutableObject.cc.jinja2 index accfc4808..eaec2f716 100644 --- a/python/templates/MutableObject.cc.jinja2 +++ b/python/templates/MutableObject.cc.jinja2 @@ -25,10 +25,7 @@ {{ macros.multi_relation_handling(class, OneToManyRelations + VectorMembers, use_get_syntax, with_adder=True, prefix='Mutable') }} {{ utils.if_present_with_replacement(ExtraCode, "implementation", '{name}', 'Mutable' + class.bare_type) }} -{{ utils.if_present_embed_with_replacement(ExtraCode, "implementationFile",'{name}', 'Mutable' + class.bare_type) }} {{ utils.if_present_with_replacement(MutableExtraCode, "implementation", '{name}', 'Mutable' + class.bare_type) }} -{{ utils.if_present_embed_with_replacement(MutableExtraCode, "implementationFile",'{name}', 'Mutable' + class.bare_type) }} - {{ macros.common_object_funcs(class, prefix='Mutable') }} diff --git a/python/templates/MutableObject.h.jinja2 b/python/templates/MutableObject.h.jinja2 index 030ec36e0..19af5e3a3 100644 --- a/python/templates/MutableObject.h.jinja2 +++ b/python/templates/MutableObject.h.jinja2 @@ -47,9 +47,7 @@ public: {{ macros.single_relation_setters(OneToOneRelations, use_get_syntax) }} {{ macros.multi_relation_handling(OneToManyRelations + VectorMembers, use_get_syntax, with_adder=True) }} {{ utils.if_present(ExtraCode, "declaration") }} -{{ utils.if_present_embed(ExtraCode, "declarationFile") }} {{ utils.if_present(MutableExtraCode, "declaration") }} -{{ utils.if_present_embed(MutableExtraCode, "declarationFile") }} {{ macros.common_object_funcs(class.bare_type, prefix='Mutable') }} private: diff --git a/python/templates/Object.cc.jinja2 b/python/templates/Object.cc.jinja2 index f5a5b199e..62707c79a 100644 --- a/python/templates/Object.cc.jinja2 +++ b/python/templates/Object.cc.jinja2 @@ -31,7 +31,6 @@ {{ macros.multi_relation_handling(class, OneToManyRelations + VectorMembers, use_get_syntax) }} {{ utils.if_present_with_replacement(ExtraCode, "implementation", '{name}', class.bare_type) }} -{{ utils.if_present_embed_with_replacement(ExtraCode, "implementationFile",'{name}', class.bare_type) }} {{ macros.common_object_funcs(class) }} diff --git a/python/templates/Object.h.jinja2 b/python/templates/Object.h.jinja2 index b14b003e0..acee909be 100644 --- a/python/templates/Object.h.jinja2 +++ b/python/templates/Object.h.jinja2 @@ -54,7 +54,6 @@ public: {{ macros.single_relation_getters(OneToOneRelations, use_get_syntax) }} {{ macros.multi_relation_handling(OneToManyRelations + VectorMembers, use_get_syntax) }} {{ utils.if_present(ExtraCode, "declaration") }} -{{ utils.if_present_embed(ExtraCode, "declarationFile") }} {{ macros.common_object_funcs(class.bare_type) }} private: diff --git a/python/templates/macros/utils.jinja2 b/python/templates/macros/utils.jinja2 index da4dbc1d7..56511a6fe 100644 --- a/python/templates/macros/utils.jinja2 +++ b/python/templates/macros/utils.jinja2 @@ -17,21 +17,6 @@ namespace {{ nsp }} { {% endif %} {%- endmacro %} -{% macro if_present_embed(content, field) %} -{% if content and content[field] %} -// embedded file {{ content[field] }} -{{ embed_file(content[field]) }} -// end of embedded file -{% endif %} -{%- endmacro %} - -{% macro if_present_embed_with_replacement(content, field, old, new) %} -{% if content and content[field] %} -// embedded file {{ content[field] }} -{{ embed_file(content[field]) | replace(old, new) }} -// end of embedded file -{% endif %} -{%- endmacro %} {% macro if_present_with_replacement(content, field, old, new) %} {% if content and content[field] %} diff --git a/tests/dumpmodel/CMakeLists.txt b/tests/dumpmodel/CMakeLists.txt index 737c6a987..6e9dfef1e 100644 --- a/tests/dumpmodel/CMakeLists.txt +++ b/tests/dumpmodel/CMakeLists.txt @@ -1,10 +1,6 @@ # Copy the main .clang-format to here so that the tests can always find it configure_file(${PROJECT_SOURCE_DIR}/.clang-format ${CMAKE_CURRENT_BINARY_DIR}/.clang-format COPYONLY) -# copy extra code declarations and implementations files since their paths are defined relative to the yaml file -file(CREATE_LINK ${CMAKE_CURRENT_SOURCE_DIR}/../extra_code - ${CMAKE_CURRENT_BINARY_DIR}/../root_io/extra_code COPY_ON_ERROR SYMBOLIC) - # Add tests for storing and retrieving the EDM definitions into the produced # files add_test(NAME datamodel_def_store_roundtrip_root COMMAND ${PROJECT_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh @@ -34,10 +30,6 @@ set_tests_properties( set(sio_roundtrip_tests "") if (ENABLE_SIO) - # copy extra code declarations and implementations files since their paths are defined relative to the yaml file - file(CREATE_LINK ${CMAKE_CURRENT_SOURCE_DIR}/../extra_code - ${CMAKE_CURRENT_BINARY_DIR}/../sio_io/extra_code COPY_ON_ERROR SYMBOLIC) - add_test(NAME datamodel_def_store_roundtrip_sio COMMAND ${PROJECT_SOURCE_DIR}/tests/scripts/dumpModelRoundTrip.sh ${PROJECT_BINARY_DIR}/tests/sio_io/example_frame.sio datamodel From 82307dfe46825cef95be4289a49fcc431c3284fe Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Thu, 16 May 2024 22:33:37 +0200 Subject: [PATCH 08/13] add including implementationFile and declarationFile during parsing --- python/podio_gen/podio_config_reader.py | 35 ++++++++++++++++++++----- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/python/podio_gen/podio_config_reader.py b/python/podio_gen/podio_config_reader.py index 36fc86774..a9531b0df 100644 --- a/python/podio_gen/podio_config_reader.py +++ b/python/podio_gen/podio_config_reader.py @@ -2,6 +2,7 @@ import copy import re +import os import yaml from podio_gen.generator_utils import ( @@ -448,8 +449,20 @@ def _handle_extracode(definition): """Handle the extra code definition. Currently simply returning a copy""" return copy.deepcopy(definition) + @staticmethod + def _expand_extracode(definitions, parent_path, directive): + """Expand extra code directives by including files from 'File' directives + Relative paths to files are extended with parent_path. Mutates definitions""" + if directive + "File" in definitions.keys(): + filename = definitions.pop(directive + "File") + if not os.path.isabs(filename): + filename = os.path.join(parent_path, filename) + with open(filename, "r", encoding="utf-8") as stream: + contents = stream.read() + definitions[directive] = definitions.get(directive, "") + "\n" + contents + @classmethod - def _read_component(cls, definition): + def _read_component(cls, definition, parent_path): """Read the component and put it into an easily digestible format.""" component = {} for name, category in definition.items(): @@ -458,13 +471,17 @@ def _read_component(cls, definition): for member in definition[name]: # for components we do not require a description in the members component["Members"].append(cls.member_parser.parse(member, False)) + elif name == "ExtraCode": + extra_code = copy.deepcopy(category) + cls._expand_extracode(extra_code, parent_path, "declaration") + component[name] = extra_code else: component[name] = copy.deepcopy(category) return component @classmethod - def _read_datatype(cls, value): + def _read_datatype(cls, value, parent_path): """Read the datatype and put it into an easily digestible format""" datatype = {} for category, definition in value.items(): @@ -474,6 +491,11 @@ def _read_datatype(cls, value): for member in definition: members.append(cls.member_parser.parse(member)) datatype[category] = members + elif category in ("ExtraCode", "MutableExtraCode"): + extra_code = copy.deepcopy(definition) + cls._expand_extracode(extra_code, parent_path, "implementation") + cls._expand_extracode(extra_code, parent_path, "declaration") + datatype[category] = extra_code else: datatype[category] = copy.deepcopy(definition) @@ -500,7 +522,7 @@ def _read_interface(cls, value): return interface @classmethod - def parse_model(cls, model_dict, package_name, upstream_edm=None): + def parse_model(cls, model_dict, package_name, upstream_edm=None, parent_path=None): """Parse a model from the dictionary, e.g. read from a yaml file.""" try: @@ -521,12 +543,12 @@ def parse_model(cls, model_dict, package_name, upstream_edm=None): components = {} if "components" in model_dict: for klassname, value in model_dict["components"].items(): - components[klassname] = cls._read_component(value) + components[klassname] = cls._read_component(value, parent_path) datatypes = {} if "datatypes" in model_dict: for klassname, value in model_dict["datatypes"].items(): - datatypes[klassname] = cls._read_datatype(value) + datatypes[klassname] = cls._read_datatype(value, parent_path) interfaces = {} if "interfaces" in model_dict: @@ -555,5 +577,6 @@ def read(cls, yamlfile, package_name, upstream_edm=None): """Read the datamodel definition from the yamlfile.""" with open(yamlfile, "r", encoding="utf-8") as stream: content = yaml.load(stream, yaml.SafeLoader) + parent_path = os.path.dirname(yamlfile) - return cls.parse_model(content, package_name, upstream_edm) + return cls.parse_model(content, package_name, upstream_edm, parent_path) From 0a35f305f9f3d3df884fa4d8789c6000d996300b Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Thu, 16 May 2024 23:19:22 +0200 Subject: [PATCH 09/13] Update .pre-commit-config.yaml --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index fbb879039..26225e67f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,12 +11,12 @@ repos: name: clang-tidy entry: clang-tidy -warnings-as-errors='*,-clang-diagnostic-deprecated-declarations' -p compile_commands.json types: [c++] - exclude: (tests/(datamodel|src)/.*(h|cc)|podioVersion.in.h|SIOFrame.*h|tests/extra_code/.*) + exclude: (tests/(datamodel|src|extra_code)/.*(h|cc)|podioVersion.in.h|SIOFrame.*h) language: system - id: clang-format name: clang-format entry: .github/scripts/clang-format-hook - exclude: (tests/(datamodel|src)/.*(h|cc)$|podioVersion.in.h|tests/extra_code/.*) + exclude: (tests/(datamodel|src|extra_code)/.*(h|cc)$|podioVersion.in.h) types: [c++] language: system - id: pylint From 8bdaa09d2d5c60feea1051b0091f6552be65731b Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Fri, 17 May 2024 18:07:51 +0200 Subject: [PATCH 10/13] remove checks for not yet implemented extra code, fix space Co-authored-by: tmadlener --- python/podio_gen/podio_config_reader.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/python/podio_gen/podio_config_reader.py b/python/podio_gen/podio_config_reader.py index a9531b0df..247ec3214 100644 --- a/python/podio_gen/podio_config_reader.py +++ b/python/podio_gen/podio_config_reader.py @@ -179,8 +179,6 @@ class ClassDefinitionValidator: "declarationFile", "implementationFile", ) - # documented but not yet implemented - not_yet_implemented_extra_code = () @classmethod def validate(cls, datamodel, upstream_edm=None): @@ -214,7 +212,7 @@ def _check_components(cls, datamodel, upstream_edm): for key in component["ExtraCode"]: if key not in ("declaration", "declarationFile", "includes"): raise DefinitionError( - f"'{key}' field found in 'ExtraCode' of component '{name}'." + f"'{key}' field found in 'ExtraCode' of component '{name}'. " "Only 'declaration', 'declarationFile' and 'includes' are allowed here" ) @@ -374,15 +372,8 @@ def _check_keys(cls, classname, definition): extracode = definition["ExtraCode"] invalid_keys = [k for k in extracode if k not in cls.valid_extra_code_keys] if invalid_keys: - not_yet_impl = [k for k in invalid_keys if k in cls.not_yet_implemented_extra_code] - if not_yet_impl: - not_yet_impl = f" (not yet implemented: {not_yet_impl})" - else: - not_yet_impl = "" - raise DefinitionError( - f"{classname} defines invalid 'ExtraCode' categories: " - f"{invalid_keys}{not_yet_impl}" + f"{classname} defines invalid 'ExtraCode' categories: " f"{invalid_keys}" ) @classmethod From 10de1ba096917ce37b889c9ab2f145f2597ca137 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Fri, 17 May 2024 18:09:30 +0200 Subject: [PATCH 11/13] add documentation of file paths --- doc/datamodel_syntax.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/datamodel_syntax.md b/doc/datamodel_syntax.md index 68ecab748..56faff1a3 100644 --- a/doc/datamodel_syntax.md +++ b/doc/datamodel_syntax.md @@ -129,7 +129,7 @@ The `includes` will be add to the header files of the generated classes. implementationFile: ``` -The code being provided has to use the macro `{name}` in place of the concrete name of the class. +The code being provided has to use the macro `{name}` in place of the concrete name of the class. The paths to the files given in `declarationFile` and `implementationFile` should be either absolute or relative to the datamodel yaml file. ## Definition of custom interfaces An interface type can be defined as follows (again using an example from the example datamodel) From e7592b3ccf4fd5aecc58a7c7d99515ed2aa2878d Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Sat, 18 May 2024 00:59:05 +0200 Subject: [PATCH 12/13] add DEPENDS option to datamodel generating macro and use it with extra code files --- cmake/podioMacros.cmake | 4 +++- doc/datamodel_syntax.md | 2 +- tests/CMakeLists.txt | 12 ++++++++++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/cmake/podioMacros.cmake b/cmake/podioMacros.cmake index c48f28d81..ff2d8461f 100644 --- a/cmake/podioMacros.cmake +++ b/cmake/podioMacros.cmake @@ -130,13 +130,14 @@ set_property(CACHE PODIO_USE_CLANG_FORMAT PROPERTY STRINGS AUTO ON OFF) # SCHEMA_EVOLUTION OPTIONAL: The path to the yaml file declaring the necessary schema evolution # LANG OPTIONAL: The programming language choice # Default is cpp +# DEPENDS OPTIONAL: List of files to be added as configure dependencies of the datamodel # ) # # Note that the create_${datamodel} target will always be called, but if the YAML_FILE has not changed # this is essentially a no-op, and should not cause re-compilation. #--------------------------------------------------------------------------------------------------- function(PODIO_GENERATE_DATAMODEL datamodel YAML_FILE RETURN_HEADERS RETURN_SOURCES) - CMAKE_PARSE_ARGUMENTS(ARG "" "OLD_DESCRIPTION;OUTPUT_FOLDER;UPSTREAM_EDM;SCHEMA_EVOLUTION" "IO_BACKEND_HANDLERS;LANG" ${ARGN}) + CMAKE_PARSE_ARGUMENTS(ARG "" "OLD_DESCRIPTION;OUTPUT_FOLDER;UPSTREAM_EDM;SCHEMA_EVOLUTION" "IO_BACKEND_HANDLERS;LANG;DEPENDS" ${ARGN}) IF(NOT ARG_OUTPUT_FOLDER) SET(ARG_OUTPUT_FOLDER ${CMAKE_CURRENT_SOURCE_DIR}) ENDIF() @@ -208,6 +209,7 @@ function(PODIO_GENERATE_DATAMODEL datamodel YAML_FILE RETURN_HEADERS RETURN_SOUR ${podio_PYTHON_DIR}/podio_gen/generator_base.py ${podio_PYTHON_DIR}/podio_gen/cpp_generator.py ${podio_PYTHON_DIR}/podio_gen/julia_generator.py + ${ARG_DEPENDS} ) message(STATUS "Creating '${datamodel}' datamodel") diff --git a/doc/datamodel_syntax.md b/doc/datamodel_syntax.md index 56faff1a3..c8a42d602 100644 --- a/doc/datamodel_syntax.md +++ b/doc/datamodel_syntax.md @@ -129,7 +129,7 @@ The `includes` will be add to the header files of the generated classes. implementationFile: ``` -The code being provided has to use the macro `{name}` in place of the concrete name of the class. The paths to the files given in `declarationFile` and `implementationFile` should be either absolute or relative to the datamodel yaml file. +The code being provided has to use the macro `{name}` in place of the concrete name of the class. The paths to the files given in `declarationFile` and `implementationFile` should be either absolute or relative to the datamodel yaml file. The cmake users are encouraged to specify these file in `DEPENDS` option of `PODIO_GENERATE_DATAMODEL` to add the files as configuration dependency of the datamodel and cause the datamodel re-generation when they change. ## Definition of custom interfaces An interface type can be defined as follows (again using an example from the example datamodel) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b8233c528..729248b91 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -5,9 +5,17 @@ foreach( _conf ${CMAKE_CONFIGURATION_TYPES} ) set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY_${_conf} ${CMAKE_CURRENT_BINARY_DIR} ) endforeach() +# files used in ExtraCode directives +set(extra_code extra_code/component_declarations.cc + extra_code/declarations.cc + extra_code/implementations.cc + extra_code/mutable_declarations.cc + extra_code/mutable_implementations.cc +) + PODIO_GENERATE_DATAMODEL(datamodel datalayout.yaml headers sources - IO_BACKEND_HANDLERS ${PODIO_IO_HANDLERS} - ) + IO_BACKEND_HANDLERS ${PODIO_IO_HANDLERS} DEPENDS ${extra_code} +) # Use the cmake building blocks to add the different parts (conditionally) PODIO_ADD_DATAMODEL_CORE_LIB(TestDataModel "${headers}" "${sources}") From acf9aefdba272faee4545997ce2c66ab49b6026e Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Wed, 22 May 2024 10:49:10 +0200 Subject: [PATCH 13/13] Fix typo in documentation --- doc/datamodel_syntax.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/datamodel_syntax.md b/doc/datamodel_syntax.md index c8a42d602..8181543fd 100644 --- a/doc/datamodel_syntax.md +++ b/doc/datamodel_syntax.md @@ -129,7 +129,7 @@ The `includes` will be add to the header files of the generated classes. implementationFile: ``` -The code being provided has to use the macro `{name}` in place of the concrete name of the class. The paths to the files given in `declarationFile` and `implementationFile` should be either absolute or relative to the datamodel yaml file. The cmake users are encouraged to specify these file in `DEPENDS` option of `PODIO_GENERATE_DATAMODEL` to add the files as configuration dependency of the datamodel and cause the datamodel re-generation when they change. +The code being provided has to use the macro `{name}` in place of the concrete name of the class. The paths to the files given in `declarationFile` and `implementationFile` should be either absolute or relative to the datamodel yaml file. The cmake users are encouraged to specify these file via the `DEPENDS` option of `PODIO_GENERATE_DATAMODEL` to add the files as configuration dependency of the datamodel and cause the datamodel re-generation when they change. ## Definition of custom interfaces An interface type can be defined as follows (again using an example from the example datamodel)