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

Add cmake build system #317

Closed
wants to merge 8 commits into from
Closed

Add cmake build system #317

wants to merge 8 commits into from

Conversation

Woazboat
Copy link
Contributor

@Woazboat Woazboat commented Nov 27, 2023

Add configuration files to allow cgimap to be built using cmake.

Intended as a potential replacement for the autotools based build system since it's IMO easier to use and has become the de-facto default build system for C++ projects with much better support from IDEs and other tools.
The new cmake build system should produce the same result as the current autotools system. Build system configuration/setup is necessarily slightly different.

The first few commits here are also included in #316 which is a prerequisite for this PR, but can be applied independently.


Building the project:

autotools:

./autogen.sh
./configure
make -j3

cmake:

mkdir build && cd build
cmake ..
cmake --build .

Configuration options:

autotools:
./configure --enable-static --disable-shared

cmake:
cmake .. -DBUILD_SHARED_LIBS=OFF

(This might be a good opportunity to switch to building using static libs by default?)


Building & running tests:
autotools:

./autogen.sh
./configure
make check

cmake:

Test suite can be enabled by building using the Debug build type or explicitly enabling the tests by setting the BUILD_TESTING config option

mkdir build && cd build
cmake .. -DCMAKE_BUILD_TYPE=Debug
cmake --build .
cmake --build . -t test

I've configured cmake to forbid builds directly in the source directory, but that can be changed if required.


Cmake allows using build tools other than the classical make, such as ninja which can potentially reduce build times.

Complete build (without test suite):

make ninja
time (./autogen.sh && ./configure --enable-static --disable-shared && make -j16) time (cmake .. -DBUILD_SHARED_LIBS=OFF -G Ninja && cmake --build .)
real 1m13.126s real 0m56.241s
user 13m25.754s user 11m4.512s
sys 0m44.306s sys 0m32.992s

Other new features:

  • Code coverage information for tests (optional, enable with ENABLE_COVERAGE=ON config parameter)

@Woazboat Woazboat force-pushed the cmake branch 3 times, most recently from d906622 to f6b8536 Compare November 28, 2023 21:59
Copy link
Contributor

@pnorman pnorman left a comment

Choose a reason for hiding this comment

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

I recommend switching to cmake, not maintaining two build systems in parallel.

I tend to prefer make to ninja for CMAKE_MAKE_PROGRAM and -G, as it's fewer items to install and makes it easier to get going. Does ninja have some advantages that impact cgimap?

*~
*.o
*.a
*.so
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these necessary? Wouldn't all of these files be in /build?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For cmake, yes they should all be in the build folder. Autotools creates object files directly into the source folder however. This could eventually be removed along with the autotools build system.

*~ is for temporary files produced by emacs.

@Woazboat
Copy link
Contributor Author

I recommend switching to cmake, not maintaining two build systems in parallel.

Yes, maintaining two build systems is not a good idea. Either cmake should eventually replace autotools or it shouldn't be added. Having a transition period where both are available to work out any kinks is probably good though.

I tend to prefer make to ninja for CMAKE_MAKE_PROGRAM and -G, as it's fewer items to install and makes it easier to get going. Does ninja have some advantages that impact cgimap?

Both work perfectly fine and with cmake people can choose whichever they like/have installed. Make is the default when no other generator is specified.
I tend to like make for its simplicity, but ninja has faster build times and has better CPU resource utilization by default. (make -j without a limit has a tendency to completely lock up my pc to the point where I have to perform a hard reset by cutting the power)

@mmd-osm
Copy link
Collaborator

mmd-osm commented Nov 30, 2023

I recommend switching to cmake, not maintaining two build systems in parallel.
Yes, maintaining two build systems is not a good idea. Either cmake should eventually replace autotools or it shouldn't be added.

I agree to both of you. When switching to cmake, we could also look into adding support for cpack, which would come in handy to build e.g. Debian packages. Unity build could be an option to speed up compilation times, although these are nowhere near as bad as some other projects i know. By the way, I used both options for Overpass API.

Also we need to see how Debian packaging for osm.org will work out in the future. I don't expect major issues here wrt cmake, though maybe they have some good ideas to avoid much extra effort on our end.

Ideally, I'd like to avoid changing the Launchpad PPA build for Ubuntu to use on cmake, only to change the whole packaging again a few months later. Maybe we could combine the new packaging for Debian with the move to cmake.

Then there are a few topics, I'd like to see completed before doing the switch:

  • finish Catch2 migration,
  • remove support for OAuth 1.0a and Basic auth,
  • release a new 0.9.x some time in Q1/2024.

That's just ideas, all open for discussion.

@Woazboat Woazboat force-pushed the cmake branch 2 times, most recently from 423eaa5 to 6cec57c Compare December 10, 2023 20:18
@mmd-osm
Copy link
Collaborator

mmd-osm commented Jan 4, 2024

Removing support for OAuth 1.0a and Basic auth will very likely take another 3-5 months, so i decided to move ahead with cmake. I used all commits in the PR as baseline, and did some smaller enhancements in #358 (which now supersedes this draft PR).

I'm not exactly sure what it takes to get clang-format up and running again on cmake...

@mmd-osm mmd-osm closed this Jan 6, 2024
@mmd-osm mmd-osm added this to the v0.9.0 milestone Apr 2, 2024
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.

3 participants