minisketch icon indicating copy to clipboard operation
minisketch copied to clipboard

build: Add CMake buildsystem

Open hebasto opened this issue 2 years ago • 12 comments

This PR adds a new CMake-based build system, which can be reused in downstream projects, including Bitcoin Core.

Also an MSVC CI task has been added per feedback.

Autotools vs CMake configuration option parity:

Autotools CMake
--disable-ccache N/A
--enable-tests -DMINISKETCH_TESTS
--enable-benchmark -DMINISKETCH_BENCHMARK
--enable-fields -DMINISKETCH_FIELDS

Here is a examples of output when configuring:

  • for the "Ninja Multi-Config" generator:
$ cmake -B build -G "Ninja Multi-Config" -DMINISKETCH_BENCHMARK=ON -DMINISKETCH_FIELDS=42
-- The CXX compiler identification is GNU 13.2.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test HAVE_CLMUL
-- Performing Test HAVE_CLMUL - Success
-- Performing Test HAVE_CLZ
-- Performing Test HAVE_CLZ - Success


minisketch configure summary
============================
Library type .......................... Static
Build options:
  clmul fields ........................ ENABLED
  clz implementation .................. compiler builtin
Optional binaries:
  benchmark ........................... ON
  tests ............................... ON

Cross compiling ....................... FALSE
C++ compiler .......................... GNU 13.2.0, /usr/bin/c++
Available build configurations ........ Debug, Release, RelWithDebInfo
Default build configuration ........... Debug
'Debug' build configuration:
  C++ flags ........................... -g -Wall
'Release' build configuration:
  C++ flags ........................... -O3 -DNDEBUG -Wall
'RelWithDebInfo' build configuration:
  C++ flags ........................... -O2 -g -DNDEBUG -Wall

CMake Warning at CMakeLists.txt:117 (message):
  Only compiling in support for field sizes: 42

  This means the library will lack support for other field sizes entirely.



-- Configuring done (0.5s)
-- Generating done (0.0s)
-- Build files have been written to: /home/hebasto/git/minisketch/build
  • for the "Unix Makefiles" generator (the default on Linux):
$ cmake -B build -DCMAKE_BUILD_TYPE=RelWithDebInfo -DMINISKETCH_BENCHMARK=ON -DMINISKETCH_FIELDS=42
-- The CXX compiler identification is GNU 13.2.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test HAVE_CLMUL
-- Performing Test HAVE_CLMUL - Success
-- Performing Test HAVE_CLZ
-- Performing Test HAVE_CLZ - Success


minisketch configure summary
============================
Library type .......................... Static
Build options:
  clmul fields ........................ ENABLED
  clz implementation .................. compiler builtin
Optional binaries:
  benchmark ........................... ON
  tests ............................... ON

Cross compiling ....................... FALSE
C++ compiler .......................... GNU 13.2.0, /usr/bin/c++
Build configuration ................... RelWithDebInfo
C++ flags ............................. -O2 -g -DNDEBUG -Wall

CMake Warning at CMakeLists.txt:117 (message):
  Only compiling in support for field sizes: 42

  This means the library will lack support for other field sizes entirely.



-- Configuring done (0.6s)
-- Generating done (0.0s)
-- Build files have been written to: /home/hebasto/git/minisketch/build

hebasto avatar Oct 20 '22 00:10 hebasto

Chasing for Concept (N)ACKs...

hebasto avatar Oct 20 '22 00:10 hebasto

The currently suggesting minimal CMake-based build system implementation is enough for:

add_subdirectory(minisketch EXCLUDE_FROM_ALL)
target_compile_definitions(minisketch
  PRIVATE
    DISABLE_DEFAULT_FIELDS
    ENABLE_FIELD_32
)

To became a full fledged one, the current minimal CMake-based build system requires some additional features:

  • more robustness (e.g., checking compiler/linker flags before applying them)
  • ability to install the built artifacts
  • ability to export the built artifacts and, probably, packaging of them
  • documentation

Although, those features are not required for this PR goal, i.e., providing a native Windows CI task.

hebasto avatar Oct 23 '22 09:10 hebasto

Updated 5851544f65863667075213c0f5af52a656058c4b -> d2408e0e6a4bf1ca4f857eccaf4943fe9012f8d5 (pr75.03 -> pr75.04, diff):

  • policy CMP0063 set explicitly to avoid CMake warnings
  • the default CMake build directory added to the .gitignore file

hebasto avatar Oct 31 '22 13:10 hebasto

Imo this should be renamed "Add CMake buildsystem". I looked for this a while back and didn't find it because CMake isn't mentioned in the title.

Concept ACK. Will review next week.

theuni avatar Mar 17 '23 20:03 theuni

@sipa Are you interested in CMake for this project?

theuni avatar Mar 31 '23 14:03 theuni

@theuni Yeah, will look soon.

sipa avatar Mar 31 '23 20:03 sipa

@sipa No rush. I was just looking for a concept ACK.

I worked up a CMake impl as well before seeing this PR. I think we'd benefit from parts of each, so I'll get with @hebasto on creating a combined version for review.

theuni avatar Mar 31 '23 20:03 theuni

Imo this should be renamed "Add CMake buildsystem".

Done.

hebasto avatar Apr 16 '23 12:04 hebasto

I'm happy to add a CMake build system for minisketch. I will need review from people more experienced with build systems, though.

sipa avatar Apr 11 '24 10:04 sipa

I'm happy to add a CMake build system for minisketch. I will need review from people more experienced with build systems, though.

I'm going to update this PR shortly.

hebasto avatar Apr 11 '24 11:04 hebasto

Reworked.

The PR description has been updated.

The approach from https://github.com/hebasto/bitcoin/pull/93 was used for printing summary.

hebasto avatar Apr 16 '24 17:04 hebasto

Addressed @theuni's comments.

hebasto avatar Apr 26 '24 16:04 hebasto