Skip to content

protobuf new versions #24154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 16 commits into from
May 30, 2024
Merged

protobuf new versions #24154

merged 16 commits into from
May 30, 2024

Conversation

jcar87
Copy link
Contributor

@jcar87 jcar87 commented May 29, 2024 •
edited
Loading

  • Add versions 25 and 27, using the versioning for C++ described in https://protobuf.dev/support/version-support/#cpp
  • Add dependency on abseil for versions >22, and record actual dependencies on specific abseil components
  • Expose upb option for versions 27 and higher. Set to False by default - but this might change in the future (being cautious as they are not currently used externally and they are unconditionally static)
  • Require a minimum of C++14 for versions 22 and higher (since they require abseil)
  • Refactor logic to handle defining the protobuf::protoc target, now it's done in a separate file without in-recipe patching
  • define and expose utf8 targets, ONLY when the library is built statically (or upb is required). These targets are unconditionally static and if protobuf is shared, they are private and hidden (and consumers must not link against them)

@github-actions GitHub Actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/protobuf//'.

👋 @Hopobcn you might be interested. 😉

@conan-center-bot

This comment has been minimized.

Sign in to view
This was referenced May 29, 2024
Closed
Closed
Closed
@conan-center-bot

This comment has been minimized.

Sign in to view
uilianries
uilianries reviewed May 29, 2024
View reviewed changes
jcar87 and others added 3 commits May 29, 2024 12:54
@jcar87 @uilianries
Update recipes/protobuf/all/conanfile.py
2f57c35 
Co-authored-by: Uilian Ries <uilianries@gmail.com>
@jcar87
fixes
0146de3
@jcar87
fix
59e8055
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@jcar87
Copy link
Contributor Author

jcar87 commented May 29, 2024

@coryan, if you could confirm that following this version scheme for the Conan recipes of protobuf is sound, that would be great!
Also removing older versions, after ensuring that most of the repo is on the most recent 3.21 we provided

@uilianries uilianries mentioned this pull request May 29, 2024
Closed
3 tasks
uilianries
uilianries reviewed May 29, 2024
View reviewed changes
@conan-center-bot

This comment has been minimized.

Sign in to view
coryan
coryan reviewed May 29, 2024
View reviewed changes
@coryan
Copy link
Contributor

coryan commented May 29, 2024

@coryan, if you could confirm that following this version scheme for the Conan recipes of protobuf is sound, that would be great! Also removing older versions, after ensuring that most of the repo is on the most recent 3.21 we provided

The major versions for 4.25.x and 5.27.x both look correct to me. And thanks for cleaning up older versions.

@jcar87
Copy link
Contributor Author

jcar87 commented May 29, 2024

@coryan, if you could confirm that following this version scheme for the Conan recipes of protobuf is sound, that would be great! Also removing older versions, after ensuring that most of the repo is on the most recent 3.21 we provided

The major versions for 4.25.x and 5.27.x both look correct to me. And thanks for cleaning up older versions.

Thanks so much!
After this is merged, will move on to gRPC and google-cloud-cpp - I have a feeling that there's chance finding a coherent version of abseil that works with all 3 may be tricky, already ran into issues building gRPC with the version of abseil in this PR :/

@conan-center-bot
Copy link
Contributor

Conan v1 pipeline ✔️

All green in build 11 (c6935e0edba402cd8a00c41850b02d2a46507001):

  • protobuf/3.20.3:
    All packages built successfully! (All logs)

  • protobuf/5.27.0:
    All packages built successfully! (All logs)

  • protobuf/3.21.12:
    All packages built successfully! (All logs)

  • protobuf/4.25.3:
    All packages built successfully! (All logs)

Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 12 (c6935e0edba402cd8a00c41850b02d2a46507001):

  • protobuf/5.27.0:
    All packages built successfully! (All logs)

  • protobuf/3.21.12:
    All packages built successfully! (All logs)

  • protobuf/3.20.3:
    All packages built successfully! (All logs)

  • protobuf/4.25.3:
    All packages built successfully! (All logs)

@jcar87 jcar87 marked this pull request as ready for review May 29, 2024 18:04
AbrilRBS
AbrilRBS approved these changes May 29, 2024
View reviewed changes
recipes/protobuf/all/protobuf-conan-protoc-target.cmake

# Create executable imported target protobuf::protoc
add_executable(protobuf::protoc IMPORTED)
set_property(TARGET protobuf::protoc PROPERTY IMPORTED_LOCATION ${Protobuf_PROTOC_EXECUTABLE})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL!

@AbrilRBS AbrilRBS assigned uilianries and jcar87 and unassigned uilianries May 29, 2024
uilianries
uilianries approved these changes May 30, 2024
View reviewed changes
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, good job.

@conan-center-bot conan-center-bot merged commit f2abb6a into conan-io:master May 30, 2024
@jcar87 jcar87 deleted the lcc/protobuf-new-versions branch May 30, 2024 07:14
This was referenced May 30, 2024
Closed
Closed
Closed
@saschasc
Copy link
Contributor

@jcar87 @uilianries Thanks a lot for pushing this update. Much appreciated!

valgur
valgur reviewed May 30, 2024
View reviewed changes
recipes/protobuf/all/test_package/CMakeLists.txt
Comment on lines +24 to +30
if(NOT TARGET protobuf::protoc)
message(FATAL_ERROR "protoc executable should have been defined as part of find_package(protobuf)")
endif()

if(NOT COMMAND protobuf_generate)
message(FATAL_ERROR "protobuf_generate should have been defined as part of find_package(protobuf)")
endif()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a very good compromise. Thanks! 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@valgur Will you try to move forward with a new GRPC version? As @jcar87 mentioned this should also be done in combination with the an update of google-cloud-cpp. But the first natural step would be to upgrade GRPC, I suppose.

@matlec-siemens matlec-siemens mentioned this pull request Jun 3, 2024
Closed
@kletoz kletoz mentioned this pull request Nov 21, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uilianries uilianries uilianries approved these changes

@AbrilRBS AbrilRBS AbrilRBS approved these changes

+3 more reviewers

@valgur valgur valgur left review comments

@coryan coryan coryan left review comments

@saschasc saschasc saschasc left review comments

Reviewers whose approvals may not affect merge requirements
Assignees

@jcar87 jcar87

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jcar87 @conan-center-bot @coryan @saschasc @valgur @uilianries @AbrilRBS