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 package config #3722

Merged
merged 2 commits into from
Nov 2, 2023
Merged

Add cmake package config #3722

merged 2 commits into from
Nov 2, 2023

Conversation

securitykernel
Copy link
Collaborator

@securitykernel securitykernel commented Oct 2, 2023

Adds a CMake package configuration for the library, so users of Botan using CMake to link against Botan don't need to invoke pkg-config or write a CMake find module (rnpgp/rnp#2121). Unlike suggested in #3490, we don't write a find module, as package configuration files now seem to be the recommended way. The package configuration files are installed with Botan and can then be used as follows:

# find any Botan version
find_package(Botan REQUIRED)

# find a minimum Botan version (>= 3.3.0)
find_package(Botan 3.3.0 REQUIRED)

# should be able to use ranges, too
find_package(Botan 3.0.0...3.3.0 REQUIRED)

# even check if certain modules are installed
find_package(Botan 3.3.0 REQUIRED COMPONENTS rsa ecdsa kyber)

# finally link with Botan
target_link_libraries(my_target PRIVATE Botan::Botan)

Also adds tests to the CI. Tests are provided in src/scripts/ci/cmake_tests using an example CMake project.

Closes #3490.

@coveralls
Copy link

coveralls commented Oct 9, 2023

Coverage Status

coverage: 91.714% (-0.003%) from 91.717% when pulling 3a052c4 on sk/cmake-config into 84a828a on master.

@randombit
Copy link
Owner

So the CMake goo itself is fine (well I haven't reviewed it but no objections in principle to shipping something like this) but I'd rather the test of the CMake integration not go in tests - I know it is is a test but in my view tests is "the main test suite", which this is not a part of. I think examples would actually be more appropriate since AIUI the CMakeLists.txt currently in src/tests/cmake is exactly an example of how to link to Botan using CMake.

@securitykernel
Copy link
Collaborator Author

Sure, I'll move it. Then this answers the question where to run the tests (with the examples).

@securitykernel
Copy link
Collaborator Author

Shared linking works fine already, static is a bit more complicated to set up, but I would like to give it a try. I'd also like to ask the CMake devs for a review.

Copy link

@mathstuf mathstuf left a comment

Choose a reason for hiding this comment

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

A pass over this was requested on Slack. Take as you see fit.

src/scripts/install.py Outdated Show resolved Hide resolved
src/tests/cmake/CMakeLists.txt Outdated Show resolved Hide resolved
src/build-data/botan-config.cmake.in Outdated Show resolved Hide resolved
src/build-data/botan-config.cmake.in Outdated Show resolved Hide resolved
src/build-data/botan-config.cmake.in Outdated Show resolved Hide resolved
src/build-data/botan-config.cmake.in Outdated Show resolved Hide resolved
src/build-data/botan-config.cmake.in Outdated Show resolved Hide resolved
src/build-data/botan-config.cmake.in Outdated Show resolved Hide resolved
@reneme reneme force-pushed the sk/cmake-config branch 12 times, most recently from 0cff302 to aeddb13 Compare October 27, 2023 15:31
configure.py Show resolved Hide resolved
@reneme
Copy link
Collaborator

reneme commented Oct 27, 2023

@mathstuf Thanks a lot for your review! Your hints were insightful and made this a much better package config for sure.

Regarding relocatablility: We now defined the locations of includes and libraries relative to the botan-config.cmake file. For instance, the include directory is located as ${CMAKE_CURRENT_LIST_DIR}/../../include/botan-%{version_major}. As long as package maintainers don't change the install layout that should be fine. Nevertheless, it still feels rather hacky and brittle. Is there a better way?

Maybe @thesamesam has an opinion or insight about it as well? 😄

@reneme reneme marked this pull request as ready for review October 27, 2023 15:47
@securitykernel
Copy link
Collaborator Author

@randombit I think this is ready now, so I removed the draft description. I also added a docstring and removed one leftover configure.py cmake foo.

This generates botan-config-version.cmake and botan-config.cmake files
and ships them with the library during the 'install' build step. These
files facilitate the consumption of the library using CMake's find_package()
in downstream applications.

Co-Authored-By: René Meusel <rene.meusel@rohde-schwarz.com>
@securitykernel
Copy link
Collaborator Author

Squashed and rebased now.

Copy link
Owner

@randombit randombit left a comment

Choose a reason for hiding this comment

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

Did not review the CMake configs, changes to Python look fine

@securitykernel securitykernel merged commit a1d1b46 into master Nov 2, 2023
38 checks passed
@randombit randombit deleted the sk/cmake-config branch November 7, 2023 19:08
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.

Upstream a CMake Find Module
5 participants