diff --git a/core/CMakeLists.txt b/core/CMakeLists.txt index 93288362..591d4b95 100644 --- a/core/CMakeLists.txt +++ b/core/CMakeLists.txt @@ -63,18 +63,20 @@ gz_msgs_generate_messages_impl( msgs_python ) +set(GZ_MSGS_DESC_FILENAME "gz-msgs.gz_desc") + gz_msgs_generate_desc_impl( PROTOC_EXEC ${${PROJECT_NAME}_PROTOC_EXECUTABLE} INPUT_PROTOS ${proto_files} PROTO_PATH ${PROJECT_SOURCE_DIR}/proto DEPENDENCY_DESCRIPTIONS ${depends_msgs_desc} OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/gz_msgs_gen - OUTPUT_FILENAME gz-msgs.gz_desc + OUTPUT_FILENAME ${GZ_MSGS_DESC_FILENAME} ) install(FILES ${msgs_headers} DESTINATION ${GZ_INCLUDE_INSTALL_DIR_FULL}/gz/msgs) install(FILES ${msgs_detail_headers} DESTINATION ${GZ_INCLUDE_INSTALL_DIR_FULL}/gz/msgs/details) -install(FILES ${CMAKE_CURRENT_BINARY_DIR}/gz-msgs.gz_desc +install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${GZ_MSGS_DESC_FILENAME} DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/gz/protos/) if (NOT GZ_PYTHON_INSTALL_PATH) @@ -102,7 +104,7 @@ gz_create_core_library(SOURCES src/MessageFactory.cc src/DynamicFactory.cc ${msgs_sources} - gz-msgs.gz_desc + ${GZ_MSGS_DESC_FILENAME} ) target_include_directories(${PROJECT_LIBRARY_TARGET_NAME} PUBLIC @@ -113,7 +115,7 @@ set_target_properties( PROPERTIES SOVERSION ${PROJECT_VERSION_MAJOR} VERSION ${PROJECT_VERSION_FULL} - GZ_MSGS_DESC_FILE "\$\{_IMPORT_PREFIX\}/share/gz/protos/gz-msgs.gz_desc" + GZ_MSGS_DESC_FILE "\$\{_IMPORT_PREFIX\}/share/gz/protos/${GZ_MSGS_DESC_FILENAME}" ) set_property(TARGET ${PROJECT_LIBRARY_TARGET_NAME} PROPERTY EXPORT_PROPERTIES "GZ_MSGS_DESC_FILE") diff --git a/core/include/gz/msgs/config.hh.in b/core/include/gz/msgs/config.hh.in index eef9e136..26554fe6 100644 --- a/core/include/gz/msgs/config.hh.in +++ b/core/include/gz/msgs/config.hh.in @@ -32,4 +32,6 @@ #define GZ_MSGS_VERSION_HEADER "Gazebo Msgs, version ${PROJECT_VERSION_FULL}\nCopyright (C) 2016 Open Source Robotics Foundation.\nReleased under the Apache 2.0 License.\n\n" +#define GZ_MSGS_DESC_FILENAME "${GZ_MSGS_DESC_FILENAME}" + #endif diff --git a/core/src/DynamicFactory.cc b/core/src/DynamicFactory.cc index 279fdfd6..19d21c43 100644 --- a/core/src/DynamicFactory.cc +++ b/core/src/DynamicFactory.cc @@ -27,6 +27,7 @@ #include "DynamicFactory.hh" #include "gz/utils/Environment.hh" +#include #include namespace { @@ -121,6 +122,13 @@ void DynamicFactory::LoadDescriptors(const std::string &_paths) for (const google::protobuf::FileDescriptorProto &fileDescriptorProto : fileDescriptorSet.file()) { + // Skip protos already loaded (e.g. same .gz_desc reached via + // both GZ_DESCRIPTOR_PATH and the global share directory). + if (this->pool.FindFileByName(fileDescriptorProto.name()) != nullptr) + { + continue; + } + if (!static_cast(this->pool.BuildFile(fileDescriptorProto))) { std::cerr << "DynamicFactory(). Unable to place descriptors from [" @@ -133,6 +141,8 @@ void DynamicFactory::LoadDescriptors(const std::string &_paths) } }; + const std::string ownDescFile = GZ_MSGS_DESC_FILENAME; + for (const std::string &descDir : descDirs) { if (!std::filesystem::is_directory(descDir)) @@ -141,8 +151,19 @@ void DynamicFactory::LoadDescriptors(const std::string &_paths) } else { + // Default to loading the descriptor file for this gz-msgs major + // version if it exists + auto ownDescPath = std::filesystem::path(descDir) / ownDescFile; + if (std::filesystem::exists(ownDescPath)) + { + loadDescFile(ownDescPath.string()); + } + for (auto const &dirIter : std::filesystem::directory_iterator{descDir}) { + auto filename = dirIter.path().filename().string(); + if (filename == ownDescFile) // Skip the ownDesc already loaded + continue; loadDescFile(dirIter.path().string()); } } diff --git a/test/integration/descriptors.cc b/test/integration/descriptors.cc index 4cc62291..adb31669 100644 --- a/test/integration/descriptors.cc +++ b/test/integration/descriptors.cc @@ -93,3 +93,39 @@ TEST(FactoryTest, DynamicFactory) EXPECT_NE(nullptr, gz::msgs::Factory::New("testing.BarMessage")); EXPECT_NE(nullptr, gz::msgs::Factory::New("testing.BazMessage")); } + +TEST(FactoryTest, NoDuplicateDescriptors) +{ + // Regression test for https://github.com/gazebosim/gz-msgs/issues/460 + // When the same .gz_desc file is reachable via both GZ_DESCRIPTOR_PATH + // and the global share directory, the DynamicFactory used to print: + // [libprotobuf ERROR] File already exists in database: ... + // The fix silently skips proto definitions already present in the pool. + + std::filesystem::path test_path(kMsgsTestPath); + std::string path = (test_path / "desc" / "stringmsg.desc").string(); + +#ifdef _WIN32 + std::string paths = path + ";" + path; +#else + std::string paths = path + ":" + path; +#endif + + // Capture stderr to detect any "already exists in database" errors from + // libprotobuf. Using gtest's CaptureStderr avoids depending on the + // protobuf-internal SetLogHandler API, which was removed in protobuf 4.x. + ::testing::internal::CaptureStderr(); + + // Loading the same descriptor twice must not cause protobuf errors. + // The second occurrence is skipped because the protos are already in the + // pool. + gz::msgs::Factory::LoadDescriptors(paths); + + std::string stderr_output = ::testing::internal::GetCapturedStderr(); + EXPECT_EQ(stderr_output.find("already exists in database"), std::string::npos) + << "libprotobuf ERROR emitted during duplicate descriptor load: " + << stderr_output; + + // The message type must remain accessible after duplicate loading. + EXPECT_NE(nullptr, gz::msgs::Factory::New("example.msgs.StringMsg")); +}