From bdee85a74e36dece32cb9292332e3dd79b073bd3 Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Thu, 28 Mar 2024 23:35:50 +0000 Subject: [PATCH] feat(cmake): automatically set `ctype=CORD` workarounds With Protobuf < v23.x (aka 4.23.x) we need to enable some ugly workarounds to use `ctype=CORD` fields in the storage library. With this change CMake automatically sets the workaround option if needed. Bazel assumes the version of Protobuf is >= v23 and disables the workarounds by default. That is reasonable because Bazel users update more often (or at least we believe that). --- CMakeLists.txt | 10 ----- ci/cloudbuild/builds/cmake-oldest-deps.sh | 3 +- ci/cloudbuild/builds/m32.sh | 3 +- ci/gha/builds/lib/cmake.sh | 3 -- .../google_cloud_cpp_storage_grpc.cmake | 41 ++++++++++++++++--- 5 files changed, 38 insertions(+), 22 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 82022f12618f..a4ecc75bdd0a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -114,16 +114,6 @@ option( OFF) mark_as_advanced(GOOGLE_CLOUD_CPP_INTERNAL_DOCFX) -option( - GOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND - [==[Enable the workarounds for [ctype = CORD] when using Protobuf < v23. -More details at - -https://github.com/googleapis/google-cloud-cpp/blob/main/doc/ctype-cord-workarounds.md -]==] - OFF) -mark_as_advanced(GOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND) - set(GOOGLE_CLOUD_CPP_OVERRIDE_GOOGLEAPIS_URL "" CACHE diff --git a/ci/cloudbuild/builds/cmake-oldest-deps.sh b/ci/cloudbuild/builds/cmake-oldest-deps.sh index a6a99db0d3d9..df5976e0e998 100755 --- a/ci/cloudbuild/builds/cmake-oldest-deps.sh +++ b/ci/cloudbuild/builds/cmake-oldest-deps.sh @@ -38,8 +38,7 @@ cmake -GNinja -S . -B cmake-out/build \ "-DCMAKE_TOOLCHAIN_FILE=${vcpkg_root}/scripts/buildsystems/vcpkg.cmake" \ "-DVCPKG_MANIFEST_DIR=ci/etc/oldest-deps" \ "-DVCPKG_FEATURE_FLAGS=versions,manifest" \ - "-DGOOGLE_CLOUD_CPP_ENABLE=${ENABLED_FEATURES}" \ - "-DGOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND=ON" + "-DGOOGLE_CLOUD_CPP_ENABLE=${ENABLED_FEATURES}" io::log_h2 "Building" cmake --build cmake-out/build diff --git a/ci/cloudbuild/builds/m32.sh b/ci/cloudbuild/builds/m32.sh index 37285ca99641..bac62106191d 100755 --- a/ci/cloudbuild/builds/m32.sh +++ b/ci/cloudbuild/builds/m32.sh @@ -35,8 +35,7 @@ readonly ENABLED_FEATURES # This is the build to test with -m32, which requires a toolchain file. io::run cmake "${cmake_args[@]}" \ "--toolchain" "${PROJECT_ROOT}/ci/etc/m32-toolchain.cmake" \ - -DGOOGLE_CLOUD_CPP_ENABLE="${ENABLED_FEATURES}" \ - -DGOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND=ON + -DGOOGLE_CLOUD_CPP_ENABLE="${ENABLED_FEATURES}" io::run cmake --build cmake-out mapfile -t ctest_args < <(ctest::common_args) io::run env -C cmake-out ctest "${ctest_args[@]}" -LE "integration-test" diff --git a/ci/gha/builds/lib/cmake.sh b/ci/gha/builds/lib/cmake.sh index 6ec5f1248032..703d9663e407 100755 --- a/ci/gha/builds/lib/cmake.sh +++ b/ci/gha/builds/lib/cmake.sh @@ -65,9 +65,6 @@ function cmake::vcpkg_args() { local args args=( -DCMAKE_TOOLCHAIN_FILE="${VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake" - # The version of vcpkg we are using ships with Protobuf v21.x, the - # workarounds are required until v23.x. - -DGOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND=ON ) printf "%s\n" "${args[@]}" } diff --git a/google/cloud/storage/google_cloud_cpp_storage_grpc.cmake b/google/cloud/storage/google_cloud_cpp_storage_grpc.cmake index df5667aaf4d8..9fe835b59bf9 100644 --- a/google/cloud/storage/google_cloud_cpp_storage_grpc.cmake +++ b/google/cloud/storage/google_cloud_cpp_storage_grpc.cmake @@ -21,21 +21,52 @@ if (NOT GOOGLE_CLOUD_CPP_STORAGE_ENABLE_GRPC) set_target_properties( google_cloud_cpp_storage_grpc PROPERTIES EXPORT_NAME "google-cloud-cpp::experimental-storage_grpc") - if (GOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND) - target_compile_definitions( - google_cloud_cpp_storage_grpc - INTERFACE GOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND) - endif () add_library(google_cloud_cpp_storage_protos INTERFACE) add_library(google-cloud-cpp::storage_protos ALIAS google_cloud_cpp_storage_protos) set_target_properties( google_cloud_cpp_storage_protos PROPERTIES EXPORT_NAME "google-cloud-cpp::storage_protos") + + option( + GOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND + [==[Unused, as GCS+gRPC is not enabled. + + More details at + + https://github.com/googleapis/google-cloud-cpp/blob/main/doc/ctype-cord-workarounds.md + ]==] + OFF) + mark_as_advanced(GOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND) else () include(GoogleCloudCppLibrary) google_cloud_cpp_add_library_protos(storage) + set(GOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND_DEFAULT ON) + # Protobuf versions are ... complicated. Protobuf used to call itself + # 3.21.*, 3.20.*, 3.19.*, etc. Then it starts calling itself 22.*, 23.*, + # etc. This may change in the future: + # + # https://github.com/protocolbuffers/protobuf/issues/13103 + # + # Guessing the new version scheme is tempting, but probably unwise. + # + # In any case, `ctype=CORD` is supported starting in 23.x. + if (Protobuf_VERSION VERSION_GREATER "23.x") + set(GOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND_DEFAULT OFF) + endif () + message(GOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND_DEFAULT= + ${GOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND_DEFAULT}) + option( + GOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND + [==[Enable the workarounds for [ctype = CORD] when using Protobuf < v23. + More details at + + https://github.com/googleapis/google-cloud-cpp/blob/main/doc/ctype-cord-workarounds.md + ]==] + ${GOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND_DEFAULT}) + mark_as_advanced(GOOGLE_CLOUD_CPP_ENABLE_CTYPE_CORD_WORKAROUND) + add_library( google_cloud_cpp_storage_grpc # cmake-format: sort async/client.cc