From e09cb5351f63157234001880a384d55f39b63807 Mon Sep 17 00:00:00 2001 From: Yunze Xu Date: Wed, 21 Jun 2023 15:27:20 +0800 Subject: [PATCH] Fix build failure with the Protobuf 23.3 ### 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 https://github.com/protocolbuffers/protobuf/issues/12292#issuecomment-1529680040 ### 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 https://github.com/protocolbuffers/protobuf/pull/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. --- .github/workflows/ci-pr-validation.yaml | 19 +++--------------- CMakeLists.txt | 26 +++++++++++++++++++++---- README.md | 15 ++++++-------- wireshark/CMakeLists.txt | 16 +++++++++++++-- wireshark/pulsarDissector.cc | 2 ++ 5 files changed, 47 insertions(+), 31 deletions(-) diff --git a/.github/workflows/ci-pr-validation.yaml b/.github/workflows/ci-pr-validation.yaml index 461e3f4d..d6188174 100644 --- a/.github/workflows/ci-pr-validation.yaml +++ b/.github/workflows/ci-pr-validation.yaml @@ -50,11 +50,7 @@ jobs: - name: Install deps (macOS) if: ${{ startsWith(matrix.os, 'macos') }} run: | - # Install protobuf v21.12 - wget https://raw.githubusercontent.com/Homebrew/homebrew-core/2d47ed2eac09d59ffc2c92b1d804f1c232188c88/Formula/protobuf.rb - brew install --formula ./protobuf.rb - brew install pkg-config wireshark - + brew install pkg-config wireshark protobuf - name: Build wireshark plugin run: | cmake -S wireshark -B build-wireshark @@ -270,20 +266,11 @@ jobs: uses: actions/checkout@v3 - name: Install dependencies - run: | - # Install protobuf v21.12 - wget https://raw.githubusercontent.com/Homebrew/homebrew-core/2d47ed2eac09d59ffc2c92b1d804f1c232188c88/Formula/protobuf.rb - brew install --formula ./protobuf.rb - brew install openssl boost zstd snappy googletest + run: brew install openssl protobuf boost zstd snappy googletest - name: Configure (default) shell: bash - run: | - # The latest GTest requires C++14 - cmake \ - -B ./build-macos \ - -DCMAKE_CXX_STANDARD=14 \ - -S . + run: cmake -B ./build-macos -S . - name: Compile shell: bash diff --git a/CMakeLists.txt b/CMakeLists.txt index 64b7b0ef..cde49663 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -86,7 +86,12 @@ MESSAGE(STATUS "Threads library: " ${CMAKE_THREAD_LIBS_INIT}) set(Boost_NO_BOOST_CMAKE ON) if (NOT CMAKE_CXX_STANDARD) - set(CMAKE_CXX_STANDARD 11) + if (APPLE) + # The latest Protobuf dependency on macOS requires the C++17 support + set(CMAKE_CXX_STANDARD 17) + else () + set(CMAKE_CXX_STANDARD 11) + endif () endif () set(CMAKE_C_STANDARD 11) @@ -135,7 +140,14 @@ find_package(OpenSSL REQUIRED) message("OPENSSL_INCLUDE_DIR: " ${OPENSSL_INCLUDE_DIR}) message("OPENSSL_LIBRARIES: " ${OPENSSL_LIBRARIES}) -find_package(Protobuf REQUIRED) +if (APPLE) + # See https://github.com/apache/arrow/issues/35987 + add_definitions(-DPROTOBUF_USE_DLLS) + # Use Config mode to avoid FindProtobuf.cmake does not find the Abseil library + find_package(Protobuf REQUIRED CONFIG) +else () + find_package(Protobuf REQUIRED) +endif () message("Protobuf_INCLUDE_DIRS: " ${Protobuf_INCLUDE_DIRS}) message("Protobuf_LIBRARIES: " ${Protobuf_LIBRARIES}) @@ -173,7 +185,7 @@ if (LINK_STATIC AND NOT VCPKG_TRIPLET) add_definitions(-DCURL_STATICLIB) endif() elseif (LINK_STATIC AND VCPKG_TRIPLET) - find_package(protobuf REQUIRED) + find_package(Protobuf REQUIRED) message(STATUS "Found protobuf static library: " ${Protobuf_LIBRARIES}) if (MSVC AND (${CMAKE_BUILD_TYPE} STREQUAL Debug)) find_library(ZLIB_LIBRARIES NAMES zlibd) @@ -296,7 +308,6 @@ set(COMMON_LIBS ${Boost_REGEX_LIBRARY} ${Boost_SYSTEM_LIBRARY} ${Boost_DATE_TIME_LIBRARY} - ${Protobuf_LIBRARIES} ${CURL_LIBRARIES} ${OPENSSL_LIBRARIES} ${ZLIB_LIBRARIES} @@ -304,6 +315,13 @@ set(COMMON_LIBS ${CMAKE_DL_LIBS} ) +if (APPLE) + # Protobuf_LIBRARIES is empty when finding Protobuf in Config mode + set(COMMON_LIBS ${COMMON_LIBS} protobuf::libprotobuf) +else () + set(COMMON_LIBS ${COMMON_LIBS} ${Protobuf_LIBRARIES}) +endif () + if (MSVC) set(COMMON_LIBS ${COMMON_LIBS} diff --git a/README.md b/README.md index 014055c1..055fb8fd 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,10 @@ The [dependencies.yaml](./dependencies.yaml) file provides the recommended depen cmake . -DCMAKE_CXX_STANDARD=14 ``` +> **Note**: +> +> On macOS, the default C++ standard is 17 because the latest Protobuf from Homebrew requires the C++17 support. + ## Platforms Pulsar C++ Client Library has been tested on: @@ -135,21 +139,14 @@ brew install cmake openssl protobuf boost googletest zstd snappy #### Compile Pulsar client library: ```shell -cmake . -DCMAKE_CXX_STANDARD=14 -make -``` - -You need to configure `CMAKE_CXX_STANDARD` with 14 because the latest `googletest` dependency from HomeBrew requires the C++14 support. If you don't want to build tests, you can run: - -```bash -cmake . -DBUILD_TESTS=OFF +cmake . make ``` If you want to build performance tools, you need to run: ```shell -cmake . -DBUILD_PERF_TOOLS=ON -DCMAKE_CXX_STANDARD=14 +cmake . -DBUILD_PERF_TOOLS=ON make ``` diff --git a/wireshark/CMakeLists.txt b/wireshark/CMakeLists.txt index dbfb0faa..add8eb7a 100644 --- a/wireshark/CMakeLists.txt +++ b/wireshark/CMakeLists.txt @@ -20,7 +20,14 @@ cmake_minimum_required(VERSION 3.7) project(pulsar-cpp-wireshark) -set(CMAKE_CXX_STANDARD 11) +if (NOT CMAKE_CXX_STANDARD) + if (APPLE) + # The latest Protobuf dependency on macOS requires the C++17 support + set(CMAKE_CXX_STANDARD 17) + else () + set(CMAKE_CXX_STANDARD 11) + endif () +endif () if (CMAKE_SYSTEM_NAME STREQUAL "Darwin" AND CMAKE_SYSTEM_PROCESSOR STREQUAL "arm64") execute_process(COMMAND sh -c "find $(brew --prefix)/Cellar/wireshark -name 'include' -type d" @@ -45,7 +52,12 @@ pkg_check_modules(GLIB glib-2.0) MESSAGE(STATUS "Use WIRESHARK_INCLUDE_PATH: ${WIRESHARK_INCLUDE_PATH}") MESSAGE(STATUS "Use GLIB_INCLUDE_DIRS: ${GLIB_INCLUDE_DIRS}") -find_package(Protobuf REQUIRED) +set(protobuf_MODULE_COMPATIBLE ON CACHE BOOL "") +if (APPLE) + find_package(Protobuf REQUIRED CONFIG) +else () + find_package(Protobuf REQUIRED) +endif () set(PROTO_SOURCES PulsarApi.pb.cc) protobuf_generate_cpp(${PROTO_SOURCES} diff --git a/wireshark/pulsarDissector.cc b/wireshark/pulsarDissector.cc index c84741cb..3e9087fc 100644 --- a/wireshark/pulsarDissector.cc +++ b/wireshark/pulsarDissector.cc @@ -34,6 +34,8 @@ constexpr int kWiresharkMinorVersion = VERSION_MINOR; #include #include +#include + #include "PulsarApi.pb.h" #ifdef VERSION