From 57781ae656229d321871cd546156434f372543c7 Mon Sep 17 00:00:00 2001 From: Jose Luis Rivero Date: Mon, 2 Mar 2026 18:37:02 +0100 Subject: [PATCH 1/8] Load always the own gz_msgs*.gz_desc first and avoid duplicates Signed-off-by: Jose Luis Rivero --- core/src/DynamicFactory.cc | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/core/src/DynamicFactory.cc b/core/src/DynamicFactory.cc index 851f338d..d8b80851 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,12 @@ 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 [" @@ -141,8 +148,20 @@ void DynamicFactory::LoadDescriptors(const std::string &_paths) } else { + // Default to loading the descriptor file for this gz-msgs major version if it exists + const std::string ownDescFile = + "gz-msgs" + std::to_string(GZ_MSGS_MAJOR_VERSION) + ".gz_desc"; + 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()); } } From 4133fbceb2c9940300583de44913fedad178f538 Mon Sep 17 00:00:00 2001 From: Jose Luis Rivero Date: Wed, 4 Mar 2026 18:32:25 +0100 Subject: [PATCH 2/8] Add regression test for duplicate descriptor loading (issue #460) Adds NoDuplicateDescriptors test that reproduces the scenario where the same .gz_desc file is reachable from multiple paths (e.g. GZ_DESCRIPTOR_PATH and global share dir), verifying no libprotobuf "File already exists" errors. Use protobuf log handler to properly detect duplicate descriptor errors Generated-By: Claude Sonnet 4.6 --- test/integration/descriptors.cc | 44 +++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/integration/descriptors.cc b/test/integration/descriptors.cc index 4cc62291..afa0e570 100644 --- a/test/integration/descriptors.cc +++ b/test/integration/descriptors.cc @@ -19,6 +19,8 @@ #include +#include + #include "gz/msgs/Factory.hh" static constexpr const char * kMsgsTestPath = GZ_MSGS_TEST_PATH; @@ -93,3 +95,45 @@ 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 + + // SetLogHandler requires a plain function pointer (no captures), so use a + // static flag to detect ERROR-level messages from libprotobuf during loading. + static bool s_protobuf_error_seen; + s_protobuf_error_seen = false; + auto * old_handler = google::protobuf::SetLogHandler( + [](google::protobuf::LogLevel level, const char *, int, + const std::string &) { + if (level == google::protobuf::LOGLEVEL_ERROR) { + s_protobuf_error_seen = true; + } + }); + + // 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); + + google::protobuf::SetLogHandler(old_handler); + + EXPECT_FALSE(s_protobuf_error_seen) + << "libprotobuf ERROR emitted during duplicate descriptor load"; + + // The message type must remain accessible after duplicate loading. + EXPECT_NE(nullptr, gz::msgs::Factory::New("example.msgs.StringMsg")); +} From 20999342b14da5986dacb93c6e7bb69594140bf8 Mon Sep 17 00:00:00 2001 From: Jose Luis Rivero Date: Wed, 4 Mar 2026 19:14:15 +0100 Subject: [PATCH 3/8] Place const outside the loop Signed-off-by: Jose Luis Rivero --- core/src/DynamicFactory.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/DynamicFactory.cc b/core/src/DynamicFactory.cc index d8b80851..03864c2e 100644 --- a/core/src/DynamicFactory.cc +++ b/core/src/DynamicFactory.cc @@ -140,6 +140,9 @@ void DynamicFactory::LoadDescriptors(const std::string &_paths) } }; + const std::string ownDescFile = + "gz-msgs" + std::to_string(GZ_MSGS_MAJOR_VERSION) + ".gz_desc"; + for (const std::string &descDir : descDirs) { if (!std::filesystem::is_directory(descDir)) @@ -149,8 +152,6 @@ void DynamicFactory::LoadDescriptors(const std::string &_paths) else { // Default to loading the descriptor file for this gz-msgs major version if it exists - const std::string ownDescFile = - "gz-msgs" + std::to_string(GZ_MSGS_MAJOR_VERSION) + ".gz_desc"; auto ownDescPath = std::filesystem::path(descDir) / ownDescFile; if (std::filesystem::exists(ownDescPath)) { From c922c72de2f7466a44a479eb4243cecb2b2bebdf Mon Sep 17 00:00:00 2001 From: Jose Luis Rivero Date: Wed, 4 Mar 2026 19:27:19 +0100 Subject: [PATCH 4/8] Update core/src/DynamicFactory.cc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Carlos Agüero Signed-off-by: Jose Luis Rivero --- core/src/DynamicFactory.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/DynamicFactory.cc b/core/src/DynamicFactory.cc index 03864c2e..4d7ef444 100644 --- a/core/src/DynamicFactory.cc +++ b/core/src/DynamicFactory.cc @@ -124,7 +124,8 @@ void DynamicFactory::LoadDescriptors(const std::string &_paths) { // 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) { + if (this->pool.FindFileByName(fileDescriptorProto.name()) != nullptr) + { continue; } From e49984e717b1f03c7058f05450274aa74b38c4ae Mon Sep 17 00:00:00 2001 From: Jose Luis Rivero Date: Wed, 4 Mar 2026 23:00:43 +0100 Subject: [PATCH 5/8] Use gtest artifacts instead of protobuf internal headers Signed-off-by: Jose Luis Rivero --- test/integration/descriptors.cc | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/test/integration/descriptors.cc b/test/integration/descriptors.cc index afa0e570..1d204bfe 100644 --- a/test/integration/descriptors.cc +++ b/test/integration/descriptors.cc @@ -19,8 +19,6 @@ #include -#include - #include "gz/msgs/Factory.hh" static constexpr const char * kMsgsTestPath = GZ_MSGS_TEST_PATH; @@ -113,26 +111,19 @@ TEST(FactoryTest, NoDuplicateDescriptors) std::string paths = path + ":" + path; #endif - // SetLogHandler requires a plain function pointer (no captures), so use a - // static flag to detect ERROR-level messages from libprotobuf during loading. - static bool s_protobuf_error_seen; - s_protobuf_error_seen = false; - auto * old_handler = google::protobuf::SetLogHandler( - [](google::protobuf::LogLevel level, const char *, int, - const std::string &) { - if (level == google::protobuf::LOGLEVEL_ERROR) { - s_protobuf_error_seen = true; - } - }); + // 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); - google::protobuf::SetLogHandler(old_handler); - - EXPECT_FALSE(s_protobuf_error_seen) - << "libprotobuf ERROR emitted during duplicate descriptor load"; + 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")); From 2e2483c90adb16f55573f6a8b122b0fd04019fb9 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Thu, 12 Mar 2026 15:36:34 -0500 Subject: [PATCH 6/8] Bump main to 13.0.0~pre1 (#543) Signed-off-by: Addisu Z. Taddese Co-authored-by: Addisu Z. Taddese --- CMakeLists.txt | 2 +- Changelog.md | 4 ++++ Migration.md | 6 +++++- package.xml | 2 +- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index af414ff8..67a95024 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.22.1 FATAL_ERROR) #============================================================================ # Initialize the project #============================================================================ -project(gz-msgs VERSION 12.0.0) +project(gz-msgs VERSION 13.0.0) #============================================================================ # Find gz-cmake diff --git a/Changelog.md b/Changelog.md index 3eb7f721..582484f0 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,7 @@ +## Gazebo Msgs 13.x + +### Gazebo Msgs 13.0.0 (20XX-XX-XX) + ## Gazebo Msgs 12.x ### Gazebo Msgs 12.0.0 (2025-09-XX) diff --git a/Migration.md b/Migration.md index 4069501f..061f6f1e 100644 --- a/Migration.md +++ b/Migration.md @@ -5,6 +5,10 @@ Deprecated code produces compile-time warnings. These warning serve as notification to users that their code should be upgraded. The next major release will remove the deprecated code. + +## Gazebo Msgs 12.X to 13.X + + ## Gazebo Msgs 11.X to 12.X 1. The major version has been removed from the cmake project name and the @@ -250,4 +254,4 @@ release will remove the deprecated code. 1. **Generator.hh** + This file is no longer installed. It served only to make an internal - protc plugin that customized the protobuf compiler output. + protc plugin that customized the protobuf compiler output. \ No newline at end of file diff --git a/package.xml b/package.xml index eceb6b1c..3fa54148 100644 --- a/package.xml +++ b/package.xml @@ -1,7 +1,7 @@ gz-msgs - 12.0.0 + 13.0.0 Gazebo Messages: Protobuf messages and functions for robot applications Carlos Agüero Apache License 2.0 From 0b7dbd2f16701dda44f7ca682cfc48e35c212efe Mon Sep 17 00:00:00 2001 From: Jose Luis Rivero Date: Fri, 13 Mar 2026 16:32:38 +0100 Subject: [PATCH 7/8] Extract GZ_MSGS_DESC_FILENAME variable to avoid hardcoded desc filenames After the desc filename was changed to drop the major version number, refactor the filename into a CMake variable and a C++ preprocessor define so it is defined in a single place. This also fixes the DynamicFactory.cc runtime construction which was still using the old versioned filename format. Generated-by: Claude Opus 4.6 Signed-off-by: Jose Luis Rivero --- core/CMakeLists.txt | 10 ++++++---- core/include/gz/msgs/config.hh.in | 2 ++ core/src/DynamicFactory.cc | 3 +-- 3 files changed, 9 insertions(+), 6 deletions(-) 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 7506adc6..ae10963b 100644 --- a/core/src/DynamicFactory.cc +++ b/core/src/DynamicFactory.cc @@ -141,8 +141,7 @@ void DynamicFactory::LoadDescriptors(const std::string &_paths) } }; - const std::string ownDescFile = - "gz-msgs" + std::to_string(GZ_MSGS_MAJOR_VERSION) + ".gz_desc"; + const std::string ownDescFile = GZ_MSGS_DESC_FILENAME; for (const std::string &descDir : descDirs) { From 5eb70fa40f06eb48c9ab78c944aef1634226a78d Mon Sep 17 00:00:00 2001 From: Jose Luis Rivero Date: Fri, 13 Mar 2026 18:39:39 +0100 Subject: [PATCH 8/8] Fix linting Signed-off-by: Jose Luis Rivero --- core/src/DynamicFactory.cc | 3 ++- test/integration/descriptors.cc | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/DynamicFactory.cc b/core/src/DynamicFactory.cc index ae10963b..19d21c43 100644 --- a/core/src/DynamicFactory.cc +++ b/core/src/DynamicFactory.cc @@ -151,7 +151,8 @@ void DynamicFactory::LoadDescriptors(const std::string &_paths) } else { - // Default to loading the descriptor file for this gz-msgs major version if it exists + // 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)) { diff --git a/test/integration/descriptors.cc b/test/integration/descriptors.cc index 1d204bfe..adb31669 100644 --- a/test/integration/descriptors.cc +++ b/test/integration/descriptors.cc @@ -117,7 +117,8 @@ TEST(FactoryTest, NoDuplicateDescriptors) ::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. + // 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();