Skip to content

Add single CMakeLists.txt with minimal CMake build configuration#1124

Merged
kevinkreiser merged 1 commit intovalhalla:masterfrom
mloskot:ml/minimal-cmake-configuration
Mar 6, 2018
Merged

Add single CMakeLists.txt with minimal CMake build configuration#1124
kevinkreiser merged 1 commit intovalhalla:masterfrom
mloskot:ml/minimal-cmake-configuration

Conversation

@mloskot
Copy link
Copy Markdown
Collaborator

@mloskot mloskot commented Feb 28, 2018

Builds libvalhalla and minimal collection of programs.

This is

  • NOT equivalent to official build based on GNU Autotools.
  • NOT suitable for building complete Valhalla suite.
  • secondary build configuration for convenient development on Windows and using CMake-enabled IDEs.

Ignore CMakeSettings.json, local configuration for CMake integration with Visual Studio 2017.


Although perhaps not a typical CMake configuration, it is intentionally provided in single-file because it is direct mapping of the main Makefile.am and it is easier to track and port any upcoming changes between 1:1 file, than between 1:hierarchy of files.

I can confirm that, using Visual Studio 2017, this setup allows to successfully build the following

  • libvalhalla
  • valhalla_build_tiles
  • valhalla_run_route
  • valhalla_run_isochrone

All the programs run, generate tiles and calculate route/isochrone.

NOTE: This CMake-based build on Windows/VS2017 based on the current master. It requires all the recent C++ source code changes submitted as PRs and merge into the current Valhalla master branch. Copying just these CMake files into Valhalla 2.4.7 (or earlier) will not be enough to successfully build released Valhalla sources on Windows using Visual Studio.

Builds libvalhalla and minimal collection of programs.
This is
  * NOT equivalent to official build based on GNU Autotools.
  * NOT suitable for building complete Valhalla suite.
  * secondary build configuration for convenient development
    on Windows and using CMake-enabled IDEs.
Ignore CMakeSettings.json, local configuration for CMake
integration with Visual Studio 2017.
@kevinkreiser
Copy link
Copy Markdown
Member

This is cool. I was considering doing the full thing at some point, maybe this will be a good start?

@mloskot
Copy link
Copy Markdown
Collaborator Author

mloskot commented Feb 28, 2018

@kevinkreiser Yes, I think so. It surely is a proof of concept.

Having no insight into Valhalla team preferences/plans, I wanted to keep the proposal minimal and as non-intrusive as possible.

For the full thing CMake setup, it may be a good idea to restructure the sources a bit, splitting into separate folders

  • library public headers
  • library sources (including private non-installable headers)
  • programs

Then, the hand-rolled source files list could be replaced with, for example:

file(GLOB build_tiles_src src/toolss/build_tiles/*.cc)
file(GLOB run_route_src src/tools/run_route/*.cc)

for saner maintenance.

@mloskot
Copy link
Copy Markdown
Collaborator Author

mloskot commented Mar 6, 2018

Any feedback on this one? Any work needed?

@kevinkreiser
Copy link
Copy Markdown
Member

I think all your observations are correct. To me this is good enough for now and we can make an issue to fully move to cmake in the future. Of particular interest to us would be how to make sure this does ALL THE THINGS that autotools does for us in the scenario where we are using it for debian packaging. I mean calling cmake will be no problem but there are a lot of details that autotools handles for us in that case. So there will just need to be some extra work there and of course to add the rest of the library as you mentioned. I think your insight on moving certain headers out of the public interface is also a good one!

@kevinkreiser kevinkreiser merged commit f4e6b60 into valhalla:master Mar 6, 2018
@mloskot mloskot deleted the ml/minimal-cmake-configuration branch March 6, 2018 13:43
@mloskot
Copy link
Copy Markdown
Collaborator Author

mloskot commented Mar 6, 2018

@kevinkreiser Thanks for accepting this PR.

I am CMake user as a developer and I've never used it a package maintainer, so I can't tell step by step how to replace autotools with CMake. I only know CPack, which is part of CMake, comes with built-in generator for deb packages.

kevinkreiser pushed a commit that referenced this pull request Jan 17, 2019
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.
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.

2 participants