Skip to content

Commit

Permalink
fix up Cmake/protobuf disaster (valhalla#4207)
Browse files Browse the repository at this point in the history
* use protobuf's CMake config instead of CMake's own

* add todo

* ugly workaround

* manually find & link abseil to protobuf

* changelog

* comment out the dll line

* boil down to linking only the used abseil library
  • Loading branch information
nilsnolde authored Aug 11, 2023
1 parent 753c8e7 commit 45d10fa
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* FIXED: when reclassifying ferry edges, remove destonly from ways only if the connecting way was destonly [#4118](https://github.com/valhalla/valhalla/pull/4118)
* FIXED: typo in use value of map matching API (`platform_connection` was misspelled) [#4174](https://github.com/valhalla/valhalla/pull/4174)
* FIXED: fix crash in timedistancebssmatrix.cc [#4244](https://github.com/valhalla/valhalla/pull/4244)
* FIXED: missing protobuf CMake configuration to link abseil for protobuf >= 3.22.0 [#4207](https://github.com/valhalla/valhalla/pull/4207)
* **Enhancement**
* UPDATED: French translations, thanks to @xlqian [#4159](https://github.com/valhalla/valhalla/pull/4159)
* CHANGED: -j flag for multithreaded executables to override mjolnir.concurrency [#4168](https://github.com/valhalla/valhalla/pull/4168)
Expand Down
28 changes: 28 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,34 @@ endif()

if(NOT Protobuf_FOUND)
find_package(Protobuf REQUIRED)

# orchestrated effort of both protobuf & cmake are responsible for the next lines..

# TODO: monitor how protobuf Linux packages evolve. Eventually everyone should use
# the Apple configuration, but e.g. apt does not install CMake config with protobuf
if(Protobuf_VERSION VERSION_GREATER_EQUAL 4.22.0)
# this might be needed for some Apple/Linux protobuf versions
# add_definitions(-DPROTOBUF_USE_DLLS)
if(APPLE OR WIN32)
# CONFIG mode is ideal for new abseil protobuf builds, which uses the protobuf-provided
# CMake configuration instead of CMake internal one, as that's not looking for abseil:
# https://github.com/valhalla/valhalla/issues/4173#issuecomment-1638067950
# Newer protobuf versions require a protobuf_MODULE_COMPATIBLE to be able
# to see the protobuf_generate_cpp() CMake function
set(protobuf_MODULE_COMPATIBLE ON CACHE BOOL "")
find_package(Protobuf REQUIRED CONFIG)
else()
# linux might not have CMake config installed (e.g. Debian derivatives)
# need to link protobuf to abseil manually
find_package(Protobuf REQUIRED)
find_package(absl REQUIRED)
if(TARGET protobuf::libprotobuf-lite)
target_link_libraries(protobuf::libprotobuf-lite INTERFACE "absl::absl_log")
elseif(TARGET protobuf::libprotobuf)
target_link_libraries(protobuf::libprotobuf INTERFACE "absl::absl_log")
endif()
endif()
endif()
endif()

message(STATUS "Using pbf headers from ${PROTOBUF_INCLUDE_DIR}")
Expand Down
1 change: 1 addition & 0 deletions proto/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ if(ENABLE_DATA_TOOLS)
${VALHALLA_SOURCE_DIR}/third_party/OSM-binary/src/osmformat.proto)
endif()

# TODO: this seems to be deprecated, see https://github.com/valhalla/valhalla/pull/4207
protobuf_generate_cpp(protobuf_srcs protobuf_hdrs ${protobuf_descriptors})

valhalla_module(NAME proto
Expand Down

0 comments on commit 45d10fa

Please sign in to comment.