Skip to content

Link libosrm_guidance correctly and work around circular dependencies #6955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mattwigway wants to merge 3 commits into from
Closed

Link libosrm_guidance correctly and work around circular dependencies #6955

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
96cc1cf
link libosrm_guidance correctly and work around circular dependencies…
mattwigway Jun 17, 2024
7745907
changelog entry for #6955
mattwigway Jun 17, 2024
4783b91
link osrm-extract to osrm_guidance when using dynamic libraries
mattwigway Jun 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- ADDED: Add support for opposite approach request parameter. [#6842](https://github.com/Project-OSRM/osrm-backend/pull/6842)
- ADDED: Add support for accessing edge flags in `process_segment` [#6658](https://github.com/Project-OSRM/osrm-backend/pull/6658)
- Build:
- FIXED: Avoid undefined symbol errors when building shared libraries on macOS [#6955](https://github.com/Project-OSRM/osrm-backend/6955)
- CHANGED: Upgrade clang-format to version 15. [#6919](https://github.com/Project-OSRM/osrm-backend/pull/6919)
- CHANGED: Use Debian Bookworm as base Docker image [#6904](https://github.com/Project-OSRM/osrm-backend/pull/6904)
- CHANGED: Upgrade CI actions to latest versions [#6893](https://github.com/Project-OSRM/osrm-backend/pull/6893)
Expand Down
12 changes: 11 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,17 @@ set(UTIL_LIBRARIES
target_link_libraries(osrm ${ENGINE_LIBRARIES})
target_link_libraries(osrm_update ${UPDATER_LIBRARIES})
target_link_libraries(osrm_contract ${CONTRACTOR_LIBRARIES} osrm_update osrm_store)
target_link_libraries(osrm_extract osrm_guidance ${EXTRACTOR_LIBRARIES})

# There is a circular dependency between osrm_extract and osrm_guidance. Tell the linker to trust us
# that the two osrm::guidance functions used in osrm_extract will be available at runtime.
# We don't need to do this for osrm_guidance, as osrm_extract will already be built when that gets
# built and symbols can be looked up in osrm_extract directly.
# https://stackoverflow.com/questions/25421479/
target_link_libraries(osrm_extract
Copy link
Member

@SiarheiFedartsou SiarheiFedartsou Jun 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hey!
Any chance it is possible to resolve this somehow differently (in a less hacky way)? May be we could easily extract those 2 functions to separate library?

Copy link
Member

@TheMarex TheMarex Jan 8, 2026

Choose a reason for hiding this comment

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

I think the easy fix here would be to bundle osrm_guidance in osrm_extract. There were some historical reasons to keep them separately (we wanted to split that from the actual extract process and make it optional) but it is quite tangled by now. We can easily change the GLOB and just be done with it for now.

Copy link
Collaborator

@DennisOSRM DennisOSRM Jan 8, 2026

Choose a reason for hiding this comment

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

The pragmatic fix sounds reasonable to me (at least).

"-Wl,-U,__ZN4osrm8guidance13annotateTurnsERKNS_4util12DynamicGraphINS1_17NodeBasedEdgeDataEEERKNS_9extractor6detail30EdgeBasedNodeDataContainerImplILNS_7storage9OwnershipE0EEERKNSt3__16vectorINS1_10CoordinateENSF_9allocatorISH_EEEERKNS7_23CompressedEdgeContainerERKNSF_13unordered_setIjNSF_4hashIjEENSF_8equal_toIjEENSI_IjEEEERKNS7_18NodeRestrictionMapINS7_17UnconditionalOnlyEEERKNS7_17WayRestrictionMapERKNS8_13NameTableImplILSB_0EEERKNS7_11SuffixTableERKNSF_5tupleIJNSG_IjSV_EENSG_ItNSI_ItEEEEEEERNS1_15ConcurrentIDMapIS1H_tNSR_IS1H_EEEERNS1L_INS1_8guidance15LaneTupleIdPairEtNSR_IS1Q_EEEERNS0_6detail21TurnDataContainerImplILSB_2EEERS1F_RNS1L_INS1P_12BearingClassEjNSR_IS1Z_EEEERNS1L_INS1P_10EntryClassEtNSR_IS23_EEEERj"
"-Wl,-U,__ZN4osrm8guidance19findSegregatedNodesERKNS_9extractor21NodeBasedGraphFactoryERKNS1_6detail13NameTableImplILNS_7storage9OwnershipE0EEE"
${EXTRACTOR_LIBRARIES})
target_link_libraries(osrm_guidance ${GUIDANCE_LIBRARIES} osrm_extract)
target_link_libraries(osrm_partition ${PARTITIONER_LIBRARIES})
target_link_libraries(osrm_customize ${CUSTOMIZER_LIBRARIES} osrm_update osrm_store)
target_link_libraries(osrm_store ${STORAGE_LIBRARIES})
Expand Down