add CMake config support

ECP-VeloC / VELOC

Very-Low Overhead Checkpointing System

http://veloc.rtfd.io

MIT License

52 stars 21 forks source link

add CMake config support #43

Closed nmm0 closed 1 year ago

nmm0 commented 1 year ago

Recently I submitted PRs for AXL, er, KVTree, rankstr, redset, and shuffile to export their own CMake configs since they should be managed upstream. This PR does the same:

Exports a config for VELOC in the install directory
Uses the find configs from the upstream dependencies and removes the Find* modules
Prefers dependencies targets (i.e. Threads::Threads and Boost::boost)
Updates CMake version to one more recent (1.14) which is in line with the dependencies
Uses target properties instead of global statements (i.e. target_include_directories vs include_directories)
Exports the config to the build tree so that you don't need to install the source

I'm happy to tweak anything, especially names/conventions/etc to your liking so please let me know :)

adammoody commented 1 year ago

Looks good to me. Thank you again, @nmm0 !

nmm0 commented 1 year ago

There was a small bug with the build-tree export missing some include directories. Should be fixed now!

Matthew-Whitlock commented 1 year ago

This currently breaks auto-install.py - I just needed to change the versions required on the dependencies to each require 'main' instead. Switching from a branch to a specific commit would be a better solution though, until new releases are made of the dependencies.

bnicolae commented 1 year ago

All, we need to make sure this does not break auto-install.py. @Matthew-Whitlock: did you check this works correctly?

Matthew-Whitlock commented 1 year ago

All, we need to make sure this does not break auto-install.py. @Matthew-Whitlock: did you check this works correctly?

@bnicolae I did check that the cmake changes work for a standard install, and that fixing auto-install.py by switching the tags listed for each dependency to main would also work.

nmm0 commented 1 year ago

All, we need to make sure this does not break auto-install.py. @Matthew-Whitlock: did you check this works correctly?

@bnicolae I did check that the cmake changes work for a standard install, and that fixing auto-install.py by switching the tags listed for each dependency to main would also work.

@Matthew-Whitlock Can you post the patch? I can update the PR to ensure this doesn't break auto-install. Thanks!

nmm0 commented 1 year ago

@bnicolae Updating the references to main could cause problems in the future since it's a moving target -- maybe we can see if the dependency libraries can issue a minor release or a fixed tag that we can use?

bnicolae commented 1 year ago

Ok, let me ping the folks to release a config-enabled version for the deps

nmm0 commented 1 year ago

Ok, let me ping the folks to release a config-enabled version for the deps

@bnicolae thanks for doing this! I saw they released a new version so I added a commit that should fix the autoinstall issue. I tested on my local machine and didn't have problems. @Matthew-Whitlock can you check if this solves your issues?

Matthew-Whitlock commented 1 year ago

This solves for me!