Multiple CMake fixes for missing Protobuf and Boost requirements#1605
Multiple CMake fixes for missing Protobuf and Boost requirements#1605kevinkreiser merged 4 commits intovalhalla:masterfrom mloskot:ml/cmake-fixes
Conversation
|
@kevinkreiser FYI, I'm still applying some tweaks to this PR |
|
@kevinkreiser This is ready for review now. Together with the bunch of recent MSVC-specific PRs, it restores support for building on Windows with VS2017. |
|
@mloskot amazing work we owe you a 🍺 and then some! |
|
Thanks :) The patch is yet to be checked at runtime - this or next week. I'm not sure of the Header2Binary thing is still sound. I'll welcome any feedback of course. |
|
@mloskot I see that you still have |
|
@kevinkreiser I added |
|
@mloskot ok, ill try it again! last time i did this i noticed that it was adding the linker bits on linux which is not desirable. I haven't pulled this down in a while so let me do that! |
|
I think that would be a |
Configure custom protobuf:: targets only if CMake <3.7. Add missing dependency on libprotobuf target to some modules: - valhalla_module does not seem to generate valhalla::proto target_link_libraries-linked against protobuf targets - clients of valhalla::proto do not transitively receive include directories, etc. Add missing dependency on Boost::boost where modules #include Boost headers. Add missing dependency of baldr on third-party dirent. Configure libvalhalla.pc only if PKG_CONFIG_FOUND. Call Unix-specific CMake command create_symlink only if UNIX. Add MSVC-specific fixes to cmake/Binary2Header.cmake: - add alternative workaround based on generating concatenated string literal to allow sub 64K byte long string literals - since 64K may be not enough, restore `xxd -i` behaviour (see minimal CMake configuration in #1124). Although it's not specific to Windows, those are helpful when building Valhalla on Windows or any environment where Protobuf or Boost are not deployed in standard prefixes eg. /usr or /usr/local, where they can be found even if target dependencies configuration is incomplete.
|
I havent forgotten about this I just havent gotten to it yet. I'll get this merged in the next week or so! Sorry for the delay! |
|
@kevinkreiser No rush, thanks. |
|
I'm going to merge this and deal with any downstream issues in a separate PR this has hanged out there long enough. Sorry for the wait @mloskot and thank you for your continued contributions to windows builds! |
Configure custom
protobuf:: targetsonly if CMake <3.7.Add missing dependency on
libprotobuftarget to some modules:valhalla_moduledoes not seem to generatevalhalla::prototarget_link_libraries-linked against Protobuf targetsvalhalla::protodo not transitively receive include directories, etc.Add missing Boost::boost where modules #include Boost headers.
Add missing dependency of baldr on third-party dirent.
Configure
libvalhalla.pconly ifPKG_CONFIG_FOUNDCall Unix-specific CMake command
create_symlinkonly if UNIX.Add MSVC-specific fixes to
cmake/Binary2Header.cmake:xxd -ibehaviour (see minimal CMake configuration in Add single CMakeLists.txt with minimal CMake build configuration #1124).Note, although not specific to Windows, those are helpful when building Valhalla on Windows or any environment where Protobuf or Boost are not deployed in standard prefixes eg.
/usror/usr/local, where they can be found even if target dependencies configuration is incomplete.Issue
Tasklist