From bcb94089c48c96b7d4ec617b875378c6e6610d5b Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Wed, 11 Jun 2025 16:07:41 -0700 Subject: [PATCH 1/4] Add support for loading plugin from static plugin registry Signed-off-by: Shameek Ganguly --- src/SystemLoader.cc | 77 +++++++++++++--- test/BUILD.bazel | 28 ++++++ .../load_system_static_registry.cc | 89 +++++++++++++++++++ test/plugins/TestStaticModelSystem.cc | 25 ++++++ 4 files changed, 208 insertions(+), 11 deletions(-) create mode 100644 test/integration/load_system_static_registry.cc create mode 100644 test/plugins/TestStaticModelSystem.cc diff --git a/src/SystemLoader.cc b/src/SystemLoader.cc index 5ab522edfd..14f5fb4fa9 100644 --- a/src/SystemLoader.cc +++ b/src/SystemLoader.cc @@ -39,6 +39,10 @@ using namespace gz::sim; class gz::sim::SystemLoaderPrivate { + ////////////////////////////////////////////////// + public: static constexpr std::string_view kStaticPluginFilenamePrefix = + "static://"; + ////////////////////////////////////////////////// public: explicit SystemLoaderPrivate() = default; @@ -60,6 +64,61 @@ class gz::sim::SystemLoaderPrivate return systemPaths.PluginPaths(); } + ////////////////////////////////////////////////// + public: std::string FixDeprecatedPluginName(const std::string& _pluginName) + { + std::string newPluginName = _pluginName; + constexpr std::string_view deprecatedPluginNamePrefix{"ignition::gazebo"}; + if (auto pos = _pluginName.find(deprecatedPluginNamePrefix); + pos != std::string::npos) + { + newPluginName.replace(pos, deprecatedPluginNamePrefix.size(), "gz::sim"); + gzwarn << "Trying to load deprecated plugin name [" << _pluginName + << "]. Using [" << newPluginName << "] instead." + << std::endl; + } + return newPluginName; + } + + ////////////////////////////////////////////////// + public: bool InstantiateStaticSystemPlugin(const sdf::Plugin &_sdfPlugin, + gz::plugin::PluginPtr &_gzPlugin) + { + const size_t prefixLen = kStaticPluginFilenamePrefix.size(); + const std::string filenameWoPrefix = + _sdfPlugin.Filename().substr(prefixLen); + std::string pluginToInstantiate = FixDeprecatedPluginName(filenameWoPrefix); + + _gzPlugin = this->loader.Instantiate(pluginToInstantiate); + + if (!_gzPlugin) + { + gzerr << "Failed to load system plugin: " + << "(Reason: static plugin registry does not contain the requested " + "plugin)\n" + << "- Requested plugin name: [" << _sdfPlugin.Name() << "]\n" + << "- Requested library name: [" << _sdfPlugin.Filename() << "]\n"; + return false; + } + + if (!_gzPlugin->HasInterface()) + { + std::stringstream ss; + ss << "Failed to load system plugin: " + << "(Reason: plugin does not implement System interface)\n" + << "- Requested plugin name: [" << _sdfPlugin.Name() << "]\n" + << "- Requested library name: [" << _sdfPlugin.Filename() << "]\n" + << "- Plugin Interfaces Implemented:\n"; + for (const auto &interfaceIt : this->loader.InterfacesImplemented()) + { + ss << " - " << interfaceIt << "\n"; + } + return false; + } + + return true; + } + ////////////////////////////////////////////////// public: bool InstantiateSystemPlugin(const sdf::Plugin &_sdfPlugin, gz::plugin::PluginPtr &_gzPlugin) @@ -75,6 +134,12 @@ class gz::sim::SystemLoaderPrivate << "]. Using [" << filename << "] instead." << std::endl; } + if (filename.substr(0, kStaticPluginFilenamePrefix.size()) == + kStaticPluginFilenamePrefix) + { + return InstantiateStaticSystemPlugin(_sdfPlugin, _gzPlugin); + } + const std::list paths = this->PluginPaths(); common::SystemPaths systemPaths; for (const auto &p : paths) @@ -125,17 +190,7 @@ class gz::sim::SystemLoaderPrivate pluginName : _sdfPlugin.Name(); // Deprecated: accept ignition plugins. - std::string deprecatedPluginNamePrefix{"ignition::gazebo"}; - pos = pluginToInstantiate.find(deprecatedPluginNamePrefix); - if (pos != std::string::npos) - { - auto origPluginName = pluginToInstantiate; - pluginToInstantiate.replace(pos, deprecatedPluginNamePrefix.size(), - "gz::sim"); - gzwarn << "Trying to load deprecated plugin name [" << origPluginName - << "]. Using [" << pluginToInstantiate << "] instead." - << std::endl; - } + pluginToInstantiate = FixDeprecatedPluginName(pluginToInstantiate); _gzPlugin = this->loader.Instantiate(pluginToInstantiate); if (!_gzPlugin) diff --git a/test/BUILD.bazel b/test/BUILD.bazel index 9fece30f3a..537c527231 100644 --- a/test/BUILD.bazel +++ b/test/BUILD.bazel @@ -53,6 +53,34 @@ cc_library( ], ) +cc_library( + name = "TestStaticModelSystem", + testonly = 1, + srcs = [ + "plugins/TestModelSystem.hh", + "plugins/TestStaticModelSystem.cc", + ], + deps = [ + "//:gz-sim", + "@gz-plugin//:register", + ], + alwayslink = 1, +) + +cc_test( + name = "INTEGRATION_load_system_static_registry", + srcs = ["integration/load_system_static_registry.cc"], + deps = [ + ":Helpers", + ":MockSystem", + ":TestStaticModelSystem", + "//:gz-sim", + "@googletest//:gtest", + "@googletest//:gtest_main", + "@gz-common", + ], +) + filegroup( name = "worlds", srcs = glob(["worlds/**"]), diff --git a/test/integration/load_system_static_registry.cc b/test/integration/load_system_static_registry.cc new file mode 100644 index 0000000000..ab214c7af6 --- /dev/null +++ b/test/integration/load_system_static_registry.cc @@ -0,0 +1,89 @@ +/* + * Copyright (C) 2025 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 + +#include "../helpers/EnvTestFixture.hh" +#include "gz/sim/Server.hh" +#include "plugins/MockSystem.hh" + +using namespace gz; +using namespace sim; + +using ::testing::Eq; +using ::testing::Optional; + +/// \brief Test loading system from static plugin registry +class LoadSystemStaticRegistryTest : public InternalFixture<::testing::Test> {}; + +TEST_F(LoadSystemStaticRegistryTest, LoadWorks) +{ + // Start server + ServerConfig serverConfig; + serverConfig.SetSdfString(R"( + + + + + true + + + 98765 + + + + true + + + + 54321 + + + + )"); + + Server server(serverConfig); + + // Verify that server was initialized correctly + ASSERT_THAT(server.IterationCount(), Optional(Eq(0))); + std::optional model1Id = server.EntityByName("test_model1"); + ASSERT_NE(model1Id, std::nullopt); + std::optional model2Id = server.EntityByName("test_model2"); + ASSERT_NE(model2Id, std::nullopt); + + // Verify that the System was instantiated for both models by checking the ECM + // for the configured `ModelPluginComponent`. + auto mockSystem = std::make_shared(); + mockSystem->postUpdateCallback = + [model1Id, model2Id](const sim::UpdateInfo &, + const sim::EntityComponentManager &_ecm) + { + const std::string modelComponentName{"ModelPluginComponent"}; + auto modelComponentId = common::hash64(modelComponentName); + EXPECT_TRUE(_ecm.HasComponentType(modelComponentId)); + EXPECT_TRUE(_ecm.EntityHasComponentType(*model1Id, modelComponentId)); + EXPECT_TRUE(_ecm.EntityHasComponentType(*model2Id, modelComponentId)); + }; + ASSERT_TRUE(server.AddSystem(mockSystem)); + server.RunOnce(); + EXPECT_EQ(1, mockSystem->postUpdateCallCount); +} diff --git a/test/plugins/TestStaticModelSystem.cc b/test/plugins/TestStaticModelSystem.cc new file mode 100644 index 0000000000..d54a74fc1d --- /dev/null +++ b/test/plugins/TestStaticModelSystem.cc @@ -0,0 +1,25 @@ +/* + * Copyright (C) 2025 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 "TestModelSystem.hh" + +GZ_ADD_STATIC_PLUGIN(gz::sim::TestModelSystem, + gz::sim::System, + gz::sim::TestModelSystem::ISystemConfigure) + +GZ_ADD_STATIC_PLUGIN_ALIAS(gz::sim::TestModelSystem, "StaticTestModelSystem") From b1b9f482df1faf95ec5c628d3e2b0c9999484825 Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Fri, 13 Jun 2025 12:14:08 -0700 Subject: [PATCH 2/4] Add test in cmake build Signed-off-by: Shameek Ganguly --- test/integration/CMakeLists.txt | 24 +++++++++++++++++++ .../load_system_static_registry.cc | 9 ++++--- test/plugins/CMakeLists.txt | 7 ++++++ 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/test/integration/CMakeLists.txt b/test/integration/CMakeLists.txt index 568bb53289..2eb075520e 100644 --- a/test/integration/CMakeLists.txt +++ b/test/integration/CMakeLists.txt @@ -47,6 +47,7 @@ set(tests light.cc lighter_than_air_dynamics.cc link.cc + load_system_static_registry.cc logical_camera_system.cc logical_audio_sensor_plugin.cc magnetometer_system.cc @@ -249,3 +250,26 @@ if (TARGET INTEGRATION_python_system_loader) set_tests_properties(INTEGRATION_python_system_loader PROPERTIES ENVIRONMENT "PYTHONPATH=${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/python/:$ENV{PYTHONPATH}") endif() + +if (TARGET INTEGRATION_load_system_static_registry) + # 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") + target_link_libraries(INTEGRATION_load_system_static_registry + -WHOLEARCHIVE:$) + + # The whole-archive invocation doesn't correctly compute dependencies, + # so explicitly require the plugin before the test can build. + add_dependencies(INTEGRATION_load_system_static_registry + TestStaticModelSystem) + else() + target_link_libraries(INTEGRATION_load_system_static_registry + $<$:-Wl,--whole-archive> + $<$:-force_load> + $<$:-force_load> TestStaticModelSystem + $<$:-Wl,--no-whole-archive>) + endif() +endif() diff --git a/test/integration/load_system_static_registry.cc b/test/integration/load_system_static_registry.cc index ab214c7af6..43ce2787a4 100644 --- a/test/integration/load_system_static_registry.cc +++ b/test/integration/load_system_static_registry.cc @@ -15,7 +15,6 @@ * */ -#include #include #include @@ -27,9 +26,6 @@ using namespace gz; using namespace sim; -using ::testing::Eq; -using ::testing::Optional; - /// \brief Test loading system from static plugin registry class LoadSystemStaticRegistryTest : public InternalFixture<::testing::Test> {}; @@ -64,7 +60,10 @@ TEST_F(LoadSystemStaticRegistryTest, LoadWorks) Server server(serverConfig); // Verify that server was initialized correctly - ASSERT_THAT(server.IterationCount(), Optional(Eq(0))); + auto iterationCount = server.IterationCount(); + ASSERT_NE(iterationCount, std::nullopt); + ASSERT_EQ(*iterationCount, 0); + std::optional model1Id = server.EntityByName("test_model1"); ASSERT_NE(model1Id, std::nullopt); std::optional model2Id = server.EntityByName("test_model2"); diff --git a/test/plugins/CMakeLists.txt b/test/plugins/CMakeLists.txt index f2cbb55595..e281386e11 100644 --- a/test/plugins/CMakeLists.txt +++ b/test/plugins/CMakeLists.txt @@ -49,3 +49,10 @@ if(BUILD_TESTING) endif() endforeach (src) endif() + +# Statically linked test plugin +add_library(TestStaticModelSystem STATIC TestStaticModelSystem.cc) +target_link_libraries(TestStaticModelSystem PRIVATE + gz-plugin${GZ_PLUGIN_VER}::register + gz-transport${GZ_TRANSPORT_VER}::gz-transport${GZ_TRANSPORT_VER} + gz-sim${PROJECT_VERSION_MAJOR}) From 8d45413a07ac9734deaa8421f3b2291bf1219c22 Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Fri, 13 Jun 2025 13:16:43 -0700 Subject: [PATCH 3/4] Clang bazel fixes Signed-off-by: Shameek Ganguly --- test/BUILD.bazel | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/BUILD.bazel b/test/BUILD.bazel index 537c527231..c739c1ad5d 100644 --- a/test/BUILD.bazel +++ b/test/BUILD.bazel @@ -62,7 +62,9 @@ cc_library( ], deps = [ "//:gz-sim", + "@gz-msgs//:gzmsgs_cc_proto", "@gz-plugin//:register", + "@gz-transport", ], alwayslink = 1, ) From da493c6980a823a7fffdce751ee7f98cf681b677 Mon Sep 17 00:00:00 2001 From: Shameek Ganguly Date: Fri, 13 Jun 2025 16:05:02 -0700 Subject: [PATCH 4/4] Review style fixes Signed-off-by: Shameek Ganguly --- src/SystemLoader.cc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/SystemLoader.cc b/src/SystemLoader.cc index 14f5fb4fa9..aaede7742e 100644 --- a/src/SystemLoader.cc +++ b/src/SystemLoader.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -65,7 +66,7 @@ class gz::sim::SystemLoaderPrivate } ////////////////////////////////////////////////// - public: std::string FixDeprecatedPluginName(const std::string& _pluginName) + public: std::string FixDeprecatedPluginName(const std::string &_pluginName) { std::string newPluginName = _pluginName; constexpr std::string_view deprecatedPluginNamePrefix{"ignition::gazebo"}; @@ -87,7 +88,8 @@ class gz::sim::SystemLoaderPrivate const size_t prefixLen = kStaticPluginFilenamePrefix.size(); const std::string filenameWoPrefix = _sdfPlugin.Filename().substr(prefixLen); - std::string pluginToInstantiate = FixDeprecatedPluginName(filenameWoPrefix); + std::string pluginToInstantiate = + this->FixDeprecatedPluginName(filenameWoPrefix); _gzPlugin = this->loader.Instantiate(pluginToInstantiate); @@ -137,7 +139,7 @@ class gz::sim::SystemLoaderPrivate if (filename.substr(0, kStaticPluginFilenamePrefix.size()) == kStaticPluginFilenamePrefix) { - return InstantiateStaticSystemPlugin(_sdfPlugin, _gzPlugin); + return this->InstantiateStaticSystemPlugin(_sdfPlugin, _gzPlugin); } const std::list paths = this->PluginPaths(); @@ -190,7 +192,8 @@ class gz::sim::SystemLoaderPrivate pluginName : _sdfPlugin.Name(); // Deprecated: accept ignition plugins. - pluginToInstantiate = FixDeprecatedPluginName(pluginToInstantiate); + pluginToInstantiate = + this->FixDeprecatedPluginName(pluginToInstantiate); _gzPlugin = this->loader.Instantiate(pluginToInstantiate); if (!_gzPlugin)