From 8ead09ec8607d703ccc2bcf398cee01fb5a63c6b Mon Sep 17 00:00:00 2001 From: ZakayZ Date: Thu, 15 Oct 2026 19:30:02 +0300 Subject: [PATCH 1/2] Support configured generic factories and portable uninstall --- COLA.hh | 9 +++- README.md | 8 +++ cmake/COLAQualityTools.cmake | 15 +++--- cmake/uninstall.cmake | 25 ++++++++++ tests/CMakeLists.txt | 8 +++ tests/cola_test.cc | 66 ++++++++++++++++++++++++ tests/uninstall_test.cmake | 97 ++++++++++++++++++++++++++++++++++++ 7 files changed, 217 insertions(+), 11 deletions(-) create mode 100644 cmake/uninstall.cmake create mode 100644 tests/uninstall_test.cmake diff --git a/COLA.hh b/COLA.hh index b103456..9fb86dc 100644 --- a/COLA.hh +++ b/COLA.hh @@ -193,8 +193,13 @@ namespace cola { "kName variable must be a string"); } - std::unique_ptr Create(const std::unordered_map& /* metaData */) override { - return std::make_unique(); + // Prefer the metadata constructor when available; keep default-only filters compatible. + std::unique_ptr Create(const std::unordered_map& meta_data) override { + if constexpr (std::is_constructible_v) { + return std::make_unique(meta_data); + } else { + return std::make_unique(); + } } const std::string& GetFilterName() const final { return Filter::kName; } diff --git a/README.md b/README.md index 74dabe8..f547ff8 100644 --- a/README.md +++ b/README.md @@ -34,4 +34,12 @@ Generated documentation will be located in the ```docs/compiled/``` folder. ## COLA Modules +`GenericFactory` passes XML attributes to a filter constructor accepting +`const std::unordered_map&`, when available. Otherwise, +it uses the default constructor. If both constructors exist, the metadata constructor +takes precedence; validation errors from the constructor propagate to the caller. + +The `uninstall` build target removes files listed in the build's `install_manifest.txt`. +It supports paths containing spaces and uses `DESTDIR` when uninstalling a staged installation. + Currently there is a [minimal module + program example](https://github.com/apBUSampK/COLA-min-example) with some guidelines on module writing, as well as [AAMCC-COLA](https://github.com/Spectator-matter-group-INR-RAS/AAMCC-COLA), a COLA implementation of AAMCC-MST model, along with relevant modules. diff --git a/cmake/COLAQualityTools.cmake b/cmake/COLAQualityTools.cmake index fb058dd..7e11d91 100644 --- a/cmake/COLAQualityTools.cmake +++ b/cmake/COLAQualityTools.cmake @@ -1,3 +1,6 @@ +# Resolve beside this module, including when it is used by an installed COLA consumer. +set(_COLA_UNINSTALL_SCRIPT "${CMAKE_CURRENT_LIST_DIR}/uninstall.cmake") + function (setup_cola_quality_tools) cmake_parse_arguments(QUALITY "" "" "EXCLUDE_PATTERNS" ${ARGN}) @@ -79,16 +82,10 @@ function (setup_cola_quality_tools) add_custom_target( uninstall - COMMAND - bash -c " - if [ -f ${CMAKE_BINARY_DIR}/install_manifest.txt ]; then - cat ${CMAKE_BINARY_DIR}/install_manifest.txt | xargs rm -f - echo 'Uninstall completed' - else - echo 'No install_manifest.txt found' - fi - " + COMMAND ${CMAKE_COMMAND} "-DCOLA_INSTALL_MANIFEST=${CMAKE_BINARY_DIR}/install_manifest.txt" + -P "${_COLA_UNINSTALL_SCRIPT}" COMMENT "Uninstalling project" + VERBATIM ) endfunction () diff --git a/cmake/uninstall.cmake b/cmake/uninstall.cmake new file mode 100644 index 0000000..be2d3e2 --- /dev/null +++ b/cmake/uninstall.cmake @@ -0,0 +1,25 @@ +if (NOT DEFINED COLA_INSTALL_MANIFEST) + set(COLA_INSTALL_MANIFEST "${CMAKE_BINARY_DIR}/install_manifest.txt") +endif () + +if (NOT EXISTS "${COLA_INSTALL_MANIFEST}") + message(STATUS "No install_manifest.txt found: ${COLA_INSTALL_MANIFEST}") + return() +endif () + +file(STRINGS "${COLA_INSTALL_MANIFEST}" installed_files) +foreach (installed_file IN LISTS installed_files) + set(staged_file "$ENV{DESTDIR}${installed_file}") + if (EXISTS "${staged_file}" OR IS_SYMLINK "${staged_file}") + execute_process( + COMMAND "${CMAKE_COMMAND}" -E remove "${staged_file}" + RESULT_VARIABLE remove_result + ERROR_VARIABLE remove_error + ) + if (NOT remove_result EQUAL 0) + message(FATAL_ERROR "Could not remove ${staged_file}: ${remove_error}") + endif () + message(STATUS "Removed: ${staged_file}") + endif () +endforeach () +message(STATUS "Uninstall completed") diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a6deb02..adc04e2 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -36,3 +36,11 @@ target_link_libraries(COLATest COLA) target_link_libraries(COLATest GTest::GTest GTest::Main) gtest_discover_tests(COLATest) + +add_test( + NAME ColaInstall.Uninstall + COMMAND + ${CMAKE_COMMAND} "-DCOLA_SOURCE_DIR=${PROJECT_SOURCE_DIR}" + "-DTEST_ROOT=${CMAKE_CURRENT_BINARY_DIR}/uninstall integration" -P + "${CMAKE_CURRENT_SOURCE_DIR}/uninstall_test.cmake" +) diff --git a/tests/cola_test.cc b/tests/cola_test.cc index 77d0bdc..910d868 100644 --- a/tests/cola_test.cc +++ b/tests/cola_test.cc @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -116,6 +117,71 @@ class TestWriter : public VWriter { } }; +namespace { + + class ConfiguredConverter : public VConverter { + public: + // GenericFactory requires a std::string name, matching the other test filters. + // NOLINTNEXTLINE(bugprone-throwing-static-initialization) + static inline const std::string kName = "ConfiguredConverter"; + + explicit ConfiguredConverter(const std::unordered_map& parameters) + : factor_(std::stod(parameters.at("factor"))) {} + + std::unique_ptr operator()(std::unique_ptr&& data) override { + for (auto& particle : data->particles) { + particle.momentum.x *= factor_; + } + return std::move(data); + } + + private: + double factor_; + }; + + class OptionalConfiguredConverter : public ConfiguredConverter { + public: + OptionalConfiguredConverter() : ConfiguredConverter({{"factor", "1"}}) {} + using ConfiguredConverter::ConfiguredConverter; + }; + +} // namespace + +TEST(ColaFactory, MetadataConstructorPreferredOverDefault) { + GenericFactory factory; + auto filter = factory.Create({{"factor", "3"}}); + auto* converter = dynamic_cast(filter.get()); + ASSERT_NE(converter, nullptr); + TestGenerator generator; + const auto result = (*converter)(generator()); + EXPECT_DOUBLE_EQ(result->particles[0].momentum.x, 0.1 * 3); +} + +TEST(ColaFactory, InvalidMetadataPropagates) { + GenericFactory factory; + EXPECT_THROW(factory.Create({}), std::out_of_range); + EXPECT_THROW(factory.Create({{"factor", "invalid"}}), std::invalid_argument); +} + +TEST(ColaFactory, XmlConfiguresNonDefaultConstructibleFilter) { + auto xml = std::stringstream(R"( + + + + )"); + MetaProcessor processor; + processor.Register(std::make_unique>()); + processor.Register(std::make_unique>()); + processor.Register(std::make_unique>()); + auto ensemble = processor.Parse(xml); + auto* writer = dynamic_cast(ensemble.writer.get()); + ASSERT_NE(writer, nullptr); + ColaRunManager manager(std::move(ensemble)); + manager.Run(); + ASSERT_EQ(writer->recorded_events.size(), 1); + EXPECT_DOUBLE_EQ(writer->recorded_events[0]->particles[0].momentum.x, 0.4); +} + TEST(ColaPdgTest, CircularConsistency) { for (uint16_t atomic_mass = 1; atomic_mass < 100; ++atomic_mass) { for (uint16_t charge_number = 0; charge_number <= atomic_mass; ++charge_number) { diff --git a/tests/uninstall_test.cmake b/tests/uninstall_test.cmake new file mode 100644 index 0000000..883e3c1 --- /dev/null +++ b/tests/uninstall_test.cmake @@ -0,0 +1,97 @@ +cmake_minimum_required(VERSION 3.13) + +function (run) + execute_process( + COMMAND ${ARGV} + RESULT_VARIABLE result + OUTPUT_VARIABLE output + ERROR_VARIABLE error + ) + if (NOT result EQUAL 0) + message(FATAL_ERROR "Command failed: ${ARGV}\n${output}\n${error}") + endif () +endfunction () + +file(REMOVE_RECURSE "${TEST_ROOT}") +file(MAKE_DIRECTORY "${TEST_ROOT}/sdk/cmake" "${TEST_ROOT}/consumer") +# The consumer has no cmake/uninstall.cmake: the installed module must find its own sibling. +file(COPY "${COLA_SOURCE_DIR}/cmake/COLAQualityTools.cmake" + "${COLA_SOURCE_DIR}/cmake/uninstall.cmake" DESTINATION "${TEST_ROOT}/sdk/cmake" +) +file(WRITE "${TEST_ROOT}/consumer/payload with spaces.txt" "installed payload\n") +file( + WRITE "${TEST_ROOT}/consumer/CMakeLists.txt" + [=[ +cmake_minimum_required(VERSION 3.13) +project(UninstallConsumer LANGUAGES NONE) +include("${QUALITY_MODULE}") +setup_cola_quality_tools() +install(FILES "${CMAKE_CURRENT_SOURCE_DIR}/payload with spaces.txt" DESTINATION share) +]=] +) + +set(build_dir "${TEST_ROOT}/build") +set(prefix "${TEST_ROOT}/install prefix") +run("${CMAKE_COMMAND}" + -S + "${TEST_ROOT}/consumer" + -B + "${build_dir}" + "-DCMAKE_INSTALL_PREFIX=${prefix}" + "-DQUALITY_MODULE=${TEST_ROOT}/sdk/cmake/COLAQualityTools.cmake" +) + +# No manifest is a successful no-op, as in the previous implementation. +run("${CMAKE_COMMAND}" --build "${build_dir}" --target uninstall) +run("${CMAKE_COMMAND}" --build "${build_dir}" --target install) +if (NOT EXISTS "${prefix}/share/payload with spaces.txt") + message(FATAL_ERROR "Fixture was not installed") +endif () +file(WRITE "${prefix}/keep.txt" "not owned by this installation\n") +run("${CMAKE_COMMAND}" --build "${build_dir}" --target uninstall) +if (EXISTS "${prefix}/share/payload with spaces.txt" OR NOT EXISTS "${prefix}/keep.txt") + message(FATAL_ERROR "Uninstall did not respect the manifest") +endif () +run("${CMAKE_COMMAND}" --build "${build_dir}" --target uninstall) + +# DESTDIR must select staged files, leaving the unstaged installation alone. +file(WRITE "${prefix}/share/payload with spaces.txt" "unstaged sentinel\n") +set(stage "${TEST_ROOT}/stage") +run("${CMAKE_COMMAND}" + -E + env + "DESTDIR=${stage}" + "${CMAKE_COMMAND}" + --build + "${build_dir}" + --target + install +) +if (NOT EXISTS "${stage}${prefix}/share/payload with spaces.txt") + message(FATAL_ERROR "Staged fixture was not installed") +endif () +run("${CMAKE_COMMAND}" + -E + env + "DESTDIR=${stage}" + "${CMAKE_COMMAND}" + --build + "${build_dir}" + --target + uninstall +) +if (EXISTS "${stage}${prefix}/share/payload with spaces.txt" + OR NOT EXISTS "${prefix}/share/payload with spaces.txt" +) + message(FATAL_ERROR "Uninstall did not respect DESTDIR") +endif () + +if (UNIX) + set(link "${prefix}/broken link") + run("${CMAKE_COMMAND}" -E create_symlink "missing-target" "${link}") + file(APPEND "${build_dir}/install_manifest.txt" "\n${link}\n") + run("${CMAKE_COMMAND}" --build "${build_dir}" --target uninstall) + if (IS_SYMLINK "${link}") + message(FATAL_ERROR "Uninstall did not remove a dangling symlink") + endif () +endif () From 33304d68776dc414162fcc6831558ffc49e9d941 Mon Sep 17 00:00:00 2001 From: ZakayZ Date: Thu, 22 Oct 2026 02:31:11 +0300 Subject: [PATCH 2/2] Keep factory PR focused on configured construction --- COLA.hh | 1 - README.md | 8 --- cmake/COLAQualityTools.cmake | 15 +++--- cmake/uninstall.cmake | 25 ---------- tests/CMakeLists.txt | 8 --- tests/cola_test.cc | 1 - tests/uninstall_test.cmake | 97 ------------------------------------ 7 files changed, 9 insertions(+), 146 deletions(-) delete mode 100644 cmake/uninstall.cmake delete mode 100644 tests/uninstall_test.cmake diff --git a/COLA.hh b/COLA.hh index 9fb86dc..455c7ff 100644 --- a/COLA.hh +++ b/COLA.hh @@ -193,7 +193,6 @@ namespace cola { "kName variable must be a string"); } - // Prefer the metadata constructor when available; keep default-only filters compatible. std::unique_ptr Create(const std::unordered_map& meta_data) override { if constexpr (std::is_constructible_v) { return std::make_unique(meta_data); diff --git a/README.md b/README.md index f547ff8..74dabe8 100644 --- a/README.md +++ b/README.md @@ -34,12 +34,4 @@ Generated documentation will be located in the ```docs/compiled/``` folder. ## COLA Modules -`GenericFactory` passes XML attributes to a filter constructor accepting -`const std::unordered_map&`, when available. Otherwise, -it uses the default constructor. If both constructors exist, the metadata constructor -takes precedence; validation errors from the constructor propagate to the caller. - -The `uninstall` build target removes files listed in the build's `install_manifest.txt`. -It supports paths containing spaces and uses `DESTDIR` when uninstalling a staged installation. - Currently there is a [minimal module + program example](https://github.com/apBUSampK/COLA-min-example) with some guidelines on module writing, as well as [AAMCC-COLA](https://github.com/Spectator-matter-group-INR-RAS/AAMCC-COLA), a COLA implementation of AAMCC-MST model, along with relevant modules. diff --git a/cmake/COLAQualityTools.cmake b/cmake/COLAQualityTools.cmake index 7e11d91..fb058dd 100644 --- a/cmake/COLAQualityTools.cmake +++ b/cmake/COLAQualityTools.cmake @@ -1,6 +1,3 @@ -# Resolve beside this module, including when it is used by an installed COLA consumer. -set(_COLA_UNINSTALL_SCRIPT "${CMAKE_CURRENT_LIST_DIR}/uninstall.cmake") - function (setup_cola_quality_tools) cmake_parse_arguments(QUALITY "" "" "EXCLUDE_PATTERNS" ${ARGN}) @@ -82,10 +79,16 @@ function (setup_cola_quality_tools) add_custom_target( uninstall - COMMAND ${CMAKE_COMMAND} "-DCOLA_INSTALL_MANIFEST=${CMAKE_BINARY_DIR}/install_manifest.txt" - -P "${_COLA_UNINSTALL_SCRIPT}" + COMMAND + bash -c " + if [ -f ${CMAKE_BINARY_DIR}/install_manifest.txt ]; then + cat ${CMAKE_BINARY_DIR}/install_manifest.txt | xargs rm -f + echo 'Uninstall completed' + else + echo 'No install_manifest.txt found' + fi + " COMMENT "Uninstalling project" - VERBATIM ) endfunction () diff --git a/cmake/uninstall.cmake b/cmake/uninstall.cmake deleted file mode 100644 index be2d3e2..0000000 --- a/cmake/uninstall.cmake +++ /dev/null @@ -1,25 +0,0 @@ -if (NOT DEFINED COLA_INSTALL_MANIFEST) - set(COLA_INSTALL_MANIFEST "${CMAKE_BINARY_DIR}/install_manifest.txt") -endif () - -if (NOT EXISTS "${COLA_INSTALL_MANIFEST}") - message(STATUS "No install_manifest.txt found: ${COLA_INSTALL_MANIFEST}") - return() -endif () - -file(STRINGS "${COLA_INSTALL_MANIFEST}" installed_files) -foreach (installed_file IN LISTS installed_files) - set(staged_file "$ENV{DESTDIR}${installed_file}") - if (EXISTS "${staged_file}" OR IS_SYMLINK "${staged_file}") - execute_process( - COMMAND "${CMAKE_COMMAND}" -E remove "${staged_file}" - RESULT_VARIABLE remove_result - ERROR_VARIABLE remove_error - ) - if (NOT remove_result EQUAL 0) - message(FATAL_ERROR "Could not remove ${staged_file}: ${remove_error}") - endif () - message(STATUS "Removed: ${staged_file}") - endif () -endforeach () -message(STATUS "Uninstall completed") diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index adc04e2..a6deb02 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -36,11 +36,3 @@ target_link_libraries(COLATest COLA) target_link_libraries(COLATest GTest::GTest GTest::Main) gtest_discover_tests(COLATest) - -add_test( - NAME ColaInstall.Uninstall - COMMAND - ${CMAKE_COMMAND} "-DCOLA_SOURCE_DIR=${PROJECT_SOURCE_DIR}" - "-DTEST_ROOT=${CMAKE_CURRENT_BINARY_DIR}/uninstall integration" -P - "${CMAKE_CURRENT_SOURCE_DIR}/uninstall_test.cmake" -) diff --git a/tests/cola_test.cc b/tests/cola_test.cc index 910d868..ee6cca3 100644 --- a/tests/cola_test.cc +++ b/tests/cola_test.cc @@ -121,7 +121,6 @@ namespace { class ConfiguredConverter : public VConverter { public: - // GenericFactory requires a std::string name, matching the other test filters. // NOLINTNEXTLINE(bugprone-throwing-static-initialization) static inline const std::string kName = "ConfiguredConverter"; diff --git a/tests/uninstall_test.cmake b/tests/uninstall_test.cmake deleted file mode 100644 index 883e3c1..0000000 --- a/tests/uninstall_test.cmake +++ /dev/null @@ -1,97 +0,0 @@ -cmake_minimum_required(VERSION 3.13) - -function (run) - execute_process( - COMMAND ${ARGV} - RESULT_VARIABLE result - OUTPUT_VARIABLE output - ERROR_VARIABLE error - ) - if (NOT result EQUAL 0) - message(FATAL_ERROR "Command failed: ${ARGV}\n${output}\n${error}") - endif () -endfunction () - -file(REMOVE_RECURSE "${TEST_ROOT}") -file(MAKE_DIRECTORY "${TEST_ROOT}/sdk/cmake" "${TEST_ROOT}/consumer") -# The consumer has no cmake/uninstall.cmake: the installed module must find its own sibling. -file(COPY "${COLA_SOURCE_DIR}/cmake/COLAQualityTools.cmake" - "${COLA_SOURCE_DIR}/cmake/uninstall.cmake" DESTINATION "${TEST_ROOT}/sdk/cmake" -) -file(WRITE "${TEST_ROOT}/consumer/payload with spaces.txt" "installed payload\n") -file( - WRITE "${TEST_ROOT}/consumer/CMakeLists.txt" - [=[ -cmake_minimum_required(VERSION 3.13) -project(UninstallConsumer LANGUAGES NONE) -include("${QUALITY_MODULE}") -setup_cola_quality_tools() -install(FILES "${CMAKE_CURRENT_SOURCE_DIR}/payload with spaces.txt" DESTINATION share) -]=] -) - -set(build_dir "${TEST_ROOT}/build") -set(prefix "${TEST_ROOT}/install prefix") -run("${CMAKE_COMMAND}" - -S - "${TEST_ROOT}/consumer" - -B - "${build_dir}" - "-DCMAKE_INSTALL_PREFIX=${prefix}" - "-DQUALITY_MODULE=${TEST_ROOT}/sdk/cmake/COLAQualityTools.cmake" -) - -# No manifest is a successful no-op, as in the previous implementation. -run("${CMAKE_COMMAND}" --build "${build_dir}" --target uninstall) -run("${CMAKE_COMMAND}" --build "${build_dir}" --target install) -if (NOT EXISTS "${prefix}/share/payload with spaces.txt") - message(FATAL_ERROR "Fixture was not installed") -endif () -file(WRITE "${prefix}/keep.txt" "not owned by this installation\n") -run("${CMAKE_COMMAND}" --build "${build_dir}" --target uninstall) -if (EXISTS "${prefix}/share/payload with spaces.txt" OR NOT EXISTS "${prefix}/keep.txt") - message(FATAL_ERROR "Uninstall did not respect the manifest") -endif () -run("${CMAKE_COMMAND}" --build "${build_dir}" --target uninstall) - -# DESTDIR must select staged files, leaving the unstaged installation alone. -file(WRITE "${prefix}/share/payload with spaces.txt" "unstaged sentinel\n") -set(stage "${TEST_ROOT}/stage") -run("${CMAKE_COMMAND}" - -E - env - "DESTDIR=${stage}" - "${CMAKE_COMMAND}" - --build - "${build_dir}" - --target - install -) -if (NOT EXISTS "${stage}${prefix}/share/payload with spaces.txt") - message(FATAL_ERROR "Staged fixture was not installed") -endif () -run("${CMAKE_COMMAND}" - -E - env - "DESTDIR=${stage}" - "${CMAKE_COMMAND}" - --build - "${build_dir}" - --target - uninstall -) -if (EXISTS "${stage}${prefix}/share/payload with spaces.txt" - OR NOT EXISTS "${prefix}/share/payload with spaces.txt" -) - message(FATAL_ERROR "Uninstall did not respect DESTDIR") -endif () - -if (UNIX) - set(link "${prefix}/broken link") - run("${CMAKE_COMMAND}" -E create_symlink "missing-target" "${link}") - file(APPEND "${build_dir}/install_manifest.txt" "\n${link}\n") - run("${CMAKE_COMMAND}" --build "${build_dir}" --target uninstall) - if (IS_SYMLINK "${link}") - message(FATAL_ERROR "Uninstall did not remove a dangling symlink") - endif () -endif ()