build: ship libosrm as a CMake Config Package#7524
Open
tete17 wants to merge 1 commit intoProject-OSRM:masterfrom
Open
build: ship libosrm as a CMake Config Package#7524tete17 wants to merge 1 commit intoProject-OSRM:masterfrom
tete17 wants to merge 1 commit intoProject-OSRM:masterfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR modernizes downstream consumption of libosrm by replacing the old FindLibOSRM.cmake find-module (pkg-config wrapper + flag variables) with a proper installed CMake Config Package that exposes an imported target (osrm::osrm) and re-finds transitive dependencies for consumers.
Changes:
- Install/export
osrmviaLibOSRMTargets.cmakeand generate/installLibOSRMConfig.cmake+LibOSRMConfigVersion.cmake. - Update the
example/consumer to usefind_package(LibOSRM)+target_link_libraries(... osrm::osrm). - Delete the legacy
example/cmake/FindLibOSRM.cmakemodule.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| CMakeLists.txt | Adds exported install set + config/version package generation; adjusts include dirs and switches some deps to imported targets. |
| cmake/LibOSRMConfig.cmake.in | New installed package config template that re-finds dependencies and includes exported targets. |
| example/CMakeLists.txt | Simplifies example consumer to link against osrm::osrm via find_package(LibOSRM). |
| example/cmake/FindLibOSRM.cmake | Removes legacy find-module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # internally, so the consumer needs both <prefix>/include and <prefix>/include/osrm | ||
| # on the include path (mirroring the historical pkg-config behaviour). | ||
| target_include_directories(osrm INTERFACE | ||
| $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include> |
Comment on lines
540
to
544
| set(EXTRACTOR_LIBRARIES | ||
| ${BZIP2_LIBRARIES} | ||
| ${BOOST_BASE_LIBRARIES} | ||
| ${CMAKE_THREAD_LIBS_INIT} | ||
| Threads::Threads | ||
| ${EXPAT_LIBRARIES} |
Comment on lines
523
to
+527
| find_package(ZLIB REQUIRED) | ||
| add_dependency_includes(${ZLIB_INCLUDE_DIRS}) | ||
| set(ZLIB_LIBRARY ${ZLIB_LIBRARIES}) | ||
| # Use the imported target (not the file-path variable) so it can serialize | ||
| # cleanly into the exported LibOSRMTargets.cmake for downstream consumers. | ||
| set(ZLIB_LIBRARY ZLIB::ZLIB) |
|
|
||
| # osrm is built as a static library and links against all of the | ||
| # below, so consumers need them re-resolved at link time. | ||
| find_dependency(Boost 1.70 CONFIG COMPONENTS date_time iostreams regex thread) |
| find_dependency(Boost 1.70 CONFIG COMPONENTS date_time iostreams regex thread) | ||
| find_dependency(TBB CONFIG) | ||
| find_dependency(Threads) | ||
| find_dependency(ZLIB) |
Before this change, downstream CMake consumers of libosrm had to copy example/cmake/FindLibOSRM.cmake into their project and prepend it to CMAKE_MODULE_PATH. That find-module was a hand-rolled pkg-config wrapper that returned flag-string variables — no imported targets, no transitive usage requirements, no auto-discovery. Ship a proper CMake Config Package instead, matching the pattern used by every modern C++ library we depend on via vcpkg (flatbuffers, fmt, nlohmann_json, protozero, TBB). Consumers now only need: find_package(LibOSRM REQUIRED) target_link_libraries(myapp PRIVATE osrm::osrm) No CMAKE_MODULE_PATH tweak, no pkg-config, no manual dependency find_package() calls — LibOSRMConfig.cmake re-declares Boost, TBB, Threads, and ZLIB via find_dependency() on behalf of the consumer. Changes: - New cmake/LibOSRMConfig.cmake.in template consumed by configure_package_config_file(); paired with write_basic_package_version_file(VERSION OSRM_VERSION COMPATIBILITY SameMinorVersion) for version-range checks. - install(TARGETS osrm ... EXPORT LibOSRMTargets ...) + install(EXPORT LibOSRMTargets NAMESPACE osrm::) writes lib/cmake/LibOSRM/LibOSRMTargets.cmake with the imported target definition, IMPORTED_LOCATION, INTERFACE_INCLUDE_DIRECTORIES, and INTERFACE_LINK_LIBRARIES baked in. - target_include_directories(osrm INTERFACE ...) exposes the public include dir via BUILD_INTERFACE (subproject builds) and two INSTALL_INTERFACE entries — both include and include/osrm — so consumer code using either #include <osrm/...> or the internal "engine/api/..." non-prefixed paths resolves after install. - ZLIB_LIBRARY and CMAKE_THREAD_LIBS_INIT uses replaced with their imported-target equivalents (ZLIB::ZLIB, Threads::Threads) so ENGINE_LIBRARIES serialises cleanly into LibOSRMTargets.cmake instead of absolute file paths that would be non-portable across install prefixes. Scope: only osrm (the routing engine) is exposed as a Config Package target. The other six osrm_* libraries are build-internal support for the command-line tools and keep their existing plain install(TARGETS ... DESTINATION lib) rules; exposing them as imported targets would require modernising the OSMIUM_LIBRARIES / EXPAT / BZIP2 / LUA variables to imported targets, which is left for a follow-up. This matches the scope of the removed find-module, which only ever covered the routing library. libosrm.pc is kept as-is for Make/Meson/autotools consumers that have no CMake. example/cmake/FindLibOSRM.cmake is deleted — superseded by LibOSRMConfig.cmake. example/CMakeLists.txt is rewritten from the variable-splicing dance down to find_package + target_link_libraries. Verified end-to-end: - cmake --install to a scratch prefix produces the expected four files under lib/cmake/LibOSRM/ (Config, ConfigVersion, Targets, Targets-release). - A fresh consumer project (stripped copy of example/) with only CMAKE_PREFIX_PATH pointed at the scratch prefix configures, compiles, and links osrm-example against osrm::osrm without any other hints. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tete17
force-pushed
the
build/libosrm-cmake-config-package
branch
from
262d197 to
01f89f5
Compare
This file contains hidden or 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
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.
Summary
Replace the hand-rolled
example/cmake/FindLibOSRM.cmakefind-module with a proper CMake Config Package, matching the pattern used by every modern C++ library we already depend on via vcpkg (flatbuffers, fmt, nlohmann_json, protozero, TBB).Before
Downstream CMake consumers had to copy
example/cmake/FindLibOSRM.cmakeinto their project and prepend it toCMAKE_MODULE_PATH. That find-module was a pkg-config wrapper that returned flag-string variables — no imported targets, no transitive usage requirements, no auto-discovery.After
No
CMAKE_MODULE_PATHtweak, no pkg-config, no manualfind_package()calls for transitive deps —LibOSRMConfig.cmakere-declares Boost, TBB, Threads, and ZLIB viafind_dependency()on behalf of the consumer.How it works
cmake/LibOSRMConfig.cmake.intemplate consumed byconfigure_package_config_file(); paired withwrite_basic_package_version_file(VERSION ${OSRM_VERSION} COMPATIBILITY SameMinorVersion)so consumers can use version ranges.install(TARGETS osrm ... EXPORT LibOSRMTargets ...)+install(EXPORT LibOSRMTargets NAMESPACE osrm::)writeslib/cmake/LibOSRM/LibOSRMTargets.cmakewith the imported target'sIMPORTED_LOCATION,INTERFACE_INCLUDE_DIRECTORIES, andINTERFACE_LINK_LIBRARIESbaked in.target_include_directories(osrm INTERFACE ...)exposes the public include dir viaBUILD_INTERFACE(subproject builds) and twoINSTALL_INTERFACEentries — bothincludeandinclude/osrm— so consumer code using either#include <osrm/...>or the internal"engine/api/..."non-prefixed paths resolves correctly after install.ZLIB_LIBRARYandCMAKE_THREAD_LIBS_INITreplaced with their imported-target equivalents (ZLIB::ZLIB,Threads::Threads) soENGINE_LIBRARIESserialises cleanly intoLibOSRMTargets.cmakeinstead of absolute file paths that would be non-portable across install prefixes.example/CMakeLists.txtrewritten from variable-splicing dance down tofind_package(LibOSRM)+target_link_libraries(... osrm::osrm).example/cmake/FindLibOSRM.cmakedeleted.Scope
Only
osrm(the routing engine) is exposed as a Config Package target — same scope as the deleted find-module. The other sixosrm_*libraries remain build-internal support for the CLI tools and keep their existing plaininstall(TARGETS ... DESTINATION lib)rules. Exposing them as imported targets would require modernisingOSMIUM_LIBRARIES/EXPAT/BZIP2/LUAto imported targets, which is left for a follow-up.libosrm.pcis kept as-is for Make/Meson/autotools consumers that have no CMake.Net diff: +74 / -72 across 4 files.
Test plan
cmake --installto a scratch prefix produces the four expected files underlib/cmake/LibOSRM/(Config,ConfigVersion,Targets,Targets-release).example/) with onlyCMAKE_PREFIX_PATHpointed at the scratch prefix configures, compiles, and linksosrm-exampleagainstosrm::osrmwithout any other hints.