Skip to content
This repository has been archived by the owner on Dec 26, 2023. It is now read-only.

Adding a CMake interface library target #52

Closed
oddlama opened this issue Nov 1, 2019 · 3 comments
Closed

Adding a CMake interface library target #52

oddlama opened this issue Nov 1, 2019 · 3 comments
Assignees

Comments

@oddlama
Copy link

oddlama commented Nov 1, 2019

As this is a library, it would be awesome to add a library definition to the CMake target for easier inclusion in other projects. As usually, you can include libraries from a cloned repository with a single call to add_subdirectory(external/somelibrary) without also having it building tests or other binaries.

If you like, I will gladly provide a PR for this. Though I must note that I'm not really familiar with your specific structuring of the CMake part. Therefore, I think it might be beneficial if you were to implement it, or first gave your opinion on some of the specifics.

Implementation

While ideally the library targets would also be used internally to "link" against the headers in all tests and examples, it can be considered a convention and thus is not strictly necessary.

Essentially it boils down to adding something along these lines to CMakeLists.txt (or src/CMakeLists.txt depending on your conventions). I'm assuming robin_hood as a library name:

add_library(robin_hood INTERFACE)
add_library(robin_hood::robin_hood ALIAS robin_hood)
target_compile_features(robin_hood INTERFACE ${RH_cxx_standard})

target_include_directories(robin_hood INTERFACE
	$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/src/include>
	$<INSTALL_INTERFACE:src/include>
)

In case you need a reference implementation for the CMake part, you can have a look at the CMakeLists.txt of other great header-only libraries like foonathan/type_safe, microsoft/GSL.

If you do want to build binaries by default (probably useful when developing the library), but only when this project is NOT included by another project, it might be beneficial to use a master project detection like the following, and to initialize your options accordingly:

# Determine if built as a subproject (using add_subdirectory)
# or if it is the master project.
set(MASTER_PROJECT OFF)
if(CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR)
	set(MASTER_PROJECT ON)
endif()

option(BUILD_TESTS "Build the tests" ${MASTER_PROJECT})
@martinus martinus self-assigned this Nov 4, 2019
@martinus
Copy link
Owner

martinus commented Nov 4, 2019

Hi @nyronium , thanks for the detailed description! I've tried to add a library definition as you've described, but I'm not really experienced with cmake so I'm not sure if it works as it should. Can you give it a try? It's only in master for now.

@oddlama
Copy link
Author

oddlama commented Nov 5, 2019

@martinus Thanks for implementing this, I really appreciate it! Your changes are mostly fine, though there are just two small things that kind of prevent using your library properly. Both of them are easily fixed though.

Wrong compile option

target_compile_features(robin_hood INTERFACE ${RH_cxx_standard})

This line breaks compilation, as it requests a feature called "14", which is not a thing.
To do this properly, use cxx_std_14 instead of ${RH_cxx_standard}. Usually you don't have to store the required standard in a variable, as it can be deduced from the dependencies of a target and its compile options.

-target_compile_features(robin_hood INTERFACE ${RH_cxx_standard})
+target_compile_features(robin_hood INTERFACE cxx_std_14)

cxx_std_14 is a compile_feature offered by cmake to require c++14 for your target.
This is where it would come in handy if you had used the target in your local tests.
Then you could completely delete the RH_cxx_standard variable, and CMake would figure it out automatically.

Global CMAKE state

set(CMAKE_CXX_STANDARD ${RH_cxx_standard})
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)

Setting global cmake variables is generally not a good practice, as it alters the global state for the whole project, not just for your definitions. In this case it will change the default standard for newly created targets, disable fallback to older standards and disable the use of any c++ extensions (such as gnu++14, ...).

Either way you can (and should) completely delete these lines, and instead either use the exported target as a dependency in your test target, or alternatively you can simply replace ${RH_cxx_standard} with cxx_std_14 in your test CMake files.

FYI (CMake intro)

I can understand that CMake does not seem very intuitive in the beginning, so
as you have said that you are not really experienced in CMake, I really want to recommend you to read through this short Introduction to modern CMake.

Especially the paragraphs "Dos and Don'ts", and "How to structure your project" might provide some useful insights.

I think adhering to some of those rules could both greatly simplify your setup and make it intuitive for other CMake users to use your project.

Even in case you don't want to change your structure, it will certainly give you a good understanding of basic CMake and why things are usually done a certain way. I hope you'll find useful!

@martinus
Copy link
Owner

Ok I think I got it now. I've got a minimal example in src/external_cmake that makes use of the library, and it seems to work for me. Thanks again!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants