smarco / WFA2-lib

WFA-lib: Wavefront alignment algorithm library v2
Other
162 stars 36 forks source link

Easily include WFA2-lib from other CMake-based projects #48

Closed marcelm closed 1 year ago

marcelm commented 1 year ago

Hi, we’re currently writing a new, fast read aligner called strobealign and are exploring using WFA2. I was happy to see that @pjotrp recently added a CMakeLists.txt file because our project also uses CMake. CMake has a nice mechanism called FetchContent that makes it easy to include external projects. This PR makes a couple of changes that would be necessary to make WFA2-lib usable via FetchContent:

  1. Build a C++ shared and static libraries libwfa2cpp.so/libwfa2cpp.a that contain the C++ binding code, and install the binding/cpp/WFAligner.hpp header in addition to the C headers.
  2. Use target_include_directories() instead of include_directories().
  3. Add wfa2::-prefixed aliases for all the library targets (such as wfa2::wfa2cpp). This seems to be the standard way of naming targets to be used via FetchContent.
  4. Do not install anything when the project is used via FetchContent. Otherwise, when the including/parent project is installed, all the WFA2-lib headers and libraries would be installed with it.

I’m not a CMake expert myself and had to read a couple of tutorials and examples before I got this to work. I’m sure this isn’t perfect, but it does work for strobealign.

With the changes in this PR, incorporating WFA2-lib into a CMake-based project would require adding this to its CMakeLists.txt:

FetchContent_Declare(wfa2repo
  GIT_REPOSITORY https://github.com/smarco/WFA2-lib.git
  GIT_TAG d71685bf70bb8977b084e036efb15e74562c16fc
)
FetchContent_MakeAvailable(wfa2repo)

and then this for a target that needs to link against WFA2-lib:

target_link_libraries(MyTarget PRIVATE wfa2::wfa2cpp_static)

CC @ksahlin

@pjotrp Perhaps you want to review this to ensure this is still good for Debian?

pjotrp commented 1 year ago

Not that much of an expert myself ;). I am not sure what the Debian folks think about fetchcontent. They probably won't like it because it fetches during build time, so they'll want to install the projects separately. I can probably apply something like this to vcflib too.

https://github.com/tillea do you have any suggestions?

pjotrp commented 1 year ago

OK, I tested the build against vcflib and it is fine even if you don't use fetchcontent. @smarco, I think it is fine to merge.

marcelm commented 1 year ago

Thanks!