Fix build failure with the Protobuf 23.3 #290
Merged
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
BewareMyPower
force-pushed
the
bewaremypower/fix-latest-protobuf
branch
from
1b4adae to
53ca69d
Compare
|
In this way, the macOS developer needs to use C++ 17 to compile this project. We need to update README. |
Yes. BTW, I think we can make C++17 as the default standard for macOS users because it's now required by the library dependency, not the test dependency. I marked this PR as drafted first. |
### Motivation The current CMakeLists.txt does not work with the latest Protobuf (23.3.0). It's because currently the Module mode is used to find Protobuf, while the `FindProtobuf.cmake` is not updated to find the Abseil dependency. See protocolbuffers/protobuf#12292 (comment) ### Modifications For macOS, use the Config mode to find Protobuf. It's because in other systems, the Module mode works well. Besides, enable `PROTOBUF_USE_DLLS` as a workaround for protocolbuffers/protobuf#12983 is not released. Pin the default C++ standard to 17 for macOS so that users don't need to set the C++ standard manually.
BewareMyPower
force-pushed
the
bewaremypower/fix-latest-protobuf
branch
from
53ca69d to
e09cb53
Compare
|
@shibd I've updated the README and pinned the C++ standard to 17 for macOS so that we don't need to set |
shibd
approved these changes
Jun 25, 2023
BewareMyPower
added a commit
to BewareMyPower/pulsar-client-python
that referenced
this pull request
Aug 11, 2023
### Modifications Upgrade the C++ client to 3.3.0 and deprecate the `log_conf_file_path` config due to apache/pulsar-client-cpp#283. There is another issue that after apache/pulsar-client-cpp#290, the CMakeLists.txt from the C++ client finds the protobuf package with config mode. To fix it, install the OpenSSL via CMake instead of the autotools.
BewareMyPower
added a commit
to BewareMyPower/pulsar-client-python
that referenced
this pull request
Aug 11, 2023
### Modifications Upgrade the C++ client to 3.3.0 and deprecate the `log_conf_file_path` config due to apache/pulsar-client-cpp#283. There is another issue that after apache/pulsar-client-cpp#290, the CMakeLists.txt from the C++ client finds the protobuf package with config mode. To fix it, install the OpenSSL via CMake instead of the autotools.
BewareMyPower
added a commit
to apache/pulsar-client-python
that referenced
this pull request
Aug 11, 2023
### Modifications Upgrade the C++ client to 3.3.0 and deprecate the `log_conf_file_path` config due to apache/pulsar-client-cpp#283. There is another issue that after apache/pulsar-client-cpp#290, the CMakeLists.txt from the C++ client finds the protobuf package with config mode. To fix it, install the OpenSSL via CMake instead of the autotools.
BewareMyPower
added a commit
to BewareMyPower/pulsar-client-cpp
that referenced
this pull request
Nov 21, 2023
…n macOS ### Motivation apache#290 brings a regression that on macOS, Protobuf is always found with CMake Config mode, which does not set the `Protobuf_LIBRARIES` variable so that the libpulsarwithdeps.a misses the symbols of Protobuf. ### Modifications When `LINK_STATIC` is ON, use CMake Module mode to find the Protobuf. Add `build-static-library.sh` to build libraries with static dependencies and verify these libraries in PR workflow. Upload the pre-built binaries in the build workflow.
BewareMyPower
added a commit
to BewareMyPower/pulsar-client-cpp
that referenced
this pull request
Nov 21, 2023
…n macOS ### Motivation apache#290 brings a regression that on macOS, Protobuf is always found with CMake Config mode, which does not set the `Protobuf_LIBRARIES` variable so that the libpulsarwithdeps.a misses the symbols of Protobuf. ### Modifications When `LINK_STATIC` is ON, use CMake Module mode to find the Protobuf. Add `build-static-library.sh` to build libraries with static dependencies and verify these libraries in PR workflow. Upload the pre-built binaries in the build workflow.
BewareMyPower
added a commit
to BewareMyPower/pulsar-client-cpp
that referenced
this pull request
Nov 21, 2023
…n macOS ### Motivation apache#290 brings a regression that on macOS, Protobuf is always found with CMake Config mode, which does not set the `Protobuf_LIBRARIES` variable so that the libpulsarwithdeps.a misses the symbols of Protobuf. ### Modifications When `LINK_STATIC` is ON, use CMake Module mode to find the Protobuf. Add `build-static-library.sh` to build libraries with static dependencies and verify these libraries in PR workflow. Upload the pre-built binaries in the build workflow.
BewareMyPower
added a commit
to BewareMyPower/pulsar-client-cpp
that referenced
this pull request
Nov 21, 2023
…n macOS ### Motivation apache#290 brings a regression that on macOS, Protobuf is always found with CMake Config mode, which does not set the `Protobuf_LIBRARIES` variable so that the libpulsarwithdeps.a misses the symbols of Protobuf. ### Modifications When `LINK_STATIC` is ON, use CMake Module mode to find the Protobuf. Add `build-static-library.sh` to build libraries with static dependencies and verify these libraries in PR workflow. Upload the pre-built binaries in the build workflow.
BewareMyPower
added a commit
to BewareMyPower/pulsar-client-cpp
that referenced
this pull request
Nov 21, 2023
…n macOS ### Motivation apache#290 brings a regression that on macOS, Protobuf is always found with CMake Config mode, which does not set the `Protobuf_LIBRARIES` variable so that the libpulsarwithdeps.a misses the symbols of Protobuf. ### Modifications When `LINK_STATIC` is ON, use CMake Module mode to find the Protobuf. Add `build-static-library.sh` to build libraries with static dependencies and verify these libraries in PR workflow. Upload the pre-built binaries in the build workflow.
BewareMyPower
added a commit
that referenced
this pull request
Nov 21, 2023
…n macOS (#354) ### Motivation #290 brings a regression that on macOS, Protobuf is always found with CMake Config mode, which does not set the `Protobuf_LIBRARIES` variable so that the libpulsarwithdeps.a misses the symbols of Protobuf. ### Modifications When `LINK_STATIC` is ON, use CMake Module mode to find the Protobuf. Add `build-static-library.sh` to build libraries with static dependencies and verify these libraries in PR workflow. Upload the pre-built binaries in the build workflow.
BewareMyPower
added a commit
that referenced
this pull request
Nov 21, 2023
…n macOS (#354) ### Motivation #290 brings a regression that on macOS, Protobuf is always found with CMake Config mode, which does not set the `Protobuf_LIBRARIES` variable so that the libpulsarwithdeps.a misses the symbols of Protobuf. ### Modifications When `LINK_STATIC` is ON, use CMake Module mode to find the Protobuf. Add `build-static-library.sh` to build libraries with static dependencies and verify these libraries in PR workflow. Upload the pre-built binaries in the build workflow. (cherry picked from commit f75b39b)
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.
Motivation
The current CMakeLists.txt does not work with the latest Protobuf (23.3.0). It's because currently the Module mode is used to find Protobuf, while the
FindProtobuf.cmakeis not updated to find the Abseil dependency.See protocolbuffers/protobuf#12292 (comment)
Modifications
For macOS, use the Config mode to find Protobuf. It's because in other systems, the Module mode works well. Besides, enable
PROTOBUF_USE_DLLSas a workaround forprotocolbuffers/protobuf#12983 is not released.
Pin the default C++ standard to 17 for macOS so that users don't need to set the C++ standard manually.
Documentation
doc-required(Your PR needs to update docs and you will update later)
doc-not-needed(Please explain why)
doc(Your PR contains doc changes)
doc-complete(Docs have been already added)