Skip to content
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.

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

[OSRM] New Binary #6106

Closed
wants to merge 2,392 commits into from
Closed

Conversation

jeremiahpslewis
Copy link
Contributor

@jeremiahpslewis jeremiahpslewis commented Jan 18, 2023

Release plan:

O/OSRM/build_tarballs.jl Outdated Show resolved Hide resolved
O/OSRM/build_tarballs.jl Outdated Show resolved Hide resolved
O/OSRM/build_tarballs.jl Outdated Show resolved Hide resolved
O/OSRM/build_tarballs.jl Outdated Show resolved Hide resolved
@jeremiahpslewis
Copy link
Contributor Author

jeremiahpslewis commented Jan 19, 2023

@mattwigway Hey! Once this PR gets to ✅ and OSRM tags a new release, this should be the OSRM_jll.jl package you seem to have been looking for. :)

(note, at the moment only packaging the executables, do you also need the lib's?)

@mattwigway
Copy link
Contributor

@jeremiahpslewis that's great! Yes, I need the libs as well. Let me know if there's anything I can do to help. I've been working on creating a JLL myself that's nearly done, perhaps we can combine efforts.

@jeremiahpslewis
Copy link
Contributor Author

@jeremiahpslewis that's great! Yes, I need the libs as well. Let me know if there's anything I can do to help. I've been working on creating a JLL myself that's nearly done, perhaps we can combine efforts.

Gladly, perhaps this PR is a good starting point for a collaboration? Do you have any idea how to get the osrm build to output dynamically linked libraries?

@jeremiahpslewis
Copy link
Contributor Author

@mattwigway Just gave you access to the branch / fork so you can make changes.

@mattwigway
Copy link
Contributor

@jeremiahpslewis thanks, I'll take a look. Allegedly -DBUILD_SHARED_LIBS=ON should do the trick, but I'm getting some linker errors when I try that.

@mattwigway
Copy link
Contributor

I also found that using the toolchain file for CMake avoided me needing to specify all the library locations: https://github.com/JuliaPackaging/BinaryBuilder.jl/blob/master/docs/src/build_tips.md#cmake-builds

@mattwigway
Copy link
Contributor

Attempting a build with the shared libraries now.

@jeremiahpslewis
Copy link
Contributor Author

I just ran it locally with your -D command, seems to work, so I've added the library products as well. :)

@mattwigway
Copy link
Contributor

It's possible the error I was running into is apple M1 specific - will see momentarily

@mattwigway
Copy link
Contributor

I was thinking it might also make sense to add the profiles (car.lua and friends) as FileProducts since OSRM is not very useful without these.

@jeremiahpslewis
Copy link
Contributor Author

@mattwigway Apple M1 is brilliant for Julia but not so good for BinaryBuild-ing. Have you tried using Yggdrasil via GitHub codespaces? It's all wired up so relatively painless. :)

@jeremiahpslewis
Copy link
Contributor Author

I was thinking it might also make sense to add the profiles (car.lua and friends) as FileProducts since OSRM is not very useful without these.

Sure, fine by me. Want to add it to the PR yourself?

@mattwigway
Copy link
Contributor

Will do.

@mattwigway
Copy link
Contributor

I'm building on x86, but the aarch64 mac target is failing. I think this is an upstream issue though. I'm trying to build OSRM outside of BinaryBuilder on an actual M1 Mac, and if that fails too I'll file an issue with the OSRM project. I think if that's the case we should just remove aarch64-apple-darwin as a supported platform from the jll for now.

@mattwigway
Copy link
Contributor

Yep, same error when building natively on M1.

@jeremiahpslewis
Copy link
Contributor Author

@mattwigway ok, so we are ✅ for linux, but windows and Mac seem to be blocked (upstream?)

Release plan:

@mattwigway
Copy link
Contributor

mattwigway commented Jan 20, 2023 via email

@mattwigway
Copy link
Contributor

No luck getting the Windows binaries built - endless errors with mingw redefining GCC internal functions, and TBB not finding things from float.h. I'm giving up on Windows for now.

@jeremiahpslewis
Copy link
Contributor Author

@mattwigway the latest oneTBB is also a disaster for windows, so until someone upstream makes this happen, think we‘ll have to stay content with Linux (and Mac someday?)

do you know anyone you could ping to get a new version tagged of osrm?

@giordano
Copy link
Member

and TBB not finding things from float.h

#6115 (comment)

@jeremiahpslewis
Copy link
Contributor Author

jeremiahpslewis commented Jan 22, 2023

Mac (aarch64 error); x86_64 apple has the same error when the SDK is upgraded to 10.15:

Current OS X Error:

Entering directory '/workspace/srcdir/osrm-backend/build'

[ 86%] Linking CXX shared library libosrm_guidance.dylib

/usr/bin/cmake -E cmake_link_script CMakeFiles/osrm_guidance.dir/link.txt --verbose=1
/opt/bin/aarch64-apple-darwin20-libgfortran5-cxx11/aarch64-apple-darwin20-clang++ --sysroot=/opt/aarch64-apple-darwin20/aarch64-apple-darwin20/sys-root  -Wall -Wextra -Wpedantic -Werror -Wstrict-overflow=2 -Wsuggest-override -Wsuggest-destructor-override -Wunused -Wunreachable-code -Wdelete-incomplete -Wdisabled-optimization -Winit-self -Wlogical-not-parentheses -Wmisleading-indentation -Wodr -Wpointer-arith -Wredundant-decls -Wreorder -Wshift-negative-value -Wsizeof-array-argument -Wswitch-bool -Wtautological-compare -Wno-c++17-extensions -Wno-implicit-int-conversion -Wno-implicit-float-conversion -Wno-unused-member-function -Wno-old-style-cast -Wno-non-virtual-dtor -Wno-float-conversion -Wno-sign-conversion -Wno-shorten-64-to-32 -Wno-padded -Wno-missing-noreturn -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fPIC -fcolor-diagnostics -ftemplate-depth=1024 -O3 -DNDEBUG -mmacosx-version-min=11.0 -dynamiclib -Wl,-headerpad_max_install_names -o libosrm_guidance.dylib -install_name @rpath/libosrm_guidance.dylib CMakeFiles/GUIDANCE.dir/src/extractor/intersection/coordinate_extractor.cpp.o CMakeFiles/GUIDANCE.dir/src/extractor/intersection/have_identical_names.cpp.o CMakeFiles/GUIDANCE.dir/src/extractor/intersection/intersection.cpp.o CMakeFiles/GUIDANCE.dir/src/extractor/intersection/intersection_analysis.cpp.o CMakeFiles/GUIDANCE.dir/src/extractor/intersection/mergable_road_detector.cpp.o CMakeFiles/GUIDANCE.dir/src/extractor/intersection/node_based_graph_walker.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/driveway_handler.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/guidance_processing.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/intersection_handler.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/motorway_handler.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/roundabout_handler.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/segregated_intersection_classification.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/sliproad_handler.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/suppress_mode_handler.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/turn_analysis.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/turn_classification.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/turn_discovery.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/turn_handler.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/turn_lane_augmentation.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/turn_lane_data.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/turn_lane_handler.cpp.o CMakeFiles/GUIDANCE.dir/src/guidance/turn_lane_matcher.cpp.o CMakeFiles/UTIL.dir/src/util/assert.cpp.o CMakeFiles/UTIL.dir/src/util/conditional_restrictions.cpp.o CMakeFiles/UTIL.dir/src/util/coordinate.cpp.o CMakeFiles/UTIL.dir/src/util/coordinate_calculation.cpp.o CMakeFiles/UTIL.dir/src/util/exception.cpp.o CMakeFiles/UTIL.dir/src/util/fingerprint.cpp.o CMakeFiles/UTIL.dir/src/util/geojson_debug_policies.cpp.o CMakeFiles/UTIL.dir/src/util/guidance/bearing_class.cpp.o CMakeFiles/UTIL.dir/src/util/guidance/entry_class.cpp.o CMakeFiles/UTIL.dir/src/util/guidance/turn_lanes.cpp.o CMakeFiles/UTIL.dir/src/util/log.cpp.o CMakeFiles/UTIL.dir/src/util/opening_hours.cpp.o CMakeFiles/UTIL.dir/src/util/timed_histogram.cpp.o CMakeFiles/UTIL.dir/src/util/timezones.cpp.o  -Wl,-rpath,@loader_path 

...

[31m  "_crc32_combine", referenced from:�[39m

[31m      __ZN3tbb6detail2d115concrete_filterINSt3__110shared_ptrIZN4osrm8guidance13annotateTurnsERKNS5_4util12DynamicGraphINS7_17NodeBasedEdgeDataEEERKNS5_9extractor6detail30EdgeBasedNodeDataContainerImplILNS5_7storage9OwnershipE0EEERKNS3_6vectorINS7_10CoordinateENS3_9allocatorISM_EEEERKNSD_23CompressedEdgeContainerERKNS3_13unordered_setIjNS3_4hashIjEENS3_8equal_toIjEENSN_IjEEEERKNSD_18NodeRestrictionMapINSD_17UnconditionalOnlyEEERKNSD_17WayRestrictionMapERKNSE_13NameTableImplILSH_0EEERKNSD_11SuffixTableERKNS3_5tupleIJNSL_IjS10_EENSL_ItNSN_ItEEEEEEERNS7_15ConcurrentIDMapIS1M_tNSD_24TurnLaneDescription_hashEEERNS1Q_INS7_8guidance15LaneTupleIdPairEtN5boost4hashIS1V_EEEERNS6_6detail21TurnDataContainerImplILSH_2EEERS1K_RNS1Q_INS1U_12BearingClassEjNSW_IS26_EEEERNS1Q_INS1U_10EntryClassEtNSW_IS2A_EEEERjE19TurnsPipelineBufferEEvZNS6_13annotateTurnsESC_SK_SR_SU_S13_S18_S1B_S1F_S1I_S1P_S1T_S20_S24_S25_S29_S2D_S2E_E3$_2EclEPv in guidance_processing.cpp.o�[39m

[31mld: symbol(s) not found for architecture arm64�[39m

@giordano
Copy link
Member

#5425 (comment)

If you want to make everybody's life easier, when you show linking errors you also want to show the compiler invocation line. Because the undefined reference to error means that the linker can't find the mentioned symbol in any of the linked libraries, which often simply means that the library which is supposed to implement the missing symbol wasn't passed as argument. But if I don't see the compiler invocation I can't tell you "see, that's the problem".

@jeremiahpslewis
Copy link
Contributor Author

#5425 (comment)

If you want to make everybody's life easier, when you show linking errors you also want to show the compiler invocation line. Because the undefined reference to error means that the linker can't find the mentioned symbol in any of the linked libraries, which often simply means that the library which is supposed to implement the missing symbol wasn't passed as argument. But if I don't see the compiler invocation I can't tell you "see, that's the problem".

Thanks, will update!

@jeremiahpslewis
Copy link
Contributor Author

jeremiahpslewis commented Jan 22, 2023

Mingw error (x86-64-w64-mingw32-cxx03), which also pops up on apple-darwin; adding -fext-numeric-literals to CMAKE_CXX_FLAGS didn't seem to help.

/opt/x86_64-w64-mingw32/x86_64-w64-mingw32/sys-root/usr/local/include/boost/math/constants/constants.hpp:246:3: error: unable to find numeric literal operator ‘operator""Q’�[39m

  246 |   BOOST_DEFINE_MATH_CONSTANT(half, 5.000000000000000000000000000000000000e-01, "5.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000e-01")�[39m

      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~�[39m

/opt/x86_64-w64-mingw32/x86_64-w64-mingw32/sys-root/usr/local/include/boost/math/constants/constants.hpp:246:3: note: use ‘-fext-numeric-literals’ to enable more built-in suffixes

enzyme-ci-bot bot and others added 29 commits June 14, 2024 09:59
* Upgrade enzyme to refs/tags/v0.0.120

* Update build_tarballs.jl

---------

Co-authored-by: wsmoses <[email protected]>
Co-authored-by: William Moses <[email protected]>
* [ReferenceBLAS] Fix the suffix of xerbla_array
* [LAPACK] Update symbols to add missing suffix _64

* Add missing symbols
* Upgrade enzyme to refs/tags/v0.0.121

* Update build_tarballs.jl

---------

Co-authored-by: wsmoses <[email protected]>
Co-authored-by: William Moses <[email protected]>
* Upgrade to v3.6.1 to hopefully get shtns_set_batch

* Found more recent v3.6.6 on the Gitlab of Grenoble University

* Try with SHTns v3.6.5

* Fix path to build directory

* Upgrade to v3.6.6

---------

Co-authored-by: Thomas Dubos <[email protected]>
* add build_script

* add missing scripts

* update

* fix typo

* Update A/ADOLC/build_tarballs.jl

Co-authored-by: Max Horn <[email protected]>

* Update A/ADOLC/build_tarballs.jl

Co-authored-by: Max Horn <[email protected]>

* Re-introduce libjulia platforms

* change platforms

* Updated platforms.
* Exclude musl due to compile error in ADOLC
* Exclude Julia 1.11 due to (probably) pending adaptation of CxxWrap.jl

* Update for Julia 1.11.

* Update A/ADOLC/build_tarballs.jl

ADOLC: chdir to source dir in one step

Co-authored-by: Mosè Giordano <[email protected]>

* ADOLC: use {prefix} instead of {WORKSPACE}/destdir

Co-authored-by: Mosè Giordano <[email protected]>

* ADOLC: remove superfluous empty line

Co-authored-by: Mosè Giordano <[email protected]>

* ADOLC: further streamlining
* Remove version restrictions on llvm, libjulia, libcxxwrap_julia
* Make libjulia, libcxxwrap_julia BinaryDependencies

* ADOLC: add `clang_use_lld` = false

Co-authored-by: Mosè Giordano <[email protected]>

* ADOLC: reinstate compat bound for libcxxjulia, make it Dependency again
* Bump Julia compat to 1.9+

* ADOLC: add expand_cxxstring_abis

Co-authored-by: Mosè Giordano <[email protected]>

---------

Co-authored-by: Max Horn <[email protected]>
Co-authored-by: Jürgen Fuhrmann <[email protected]>
Co-authored-by: Mosè Giordano <[email protected]>
* build LightGBM v4.3.0 with OpenCL/CUDA support

* fix: replace FindCuda with FindCUDAToolkit

* 😶 oops

* patch switch to findcudatoolkit

* boost pin compat

* Update build_tarballs.jl

* fix: add libs to CMAKE_CUDA_FLAGS

* Update L/LightGBM/build_tarballs.jl

---------

Co-authored-by: Mosè Giordano <[email protected]>
* [libunwind] Add v1.8.0

* [libunwind 1.8.0] Amend patch

* [libunwind 1.8.0] Set preferred GCC version to 6

* [libunwind 1.8.0] idk, how about gcc 12

* [libunwind] 1.8.0 -> 1.8.1

* [[email protected]] Try `-fomit-frame-pointer` on Linux AArch64

Seems people building TensorFlow on Linux AArch64 hit the same inline
assembly issue we're seeing here and this (plus
`-flax-vector-conversion`) seems to fix it for them. This flag seems sus
for libunwind but worth a try.

* Thank you, Mosè

Co-authored-by: Mosè Giordano <[email protected]>

* [[email protected]] rm `-fomit-frame-pointer`, prefer GCC 12

Also add line breaks for long line

* [[email protected]] Include patch for PR 748

Should hopefully fix the inline assembly issue on AArch64?

* [[email protected]] Try re-lowering the preferred GCC version

12 seems excessive; 1.7.2 uses 5, let's try that

* Update L/LibUnwind/[email protected]/build_tarballs.jl

* Revert "Update L/LibUnwind/[email protected]/build_tarballs.jl"

This reverts commit c37f8f9.

---------

Co-authored-by: Mosè Giordano <[email protected]>
Co-authored-by: Mosè Giordano <[email protected]>
Also fix GAC_LDFLAGS on macOS, drop a redundant filter!
The previous release had a bug with the `LBT_USE_RTLD_DEEPBIND` environment variable
* Upgrade enzyme to refs/tags/v0.0.122

* Update build_tarballs.jl

---------

Co-authored-by: wsmoses <[email protected]>
Co-authored-by: William Moses <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.