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

improve cmake integration #105

Merged
merged 1 commit into from
Jan 5, 2021
Merged

Conversation

Ryan-rsm-McKenzie
Copy link
Contributor

export targets, add versioning, make standalone mode optional

export targets, add versioning, make standalone mode optional
install(
EXPORT ${LIB_NAME}-targets
NAMESPACE ${LIB_NAMESPACE}::
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/${LIB_NAME}
Copy link

Choose a reason for hiding this comment

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

Install header-only library in non standalone mode by default is are bad idea in my opinion.
Please add an option to disable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do these lines have to do with standalone mode? Unless you're talking about RH_STANDALONE_PROJECT, whose default value is exactly what it was before, except that now it can be overridden as part of the invocation to cmake.

Copy link

Choose a reason for hiding this comment

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

You added "install" if RH_STANDALONE_PROJECT == OFF, this changes the behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You have to install the library or else it can't be consumed by other cmake projects

Copy link

Choose a reason for hiding this comment

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

If RH_STANDALONE_PROJECT == ON - yes.
If RH_STANDALONE_PROJECT == OFF I use add_subdirectory(path/to/robin-hood-hashing) to add library in my cmake project. In this case, installation of headers is not required to use the library. robin-hood-hashing - is header-only library.

Copy link

Choose a reason for hiding this comment

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

CMakeLists.txt:

cmake_minimum_required(VERSION 3.5)
project(test LANGUAGES CXX)
add_subdirectory(3rdparty/robin-hood-hashing)

Cli:

$ mkdir 3rdparty && cd 3rdparty
$ git clone https://github.com/Ryan-rsm-McKenzie/robin-hood-hashing.git
$ cd ../..
$ mkdir build && cd build
$ cmake -DCMAKE_INSTALL_PREFIX=/Users/phprus/Devel/install ..
$ make install
Install the project...
-- Install configuration: ""
-- Installing: /Users/phprus/Devel/install/include/robin_hood.h
-- Installing: /Users/phprus/Devel/install/lib/cmake/robin_hood/robin_hoodConfig.cmake
-- Installing: /Users/phprus/Devel/install/lib/cmake/robin_hood/robin_hood-targets.cmake

What is it files on add_subdirectory case???

For the find_library case, the install commands should be in the RH_STANDALONE_PROJECT == ON branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These files describe your transitive dependencies. If someone uses your library, they will also need your dependencies. These files describe those dependencies. This is working exactly as intended.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Furthermore, the RH_STANDALONE_PROJECT=ON is the development branch and is clearly not intended for consumption by third parties.

Copy link

Choose a reason for hiding this comment

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

In my example, NOBODY is using the library, but the unneeded files in this scenario were created by the install commands.

Please, see https://github.com/Cyan4973/xxHash/blob/dev/cmake_unofficial/CMakeLists.txt#L121 for example.
install commands used only if "NOT XXHASH_BUNDLED_MODE"
XXHASH_BUNDLED_MODE option is equivalent (NOT RH_STANDALONE_PROJECT)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you don't want subdirectories to run install commands, then you can use add_subdirectory (robin_hood EXCLUDE_FROM_ALL)

@martinus
Copy link
Owner

martinus commented Jan 5, 2021

Hi, sorry for the late reply! I can't really review this change as I don't know much about cmake. It works for me though, so I'll merge it.

@martinus martinus merged commit d96d77f into martinus:master Jan 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants