From a6cf1aa386067e26d582cc1d1e327787595c9f13 Mon Sep 17 00:00:00 2001 From: Robert Edmonds Date: Wed, 20 Mar 2024 21:48:10 -0400 Subject: [PATCH 01/11] FileGenerator::GenerateHeader(): Set `min_header_version` unconditionally Previously, we were conditionally trying to set `min_header_version` to the lowest possible value, and relying on a "legacy" Google interface to determine the file descriptor's syntax version as part of that determination. Instead, simply bump the minimum version to 1003000 (1.3.0). This release was almost 7 years ago. In practice protobuf-c users should not be shipping pre-compiled .pb-c.c/.pb-c.h files, anyway. --- protoc-c/c_file.cc | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/protoc-c/c_file.cc b/protoc-c/c_file.cc index ca0ad34e..c6d8a240 100644 --- a/protoc-c/c_file.cc +++ b/protoc-c/c_file.cc @@ -117,14 +117,7 @@ FileGenerator::~FileGenerator() {} void FileGenerator::GenerateHeader(io::Printer* printer) { std::string filename_identifier = FilenameIdentifier(file_->name()); - int min_header_version = 1000000; -#if GOOGLE_PROTOBUF_VERSION >= 4023000 - if (FileDescriptorLegacy(file_).syntax() == FileDescriptorLegacy::SYNTAX_PROTO3) { -#else - if (file_->syntax() == FileDescriptor::SYNTAX_PROTO3) { -#endif - min_header_version = 1003000; - } + const int min_header_version = 1003000; // Generate top of header. printer->Print( From ee3d9e5423c93ee6b828fdda8e7fef13a77634eb Mon Sep 17 00:00:00 2001 From: Robert Edmonds Date: Wed, 20 Mar 2024 22:25:54 -0400 Subject: [PATCH 02/11] Reimplement FieldSyntax() to maximize compatibility across protobuf versions Recent versions of Google protobuf have broken the interfaces for determining the syntax version of a .proto file. The current protobuf-c 1.5.0 release does not compile with Google protobuf 26.0 due to the most recentage breakage. There is a possible workaround involving the Google protobuf `FileDescriptorLegacy` class, which is documented as: // TODO Remove this deprecated API entirely. So we probably shouldn't rely on it. Instead, this commit obtains the `FileDescriptorProto` corresponding to the passed in `FieldDescriptor` and interrogates the `syntax` field directly. This is a single implementation with no version-specific workarounds. Hopefully this won't break in the next Google protobuf release. I tested the `FieldSyntax()` implementation in this commit across a number of different Google protobuf releases and found that it worked (`make && make check`) on all of them: - Google protobuf 3.6.1.3 (Ubuntu 20.04) - Google protobuf 3.12.4 (Ubuntu 22.04) - Google protobuf 3.21.12 (Debian 12 + Debian unstable) - Google protobuf 3.25.2 (Debian experimental) - Google protobuf 26.1-dev --- protoc-c/c_helpers.h | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/protoc-c/c_helpers.h b/protoc-c/c_helpers.h index 062d330b..be28b601 100644 --- a/protoc-c/c_helpers.h +++ b/protoc-c/c_helpers.h @@ -70,10 +70,6 @@ #include #include -#if GOOGLE_PROTOBUF_VERSION >= 4023000 -# include -#endif - namespace google { namespace protobuf { namespace compiler { @@ -173,13 +169,21 @@ struct NameIndex int compare_name_indices_by_name(const void*, const void*); // Return the syntax version of the file containing the field. -// This wrapper is needed to be able to compile against protobuf2. inline int FieldSyntax(const FieldDescriptor* field) { -#if GOOGLE_PROTOBUF_VERSION >= 4023000 - return FileDescriptorLegacy(field->file()).syntax() == FileDescriptorLegacy::SYNTAX_PROTO3 ? 3 : 2; -#else - return field->file()->syntax() == FileDescriptor::SYNTAX_PROTO3 ? 3 : 2; -#endif + auto proto = FileDescriptorProto(); + field->file()->CopyTo(&proto); + + if (proto.has_syntax()) { + auto syntax = proto.syntax(); + assert(syntax == "proto2" || syntax == "proto3"); + if (syntax == "proto2") { + return 2; + } else if (syntax == "proto3") { + return 3; + } + } + + return 2; } // Work around changes in protobuf >= 22.x without breaking compilation against