Test build of #711 with #714 #715
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ally 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.
…ersions 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
…roto files
The Google protobuf project is currently experimenting with a new syntax
for .proto files called "editions". Since protobuf-c is a proto2/proto3
compiler, after the previous commit reimplementing `FieldSyntax()`, the
protobuf compiler will abort like this if presented with an "editions"
syntax .proto file due to the safety check in `FieldSyntax()`:
$ protoc --experimental_editions --c_out=. test.proto
protoc-gen-c: ./protoc-c/c_helpers.h:178: int google::protobuf::compiler::c::FieldSyntax(const google::protobuf::FieldDescriptor*): Assertion `syntax == "proto2" || syntax == "proto3"' failed.
--c_out: protoc-gen-c: Plugin killed by signal 6.
On protobuf 26, our `CodeGenerator` can implement certain methods to
declare that we "support" editions, and then reject any other edition
except proto2 and proto3, which have apparently been retroactively
declared to be "editions". Of course this needs to be wrapped in a
version guard.
With this protection in place, the protobuf compiler cleanly exits with
a nice error message like this:
$ protoc --experimental_editions --c_out=. test.proto
WARNING: All log messages before absl::InitializeLog() is called are written to STDERR
E0000 00:00:1710988958.296200 20022 descriptor.cc:4620] Invalid proto descriptor for file "test.proto":
E0000 00:00:1710988958.296239 20022 descriptor.cc:4623] test.proto: Edition 2023 is later than the maximum supported edition PROTO3
--c_out: protoc-gen-c: Plugin failed with status code 1.
The CMake module is used by default but isn't compatible with recent protobuf version. Try to first look for a protobuf config then fallback to legacy cmake module. Signed-off-by: Clément Péron <peron.clem@gmail.com>
APT cache could be out of date leading to a: E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing? Fix this by always calling apt-get update before an apt-get install. Signed-off-by: Clément Péron <peron.clem@gmail.com>
Keep the DART compatibility enabled until test are moved to CTest. Signed-off-by: Clément Péron <peron.clem@gmail.com>
PROTOBUF_PROTOC_EXECUTABLE is not set in the Protobuf CMake config file. Set it properly in case we use CMake protobuf config file. Signed-off-by: Clément Péron <peron.clem@gmail.com>
…uf-26-fixes_plus_pull-714
edmonds
force-pushed
the
edmonds/test/google-protobuf-26-fixes_plus_pull-714
branch
from
27c10cf
to
fdd163f
Compare
This job builds abseil-cpp and protobuf from source using Cmake and tries to build protobuf-c against those dependencies using Cmake. The "latest" build dependencies are being targeted, currently Ubuntu 22.04, abseil-cpp from the lts_2024_01_16 branch, protobuf from the 27.x branch. Update as newer versions become available. This uses Cmake's Ninja generator which should be equivalent to the legacy makefiles that it generates. The Cmake Ninja generator is useful primarily because it automatically compiles using the maximum amount of concurrency and it actually shows you the command that failed by default.
edmonds
force-pushed
the
edmonds/test/google-protobuf-26-fixes_plus_pull-714
branch
from
fdd163f
to
9bdd24f
Compare
This reverts commit 1faae3a.
Apparently ${PROTOBUF_LIBRARY} is empty?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Testing #714 on top of #711, do not merge.