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 support for CMake find_package Config #96

Merged
merged 1 commit into from
Jul 26, 2023

Conversation

pr0g
Copy link
Contributor

@pr0g pr0g commented Jun 13, 2023

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 dougbinks self-requested a review June 14, 2023 08:46
@dougbinks
Copy link
Owner

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
Copy link
Contributor Author

pr0g commented Jun 14, 2023

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
Copy link
Contributor Author

pr0g commented Jul 2, 2023

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
Copy link
Owner

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

@dougbinks dougbinks self-assigned this Jul 2, 2023
@pr0g
Copy link
Contributor Author

pr0g commented Jul 2, 2023

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 dougbinks merged commit 0d0b53b into dougbinks:master Jul 26, 2023
4 checks passed
@dougbinks
Copy link
Owner

I've now merged this PR, many thanks.

@pr0g
Copy link
Contributor Author

pr0g commented Jul 26, 2023

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

@pr0g pr0g deleted the install branch July 26, 2023 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants