From b89567bf4730671ba42a18b78af157151b4c68f5 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sat, 27 Jun 2026 17:18:12 +0200 Subject: [PATCH 1/3] feat(catalog): scan an ordered list of plugin folders with dedup-by-id Add PluginRuntimeCatalog::setPluginDirs() so the host can configure an ordered list of plugin directories. scanDirectory() and reload() now scan every configured directory in order and de-duplicate by manifest id: when the same plugin id appears in more than one folder, the first (highest-priority) folder wins and the later copies are skipped with an info diagnostic. The single-directory API (setPluginDir/ctor) is preserved and maps to a one-element list. Co-Authored-By: Claude Opus 4.8 --- .../host/plugin_runtime_catalog.hpp | 14 +++- pj_plugins/src/plugin_runtime_catalog.cpp | 65 +++++++++++++------ 2 files changed, 59 insertions(+), 20 deletions(-) diff --git a/pj_plugins/include/pj_plugins/host/plugin_runtime_catalog.hpp b/pj_plugins/include/pj_plugins/host/plugin_runtime_catalog.hpp index 5129a33..eb2ec09 100644 --- a/pj_plugins/include/pj_plugins/host/plugin_runtime_catalog.hpp +++ b/pj_plugins/include/pj_plugins/host/plugin_runtime_catalog.hpp @@ -72,6 +72,13 @@ class PluginRuntimeCatalog { // Replaces the directory scanned by scanDirectory() and reload(). void setPluginDir(std::filesystem::path plugin_dir); + // Replaces the ordered list of directories scanned by scanDirectory() and + // reload(). Directories are scanned in order and de-duplicated by manifest id: + // when the same plugin id appears in more than one directory, the first + // (highest-priority) directory wins and the later copies are skipped with an + // info diagnostic. Empty entries are ignored. + void setPluginDirs(std::vector plugin_dirs); + // Replaces the optional diagnostic sink. void setDiagnosticSink(DiagnosticSink sink); @@ -135,6 +142,11 @@ class PluginRuntimeCatalog { [[nodiscard]] std::string listAvailableEncodings() const; private: + // Scans every directory in plugin_dirs_ (in order) and returns the loadable + // descriptors de-duplicated by manifest id (first directory wins). Reports + // scan diagnostics and one info diagnostic per skipped duplicate. + [[nodiscard]] std::vector collectDeduplicatedPlugins() const; + // Loads a descriptor using the family-specific loader. bool loadAndRegister(const PluginDescriptor& descriptor); @@ -159,7 +171,7 @@ class PluginRuntimeCatalog { // Emits diagnostics produced by DSO discovery. void reportScanDiagnostics(const PluginScanResult& scan) const; - std::filesystem::path plugin_dir_; + std::vector plugin_dirs_; DiagnosticSink sink_; std::string diagnostic_source_; std::vector data_sources_; diff --git a/pj_plugins/src/plugin_runtime_catalog.cpp b/pj_plugins/src/plugin_runtime_catalog.cpp index c6e045b..f43a9a6 100644 --- a/pj_plugins/src/plugin_runtime_catalog.cpp +++ b/pj_plugins/src/plugin_runtime_catalog.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include "pj_base/data_source_protocol.h" @@ -60,29 +61,60 @@ std::vector constPtrs(const std::vector& plugins, uint6 PluginRuntimeCatalog::PluginRuntimeCatalog( std::filesystem::path plugin_dir, DiagnosticSink sink, std::string diagnostic_source) - : plugin_dir_(std::move(plugin_dir)), sink_(std::move(sink)), diagnostic_source_(std::move(diagnostic_source)) {} + : sink_(std::move(sink)), diagnostic_source_(std::move(diagnostic_source)) { + if (!plugin_dir.empty()) { + plugin_dirs_.push_back(std::move(plugin_dir)); + } +} void PluginRuntimeCatalog::setPluginDir(std::filesystem::path plugin_dir) { - plugin_dir_ = std::move(plugin_dir); + plugin_dirs_.clear(); + if (!plugin_dir.empty()) { + plugin_dirs_.push_back(std::move(plugin_dir)); + } +} + +void PluginRuntimeCatalog::setPluginDirs(std::vector plugin_dirs) { + plugin_dirs_ = std::move(plugin_dirs); } void PluginRuntimeCatalog::setDiagnosticSink(DiagnosticSink sink) { sink_ = std::move(sink); } +std::vector PluginRuntimeCatalog::collectDeduplicatedPlugins() const { + std::vector winners; + std::unordered_set seen_ids; + for (const std::filesystem::path& dir : plugin_dirs_) { + if (dir.empty()) { + continue; + } + auto scan = scanPluginDsos(dir); + if (!scan) { + report(DiagnosticLevel::kError, {}, scan.error()); + continue; + } + reportScanDiagnostics(*scan); + for (const PluginDescriptor& descriptor : scan->plugins) { + if (!seen_ids.insert(descriptor.id).second) { + report( + DiagnosticLevel::kInfo, descriptor.id, + descriptor.dso_path.string() + ": ignoring duplicate plugin id \"" + descriptor.id + + "\" (already provided by a higher-priority folder)"); + continue; + } + winners.push_back(descriptor); + } + } + return winners; +} + void PluginRuntimeCatalog::scanDirectory() { data_sources_.clear(); message_parsers_.clear(); toolbox_plugins_.clear(); - auto scan = scanPluginDsos(plugin_dir_); - if (!scan) { - report(DiagnosticLevel::kError, {}, scan.error()); - return; - } - reportScanDiagnostics(*scan); - - for (const PluginDescriptor& descriptor : scan->plugins) { + for (const PluginDescriptor& descriptor : collectDeduplicatedPlugins()) { if (!loadAndRegister(descriptor)) { report( DiagnosticLevel::kError, descriptor.id, @@ -92,16 +124,11 @@ void PluginRuntimeCatalog::scanDirectory() { } bool PluginRuntimeCatalog::reload() { - auto scan = scanPluginDsos(plugin_dir_); - if (!scan) { - report(DiagnosticLevel::kError, {}, scan.error()); - return false; - } - reportScanDiagnostics(*scan); + const std::vector plugins = collectDeduplicatedPlugins(); std::vector on_disk; - on_disk.reserve(scan->plugins.size()); - for (const PluginDescriptor& descriptor : scan->plugins) { + on_disk.reserve(plugins.size()); + for (const PluginDescriptor& descriptor : plugins) { on_disk.push_back(canonicalPath(descriptor.dso_path)); } @@ -121,7 +148,7 @@ bool PluginRuntimeCatalog::reload() { drop_missing(message_parsers_, "MessageParser"); drop_missing(toolbox_plugins_, "Toolbox"); - for (const PluginDescriptor& descriptor : scan->plugins) { + for (const PluginDescriptor& descriptor : plugins) { const std::string path = canonicalPath(descriptor.dso_path); const auto disk_mtime = safeMtime(descriptor.dso_path); if (disk_mtime == std::filesystem::file_time_type{}) { From 7ea255c9f65b1035e74f12f57433ac7ff821aff6 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sat, 27 Jun 2026 18:31:02 +0200 Subject: [PATCH 2/3] test(catalog): cover PluginRuntimeCatalog multi-dir dedup-by-id Add two cases to plugin_catalog_test: the same plugin id in two folders loads once from the first (higher-priority) folder; distinct ids across folders all load. Links the test against pj_plugin_runtime_catalog. Co-Authored-By: Claude Opus 4.8 --- pj_plugins/CMakeLists.txt | 2 +- pj_plugins/tests/plugin_catalog_test.cpp | 40 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/pj_plugins/CMakeLists.txt b/pj_plugins/CMakeLists.txt index f052da2..4c94110 100644 --- a/pj_plugins/CMakeLists.txt +++ b/pj_plugins/CMakeLists.txt @@ -386,7 +386,7 @@ target_compile_definitions(plugin_catalog_test PRIVATE ) target_compile_options(plugin_catalog_test PRIVATE ${PJ_WARNING_FLAGS}) target_link_libraries(plugin_catalog_test PRIVATE - pj_plugin_catalog GTest::gtest_main + pj_plugin_catalog pj_plugin_runtime_catalog GTest::gtest_main ) add_dependencies(plugin_catalog_test mock_data_source_plugin mock_json_parser_plugin mock_toolbox_plugin mock_dialog_plugin missing_id_data_source_plugin diff --git a/pj_plugins/tests/plugin_catalog_test.cpp b/pj_plugins/tests/plugin_catalog_test.cpp index 9b9a101..2709647 100644 --- a/pj_plugins/tests/plugin_catalog_test.cpp +++ b/pj_plugins/tests/plugin_catalog_test.cpp @@ -5,6 +5,8 @@ #include +#include "pj_plugins/host/plugin_runtime_catalog.hpp" + #include #include #include @@ -178,5 +180,43 @@ TEST_F(PluginCatalogTest, FamilyToStringRoundTrip) { EXPECT_EQ(toString(PluginFamily::kUnknown), "unknown"); } +TEST_F(PluginCatalogTest, RuntimeCatalogDedupsDuplicateIdFirstFolderWins) { + // The same data-source DSO (manifest id "mock-data-source") placed in two + // folders: setPluginDirs scans them in order and must load it exactly once, + // from the first (higher-priority) folder. + const std::filesystem::path dir_a = dir_ / "a"; + const std::filesystem::path dir_b = dir_ / "b"; + std::filesystem::create_directories(dir_a); + std::filesystem::create_directories(dir_b); + std::filesystem::copy_file(PJ_MOCK_DATA_SOURCE_PLUGIN_PATH, dir_a / pluginFileName("ds")); + std::filesystem::copy_file(PJ_MOCK_DATA_SOURCE_PLUGIN_PATH, dir_b / pluginFileName("ds")); + + PluginRuntimeCatalog catalog; + catalog.setPluginDirs({dir_a, dir_b}); + catalog.scanDirectory(); + + ASSERT_EQ(catalog.dataSources().size(), 1U); + EXPECT_EQ(catalog.dataSources()[0].id, "mock-data-source"); + EXPECT_NE(catalog.dataSources()[0].path.find(dir_a.string()), std::string::npos) + << "the winner must come from the first folder; got " << catalog.dataSources()[0].path; +} + +TEST_F(PluginCatalogTest, RuntimeCatalogLoadsDistinctIdsFromMultipleFolders) { + // Different plugins in different folders all load. + const std::filesystem::path dir_a = dir_ / "a"; + const std::filesystem::path dir_b = dir_ / "b"; + std::filesystem::create_directories(dir_a); + std::filesystem::create_directories(dir_b); + std::filesystem::copy_file(PJ_MOCK_DATA_SOURCE_PLUGIN_PATH, dir_a / pluginFileName("ds")); + std::filesystem::copy_file(PJ_MOCK_TOOLBOX_PLUGIN_PATH, dir_b / pluginFileName("tb")); + + PluginRuntimeCatalog catalog; + catalog.setPluginDirs({dir_a, dir_b}); + catalog.scanDirectory(); + + EXPECT_EQ(catalog.dataSources().size(), 1U); + EXPECT_EQ(catalog.toolboxes().size(), 1U); +} + } // namespace } // namespace PJ From 9ad8e0867b7716825d1e3078afec3590990d1c83 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sun, 28 Jun 2026 16:18:41 +0200 Subject: [PATCH 3/3] fix(catalog test): make dedup winner-folder check Windows-robust The RuntimeCatalogDedupsDuplicateIdFirstFolderWins assertion used a substring match of dir_a.string() against the loaded plugin path, which failed on Windows (slash/drive-case/canonicalisation differences) even though the dedup is correct. Compare at the filesystem level with std::filesystem::equivalent(winner.parent_path(), dir_a) instead. Also applies clang-format (the prior commit skipped the submodule hook). Co-Authored-By: Claude Opus 4.8 --- pj_plugins/tests/plugin_catalog_test.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pj_plugins/tests/plugin_catalog_test.cpp b/pj_plugins/tests/plugin_catalog_test.cpp index 2709647..b6f8b9e 100644 --- a/pj_plugins/tests/plugin_catalog_test.cpp +++ b/pj_plugins/tests/plugin_catalog_test.cpp @@ -5,14 +5,14 @@ #include -#include "pj_plugins/host/plugin_runtime_catalog.hpp" - #include #include #include #include #include +#include "pj_plugins/host/plugin_runtime_catalog.hpp" + namespace PJ { namespace { @@ -197,8 +197,12 @@ TEST_F(PluginCatalogTest, RuntimeCatalogDedupsDuplicateIdFirstFolderWins) { ASSERT_EQ(catalog.dataSources().size(), 1U); EXPECT_EQ(catalog.dataSources()[0].id, "mock-data-source"); - EXPECT_NE(catalog.dataSources()[0].path.find(dir_a.string()), std::string::npos) - << "the winner must come from the first folder; got " << catalog.dataSources()[0].path; + // The winner must come from the first folder. Compare at the filesystem level + // (std::filesystem::equivalent) so it holds regardless of how the stored path + // is spelled — Windows back/forward slashes, drive-letter case, canonicalisation. + const std::filesystem::path winner(catalog.dataSources()[0].path); + EXPECT_TRUE(std::filesystem::equivalent(winner.parent_path(), dir_a)) + << "winner " << winner << " is not in the first folder " << dir_a; } TEST_F(PluginCatalogTest, RuntimeCatalogLoadsDistinctIdsFromMultipleFolders) {