From b10ceae9d96a7a6d3c29fc272619a1f9015b4619 Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Wed, 20 Jul 2022 13:11:47 -0700 Subject: [PATCH 01/14] Refactor loader in preparation to support statically linked plugins Signed-off-by: Shameek Ganguly --- loader/CMakeLists.txt | 3 +- loader/include/gz/plugin/detail/Registry.hh | 174 ++++++++++++++ loader/src/Loader.cc | 214 ++--------------- loader/src/detail/Registry.cc | 242 ++++++++++++++++++++ 4 files changed, 436 insertions(+), 197 deletions(-) create mode 100644 loader/include/gz/plugin/detail/Registry.hh create mode 100644 loader/src/detail/Registry.cc diff --git a/loader/CMakeLists.txt b/loader/CMakeLists.txt index b4cd4751..4910373b 100644 --- a/loader/CMakeLists.txt +++ b/loader/CMakeLists.txt @@ -8,8 +8,9 @@ if (MSVC OR NOT GZ_TOOLS_PROGRAM) endif() # Create the library target +file(GLOB detail_sources RELATIVE "${CMAKE_CURRENT_LIST_DIR}" "src/detail/*.cc") gz_add_component(loader - SOURCES ${sources} + SOURCES ${sources} ${detail_sources} GET_TARGET_NAME loader) target_link_libraries(${loader} diff --git a/loader/include/gz/plugin/detail/Registry.hh b/loader/include/gz/plugin/detail/Registry.hh new file mode 100644 index 00000000..0f7173b7 --- /dev/null +++ b/loader/include/gz/plugin/detail/Registry.hh @@ -0,0 +1,174 @@ +/* + * Copyright (C) 2022 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + + +#ifndef GZ_PLUGIN_DETAIL_REGISTRY_HH_ +#define GZ_PLUGIN_DETAIL_REGISTRY_HH_ + +#include +#include +#include +#include +#include +#include + +#include + +namespace gz +{ + namespace plugin + { + /// \brief Manages a set of plugin Infos and allows querying by name or + /// by alias. + class Registry { + public: Registry() = default; + + public: virtual ~Registry() = default; + + /// \brief Makes a printable string with info about plugins + /// + /// \returns A pretty string + public: std::string PrettyStr() const; + + /// \brief Get demangled names of interfaces that the registry has plugins + /// for. + /// + /// \returns Demangled names of the interfaces that are implemented + public: std::unordered_set InterfacesImplemented() const; + + /// \brief Get plugin names that implement the specified interface + /// + /// \return names of plugins that implement the interface. + public: template + std::unordered_set PluginsImplementing() const + { + return this->PluginsImplementing(typeid(Interface).name(), false); + } + + /// \brief Get plugin names that implement the specified interface string. + /// Note that the templated version of this function is recommended + /// instead of this version to avoid confusion about whether a mangled or + /// demangled version of a string is being used. Note that the function + /// InterfacesImplemented() returns demangled versions of the interface + /// names. + /// + /// If you want to pass in a mangled version of an interface name, e.g. + /// the result that would be produced by typeid(T).name(), then set + /// `demangled` to false. + /// + /// \param[in] _interface + /// Name of an interface + /// + /// \param[in] _demangled + /// Specify whether the _interface string is demangled (default, true) + /// or mangled (false). + /// + /// \returns Names of plugins that implement the interface + public: std::unordered_set PluginsImplementing( + const std::string &_interface, + const bool _demangled = true) const; + + /// \brief Get plugin names that correspond to the specified alias string. + /// + /// If there is more than one entry in this set, then the alias cannot be + /// used to instantiate any of those plugins. + /// + /// If the name of a plugin matches the alias string, then that plugin + /// will be instantiated any time the string is used to instantiate a + /// plugin, no matter how many other plugins use the alias. + /// + /// \param[in] _alias + /// The std::string of the alias + /// + /// \return A set of plugins that correspond to the desired alias + public: std::set PluginsWithAlias( + const std::string &_alias) const; + + /// \brief Get the aliases of the plugin with the given name + /// + /// \param[in] _pluginName + /// The name of the desired plugin + /// + /// \return A set of aliases corresponding to the desired plugin + public: std::set AliasesOfPlugin( + const std::string &_pluginName) const; + + /// \brief Resolve the plugin name or alias into the name of the plugin + /// that it maps to. If this is a name or alias that does not uniquely map + /// to a known plugin, then the return value will be an empty string. + /// + /// \param[in] _nameOrAlias + /// The name or alias of the plugin of interest. + /// + /// \return The name of the plugin being referred to, or an empty string + /// if no such plugin is known. + public: std::string LookupPlugin(const std::string &_nameOrAlias) const; + + /// \brief Get a set of the names of all plugins that are in this registry. + /// + /// Method is virtual to allow other Info storage and memory management + /// models. + /// + /// \return A set of all plugin names in this registry. + public: virtual std::set AllPlugins() const; + + /// \brief Get info as ConstInfoPtr. + /// + /// Method is virtual to allow other Info storage and memory management + /// models. + /// + /// \param[in] _pluginName + /// Name of the plugin as returned by LookupPlugin(~). + public: virtual ConstInfoPtr GetInfo(const std::string &_pluginName) const; + + /// \brief Add a new plugin info. + /// + /// Tracked aliases in this registry are also updated. + /// + /// \param[in] _info + /// Plugin info to add. + /// + /// \return True if the info was added, false if a plugin with this name + /// already exists in the registry. + public: virtual bool AddInfo(const Info &_info); + + /// \brief Forget a plugin info. + /// + /// Tracked aliases in this registry are also updated. + /// + /// \param[in] _pluginName + /// Name of the plugin as returned by LookupPlugin(~). + public: virtual void ForgetInfo(const std::string &_pluginName); + + public: Registry(const Registry&) = delete; + + public: Registry& operator=(Registry&) = delete; + + protected: using AliasMap = std::map>; + /// \brief A map from known alias names to the plugin names that they + /// correspond to. Since an alias might refer to more than one plugin, the + /// key of this map is a set. + protected: AliasMap aliases; + + protected: using PluginMap = std::unordered_map; + /// \brief A map from known plugin names to their Info. + protected: PluginMap plugins; + }; + } +} + +#endif diff --git a/loader/src/Loader.cc b/loader/src/Loader.cc index fde3f7e4..94df8b37 100644 --- a/loader/src/Loader.cc +++ b/loader/src/Loader.cc @@ -29,7 +29,7 @@ #include #include #include - +#include #include namespace gz @@ -60,19 +60,6 @@ namespace gz /// \sa Loader::ForgetLibrary() public: bool ForgetLibrary(void *_dlHandle); - /// \brief Pass in a plugin name or alias, and this will give back the - /// plugin name that corresponds to it. If the name or alias could not be - /// found, this returns an empty string. - /// \return The demangled symbol name of the desired plugin, or an empty - /// string if no matching plugin could be found. - public: std::string LookupPlugin(const std::string &_nameOrAlias) const; - - public: using AliasMap = std::map>; - /// \brief A map from known alias names to the plugin names that they - /// correspond to. Since an alias might refer to more than one plugin, the - /// key of this map is a set. - public: AliasMap aliases; - public: using PluginToDlHandleMap = std::unordered_map< std::string, std::shared_ptr >; /// \brief A map from known plugin names to the handle of the library that @@ -90,16 +77,13 @@ namespace gz /// maintain the ordering of these member variables. public: PluginToDlHandleMap pluginToDlHandlePtrs; - public: using PluginMap = std::unordered_map; - /// \brief A map from known plugin names to their Info - /// - /// CRUCIAL DEV NOTE (MXG): `plugins` MUST come AFTER + /// CRUCIAL DEV NOTE (MXG): `filePlugins` MUST come AFTER /// `pluginToDlHandlePtrs` in this class definition. See the comment on /// pluginToDlHandlePtrs for an explanation. /// /// If you change this class definition for ANY reason, be sure to /// maintain the ordering of these member variables. - public: PluginMap plugins; + public: Registry filePlugins; using DlHandleMap = std::unordered_map< void*, std::weak_ptr >; /// \brief A map which keeps track of which shared libraries have been @@ -127,65 +111,7 @@ namespace gz ///////////////////////////////////////////////// std::string Loader::PrettyStr() const { - auto interfaces = this->InterfacesImplemented(); - std::stringstream pretty; - pretty << "Loader State" << std::endl; - pretty << "\tKnown Interfaces: " << interfaces.size() << std::endl; - for (auto const &interface : interfaces) - pretty << "\t\t" << interface << std::endl; - - pretty << "\tKnown Plugins: " << dataPtr->plugins.size() << std::endl; - for (const auto &pair : dataPtr->plugins) - { - const ConstInfoPtr &plugin = pair.second; - const std::size_t aSize = plugin->aliases.size(); - - pretty << "\t\t[" << plugin->name << "]\n"; - if (0 < aSize) - { - pretty << "\t\t\thas " - << aSize << (aSize == 1? " alias" : " aliases") << ":\n"; - for (const auto &alias : plugin->aliases) - pretty << "\t\t\t\t[" << alias << "]\n"; - } - else - { - pretty << "\t\t\thas no aliases\n"; - } - - const std::size_t iSize = plugin->interfaces.size(); - pretty << "\t\t\timplements " << iSize - << (iSize == 1? " interface" : " interfaces") << ":\n"; - for (const auto &interface : plugin->demangledInterfaces) - pretty << "\t\t\t\t" << interface << "\n"; - } - - Implementation::AliasMap badAliases; - for (const auto &entry : this->dataPtr->aliases) - { - if (entry.second.size() > 1) - { - badAliases.insert(entry); - } - } - - if (!badAliases.empty()) - { - const std::size_t aSize = badAliases.size(); - pretty << "\tThere " << (aSize == 1? "is " : "are ") << aSize - << (aSize == 1? " alias" : " aliases") << " with a " - << "name collision:\n"; - for (const auto &alias : badAliases) - { - pretty << "\t\t[" << alias.first << "] collides between:\n"; - for (const auto &name : alias.second) - pretty << "\t\t\t[" << name << "]\n"; - } - } - - pretty << std::endl; - - return pretty.str(); + return this->dataPtr->filePlugins.PrettyStr(); } ///////////////////////////////////////////////// @@ -225,17 +151,12 @@ namespace gz // Demangle the plugin name before creating an entry for it. plugin.name = DemangleSymbol(plugin.name); - // Add the plugin's aliases to the alias map - for (const std::string &alias : plugin.aliases) - this->dataPtr->aliases[alias].insert(plugin.name); - // Make a list of the demangled interface names for later convenience. for (auto const &interface : plugin.interfaces) plugin.demangledInterfaces.insert(DemangleSymbol(interface.first)); // Add the plugin to the map - this->dataPtr->plugins.insert( - std::make_pair(plugin.name, std::make_shared(plugin))); + this->dataPtr->filePlugins.AddInfo(plugin); // Add the plugin's name to the set of newPlugins newPlugins.insert(plugin.name); @@ -252,93 +173,41 @@ namespace gz ///////////////////////////////////////////////// std::unordered_set Loader::InterfacesImplemented() const { - std::unordered_set interfaces; - for (auto const &plugin : this->dataPtr->plugins) - { - for (auto const &interface : plugin.second->demangledInterfaces) - interfaces.insert(interface); - } - return interfaces; + return this->dataPtr->filePlugins.InterfacesImplemented(); } ///////////////////////////////////////////////// std::unordered_set Loader::PluginsImplementing( const std::string &_interface, - const bool demangled) const + const bool _demangled) const { - std::unordered_set plugins; - - if (demangled) - { - for (auto const &plugin : this->dataPtr->plugins) - { - if (plugin.second->demangledInterfaces.find(_interface) != - plugin.second->demangledInterfaces.end()) - plugins.insert(plugin.second->name); - } - } - else - { - for (auto const &plugin : this->dataPtr->plugins) - { - if (plugin.second->interfaces.find(_interface) != - plugin.second->interfaces.end()) - plugins.insert(plugin.second->name); - } - } - - return plugins; + return this->dataPtr->filePlugins.PluginsImplementing(_interface, _demangled); } ///////////////////////////////////////////////// std::set Loader::AllPlugins() const { - std::set result; - - for (const auto &entry : this->dataPtr->plugins) - result.insert(result.end(), entry.first); - - return result; + return this->dataPtr->filePlugins.AllPlugins(); } ///////////////////////////////////////////////// std::set Loader::PluginsWithAlias( const std::string &_alias) const { - std::set result; - - const Implementation::AliasMap::const_iterator names = - this->dataPtr->aliases.find(_alias); - - if (names != this->dataPtr->aliases.end()) - result = names->second; - - const Implementation::PluginMap::const_iterator plugin = - this->dataPtr->plugins.find(_alias); - - if (plugin != this->dataPtr->plugins.end()) - result.insert(_alias); - - return result; + return this->dataPtr->filePlugins.PluginsWithAlias(_alias); } ///////////////////////////////////////////////// std::set Loader::AliasesOfPlugin( const std::string &_pluginName) const { - const Implementation::PluginMap::const_iterator plugin = - this->dataPtr->plugins.find(_pluginName); - - if (plugin != this->dataPtr->plugins.end()) - return plugin->second->aliases; - - return {}; + return this->dataPtr->filePlugins.AliasesOfPlugin(_pluginName); } ///////////////////////////////////////////////// std::string Loader::LookupPlugin(const std::string &_nameOrAlias) const { - return this->dataPtr->LookupPlugin(_nameOrAlias); + return this->dataPtr->filePlugins.LookupPlugin(_nameOrAlias); } ///////////////////////////////////////////////// @@ -402,10 +271,9 @@ namespace gz ConstInfoPtr Loader::PrivateGetInfo( const std::string &_resolvedName) const { - const Implementation::PluginMap::const_iterator it = - this->dataPtr->plugins.find(_resolvedName); + ConstInfoPtr info = this->dataPtr->filePlugins.GetInfo(_resolvedName); - if (this->dataPtr->plugins.end() == it) + if (info == nullptr) { // LCOV_EXCL_START std::cerr << "[gz::Loader::PrivateGetInfo] A resolved name [" @@ -416,7 +284,7 @@ namespace gz // LCOV_EXCL_STOP } - return it->second; + return info; } ///////////////////////////////////////////////// @@ -639,44 +507,6 @@ namespace gz return loadedPlugins; } - ///////////////////////////////////////////////// - std::string Loader::Implementation::LookupPlugin( - const std::string &_nameOrAlias) const - { - const PluginMap::const_iterator name = this->plugins.find(_nameOrAlias); - - if (this->plugins.end() != name) - return _nameOrAlias; - - const AliasMap::const_iterator alias = this->aliases.find(_nameOrAlias); - if (this->aliases.end() != alias && !alias->second.empty()) - { - if (alias->second.size() == 1) - return *alias->second.begin(); - - // We use a stringstream because we're going to output to std::cerr, and - // we want it all to print at once, but std::cerr does not support - // buffering. - std::stringstream ss; - - ss << "[gz::plugin::Loader::LookupPlugin] Failed to resolve the " - << "alias [" << _nameOrAlias << "] because it refers to multiple " - << "plugins:\n"; - for (const std::string &plugin : alias->second) - ss << " -- [" << plugin << "]\n"; - - std::cerr << ss.str(); - - return ""; - } - - std::cerr << "[gz::plugin::Loader::LookupPlugin] Failed to get " - << "info for [" << _nameOrAlias << "]. Could not find a plugin " - << "with that name or alias.\n"; - - return ""; - } - ///////////////////////////////////////////////// bool Loader::Implementation::ForgetLibrary(void *_dlHandle) { @@ -686,14 +516,6 @@ namespace gz const std::unordered_set &forgottenPlugins = it->second; - for (const std::string &forget : forgottenPlugins) - { - // Erase each alias entry corresponding to this plugin - const ConstInfoPtr &info = plugins.at(forget); - for (const std::string &alias : info->aliases) - this->aliases.at(alias).erase(info->name); - } - for (const std::string &forget : forgottenPlugins) { // CRUCIAL DEV NOTE (MXG): Be sure to erase the Info from @@ -702,10 +524,10 @@ namespace gz // for the destructors of their `deleter` member variables. // This erase should come FIRST. - plugins.erase(forget); + this->filePlugins.ForgetInfo(forget); // This erase should come LAST. - pluginToDlHandlePtrs.erase(forget); + this->pluginToDlHandlePtrs.erase(forget); } // Dev note (MXG): We do not need to delete anything from `dlHandlePtrMap` @@ -714,7 +536,7 @@ namespace gz // Dev note (MXG): This erase call should come at the very end of this // function to ensure that the `forgottenPlugins` reference remains valid // while it is being used. - dlHandleToPluginMap.erase(it); + this->dlHandleToPluginMap.erase(it); // Dev note (MXG): We do not need to call dlclose because that will be // taken care of automatically by the std::shared_ptr that manages the diff --git a/loader/src/detail/Registry.cc b/loader/src/detail/Registry.cc new file mode 100644 index 00000000..66445d08 --- /dev/null +++ b/loader/src/detail/Registry.cc @@ -0,0 +1,242 @@ +/* + * Copyright (C) 2022 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + + +#include +#include + +#include + +namespace gz +{ + namespace plugin + { + ///////////////////////////////////////////////// + std::string Registry::PrettyStr() const + { + auto interfaces = this->InterfacesImplemented(); + std::stringstream pretty; + pretty << "Registry state" << std::endl; + pretty << "\tKnown Interfaces: " << interfaces.size() << std::endl; + for (auto const &interface : interfaces) + pretty << "\t\t" << interface << std::endl; + + std::set pluginNames = this->AllPlugins(); + + pretty << "\tKnown Plugins: " << pluginNames.size() << std::endl; + for (const auto &name : pluginNames) + { + const ConstInfoPtr &plugin = this->GetInfo(name); + const std::size_t aSize = plugin->aliases.size(); + + pretty << "\t\t[" << plugin->name << "]\n"; + if (0 < aSize) + { + pretty << "\t\t\thas " + << aSize << (aSize == 1? " alias" : " aliases") << ":\n"; + for (const auto &alias : plugin->aliases) + pretty << "\t\t\t\t[" << alias << "]\n"; + } + else + { + pretty << "\t\t\thas no aliases\n"; + } + + const std::size_t iSize = plugin->interfaces.size(); + pretty << "\t\t\timplements " << iSize + << (iSize == 1? " interface" : " interfaces") << ":\n"; + for (const auto &interface : plugin->demangledInterfaces) + pretty << "\t\t\t\t" << interface << "\n"; + } + + AliasMap badAliases; + for (const auto &entry : this->aliases) + { + if (entry.second.size() > 1) + { + badAliases.insert(entry); + } + } + + if (!badAliases.empty()) + { + const std::size_t aSize = badAliases.size(); + pretty << "\tThere " << (aSize == 1? "is " : "are ") << aSize + << (aSize == 1? " alias" : " aliases") << " with a " + << "name collision:\n"; + for (const auto &alias : badAliases) + { + pretty << "\t\t[" << alias.first << "] collides between:\n"; + for (const auto &name : alias.second) + pretty << "\t\t\t[" << name << "]\n"; + } + } + + pretty << std::endl; + + return pretty.str(); + } + + ///////////////////////////////////////////////// + std::unordered_set Registry::InterfacesImplemented() const + { + std::set pluginNames = this->AllPlugins(); + std::unordered_set interfaces; + for (auto const &name : pluginNames) + { + const ConstInfoPtr &plugin = this->GetInfo(name); + for (auto const &interface : plugin->demangledInterfaces) + interfaces.insert(interface); + } + return interfaces; + } + + ///////////////////////////////////////////////// + std::unordered_set Registry::PluginsImplementing( + const std::string &_interface, + const bool demangled) const + { + std::set allPlugins = this->AllPlugins(); + std::unordered_set plugins; + + for (auto const &name : allPlugins) + { + const ConstInfoPtr &plugin = this->GetInfo(name); + if (demangled) + { + if (plugin->demangledInterfaces.find(_interface) != + plugin->demangledInterfaces.end()) + plugins.insert(plugin->name); + } + else + { + if (plugin->interfaces.find(_interface) != + plugin->interfaces.end()) + plugins.insert(plugin->name); + } + } + + return plugins; + } + + ///////////////////////////////////////////////// + std::set Registry::PluginsWithAlias( + const std::string &_alias) const + { + std::set result; + + const AliasMap::const_iterator names = this->aliases.find(_alias); + + if (names != this->aliases.end()) + result = names->second; + + ConstInfoPtr plugin = this->GetInfo(_alias); + + if (plugin != nullptr) + result.insert(_alias); + + return result; + } + + ///////////////////////////////////////////////// + std::set Registry::AliasesOfPlugin( + const std::string &_pluginName) const + { + ConstInfoPtr plugin = this->GetInfo(_pluginName); + + if (plugin != nullptr) + return plugin->aliases; + + return {}; + } + + ///////////////////////////////////////////////// + std::string Registry::LookupPlugin(const std::string &_nameOrAlias) const + { + ConstInfoPtr pluginPtr = this->GetInfo(_nameOrAlias); + + if (pluginPtr != nullptr) + return _nameOrAlias; + + const AliasMap::const_iterator alias = this->aliases.find(_nameOrAlias); + if (this->aliases.end() != alias && !alias->second.empty()) + { + if (alias->second.size() == 1) + return *alias->second.begin(); + + // We use a stringstream because we're going to output to std::cerr, and + // we want it all to print at once, but std::cerr does not support + // buffering. + std::stringstream ss; + + ss << "[gz::plugin::Registry::LookupPlugin] Failed to resolve the " + << "alias [" << _nameOrAlias << "] because it refers to multiple " + << "plugins:\n"; + for (const std::string &plugin : alias->second) + ss << " -- [" << plugin << "]\n"; + + std::cerr << ss.str(); + + return ""; + } + + return ""; + } + + ///////////////////////////////////////////////// + std::set Registry::AllPlugins() const + { + std::set result; + + for (const auto &entry : this->plugins) + result.insert(result.end(), entry.first); + + return result; + } + + ///////////////////////////////////////////////// + ConstInfoPtr Registry::GetInfo(const std::string &_pluginName) const { + const PluginMap::const_iterator it = this->plugins.find(_pluginName); + if (this->plugins.end() == it) + return nullptr; + return it->second; + } + + ///////////////////////////////////////////////// + bool Registry::AddInfo(const Info &_info) { + for (const std::string &alias : _info.aliases) + this->aliases[alias].insert(_info.name); + + auto result = this->plugins.insert( + std::make_pair(_info.name, std::make_shared(_info))); + + return result.second; + } + + ///////////////////////////////////////////////// + void Registry::ForgetInfo(const std::string &_pluginName) { + ConstInfoPtr info = this->GetInfo(_pluginName); + if (info == nullptr) + return; + + for (const std::string &alias : info->aliases) + this->aliases.at(alias).erase(info->name); + this->plugins.erase(_pluginName); + return; + } + } +} From 5f23e1f71590cd9d2155cdbee47f407bb8fe8b39 Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Thu, 21 Jul 2022 10:52:57 -0700 Subject: [PATCH 02/14] Enable plugin instantiation without shared lib handle Signed-off-by: Shameek Ganguly --- core/include/gz/plugin/Plugin.hh | 8 ++++++++ core/include/gz/plugin/PluginPtr.hh | 5 +++++ core/include/gz/plugin/detail/PluginPtr.hh | 9 +++++++++ core/src/Plugin.cc | 21 +++++++++++++++++++-- 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/core/include/gz/plugin/Plugin.hh b/core/include/gz/plugin/Plugin.hh index 79a627ba..0b601853 100644 --- a/core/include/gz/plugin/Plugin.hh +++ b/core/include/gz/plugin/Plugin.hh @@ -157,6 +157,14 @@ namespace gz const ConstInfoPtr &_info, const std::shared_ptr &_dlHandlePtr) const; + /// \brief Create a new plugin instance based on the info provided for a + /// plugin from the static plugin loader registry. + /// \param[in] _info + /// Pointer to the Info for this plugin + private: void PrivateCreateStaticPluginInstance( + const ConstInfoPtr &_info) const; + + /// \brief Get a reference to the abstract instance being managed by this /// wrapper private: const std::shared_ptr &PrivateGetInstancePtr() const; diff --git a/core/include/gz/plugin/PluginPtr.hh b/core/include/gz/plugin/PluginPtr.hh index 605a5819..b0242829 100644 --- a/core/include/gz/plugin/PluginPtr.hh +++ b/core/include/gz/plugin/PluginPtr.hh @@ -206,6 +206,11 @@ namespace gz private: explicit TemplatePluginPtr( const ConstInfoPtr &_info, const std::shared_ptr &_dlHandlePtr); + + /// \brief Private constructor. Used by the Loader to instantiate a + /// plugin class from the static registry. + /// \param[in] _info An Info instance that was generated by Loader. + private: explicit TemplatePluginPtr(const ConstInfoPtr &_info); }; /// \brief Typical usage for TemplatePluginPtr is to just hold a generic diff --git a/core/include/gz/plugin/detail/PluginPtr.hh b/core/include/gz/plugin/detail/PluginPtr.hh index fa4e3af2..2f111f2c 100644 --- a/core/include/gz/plugin/detail/PluginPtr.hh +++ b/core/include/gz/plugin/detail/PluginPtr.hh @@ -176,6 +176,15 @@ namespace gz { dataPtr->PrivateCreatePluginInstance(_info, _dlHandlePtr); } + + ////////////////////////////////////////////////// + template + TemplatePluginPtr::TemplatePluginPtr( + const ConstInfoPtr &_info) + : dataPtr(new PluginType) + { + dataPtr->PrivateCreateStaticPluginInstance(_info); + } } } diff --git a/core/src/Plugin.cc b/core/src/Plugin.cc index 06273b80..3743b249 100644 --- a/core/src/Plugin.cc +++ b/core/src/Plugin.cc @@ -128,9 +128,13 @@ namespace gz /// \param[in] _info Information describing the plugin to initialize /// \param[in] _dlHandlePtr A reference to the dl handle that manages the /// lifecycle of the plugin library. + /// \param[in] _allowNullDlHandlePtr Allow _dlHandlePtr to be null. Only + /// set true for plugin instances created from the static + /// registry. public: void Create( const ConstInfoPtr &_info, - const std::shared_ptr &_dlHandlePtr) + const std::shared_ptr &_dlHandlePtr, + bool _allowNullDlHandlePtr=false) { this->Clear(); @@ -139,7 +143,7 @@ namespace gz this->info = _info; - if (!_dlHandlePtr) + if (!_dlHandlePtr && !_allowNullDlHandlePtr) { // LCOV_EXCL_START std::cerr << "Received Info for [" << _info->name << "], " @@ -148,6 +152,11 @@ namespace gz assert(false); return; // LCOV_EXCL_STOP + } else if (_dlHandlePtr != nullptr && _allowNullDlHandlePtr) { + std::cerr << "Trying to create a plugin from a static plugin, but " + << "a shared library handle was provided. This should " + << "never happen! Please report this bug!\n"; + assert(false); } // Create a std::shared_ptr to a struct which ensures that the @@ -359,6 +368,14 @@ namespace gz this->dataPtr->Create(_info, _dlHandlePtr); } + ////////////////////////////////////////////////// + void Plugin::PrivateCreateStaticPluginInstance( + const ConstInfoPtr &_info) const + { + this->dataPtr->Create(_info, /*_dlHandlePtr=*/nullptr, + /*_allowNullDlHandlePtr=*/true); + } + ////////////////////////////////////////////////// const std::shared_ptr &Plugin::PrivateGetInstancePtr() const { From dc606265111a9d8794df4b1d04caf80ac739fbb0 Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Thu, 21 Jul 2022 10:56:45 -0700 Subject: [PATCH 03/14] Add a singleton registry for plugin classes that are statically linked in Signed-off-by: Shameek Ganguly --- .../gz/plugin/detail/StaticRegistry.hh | 74 +++++++++++++ loader/src/detail/StaticRegistry.cc | 103 ++++++++++++++++++ 2 files changed, 177 insertions(+) create mode 100644 loader/include/gz/plugin/detail/StaticRegistry.hh create mode 100644 loader/src/detail/StaticRegistry.cc diff --git a/loader/include/gz/plugin/detail/StaticRegistry.hh b/loader/include/gz/plugin/detail/StaticRegistry.hh new file mode 100644 index 00000000..5853c640 --- /dev/null +++ b/loader/include/gz/plugin/detail/StaticRegistry.hh @@ -0,0 +1,74 @@ +/* + * Copyright (C) 2022 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + + +#ifndef GZ_PLUGIN_DETAIL_STATICREGISTRY_HH_ +#define GZ_PLUGIN_DETAIL_STATICREGISTRY_HH_ + +#include + +#include + +namespace gz +{ + namespace plugin + { + /// \brief Static registry of plugin classes populated from + /// gz/plugin/RegisterStatic.hh + class StaticRegistry final: public Registry { + /// \brief Get a reference to the StaticRegistry instance. + public: static StaticRegistry& GetInstance(); + + /// \brief Get a set of the names of all plugins that are in this registry. + /// + /// \return A set of all plugin names in this registry. + public: virtual std::set AllPlugins() const override; + + /// \brief Get info as ConstInfoPtr. + /// + /// \param[in] _pluginName + /// Name of the plugin as returned by LookupPlugin(~). + public: virtual ConstInfoPtr GetInfo(const std::string &_pluginName) const override; + + /// \brief Register Info for a new plugin. + /// + /// This happens automatically when the macros defined in + /// gz/plugin/RegisterStatic.hh are called in a plugin library. So this + /// method is assumed to be called only during program start. + /// + /// \param[in] _info + /// Info for a plugin class. + /// + /// \return Always returns true. + public: virtual bool AddInfo(const Info &_info) override; + + /// \brief This function is a no-op for the static registry. + /// + /// Registered plugins cannot be removed. + /// + /// \param[in] _pluginName + /// Name of the plugin as returned by LookupPlugin(~). + public: virtual void ForgetInfo(const std::string &_pluginName) override {} + + protected: StaticRegistry() = default; + + private: InfoMap infos; + }; + } +} + +#endif diff --git a/loader/src/detail/StaticRegistry.cc b/loader/src/detail/StaticRegistry.cc new file mode 100644 index 00000000..2c47b8ff --- /dev/null +++ b/loader/src/detail/StaticRegistry.cc @@ -0,0 +1,103 @@ +/* + * Copyright (C) 2022 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + + +#include +#include + +namespace gz +{ + namespace plugin + { + ///////////////////////////////////////////////// + StaticRegistry &StaticRegistry::GetInstance() + { + static std::unique_ptr instance(new StaticRegistry()); + return *instance; + } + + ///////////////////////////////////////////////// + std::set StaticRegistry::AllPlugins() const + { + std::set result; + + for (const auto &entry : this->infos) + result.insert(result.end(), entry.first); + + return result; + } + + ///////////////////////////////////////////////// + bool StaticRegistry::AddInfo(const Info& _info) + { + InfoMap::iterator it; + bool inserted; + + std::string pluginName = DemangleSymbol(_info.name); + + // We use insert to ensure that we do not accidentally overwrite some + // existing information for the plugin that has this name. + std::tie(it, inserted) = this->infos.insert( + std::make_pair(pluginName, _info)); + + if (inserted) + { + it->second.name = pluginName; + for (const auto &interfaceMapEntry : _info.interfaces) + { + it->second.demangledInterfaces.insert( + DemangleSymbol(interfaceMapEntry.first)); + } + } + else + { + // If the object was not inserted, then an entry already existed for + // this plugin type. We should still insert each of the interface map + // entries provided by the input info, just in case any of + // them are missing from the currently existing entry. This allows the + // user to specify different interfaces for the same plugin + // type using different macros in different locations or across + // multiple translation units. + for (const auto &interfaceMapEntry : _info.interfaces) + { + it->second.interfaces.insert(interfaceMapEntry); + it->second.demangledInterfaces.insert( + DemangleSymbol(interfaceMapEntry.first)); + } + + // Add aliases + for (const std::string &alias : _info.aliases) + it->second.aliases.insert(alias); + } + + for (const std::string &alias : _info.aliases) + this->aliases[alias].insert(pluginName); + + return true; + } + + ///////////////////////////////////////////////// + ConstInfoPtr StaticRegistry::GetInfo( + const std::string &_pluginName) const + { + const InfoMap::const_iterator it = this->infos.find(_pluginName); + if (this->infos.end() == it) + return nullptr; + return std::make_shared(it->second); + } + } +} From f4f0ae14f06ff16d2889608b518360354ce3d852 Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Thu, 21 Jul 2022 11:04:09 -0700 Subject: [PATCH 04/14] Refactor Register implementation in preparation for RegisterStatic Signed-off-by: Shameek Ganguly --- register/include/gz/plugin/detail/Common.hh | 180 ++++++++++++++++++ register/include/gz/plugin/detail/Register.hh | 149 +-------------- 2 files changed, 183 insertions(+), 146 deletions(-) create mode 100644 register/include/gz/plugin/detail/Common.hh diff --git a/register/include/gz/plugin/detail/Common.hh b/register/include/gz/plugin/detail/Common.hh new file mode 100644 index 00000000..d7f2cc75 --- /dev/null +++ b/register/include/gz/plugin/detail/Common.hh @@ -0,0 +1,180 @@ +/* + * Copyright (C) 2022 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ + + +#ifndef GZ_PLUGIN_DETAIL_COMMON_HH_ +#define GZ_PLUGIN_DETAIL_COMMON_HH_ + +#include +#include +#include +#include + +#include +#include + + +namespace gz +{ + namespace plugin + { + namespace detail + { + ////////////////////////////////////////////////// + /// \brief This default will be called when NoMoreInterfaces is an empty + /// parameter pack. When one or more Interfaces are provided, the other + /// template specialization of this class will be called. + template + struct InterfaceHelper + { + public: static void InsertInterfaces(Info::InterfaceCastingMap &) + { + // Do nothing. This is the terminal specialization of the variadic + // template class member function. + } + }; + + ////////////////////////////////////////////////// + /// \brief This specialization will be called when one or more Interfaces + /// are specified. + template + struct InterfaceHelper + { + public: static void InsertInterfaces( + Info::InterfaceCastingMap &interfaces) + { + // READ ME: If you get a compilation error here, then one of the + // interfaces that you tried to register for your plugin is not + // actually a base class of the plugin class. This is not allowed. A + // plugin class must inherit every interface class that you want it to + // provide. + static_assert(std::is_base_of::value, + "YOU ARE ATTEMPTING TO REGISTER AN INTERFACE FOR A " + "PLUGIN, BUT THE INTERFACE IS NOT A BASE CLASS OF THE " + "PLUGIN."); + + interfaces.insert(std::make_pair( + typeid(Interface).name(), + [=](void* v_ptr) + { + PluginClass *d_ptr = static_cast(v_ptr); + return static_cast(d_ptr); + })); + + InterfaceHelper + ::InsertInterfaces(interfaces); + } + }; + + ////////////////////////////////////////////////// + /// \brief This overload will be called when no more aliases remain to be + /// inserted. If one or more aliases still need to be inserted, then the + /// overload below this one will be called instead. + inline void InsertAlias(std::set &/*aliases*/) + { + // Do nothing. This is the terminal overload of the variadic template + // function. + } + + template + void InsertAlias(std::set &aliases, + const std::string &nextAlias, + Aliases&&... remainingAliases) + { + aliases.insert(nextAlias); + InsertAlias(aliases, std::forward(remainingAliases)...); + } + + ////////////////////////////////////////////////// + template + struct IfEnablePluginFromThisImpl + { + public: static void AddIt(Info::InterfaceCastingMap &_interfaces) + { + _interfaces.insert(std::make_pair( + typeid(EnablePluginFromThis).name(), + [=](void *v_ptr) + { + PluginClass *d_ptr = static_cast(v_ptr); + return static_cast(d_ptr); + })); + } + }; + + ////////////////////////////////////////////////// + template + struct IfEnablePluginFromThisImpl + { + public: static void AddIt(Info::InterfaceCastingMap &) + { + // Do nothing, because the plugin does not inherit + // the EnablePluginFromThis interface. + } + }; + + ////////////////////////////////////////////////// + template + struct IfEnablePluginFromThis + : IfEnablePluginFromThisImpl::value> + { }; // NOLINT + + ////////////////////////////////////////////////// + template + Info MakeInfo() + { + Info info; + + // Set the name of the plugin + info.name = typeid(PluginClass).name(); + + // Create a factory for generating new plugin instances + info.factory = [=]() + { + // vvvvvvvvvvvvvvvvvvvvvvvv READ ME vvvvvvvvvvvvvvvvvvvvvvvvvvvvv + // If you get a compilation error here, then you are trying to + // register an abstract class as a plugin, which is not allowed. To + // register a plugin class, every one if its virtual functions must + // have a definition. + // + // Read through the error produced by your compiler to see which + // pure virtual functions you are neglecting to provide overrides + // for. + // ^^^^^^^^^^^^^ READ ABOVE FOR COMPILATION ERRORS ^^^^^^^^^^^^^^^^ + return static_cast(new PluginClass); + }; + +GZ_UTILS_WARN_IGNORE__NON_VIRTUAL_DESTRUCTOR + // Create a deleter to clean up destroyed instances + info.deleter = [=](void *ptr) + { + delete static_cast(ptr); + }; +GZ_UTILS_WARN_RESUME__NON_VIRTUAL_DESTRUCTOR + + // Construct a map from the plugin to its interfaces + InterfaceHelper + ::InsertInterfaces(info.interfaces); + + return info; + } + } + } +} + +#endif diff --git a/register/include/gz/plugin/detail/Register.hh b/register/include/gz/plugin/detail/Register.hh index 9410e882..6cb63a85 100644 --- a/register/include/gz/plugin/detail/Register.hh +++ b/register/include/gz/plugin/detail/Register.hh @@ -19,16 +19,11 @@ #ifndef GZ_PLUGIN_DETAIL_REGISTER_HH_ #define GZ_PLUGIN_DETAIL_REGISTER_HH_ -#include -#include #include -#include #include -#include - -#include #include +#include #include @@ -246,106 +241,6 @@ namespace gz { namespace detail { - ////////////////////////////////////////////////// - /// \brief This default will be called when NoMoreInterfaces is an empty - /// parameter pack. When one or more Interfaces are provided, the other - /// template specialization of this class will be called. - template - struct InterfaceHelper - { - public: static void InsertInterfaces(Info::InterfaceCastingMap &) - { - // Do nothing. This is the terminal specialization of the variadic - // template class member function. - } - }; - - ////////////////////////////////////////////////// - /// \brief This specialization will be called when one or more Interfaces - /// are specified. - template - struct InterfaceHelper - { - public: static void InsertInterfaces( - Info::InterfaceCastingMap &interfaces) - { - // READ ME: If you get a compilation error here, then one of the - // interfaces that you tried to register for your plugin is not - // actually a base class of the plugin class. This is not allowed. A - // plugin class must inherit every interface class that you want it to - // provide. - static_assert(std::is_base_of::value, - "YOU ARE ATTEMPTING TO REGISTER AN INTERFACE FOR A " - "PLUGIN, BUT THE INTERFACE IS NOT A BASE CLASS OF THE " - "PLUGIN."); - - interfaces.insert(std::make_pair( - typeid(Interface).name(), - [=](void* v_ptr) - { - PluginClass *d_ptr = static_cast(v_ptr); - return static_cast(d_ptr); - })); - - InterfaceHelper - ::InsertInterfaces(interfaces); - } - }; - - ////////////////////////////////////////////////// - /// \brief This overload will be called when no more aliases remain to be - /// inserted. If one or more aliases still need to be inserted, then the - /// overload below this one will be called instead. - inline void InsertAlias(std::set &/*aliases*/) - { - // Do nothing. This is the terminal overload of the variadic template - // function. - } - - template - void InsertAlias(std::set &aliases, - const std::string &nextAlias, - Aliases&&... remainingAliases) - { - aliases.insert(nextAlias); - InsertAlias(aliases, std::forward(remainingAliases)...); - } - - ////////////////////////////////////////////////// - template - struct IfEnablePluginFromThisImpl - { - public: static void AddIt(Info::InterfaceCastingMap &_interfaces) - { - _interfaces.insert(std::make_pair( - typeid(EnablePluginFromThis).name(), - [=](void *v_ptr) - { - PluginClass *d_ptr = static_cast(v_ptr); - return static_cast(d_ptr); - })); - } - }; - - ////////////////////////////////////////////////// - template - struct IfEnablePluginFromThisImpl - { - public: static void AddIt(Info::InterfaceCastingMap &) - { - // Do nothing, because the plugin does not inherit - // the EnablePluginFromThis interface. - } - }; - - ////////////////////////////////////////////////// - template - struct IfEnablePluginFromThis - : IfEnablePluginFromThisImpl::value> - { }; // NOLINT - ////////////////////////////////////////////////// /// \brief This specialization of the Register class will be called when /// one or more arguments are provided to the GZ_ADD_PLUGIN(~) @@ -354,50 +249,12 @@ namespace gz template struct Registrar { - public: static Info MakeInfo() - { - Info info; - - // Set the name of the plugin - info.name = typeid(PluginClass).name(); - - // Create a factory for generating new plugin instances - info.factory = [=]() - { - // vvvvvvvvvvvvvvvvvvvvvvvv READ ME vvvvvvvvvvvvvvvvvvvvvvvvvvvvv - // If you get a compilation error here, then you are trying to - // register an abstract class as a plugin, which is not allowed. To - // register a plugin class, every one if its virtual functions must - // have a definition. - // - // Read through the error produced by your compiler to see which - // pure virtual functions you are neglecting to provide overrides - // for. - // ^^^^^^^^^^^^^ READ ABOVE FOR COMPILATION ERRORS ^^^^^^^^^^^^^^^^ - return static_cast(new PluginClass); - }; - -GZ_UTILS_WARN_IGNORE__NON_VIRTUAL_DESTRUCTOR - // Create a deleter to clean up destroyed instances - info.deleter = [=](void *ptr) - { - delete static_cast(ptr); - }; -GZ_UTILS_WARN_RESUME__NON_VIRTUAL_DESTRUCTOR - - // Construct a map from the plugin to its interfaces - InterfaceHelper - ::InsertInterfaces(info.interfaces); - - return info; - } - /// \brief This function registers a plugin along with a set of /// interfaces that it provides. public: static void Register() { // Make all info that the user has specified - Info info = MakeInfo(); + Info info = MakeInfo(); // Add the EnablePluginFromThis interface automatically if it is // inherited by PluginClass. @@ -425,7 +282,7 @@ GZ_UTILS_WARN_RESUME__NON_VIRTUAL_DESTRUCTOR "THERE IS A BUG IN THE ALIAS REGISTRATION " "IMPLEMENTATION! PLEASE REPORT THIS!"); - Info info = MakeInfo(); + Info info = MakeInfo(); // Gather up all the aliases that have been specified for this plugin. InsertAlias(info.aliases, std::forward(aliases)...); From 0ac615bdd9e99a3de8fd5271ef58ebf71ca9b9b4 Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Thu, 21 Jul 2022 11:05:13 -0700 Subject: [PATCH 05/14] Added macros for registering plugin classes statically Signed-off-by: Shameek Ganguly --- register/include/gz/plugin/RegisterStatic.hh | 76 ++++++++ .../gz/plugin/detail/RegisterStatic.hh | 173 ++++++++++++++++++ .../include/ignition/plugin/RegisterStatic.hh | 25 +++ .../ignition/plugin/detail/RegisterStatic.hh | 19 ++ 4 files changed, 293 insertions(+) create mode 100644 register/include/gz/plugin/RegisterStatic.hh create mode 100644 register/include/gz/plugin/detail/RegisterStatic.hh create mode 100644 register/include/ignition/plugin/RegisterStatic.hh create mode 100644 register/include/ignition/plugin/detail/RegisterStatic.hh diff --git a/register/include/gz/plugin/RegisterStatic.hh b/register/include/gz/plugin/RegisterStatic.hh new file mode 100644 index 00000000..13385c73 --- /dev/null +++ b/register/include/gz/plugin/RegisterStatic.hh @@ -0,0 +1,76 @@ +/* + * Copyright (C) 2022 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ + + +#ifndef GZ_PLUGIN_REGISTERSTATIC_HH_ +#define GZ_PLUGIN_REGISTERSTATIC_HH_ + +#include + + +// ------------- Add a set of plugins or a set of interfaces ------------------ + +/// \brief Add a plugin and interface from this static library. +/// +/// This macro can be put in any namespace and may be called any number of +/// times. It can be called multiple times on the same plugin class in order to +/// register multiple interfaces, e.g.: +/// +/// \code +/// GZ_ADD_STATIC_PLUGIN(PluginClass, Interface1) +/// GZ_ADD_STATIC_PLUGIN(PluginClass, Interface2) +/// +/// /* Some other code */ +/// +/// GZ_ADD_STATIC_PLUGIN(PluginClass, Interface3) +/// \endcode +/// +/// Or you can list multiple interfaces in a single call to the macro, e.g.: +/// +/// \code +/// GZ_ADD_STATIC_PLUGIN(PluginClass, Interface1, Interface2, Interface3) +/// \endcode +/// +/// This macro version is to be used when the plugin library is expected to be +/// linked statically into the same program as the loader that instantiates the +/// plugin. Multiple translation units in the same library may include the +/// gz/plugins/RegisterStatic.hh header. +#define GZ_ADD_STATIC_PLUGIN(PluginClass, ...) \ + DETAIL_GZ_ADD_STATIC_PLUGIN(PluginClass, __VA_ARGS__) + +/// \brief Add an alias for one of your plugins. +/// +/// This macro can be put in any namespace and may be called any number of +/// times. It can be called multiple times on the same plugin class in order to +/// register multiple aliases, e.g.: +/// +/// \code +/// GZ_ADD_STATIC_PLUGIN_ALIAS(PluginClass, "PluginClass") +/// GZ_ADD_STATIC_PLUGIN_ALIAS(PluginClass, "SomeOtherName", "Yet another name") +/// GZ_ADD_STATIC_PLUGIN_ALIAS(AnotherPluginClass, "Foo", "Bar", "Baz") +/// \endcode +/// +/// You can give the same alias to multiple plugins, but then that alias can no +/// longer be used to instantiate any plugin. +/// +/// If you give a plugin an alias string that matches the demangled symbol name +/// of another plugin, then the Loader will always prefer to instantiate the +/// plugin whose symbol name matches that string. +#define GZ_ADD_STATIC_PLUGIN_ALIAS(PluginClass, ...) \ + DETAIL_GZ_ADD_STATIC_PLUGIN_ALIAS(PluginClass, __VA_ARGS__) + +#endif diff --git a/register/include/gz/plugin/detail/RegisterStatic.hh b/register/include/gz/plugin/detail/RegisterStatic.hh new file mode 100644 index 00000000..1a8ecfd0 --- /dev/null +++ b/register/include/gz/plugin/detail/RegisterStatic.hh @@ -0,0 +1,173 @@ +/* + * Copyright (C) 2022 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ + + +#ifndef GZ_PLUGIN_DETAIL_REGISTERSTATIC_HH_ +#define GZ_PLUGIN_DETAIL_REGISTERSTATIC_HH_ + +#include +#include + +namespace gz +{ + namespace plugin + { + namespace detail + { + ////////////////////////////////////////////////// + /// \brief This specialization of the Register class will be called when + /// one or more arguments are provided to the GZ_ADD_STATIC_PLUGIN(~) + /// macro. + /// + /// Calling Register() creates an Info object for the plugin which + /// includes a factory function to instantiate the plugin and a deleter + /// function to delete the plugin instance with the proper derived class + /// destructor. The Info object also contains a map of all the interfaces + /// that are registered for the plugin and the aliases that the plugin + /// may be referred by. The Info object is then stored in the global + /// static plugin registry which may be accessed through the + /// StaticLoader. + template + struct StaticRegistrar + { + /// \brief This function registers a plugin along with a set of + /// interfaces that it provides. + public: static void Register() { + // Make all info that the user has specified + Info info = MakeInfo(); + + // Add the EnablePluginFromThis interface automatically if it is + // inherited by PluginClass. + IfEnablePluginFromThis::AddIt(info.interfaces); + + // Send this information as input to the global static plugin + // registry. + gz::plugin::StaticRegistry::GetInstance().AddInfo(info); + } + + public: template + static void RegisterAlias(Aliases &&...aliases) { + // Dev note (MXG): We expect the RegisterAlias function to be called + // using the GZ_ADD_PLUGIN_ALIAS(~) macro, which should never + // contain any interfaces. Therefore, this parameter pack should be + // empty. + // + // In the future, we could allow Interfaces and Aliases to be + // specified simultaneously, but that would be very tricky to do with + // macros, so for now we will enforce this assumption to make sure + // that the implementation is working as expected. + static_assert(sizeof...(Interfaces) == 0, + "THERE IS A BUG IN THE ALIAS REGISTRATION " + "IMPLEMENTATION! PLEASE REPORT THIS!"); + + Info info = MakeInfo(); + + // Gather up all the aliases that have been specified for this plugin. + InsertAlias(info.aliases, std::forward(aliases)...); + + // Send this information as input to the global static plugin + // registry. + gz::plugin::StaticRegistry::GetInstance().AddInfo(info); + } + }; + } + } +} + +////////////////////////////////////////////////// +/// This macro creates a uniquely-named class whose constructor calls the +/// gz::plugin::detail::StaticRegistrar::RegisterAlias function. It then +/// declares a uniquely-named instance of the class with static lifetime. When +/// it is constructed at program start, the Register function will be called. +#define DETAIL_GZ_ADD_STATIC_PLUGIN_HELPER(UniqueID, ...) \ + namespace gz \ + { \ + namespace plugin \ + { \ + namespace \ + { \ + struct RegisterStaticPlugin##UniqueID \ + { \ + RegisterStaticPlugin##UniqueID() \ + { \ + ::gz::plugin::detail::StaticRegistrar<__VA_ARGS__>::Register(); \ + } \ + }; \ + \ + static RegisterStaticPlugin##UniqueID execute##UniqueID; \ + } /* namespace */ \ + } \ + } + + +////////////////////////////////////////////////// +/// This macro is needed to force the __COUNTER__ macro to expand to a value +/// before being passed to the *_HELPER macro. +#define DETAIL_GZ_ADD_STATIC_PLUGIN_WITH_COUNTER(UniqueID, ...) \ + DETAIL_GZ_ADD_STATIC_PLUGIN_HELPER(UniqueID, __VA_ARGS__) + + +////////////////////////////////////////////////// +/// We use the __COUNTER__ here to give each plugin registration its own unique +/// name, which is required in order to statically initialize each one. +#define DETAIL_GZ_ADD_STATIC_PLUGIN(...) \ + DETAIL_GZ_ADD_STATIC_PLUGIN_WITH_COUNTER(__COUNTER__, __VA_ARGS__) + + +////////////////////////////////////////////////// +/// This macro creates a uniquely-named class whose constructor calls the +/// gz::plugin::detail::StaticRegistrar::RegisterAlias function. It then +/// declares a uniquely-named instance of the class with static lifetime. When +/// it is constructed at program start, the Register function will be called. +#define DETAIL_GZ_ADD_STATIC_PLUGIN_ALIAS_HELPER(UniqueID, PluginClass, ...) \ + namespace gz \ + { \ + namespace plugin \ + { \ + namespace \ + { \ + struct RegisterStaticPlugin##UniqueID \ + { \ + RegisterStaticPlugin##UniqueID() \ + { \ + ::gz::plugin::detail::StaticRegistrar::RegisterAlias( \ + __VA_ARGS__); \ + } \ + }; \ + \ + static RegisterStaticPlugin##UniqueID register##UniqueID; \ + } /* namespace */ \ + } \ + } + + +////////////////////////////////////////////////// +/// This macro is needed to force the __COUNTER__ macro to expand to a value +/// before being passed to the *_HELPER macro. +#define DETAIL_GZ_ADD_STATIC_PLUGIN_ALIAS_WITH_COUNTER( \ + UniqueID, PluginClass, ...) \ + DETAIL_GZ_ADD_STATIC_PLUGIN_ALIAS_HELPER(UniqueID, PluginClass, __VA_ARGS__) + + +////////////////////////////////////////////////// +/// We use the __COUNTER__ here to give each plugin registration its own unique +/// name, which is required in order to statically initialize each one. +#define DETAIL_GZ_ADD_STATIC_PLUGIN_ALIAS(PluginClass, ...) \ + DETAIL_GZ_ADD_STATIC_PLUGIN_ALIAS_WITH_COUNTER( \ + __COUNTER__, PluginClass, __VA_ARGS__) + +#endif \ No newline at end of file diff --git a/register/include/ignition/plugin/RegisterStatic.hh b/register/include/ignition/plugin/RegisterStatic.hh new file mode 100644 index 00000000..6d56e015 --- /dev/null +++ b/register/include/ignition/plugin/RegisterStatic.hh @@ -0,0 +1,25 @@ +/* + * Copyright (C) 2022 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include +#include + +#define IGNITION_ADD_STATIC_PLUGIN(PluginClass, ...) \ + GZ_ADD_STATIC_PLUGIN(PluginClass, __VA_ARGS__) + +#define IGNITION_ADD_STATIC_PLUGIN_ALIAS(PluginClass, ...) \ + GZ_ADD_STATIC_PLUGIN_ALIAS(PluginClass, __VA_ARGS__) diff --git a/register/include/ignition/plugin/detail/RegisterStatic.hh b/register/include/ignition/plugin/detail/RegisterStatic.hh new file mode 100644 index 00000000..1186d388 --- /dev/null +++ b/register/include/ignition/plugin/detail/RegisterStatic.hh @@ -0,0 +1,19 @@ +/* + * Copyright (C) 2022 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include +#include From 1821b926764df188ccb8cff376c0f340f139d113 Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Thu, 21 Jul 2022 11:10:00 -0700 Subject: [PATCH 06/14] Support loading plugins from the static registry through Loader Signed-off-by: Shameek Ganguly --- loader/include/gz/plugin/Loader.hh | 41 ++++++- loader/include/gz/plugin/detail/Loader.hh | 34 ++++-- loader/src/Loader.cc | 139 +++++++++++++++++++--- 3 files changed, 185 insertions(+), 29 deletions(-) diff --git a/loader/include/gz/plugin/Loader.hh b/loader/include/gz/plugin/Loader.hh index d590ca0a..4be3be9a 100644 --- a/loader/include/gz/plugin/Loader.hh +++ b/loader/include/gz/plugin/Loader.hh @@ -219,15 +219,50 @@ namespace gz /// \sa bool ForgetLibrary(const std::string &_pathToLibrary) public: bool ForgetLibraryOfPlugin(const std::string &_pluginNameOrAlias); - /// \brief Get a pointer to the Info corresponding to _pluginName. + /// \brief Specifically look up a plugin loaded from file. + /// + /// \param[in] _nameOrAlias + /// The name or alias of the plugin of interest. + /// + /// \return The name of the plugin being referred to, or an empty string + /// if no such plugin is known. + private: std::string PrivateLookupFilePlugin( + const std::string &_nameOrAlias) const; + + /// \brief Get a pointer to the Info corresponding to _pluginName for a + /// plugin loaded from file. /// /// \param[in] _resolvedName /// The resolved name, i.e. the demangled class symbol name as returned - /// by LookupPlugin(~), of the plugin that you want to instantiate. + /// by PrivateLookupFilePlugin(~), of the plugin that you want to + /// instantiate. + /// + /// \return Pointer to the corresponding Info, or nullptr if there + /// is no info for the requested _pluginName. + private: ConstInfoPtr PrivateGetInfoForFilePlugin( + const std::string &_resolvedName) const; + + /// \brief Specifically look up a plugin in the static registry. + /// + /// \param[in] _nameOrAlias + /// The name or alias of the plugin of interest. + /// + /// \return The name of the plugin being referred to, or an empty string + /// if no such plugin is known. + private: std::string PrivateLookupStaticPlugin( + const std::string &_nameOrAlias) const; + + /// \brief Get a pointer to the Info corresponding to _pluginName for a + /// plugin loaded from the static registry. + /// + /// \param[in] _resolvedName + /// The resolved name, i.e. the demangled class symbol name as returned + /// by PrivateLookupStaticPlugin(~), of the plugin that you want to + /// instantiate. /// /// \return Pointer to the corresponding Info, or nullptr if there /// is no info for the requested _pluginName. - private: ConstInfoPtr PrivateGetInfo( + private: ConstInfoPtr PrivateGetInfoForStaticPlugin( const std::string &_resolvedName) const; /// \brief Get a std::shared_ptr that manages the lifecycle of the shared diff --git a/loader/include/gz/plugin/detail/Loader.hh b/loader/include/gz/plugin/detail/Loader.hh index 8031502c..9a5c9aff 100644 --- a/loader/include/gz/plugin/detail/Loader.hh +++ b/loader/include/gz/plugin/detail/Loader.hh @@ -39,18 +39,34 @@ namespace gz PluginPtrType Loader::Instantiate( const std::string &_pluginNameOrAlias) const { - const std::string &resolvedName = this->LookupPlugin(_pluginNameOrAlias); - if (resolvedName.empty()) - return PluginPtr(); + // Higher priority for plugins loaded from file than from the static + // registry. + const std::string &resolvedNameForFilePlugin = + this->PrivateLookupFilePlugin(_pluginNameOrAlias); + + const std::string &resolvedNameForStaticPlugin = + this->PrivateLookupStaticPlugin(_pluginNameOrAlias); - PluginPtrType ptr(this->PrivateGetInfo(resolvedName), - this->PrivateGetPluginDlHandlePtr(resolvedName)); + PluginPtr ptr; + + if (!resolvedNameForFilePlugin.empty()) + { + ptr = PluginPtr(this->PrivateGetInfoForFilePlugin(resolvedNameForFilePlugin), + this->PrivateGetPluginDlHandlePtr(resolvedNameForFilePlugin)); + } + else if (!resolvedNameForStaticPlugin.empty()) + { + ptr = PluginPtr(this->PrivateGetInfoForStaticPlugin(resolvedNameForStaticPlugin)); + } + else + { + return PluginPtr(); + } - if (auto *enableFromThis = - ptr->template QueryInterface()) - enableFromThis->PrivateSetPluginFromThis(ptr); + if (auto *enableFromThis = ptr->QueryInterface()) + enableFromThis->PrivateSetPluginFromThis(ptr); - return ptr; + return ptr; } template diff --git a/loader/src/Loader.cc b/loader/src/Loader.cc index 94df8b37..b5bc79f3 100644 --- a/loader/src/Loader.cc +++ b/loader/src/Loader.cc @@ -30,6 +30,7 @@ #include #include #include +#include #include namespace gz @@ -106,19 +107,27 @@ namespace gz /// \brief A map from the shared library handle to the names of the /// plugins that it provides. public: DlHandleToPluginMap dlHandleToPluginMap; + + /// \brief Pointer to the singleton StaticRegistry + public: StaticRegistry* staticPlugins; }; ///////////////////////////////////////////////// std::string Loader::PrettyStr() const { - return this->dataPtr->filePlugins.PrettyStr(); + std::stringstream pretty; + pretty << "Loaded plugins registry: \n" + << this->dataPtr->filePlugins.PrettyStr(); + pretty << "Static plugins registry: \n" + << this->dataPtr->staticPlugins->PrettyStr(); + return pretty.str(); } ///////////////////////////////////////////////// Loader::Loader() : dataPtr(new Implementation()) { - // Do nothing. + this->dataPtr->staticPlugins = &(StaticRegistry::GetInstance()); } ///////////////////////////////////////////////// @@ -173,7 +182,13 @@ namespace gz ///////////////////////////////////////////////// std::unordered_set Loader::InterfacesImplemented() const { - return this->dataPtr->filePlugins.InterfacesImplemented(); + std::unordered_set allInterfaces = + this->dataPtr->filePlugins.InterfacesImplemented(); + std::unordered_set staticPluginInterfaces = + this->dataPtr->staticPlugins->InterfacesImplemented(); + allInterfaces.insert(staticPluginInterfaces.begin(), + staticPluginInterfaces.end()); + return allInterfaces; } ///////////////////////////////////////////////// @@ -181,44 +196,100 @@ namespace gz const std::string &_interface, const bool _demangled) const { - return this->dataPtr->filePlugins.PluginsImplementing(_interface, _demangled); + std::unordered_set allPlugins = + this->dataPtr->filePlugins.PluginsImplementing(_interface, + _demangled); + std::unordered_set staticPlugins = + this->dataPtr->staticPlugins->PluginsImplementing(_interface, + _demangled); + allPlugins.insert(staticPlugins.begin(), staticPlugins.end()); + return allPlugins; } ///////////////////////////////////////////////// std::set Loader::AllPlugins() const { - return this->dataPtr->filePlugins.AllPlugins(); + std::set allPlugins = + this->dataPtr->filePlugins.AllPlugins(); + std::set staticPlugins = + this->dataPtr->staticPlugins->AllPlugins(); + allPlugins.insert(staticPlugins.begin(), staticPlugins.end()); + return allPlugins; } ///////////////////////////////////////////////// std::set Loader::PluginsWithAlias( const std::string &_alias) const { - return this->dataPtr->filePlugins.PluginsWithAlias(_alias); + std::set allPlugins = + this->dataPtr->filePlugins.PluginsWithAlias(_alias); + std::set staticPlugins = + this->dataPtr->staticPlugins->PluginsWithAlias(_alias); + allPlugins.insert(staticPlugins.begin(), staticPlugins.end()); + return allPlugins; } ///////////////////////////////////////////////// std::set Loader::AliasesOfPlugin( const std::string &_pluginName) const { - return this->dataPtr->filePlugins.AliasesOfPlugin(_pluginName); + std::set allAliases = + this->dataPtr->filePlugins.AliasesOfPlugin(_pluginName); + std::set staticAliases = + this->dataPtr->staticPlugins->AliasesOfPlugin(_pluginName); + allAliases.insert(staticAliases.begin(), staticAliases.end()); + return allAliases; } ///////////////////////////////////////////////// std::string Loader::LookupPlugin(const std::string &_nameOrAlias) const { - return this->dataPtr->filePlugins.LookupPlugin(_nameOrAlias); + // Higher priority for plugins loaded from file than from the static + // registry. + std::string nameInFilePlugins = + this->dataPtr->filePlugins.LookupPlugin(_nameOrAlias); + if (!nameInFilePlugins.empty()) { + return nameInFilePlugins; + } + + std::string nameInStaticPlugins = + this->dataPtr->staticPlugins->LookupPlugin(_nameOrAlias); + if (!nameInStaticPlugins.empty()) { + return nameInStaticPlugins; + } + + std::cerr << "[gz::plugin::Loader::LookupPlugin] Failed to get " + << "info for [" << _nameOrAlias << "]. Could not find a plugin " + << "with that name or alias.\n"; + return ""; } ///////////////////////////////////////////////// PluginPtr Loader::Instantiate(const std::string &_pluginNameOrAlias) const { - const std::string &resolvedName = this->LookupPlugin(_pluginNameOrAlias); - if (resolvedName.empty()) - return PluginPtr(); + // Higher priority for plugins loaded from file than from the static + // registry. + const std::string &resolvedNameForFilePlugin = + this->PrivateLookupFilePlugin(_pluginNameOrAlias); + + const std::string &resolvedNameForStaticPlugin = + this->PrivateLookupStaticPlugin(_pluginNameOrAlias); + + PluginPtr ptr; - PluginPtr ptr(this->PrivateGetInfo(resolvedName), - this->PrivateGetPluginDlHandlePtr(resolvedName)); + if (!resolvedNameForFilePlugin.empty()) + { + ptr = PluginPtr(this->PrivateGetInfoForFilePlugin(resolvedNameForFilePlugin), + this->PrivateGetPluginDlHandlePtr(resolvedNameForFilePlugin)); + } + else if (!resolvedNameForStaticPlugin.empty()) + { + ptr = PluginPtr(this->PrivateGetInfoForStaticPlugin(resolvedNameForStaticPlugin)); + } + else + { + return PluginPtr(); + } if (auto *enableFromThis = ptr->QueryInterface()) enableFromThis->PrivateSetPluginFromThis(ptr); @@ -268,7 +339,13 @@ namespace gz } ///////////////////////////////////////////////// - ConstInfoPtr Loader::PrivateGetInfo( + std::string Loader::PrivateLookupFilePlugin(const std::string &_nameOrAlias) const + { + return this->dataPtr->filePlugins.LookupPlugin(_nameOrAlias); + } + + ///////////////////////////////////////////////// + ConstInfoPtr Loader::PrivateGetInfoForFilePlugin( const std::string &_resolvedName) const { ConstInfoPtr info = this->dataPtr->filePlugins.GetInfo(_resolvedName); @@ -276,9 +353,37 @@ namespace gz if (info == nullptr) { // LCOV_EXCL_START - std::cerr << "[gz::Loader::PrivateGetInfo] A resolved name [" - << _resolvedName << "] could not be found in the PluginMap. " - << "This should not be possible! Please report this bug!\n"; + std::cerr << "[gz::Loader::PrivateGetInfoForFilePlugin] A resolved " + << "name [" << _resolvedName << "] could not be found in " + << "the registry of loaded plugins. This should not be " + << "possible! Please report this bug!\n"; + assert(false); + return nullptr; + // LCOV_EXCL_STOP + } + + return info; + } + + ///////////////////////////////////////////////// + std::string Loader::PrivateLookupStaticPlugin(const std::string &_nameOrAlias) const + { + return this->dataPtr->staticPlugins->LookupPlugin(_nameOrAlias); + } + + ///////////////////////////////////////////////// + ConstInfoPtr Loader::PrivateGetInfoForStaticPlugin( + const std::string &_resolvedName) const + { + ConstInfoPtr info = this->dataPtr->staticPlugins->GetInfo(_resolvedName); + + if (info == nullptr) + { + // LCOV_EXCL_START + std::cerr << "[gz::Loader::PrivateGetInfoForStaticPlugin] A resolved " + << "name [" << _resolvedName << "] could not be found in " + << "the static plugin registry. This should not be " + << "possible! Please report this bug!\n"; assert(false); return nullptr; // LCOV_EXCL_STOP From 603b040d3c53a59c742830fb375db0505aefc6c2 Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Thu, 21 Jul 2022 11:11:52 -0700 Subject: [PATCH 07/14] Add dummy static plugin for tests Signed-off-by: Shameek Ganguly --- test/plugins/CMakeLists.txt | 9 ++++++- test/plugins/DummyStaticPlugin.cc | 39 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 test/plugins/DummyStaticPlugin.cc diff --git a/test/plugins/CMakeLists.txt b/test/plugins/CMakeLists.txt index 3acdad9d..84f12b53 100644 --- a/test/plugins/CMakeLists.txt +++ b/test/plugins/CMakeLists.txt @@ -10,6 +10,8 @@ add_library(GzDummyPlugins SHARED DummyPlugins.cc DummyPluginsOtherTranslationUnit.cc) +add_library(GzDummyStaticPlugin SHARED DummyStaticPlugin.cc) + # Create a variable for the name of the header which will contain the dummy plugin path. # This variable gets put in the cache so that it is available at generation time. foreach(plugin_target @@ -20,9 +22,14 @@ foreach(plugin_target GzBadPluginSize GzDummyPlugins GzFactoryPlugins - GzTemplatedPlugins) + GzTemplatedPlugins + GzDummyStaticPlugin) target_link_libraries(${plugin_target} PRIVATE ${PROJECT_LIBRARY_TARGET_NAME}-register) endforeach() + +# Need to link to the loader component to link static registry implementation. +target_link_libraries(GzDummyStaticPlugin PRIVATE + ${PROJECT_LIBRARY_TARGET_NAME}-loader) diff --git a/test/plugins/DummyStaticPlugin.cc b/test/plugins/DummyStaticPlugin.cc new file mode 100644 index 00000000..ce157d6f --- /dev/null +++ b/test/plugins/DummyStaticPlugin.cc @@ -0,0 +1,39 @@ +/* + * Copyright (C) 2022 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ + +#include "gz/plugin/RegisterStatic.hh" +#include "DummyPlugins.hh" + +namespace test +{ +namespace util +{ + +class DummySinglePlugin : public DummyNameBase +{ + public: virtual std::string MyNameIs() const override + { + return std::string("DummySinglePlugin"); + } +}; + +} +} + +GZ_ADD_STATIC_PLUGIN(test::util::DummySinglePlugin, test::util::DummyNameBase) +GZ_ADD_STATIC_PLUGIN_ALIAS(test::util::DummySinglePlugin, "Alternative name") +GZ_ADD_STATIC_PLUGIN_ALIAS(test::util::DummySinglePlugin, "Bar", "Baz") From e56629034acb09677d4e90d11aece1317dedc604 Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Thu, 21 Jul 2022 11:41:05 -0700 Subject: [PATCH 08/14] Add integration test for registering and loading plugins from statically linked plugin lib Signed-off-by: Shameek Ganguly --- test/integration/CMakeLists.txt | 6 ++++ test/integration/static_plugins.cc | 56 ++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 test/integration/static_plugins.cc diff --git a/test/integration/CMakeLists.txt b/test/integration/CMakeLists.txt index 5178b402..4d9d366f 100644 --- a/test/integration/CMakeLists.txt +++ b/test/integration/CMakeLists.txt @@ -40,3 +40,9 @@ foreach(test endif() endforeach() + +# The linker option -force_load needs to be specified to ensure that the global +# structs specified in the plugin module still get loaded even without any +# explicit reference to the loaded symbols (only the interfaces are +# referenced). +target_link_libraries(INTEGRATION_static_plugins -force_load GzDummyStaticPlugin) diff --git a/test/integration/static_plugins.cc b/test/integration/static_plugins.cc new file mode 100644 index 00000000..12c8fc3a --- /dev/null +++ b/test/integration/static_plugins.cc @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2022 Open Source Robotics Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ + +#include + +#include + +#include "../plugins/DummyPlugins.hh" + +///////////////////////////////////////////////// +TEST(StaticPlugins, Load) +{ + gz::plugin::Loader loader; + EXPECT_EQ(loader.AllPlugins().size(), 1); + EXPECT_EQ(loader.LookupPlugin("Alternative name"), + "test::util::DummySinglePlugin"); +} + +TEST(StaticPlugins, AdditionalAliases) +{ + gz::plugin::Loader loader; + EXPECT_EQ(loader.LookupPlugin("Baz"), "test::util::DummySinglePlugin"); + EXPECT_EQ(loader.LookupPlugin("Bar"), "test::util::DummySinglePlugin"); +} + +TEST(StaticPlugins, Interfaces) +{ + gz::plugin::Loader loader; + EXPECT_EQ(loader.PluginsImplementing().size(), + 1); +} + +TEST(StaticPlugins, Instantiate) +{ + gz::plugin::Loader loader; + std::string resolvedName = loader.LookupPlugin("Alternative name"); + auto pluginInstance = loader.Instantiate(resolvedName); + EXPECT_TRUE(pluginInstance); + auto *dummySinglePluginInterface = + pluginInstance->QueryInterface(); + EXPECT_EQ(dummySinglePluginInterface->MyNameIs(), "DummySinglePlugin"); +} From d4961fc8f1014e9863cb62943aba881baad5bce9 Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Thu, 21 Jul 2022 12:00:51 -0700 Subject: [PATCH 09/14] Address codecheck errors Signed-off-by: Shameek Ganguly --- core/src/Plugin.cc | 2 +- loader/include/gz/plugin/detail/Loader.hh | 8 +++++--- loader/include/gz/plugin/detail/Registry.hh | 13 ++++++++----- loader/include/gz/plugin/detail/StaticRegistry.hh | 13 +++++++++---- loader/src/Loader.cc | 14 +++++++++----- loader/src/detail/Registry.cc | 8 ++++---- loader/src/detail/StaticRegistry.cc | 2 +- register/include/gz/plugin/detail/Common.hh | 1 + .../include/gz/plugin/detail/RegisterStatic.hh | 4 +++- 9 files changed, 41 insertions(+), 24 deletions(-) diff --git a/core/src/Plugin.cc b/core/src/Plugin.cc index 3743b249..a282931c 100644 --- a/core/src/Plugin.cc +++ b/core/src/Plugin.cc @@ -134,7 +134,7 @@ namespace gz public: void Create( const ConstInfoPtr &_info, const std::shared_ptr &_dlHandlePtr, - bool _allowNullDlHandlePtr=false) + bool _allowNullDlHandlePtr = false) { this->Clear(); diff --git a/loader/include/gz/plugin/detail/Loader.hh b/loader/include/gz/plugin/detail/Loader.hh index 9a5c9aff..d4d647c9 100644 --- a/loader/include/gz/plugin/detail/Loader.hh +++ b/loader/include/gz/plugin/detail/Loader.hh @@ -51,12 +51,14 @@ namespace gz if (!resolvedNameForFilePlugin.empty()) { - ptr = PluginPtr(this->PrivateGetInfoForFilePlugin(resolvedNameForFilePlugin), - this->PrivateGetPluginDlHandlePtr(resolvedNameForFilePlugin)); + ptr = PluginPtr( + this->PrivateGetInfoForFilePlugin(resolvedNameForFilePlugin), + this->PrivateGetPluginDlHandlePtr(resolvedNameForFilePlugin)); } else if (!resolvedNameForStaticPlugin.empty()) { - ptr = PluginPtr(this->PrivateGetInfoForStaticPlugin(resolvedNameForStaticPlugin)); + ptr = PluginPtr( + this->PrivateGetInfoForStaticPlugin(resolvedNameForStaticPlugin)); } else { diff --git a/loader/include/gz/plugin/detail/Registry.hh b/loader/include/gz/plugin/detail/Registry.hh index 0f7173b7..2590e698 100644 --- a/loader/include/gz/plugin/detail/Registry.hh +++ b/loader/include/gz/plugin/detail/Registry.hh @@ -118,7 +118,8 @@ namespace gz /// if no such plugin is known. public: std::string LookupPlugin(const std::string &_nameOrAlias) const; - /// \brief Get a set of the names of all plugins that are in this registry. + /// \brief Get a set of the names of all plugins that are in this + /// registry. /// /// Method is virtual to allow other Info storage and memory management /// models. @@ -133,7 +134,8 @@ namespace gz /// /// \param[in] _pluginName /// Name of the plugin as returned by LookupPlugin(~). - public: virtual ConstInfoPtr GetInfo(const std::string &_pluginName) const; + public: virtual ConstInfoPtr GetInfo( + const std::string &_pluginName) const; /// \brief Add a new plugin info. /// @@ -160,11 +162,12 @@ namespace gz protected: using AliasMap = std::map>; /// \brief A map from known alias names to the plugin names that they - /// correspond to. Since an alias might refer to more than one plugin, the - /// key of this map is a set. + /// correspond to. Since an alias might refer to more than one plugin, + /// the key of this map is a set. protected: AliasMap aliases; - protected: using PluginMap = std::unordered_map; + protected: using PluginMap = + std::unordered_map; /// \brief A map from known plugin names to their Info. protected: PluginMap plugins; }; diff --git a/loader/include/gz/plugin/detail/StaticRegistry.hh b/loader/include/gz/plugin/detail/StaticRegistry.hh index 5853c640..a59a9096 100644 --- a/loader/include/gz/plugin/detail/StaticRegistry.hh +++ b/loader/include/gz/plugin/detail/StaticRegistry.hh @@ -20,6 +20,8 @@ #define GZ_PLUGIN_DETAIL_STATICREGISTRY_HH_ #include +#include +#include #include @@ -33,16 +35,18 @@ namespace gz /// \brief Get a reference to the StaticRegistry instance. public: static StaticRegistry& GetInstance(); - /// \brief Get a set of the names of all plugins that are in this registry. + /// \brief Get a set of the names of all plugins that are in this + /// registry. /// /// \return A set of all plugin names in this registry. public: virtual std::set AllPlugins() const override; - + /// \brief Get info as ConstInfoPtr. /// /// \param[in] _pluginName /// Name of the plugin as returned by LookupPlugin(~). - public: virtual ConstInfoPtr GetInfo(const std::string &_pluginName) const override; + public: virtual ConstInfoPtr GetInfo( + const std::string &_pluginName) const override; /// \brief Register Info for a new plugin. /// @@ -62,7 +66,8 @@ namespace gz /// /// \param[in] _pluginName /// Name of the plugin as returned by LookupPlugin(~). - public: virtual void ForgetInfo(const std::string &_pluginName) override {} + public: virtual void ForgetInfo( + const std::string &_pluginName) override { } protected: StaticRegistry() = default; diff --git a/loader/src/Loader.cc b/loader/src/Loader.cc index b5bc79f3..e35b80e3 100644 --- a/loader/src/Loader.cc +++ b/loader/src/Loader.cc @@ -279,12 +279,14 @@ namespace gz if (!resolvedNameForFilePlugin.empty()) { - ptr = PluginPtr(this->PrivateGetInfoForFilePlugin(resolvedNameForFilePlugin), - this->PrivateGetPluginDlHandlePtr(resolvedNameForFilePlugin)); + ptr = PluginPtr( + this->PrivateGetInfoForFilePlugin(resolvedNameForFilePlugin), + this->PrivateGetPluginDlHandlePtr(resolvedNameForFilePlugin)); } else if (!resolvedNameForStaticPlugin.empty()) { - ptr = PluginPtr(this->PrivateGetInfoForStaticPlugin(resolvedNameForStaticPlugin)); + ptr = PluginPtr( + this->PrivateGetInfoForStaticPlugin(resolvedNameForStaticPlugin)); } else { @@ -339,7 +341,8 @@ namespace gz } ///////////////////////////////////////////////// - std::string Loader::PrivateLookupFilePlugin(const std::string &_nameOrAlias) const + std::string Loader::PrivateLookupFilePlugin( + const std::string &_nameOrAlias) const { return this->dataPtr->filePlugins.LookupPlugin(_nameOrAlias); } @@ -366,7 +369,8 @@ namespace gz } ///////////////////////////////////////////////// - std::string Loader::PrivateLookupStaticPlugin(const std::string &_nameOrAlias) const + std::string Loader::PrivateLookupStaticPlugin( + const std::string &_nameOrAlias) const { return this->dataPtr->staticPlugins->LookupPlugin(_nameOrAlias); } diff --git a/loader/src/detail/Registry.cc b/loader/src/detail/Registry.cc index 66445d08..cd3d9241 100644 --- a/loader/src/detail/Registry.cc +++ b/loader/src/detail/Registry.cc @@ -157,7 +157,7 @@ namespace gz const std::string &_pluginName) const { ConstInfoPtr plugin = this->GetInfo(_pluginName); - + if (plugin != nullptr) return plugin->aliases; @@ -168,7 +168,7 @@ namespace gz std::string Registry::LookupPlugin(const std::string &_nameOrAlias) const { ConstInfoPtr pluginPtr = this->GetInfo(_nameOrAlias); - + if (pluginPtr != nullptr) return _nameOrAlias; @@ -220,10 +220,10 @@ namespace gz bool Registry::AddInfo(const Info &_info) { for (const std::string &alias : _info.aliases) this->aliases[alias].insert(_info.name); - + auto result = this->plugins.insert( std::make_pair(_info.name, std::make_shared(_info))); - + return result.second; } diff --git a/loader/src/detail/StaticRegistry.cc b/loader/src/detail/StaticRegistry.cc index 2c47b8ff..a1d49d54 100644 --- a/loader/src/detail/StaticRegistry.cc +++ b/loader/src/detail/StaticRegistry.cc @@ -86,7 +86,7 @@ namespace gz for (const std::string &alias : _info.aliases) this->aliases[alias].insert(pluginName); - + return true; } diff --git a/register/include/gz/plugin/detail/Common.hh b/register/include/gz/plugin/detail/Common.hh index d7f2cc75..28a36416 100644 --- a/register/include/gz/plugin/detail/Common.hh +++ b/register/include/gz/plugin/detail/Common.hh @@ -23,6 +23,7 @@ #include #include #include +#include #include #include diff --git a/register/include/gz/plugin/detail/RegisterStatic.hh b/register/include/gz/plugin/detail/RegisterStatic.hh index 1a8ecfd0..bd953c5f 100644 --- a/register/include/gz/plugin/detail/RegisterStatic.hh +++ b/register/include/gz/plugin/detail/RegisterStatic.hh @@ -19,6 +19,8 @@ #ifndef GZ_PLUGIN_DETAIL_REGISTERSTATIC_HH_ #define GZ_PLUGIN_DETAIL_REGISTERSTATIC_HH_ +#include + #include #include @@ -170,4 +172,4 @@ namespace gz DETAIL_GZ_ADD_STATIC_PLUGIN_ALIAS_WITH_COUNTER( \ __COUNTER__, PluginClass, __VA_ARGS__) -#endif \ No newline at end of file +#endif From 89ac288f40ee2280edcd4561bd4ea8511dd7e7a6 Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Sun, 24 Jul 2022 11:35:57 -0700 Subject: [PATCH 10/14] Added linker dependent flags for static plugin test Signed-off-by: Shameek Ganguly --- test/integration/CMakeLists.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/integration/CMakeLists.txt b/test/integration/CMakeLists.txt index 4d9d366f..2426dcdc 100644 --- a/test/integration/CMakeLists.txt +++ b/test/integration/CMakeLists.txt @@ -45,4 +45,8 @@ endforeach() # structs specified in the plugin module still get loaded even without any # explicit reference to the loaded symbols (only the interfaces are # referenced). -target_link_libraries(INTEGRATION_static_plugins -force_load GzDummyStaticPlugin) +target_link_libraries(INTEGRATION_static_plugins GzDummyStaticPlugin) +target_link_options(INTEGRATION_static_plugins PRIVATE + $<$:--whole-archive> + $<$:-force_load> + ) \ No newline at end of file From d5050439e475d3d8aa255da6bcb4638910e667bd Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Sun, 24 Jul 2022 19:10:52 -0700 Subject: [PATCH 11/14] Fix link flag for GNU to link in static plugin lib Signed-off-by: Shameek Ganguly --- test/integration/CMakeLists.txt | 17 ++++++++--------- test/plugins/CMakeLists.txt | 2 +- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/test/integration/CMakeLists.txt b/test/integration/CMakeLists.txt index 2426dcdc..c46ccb50 100644 --- a/test/integration/CMakeLists.txt +++ b/test/integration/CMakeLists.txt @@ -41,12 +41,11 @@ foreach(test endforeach() -# The linker option -force_load needs to be specified to ensure that the global -# structs specified in the plugin module still get loaded even without any -# explicit reference to the loaded symbols (only the interfaces are -# referenced). -target_link_libraries(INTEGRATION_static_plugins GzDummyStaticPlugin) -target_link_options(INTEGRATION_static_plugins PRIVATE - $<$:--whole-archive> - $<$:-force_load> - ) \ No newline at end of file +# The linker option -force_load (for Clang) or --whole-archive (for GNU) needs +# to be specified to ensure that the global structs specified in the static +# plugin module get loaded even without any explicit reference to the +# loaded symbols (only the interfaces are referenced). +target_link_libraries(INTEGRATION_static_plugins + $<$:-Wl,--whole-archive GzDummyStaticPlugin -Wl,--no-whole-archive> + $<$:-force_load GzDummyStaticPlugin> + $<$:-force_load GzDummyStaticPlugin>) diff --git a/test/plugins/CMakeLists.txt b/test/plugins/CMakeLists.txt index 84f12b53..f9de8b1b 100644 --- a/test/plugins/CMakeLists.txt +++ b/test/plugins/CMakeLists.txt @@ -10,7 +10,7 @@ add_library(GzDummyPlugins SHARED DummyPlugins.cc DummyPluginsOtherTranslationUnit.cc) -add_library(GzDummyStaticPlugin SHARED DummyStaticPlugin.cc) +add_library(GzDummyStaticPlugin STATIC DummyStaticPlugin.cc) # Create a variable for the name of the header which will contain the dummy plugin path. # This variable gets put in the cache so that it is available at generation time. From 23dd7f155f92f10e02a3df7b22e7770e9f2bf359 Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Mon, 25 Jul 2022 08:02:47 -0700 Subject: [PATCH 12/14] Fix build error due to spaces in cmake generator expressions Signed-off-by: Shameek Ganguly --- test/integration/CMakeLists.txt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/integration/CMakeLists.txt b/test/integration/CMakeLists.txt index c46ccb50..8c9bc606 100644 --- a/test/integration/CMakeLists.txt +++ b/test/integration/CMakeLists.txt @@ -46,6 +46,7 @@ endforeach() # plugin module get loaded even without any explicit reference to the # loaded symbols (only the interfaces are referenced). target_link_libraries(INTEGRATION_static_plugins - $<$:-Wl,--whole-archive GzDummyStaticPlugin -Wl,--no-whole-archive> - $<$:-force_load GzDummyStaticPlugin> - $<$:-force_load GzDummyStaticPlugin>) + $<$:-Wl,--whole-archive> + $<$:-force_load> + $<$:-force_load> GzDummyStaticPlugin + $<$:-Wl,--no-whole-archive>) From 754674d9354ec33fb94ad74e5e6454e5444c6cee Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Fri, 29 Jul 2022 15:52:10 -0500 Subject: [PATCH 13/14] Fix visibility and linkage for MSVC Signed-off-by: Michael Carroll --- loader/include/gz/plugin/detail/Registry.hh | 10 ++++++- .../gz/plugin/detail/StaticRegistry.hh | 11 ++++++-- loader/src/detail/Registry.cc | 8 +++--- test/integration/CMakeLists.txt | 27 ++++++++++++------- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/loader/include/gz/plugin/detail/Registry.hh b/loader/include/gz/plugin/detail/Registry.hh index 2590e698..71bdcc6e 100644 --- a/loader/include/gz/plugin/detail/Registry.hh +++ b/loader/include/gz/plugin/detail/Registry.hh @@ -27,6 +27,8 @@ #include #include +#include +#include namespace gz { @@ -34,9 +36,11 @@ namespace gz { /// \brief Manages a set of plugin Infos and allows querying by name or /// by alias. - class Registry { + class GZ_PLUGIN_LOADER_VISIBLE Registry { + /// \brief Constructor public: Registry() = default; + /// \brief Destructor public: virtual ~Registry() = default; /// \brief Makes a printable string with info about plugins @@ -156,10 +160,13 @@ namespace gz /// Name of the plugin as returned by LookupPlugin(~). public: virtual void ForgetInfo(const std::string &_pluginName); + /// \brief Deleted copy constructor public: Registry(const Registry&) = delete; + /// \brief Deleted copy assignment operator public: Registry& operator=(Registry&) = delete; + GZ_UTILS_WARN_IGNORE__DLL_INTERFACE_MISSING protected: using AliasMap = std::map>; /// \brief A map from known alias names to the plugin names that they /// correspond to. Since an alias might refer to more than one plugin, @@ -170,6 +177,7 @@ namespace gz std::unordered_map; /// \brief A map from known plugin names to their Info. protected: PluginMap plugins; + GZ_UTILS_WARN_RESUME__DLL_INTERFACE_MISSING }; } } diff --git a/loader/include/gz/plugin/detail/StaticRegistry.hh b/loader/include/gz/plugin/detail/StaticRegistry.hh index a59a9096..286c89ac 100644 --- a/loader/include/gz/plugin/detail/StaticRegistry.hh +++ b/loader/include/gz/plugin/detail/StaticRegistry.hh @@ -23,7 +23,10 @@ #include #include +#include #include +#include +#include namespace gz { @@ -31,7 +34,7 @@ namespace gz { /// \brief Static registry of plugin classes populated from /// gz/plugin/RegisterStatic.hh - class StaticRegistry final: public Registry { + class GZ_PLUGIN_LOADER_VISIBLE StaticRegistry final: public Registry { /// \brief Get a reference to the StaticRegistry instance. public: static StaticRegistry& GetInstance(); @@ -67,11 +70,15 @@ namespace gz /// \param[in] _pluginName /// Name of the plugin as returned by LookupPlugin(~). public: virtual void ForgetInfo( - const std::string &_pluginName) override { } + const std::string &/*_pluginName*/) override { } + /// \brief Construtor protected: StaticRegistry() = default; + /// \brief Holds info required to construct a plugin + GZ_UTILS_WARN_IGNORE__DLL_INTERFACE_MISSING private: InfoMap infos; + GZ_UTILS_WARN_RESUME__DLL_INTERFACE_MISSING }; } } diff --git a/loader/src/detail/Registry.cc b/loader/src/detail/Registry.cc index cd3d9241..40a723b8 100644 --- a/loader/src/detail/Registry.cc +++ b/loader/src/detail/Registry.cc @@ -111,7 +111,7 @@ namespace gz const bool demangled) const { std::set allPlugins = this->AllPlugins(); - std::unordered_set plugins; + std::unordered_set availablePlugins; for (auto const &name : allPlugins) { @@ -120,17 +120,17 @@ namespace gz { if (plugin->demangledInterfaces.find(_interface) != plugin->demangledInterfaces.end()) - plugins.insert(plugin->name); + availablePlugins.insert(plugin->name); } else { if (plugin->interfaces.find(_interface) != plugin->interfaces.end()) - plugins.insert(plugin->name); + availablePlugins.insert(plugin->name); } } - return plugins; + return availablePlugins; } ///////////////////////////////////////////////// diff --git a/test/integration/CMakeLists.txt b/test/integration/CMakeLists.txt index 8c9bc606..d8d429eb 100644 --- a/test/integration/CMakeLists.txt +++ b/test/integration/CMakeLists.txt @@ -41,12 +41,21 @@ foreach(test endforeach() -# The linker option -force_load (for Clang) or --whole-archive (for GNU) needs -# to be specified to ensure that the global structs specified in the static -# plugin module get loaded even without any explicit reference to the -# loaded symbols (only the interfaces are referenced). -target_link_libraries(INTEGRATION_static_plugins - $<$:-Wl,--whole-archive> - $<$:-force_load> - $<$:-force_load> GzDummyStaticPlugin - $<$:-Wl,--no-whole-archive>) +# The linker option -force_load (for Clang) or --whole-archive (for GNU) +# or /WHOLEARCHIVE: (for MSVC) needs to be specified to ensure that the +# global structs specified in the static plugin module get loaded even +# without any explicit reference to the loaded symbols +# (only the interfaces are referenced). +if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") + # MSVC link flag doesn't work with generator expressions + # TODO(mjcarroll) When CMake 3.24 is genrally available, use + # linking generator expressions as described here: + # https://cmake.org/cmake/help/latest/manual/cmake-generator-expressions.7.html#genex:LINK_LIBRARY + target_link_libraries(INTEGRATION_static_plugins -WHOLEARCHIVE:$) +else() + target_link_libraries(INTEGRATION_static_plugins + $<$:-Wl,--whole-archive> + $<$:-force_load> + $<$:-force_load> $ + $<$:-Wl,--no-whole-archive>) +endif() From 87012c85e15d44d55c63c4e2c8a80e62a7056948 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Fri, 29 Jul 2022 16:01:06 -0500 Subject: [PATCH 14/14] Fixup linkage Signed-off-by: Michael Carroll --- test/integration/CMakeLists.txt | 2 +- test/plugins/CMakeLists.txt | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/test/integration/CMakeLists.txt b/test/integration/CMakeLists.txt index d8d429eb..cea2931c 100644 --- a/test/integration/CMakeLists.txt +++ b/test/integration/CMakeLists.txt @@ -56,6 +56,6 @@ else() target_link_libraries(INTEGRATION_static_plugins $<$:-Wl,--whole-archive> $<$:-force_load> - $<$:-force_load> $ + $<$:-force_load> GzDummyStaticPlugin $<$:-Wl,--no-whole-archive>) endif() diff --git a/test/plugins/CMakeLists.txt b/test/plugins/CMakeLists.txt index f9de8b1b..df8e4792 100644 --- a/test/plugins/CMakeLists.txt +++ b/test/plugins/CMakeLists.txt @@ -27,7 +27,6 @@ foreach(plugin_target target_link_libraries(${plugin_target} PRIVATE ${PROJECT_LIBRARY_TARGET_NAME}-register) - endforeach() # Need to link to the loader component to link static registry implementation.