-
Notifications
You must be signed in to change notification settings - Fork 658
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
Make vendored libraries optional #4544
Conversation
8daf123
to
9b36b25
Compare
@@ -118,16 +123,57 @@ add_definitions(-DBOOST_NO_CXX11_SCOPED_ENUMS) | |||
add_definitions(-DBOOST_ALLOW_DEPRECATED_HEADERS) | |||
add_definitions(-DBOOST_BIND_GLOBAL_PLACEHOLDERS) | |||
|
|||
# resolve vendored libraries | |||
set(date_include_dir ${VALHALLA_SOURCE_DIR}/third_party/date/include) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the main changes, defaulting to vendored paths but trying to change them to system paths if opted in with PREFER_SYSTEM_DEPS
@@ -52,34 +72,9 @@ sudo make -C build install | |||
|
|||
### Building from Source - macOS | |||
|
|||
#### Configuring Rosetta for ARM64 MacBook | |||
|
|||
Check your architecture typing `arch` in the terminal. In case the result is `arm64` set up Rosetta terminal to emulate x86_64 behavior. Otherwise, skip this step. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rosetta should be a thing of the past I think
if(APPLE) | ||
list(APPEND system_includes ${VALHALLA_SOURCE_DIR}/third_party/date/include/date) | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed this in all CMakeLists.txt, ${VALHALLA_SOURCE_DIR}/third_party/date/include/date
is not even a valid path
…h are NOT usually found in package managers
c841036
to
da0d0d5
Compare
@@ -37,7 +37,7 @@ defaults: | |||
env: | |||
BUILD_TYPE: Release | |||
MSVC_VERSION: '2022' | |||
VCPKG_VERSION: '2024.01.12' | |||
VCPKG_VERSION: '8040303' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
newest commit on vcpkg, the "old" release failed to resolve libspatialite with pkg-config for some reason (worked until this PR and I didn't change anything regarding pkg-config or spatialite..)
based on #4541
fixes #4529
Let users decide whether to use our vendored libraries or their own for all major ones which we need for compile-/ or run-time (e.g. not
just_gtfs
, not found in any package manager).I tried a full/default vcpkg build turning all vendored libraries off, worked smoothly. To be fair, quite a few libraries wouldn't be found with other package managers, e.g.
date
ordirent.h
. But it doesn't really matter, bcs we fallback to our submodules.What this PR does:
PREFER_SYSTEM_DEPS
which, if turnedON
(defaultOFF
), will usefind_package()
to resolvedate
,rapidjson
,cxxopts
,robin-hood-hashing
,pybind11
,dirent.h
(Win only). If turnedON
and CMake doesn't find the library, we fall back to our vendored version of those libs and emit aWARNING
. This way nothing changes for anyone who doesn't want to use this featurelz4
submodule and depend on external package viapkg-config
, it's available everywhere and stablePREFER_SYSTEM_DEPS=ON
for the Windows CI build which already usesvcpkg
building.md
forvcpkg