| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
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:
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.
Sorry, something went wrong.
| set(EXTRACTOR_LIBRARIES | ||
| ${BZIP2_LIBRARIES} | ||
| ${BOOST_BASE_LIBRARIES} | ||
| ${CMAKE_THREAD_LIBS_INIT} | ||
| Threads::Threads | ||
| ${EXPAT_LIBRARIES} |
|
|
||
| # 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) |
|
Does this change the API, ie. does it require changes in the code bases that embed OSRM? |
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## master #7524 +/- ##
==========================================
- Coverage 90.80% 90.79% -0.02%
==========================================
Files 483 483
Lines 37670 37670
==========================================
- Hits 34206 34202 -4
- Misses 3464 3468 +4 ☔ View full report in Codecov by Harness.
|
Sorry, something went wrong.
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>
| Back | FazBrowse Home | New Git URL |
Summary
Replace the hand-rolled example/cmake/FindLibOSRM.cmake find-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.cmake into their project and prepend it to CMAKE_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_PATH tweak, no pkg-config, no manual find_package() calls for transitive deps — LibOSRMConfig.cmake re-declares Boost, TBB, Threads, and ZLIB via find_dependency() on behalf of the consumer.
How it works
Scope
Only osrm (the routing engine) is exposed as a Config Package target — same scope as the deleted find-module. The other six osrm_* libraries remain build-internal support for the CLI tools and keep their existing plain install(TARGETS ... DESTINATION lib) rules. Exposing them as imported targets would require modernising OSMIUM_LIBRARIES / EXPAT / BZIP2 / LUA to imported targets, which is left for a follow-up.
libosrm.pc is kept as-is for Make/Meson/autotools consumers that have no CMake.
Net diff: +74 / -72 across 4 files.
Test plan