dougbinks / enkiTS

A permissively licensed C and C++ Task Scheduler for creating parallel programs. Requires C++11 support.
zlib License
1.66k stars 138 forks source link

Add support for CMake find_package Config #96

Closed pr0g closed 11 months ago

pr0g commented 1 year ago

This change allows enkiTS to be installed and then used with find_package(enkiTS) (or more precisely find_package(enkiTS REQUIRED CONFIG).

An example usage is using CMake AddExternal_Project such as...

ExternalProject_Add(
  enkiTS
  GIT_REPOSITORY https://github.com/pr0g/enkiTS.git
  GIT_TAG cd8e8c2af2e7c3f7d7719ccbed25f147d63b71f6
  BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/enkiTS/build/${build_type_dir}
  INSTALL_DIR ${CMAKE_CURRENT_BINARY_DIR}
  CMAKE_ARGS ${build_type_arg} -DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
             "$<$<CONFIG:Debug>:-DCMAKE_DEBUG_POSTFIX=d>"
             -DENKITS_BUILD_EXAMPLES=OFF -DENKITS_INSTALL=ON)

and then consuming enkiTS in a downstream application library with...

find_package(enkiTS REQUIRED CONFIG)
...
target_link_libraries(${PROJECT_NAME} PRIVATE enkiTS::enkiTS)

It can then be used by using the above find_package command after installing locally and telling CMake where to find it with -DCMAKE_PREFIX_PATH.

I'm definitely not a CMake expert but I have used this pattern successfully in a number of projects and have written up a number of use cases here that might be useful to review - cmake examples.

If it would be useful to include a sample project showing this in action I can include an example as well. Thanks!

dougbinks commented 1 year ago

This may take me a bit of time to review and merge.

I'm personally not fond of installing code system wide, as I think it's important to have projects as self contained as possible. So whilst I'm happy to add a feature which is useful to others I'm not that familiar with this approach so I'll need to do some background reading on the cmake install approaches.

As an aside it's worth noting that enkiTS is rather tiny (all the source and headers are in one dir and only one source file is needed for C++) so just adding the source files to your build system should be trivial. For cmake, my preferred approach is how I do it for the extended examples in https://github.com/dougbinks/enkiTSExamples - use submodules (or download to a dir) and enkiTS from there with:

include_directories("enkiTS/src")
set( ENKITS_BUILD_EXAMPLES OFF CACHE BOOL "Do not build examples" )
add_subdirectory( enkiTS )
target_link_libraries(${PROJECT_NAME} enkiTS )
pr0g commented 1 year ago

Sure I understand, definitely have a look at the repo I mentioned at the bottom of the description for more info/context.

The really useful thing about installing is it decouples building your dependencies from the main application (in some cases to save time, but also for things like compilation flags/options etc... it can be super useful).

The beauty of installing is you can use -DCMAKE_INSTALL_PREFIX to specify where to install things (so they're not system wide) and then use -DCMAKE_PREFIX_PATH to tell CMake where to look.

Right now all my changes (bar one) are behind the ENKI_INSTALL option and from what I can tell installing currently is not supported correctly (I feel this PR improves this).

If you'd rather not support installing it might be worth just removing the option entirely or include in the extended examples a demonstration of using install in its current form.

I could add a new extended example showing how this works if that would be useful? It'll be very similar to what's in the cmake-examples repo mentioned above.

pr0g commented 1 year ago

Just a brief update on this. I made a very simple repo to demonstrate using enkiTS with either FetchContent or ExternalProject_Add (after installing locally). See enkiTS-example. Let me know if this is helpful to understand what this PR facilitiates.

dougbinks commented 1 year ago

Thanks - I have a lot of work on at the moment so haven't had a chance to look at it yet,

pr0g commented 1 year ago

Ok no worries at all, thank you for letting me know.

It's not urgent in the slightest, I just wanted to try and help with an example if that made reviewing the PR easier when you find time.

Cheers!

dougbinks commented 11 months ago

I've now merged this PR, many thanks.

pr0g commented 11 months ago

That's great to hear, thank you very much @dougbinks! 🙂