Skip to content

Commit

Permalink
[Build] Removed gRPC_PROTOBUF_PACKAGE_TYPE, supporting config only (#…
Browse files Browse the repository at this point in the history
…32988)

`FindProtobuf` isn't working as Protobuf began to use Abseil so gRPC is
now using `CONFIG` mode for protobuf module
(Context: https://gitlab.kitware.com/cmake/cmake/-/issues/24321)
  • Loading branch information
veblush committed May 4, 2023
1 parent 40f20c0 commit 303e568
Show file tree
Hide file tree
Showing 7 changed files with 2 additions and 16 deletions.
3 changes: 0 additions & 3 deletions CMakeLists.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 2 additions & 6 deletions cmake/protobuf.cmake
Expand Up @@ -55,12 +55,8 @@ if(gRPC_PROTOBUF_PROVIDER STREQUAL "module")
set(gRPC_INSTALL FALSE)
endif()
elseif(gRPC_PROTOBUF_PROVIDER STREQUAL "package")
find_package(Protobuf REQUIRED ${gRPC_PROTOBUF_PACKAGE_TYPE})
find_package(Protobuf REQUIRED CONFIG)

# {Protobuf,PROTOBUF}_FOUND is defined based on find_package type ("MODULE" vs "CONFIG").
# For "MODULE", the case has also changed between cmake 3.5 and 3.6.
# We use the legacy uppercase version for *_LIBRARIES AND *_INCLUDE_DIRS variables
# as newer cmake versions provide them too for backward compatibility.
if(Protobuf_FOUND OR PROTOBUF_FOUND)
if(TARGET protobuf::${_gRPC_PROTOBUF_LIBRARY_NAME})
set(_gRPC_PROTOBUF_LIBRARIES protobuf::${_gRPC_PROTOBUF_LIBRARY_NAME})
Expand Down Expand Up @@ -90,6 +86,6 @@ elseif(gRPC_PROTOBUF_PROVIDER STREQUAL "package")
set(_gRPC_PROTOBUF_PROTOC_EXECUTABLE ${PROTOBUF_PROTOC_EXECUTABLE})
endif()
endif()
set(_gRPC_FIND_PROTOBUF "if(NOT Protobuf_FOUND AND NOT PROTOBUF_FOUND)\n find_package(Protobuf ${gRPC_PROTOBUF_PACKAGE_TYPE})\nendif()")
set(_gRPC_FIND_PROTOBUF "find_dependency(Protobuf CONFIG)")
endif()
endif()

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions templates/CMakeLists.txt.template
Expand Up @@ -217,9 +217,6 @@
set(gRPC_PROTOBUF_PROVIDER "module" CACHE STRING "Provider of protobuf library")
set_property(CACHE gRPC_PROTOBUF_PROVIDER PROPERTY STRINGS "module" "package")

set(gRPC_PROTOBUF_PACKAGE_TYPE "" CACHE STRING "Algorithm for searching protobuf package")
set_property(CACHE gRPC_PROTOBUF_PACKAGE_TYPE PROPERTY STRINGS "CONFIG" "MODULE")

if(gRPC_BUILD_TESTS)
set(gRPC_BENCHMARK_PROVIDER "module" CACHE STRING "Provider of benchmark library")
set_property(CACHE gRPC_BENCHMARK_PROVIDER PROPERTY STRINGS "module" "package")
Expand Down
1 change: 0 additions & 1 deletion test/distrib/cpp/run_distrib_test_cmake.bat
Expand Up @@ -99,7 +99,6 @@ cmake ^
-DgRPC_BUILD_MSVC_MP_COUNT=-1 ^
-DgRPC_ABSL_PROVIDER=package ^
-DgRPC_CARES_PROVIDER=package ^
-DgRPC_PROTOBUF_PACKAGE_TYPE=CONFIG ^
-DgRPC_PROTOBUF_PROVIDER=package ^
-DProtobuf_USE_STATIC_LIBS=ON ^
-DgRPC_RE2_PROVIDER=package ^
Expand Down
1 change: 0 additions & 1 deletion test/distrib/cpp/run_distrib_test_cmake.sh
Expand Up @@ -75,7 +75,6 @@ cmake \
-DgRPC_BUILD_TESTS=OFF \
-DgRPC_CARES_PROVIDER=package \
-DgRPC_ABSL_PROVIDER=package \
-DgRPC_PROTOBUF_PACKAGE_TYPE=CONFIG \
-DgRPC_PROTOBUF_PROVIDER=package \
-DgRPC_RE2_PROVIDER=package \
-DgRPC_SSL_PROVIDER=package \
Expand Down
1 change: 0 additions & 1 deletion test/distrib/cpp/run_distrib_test_cmake_pkgconfig.sh
Expand Up @@ -81,7 +81,6 @@ cmake \
-DgRPC_BUILD_TESTS=OFF \
-DgRPC_ABSL_PROVIDER=package \
-DgRPC_CARES_PROVIDER=package \
-DgRPC_PROTOBUF_PACKAGE_TYPE=CONFIG \
-DgRPC_PROTOBUF_PROVIDER=package \
-Dutf8_range_DIR=/usr/local/lib/cmake/utf8_range \
-DgRPC_RE2_PROVIDER=package \
Expand Down

3 comments on commit 303e568

@barracuda156
Copy link
Contributor

@barracuda156 barracuda156 commented on 303e568 May 30, 2023

Choose a reason for hiding this comment

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

@veblush This has broken finding of protobuf.

P. S. Unsurprisingly so, since protobuf does not install *config.cmake at all, at least when not built with CMake: https://ports.macports.org/port/protobuf3-cpp/details
Specifying include and lib dirs does not seem to work either.

Earlier versions of grpc were finding protobuf with no issues.

@Allen-Webb
Copy link

Choose a reason for hiding this comment

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

@veblush This has broken finding of protobuf.

P. S. Unsurprisingly so, since protobuf does not install *config.cmake at all, at least when not built with CMake: https://ports.macports.org/port/protobuf3-cpp/details Specifying include and lib dirs does not seem to work either.

Earlier versions of grpc were finding protobuf with no issues.

It would be nice to have a workaround. As is I might patch in a revert of this commit until there is a resolution.

@barracuda156
Copy link
Contributor

Choose a reason for hiding this comment

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

@Allen-Webb We have moved to CMake in macports/macports-ports@2660da8 – but it is still desirable, I think, not to force everyone use CMake build system with protobuf.

Please sign in to comment.