[Debian-med-packaging] cmake help needed for metabat

[Andreas Tille]

Hi Alexis,

thanks a lot for your extremely helpful explanation.

On Thu, May 28, 2020 at 10:04:02PM +0200, Alexis Murzeau wrote:
> 
> There is a Debian patch that replace cmake/htslib.cmake with a
> pkg_check_modules call, which doesn't create targets, so that why it fails.
> There is the same issue with zlib, and a missing header
> "metabat_version.h" created by cmake/git-watcher.cmake.
> 
> 
> About HTSlib issue
> ------------------
> 
> With pkgconfig, you instead get just variables (no target), including
> these interesting ones:
>  - HTSlib_LIBRARIES
>  - HTSlib_INCLUDE_DIRS
> 
> (The HTSlib comes from the first argument of pkg_check_modules)
> 
> In cmake version 3.6 and later, an argument to pkg_check_modules can
> create a target to be used with target_link_libraries that will take
> care of the libraries and include dirs.
> This is the IMPORTED_TARGET option, it will create a target named
> PkgConfig::<PREFIX>.
> 
> So you can have:
> ```
> pkg_check_modules(HTSlib REQUIRED IMPORTED_TARGET htslib)
> ```
> 
> and then use it:
> ```
> target_link_libraries(target [...] PkgConfig::HTSlib)
> ```
> 
> But metabat seems to require only cmake 3.5, so it might not be allowed
> for upstream to do that.
> 
> Instead you can just do the same as done for Boost and add:
> ```
> include_directories(${HTSlib_INCLUDE_DIRS})
> ```
> and
> ```
> target_link_libraries(target [...] ${HTSlib_INCLUDE_DIRS})
> ```

Makes sense.
 
> About zlib issue
> ----------------
> 
> FindPackage(ZLIB) don't create any "zlib" target, but a "ZLIB::ZLIB"
> target instead, which can be used with target_link_libraries.
> 
> So you can just replace the add_dependencies(target zlib) with a new
> item there:
> ```
> target_link_libraries(target [...] ZLIB::ZLIB)
> ```
> 
> About the metabat_version.h header issue
> ----------------------------------------
> 
> cmake/git-watcher.cmake generate metabat_version.h from
> metabat_version.h.in.
> So as with the patch, git-watcher.cmake is not used anymore, you still
> need to handle that.
> 
> Either put fake stuff like this:
> ```
> set(PRE_CONFIGURE_FILE "metabat_version.h.in")
> set(POST_CONFIGURE_FILE "metabat_version.h")
> # include(cmake/git-watcher.cmake)
> 
> # Add this:
> set(GIT_RETRIEVED_STATE "")
> set(GIT_HEAD_SHA1 "nosha")
> set(GIT_IS_DIRTY 0)
> set(BUILD_TIMESTAMP 0)
> configure_file("${PRE_CONFIGURE_FILE}" "${POST_CONFIGURE_FILE}" @ONLY)
> include_directories("${CMAKE_CURRENT_BINARY_DIR}")
> ```
> 
> Or you change directly the code that use these defines to use different
> data (like Debian version instead).

Makes sense.  I was not up to this issue - just was dealing with errors
step by step but I guess this would have been my next question. ;-)
Thanks a lot for the proactive answer.
 
> Conclusion
> ----------
> 
> I'm attaching a patch with what I've said here.
> The build still doesn't work because of tests failure.
> The tests require data files like "contigs_depth.txt" but can't find them.

OK, I need to deal with this.

> In test/CMakeLists.txt, there is a reference to them as
> "${CMAKE_BINARY_DIR}/contigs_depth.txt", but it should be
> "${CMAKE_CURRENT_SOURCE_DIR}/contigs_depth.txt", unless the text files
> should be copied while building, but that doesn't seem the case.

I'll check upstream Git or file an issue about this.

Unfortunately your patch did not applied cleanly - no idea why.  I have
added it manually and reread your explanation which I think are
implemented correctly.  Unfortunately in my pbuilder I get the same
cmake failure as before and since I think I've now understand the issue
I'm even more in vain since its not working anyway for me but you
confirmed that it should work.  Could you be so kind to have another
look on the latest state in Git?

Thanks again

      Andreas.

> -- 
> Alexis Murzeau
> PGP: B7E6 0EBB 9293 7B06 BDBC  2787 E7BD 1904 F480 937F

> diff --git a/debian/patches/use_debian_packaged_libs.patch b/debian/patches/use_debian_packaged_libs.patch
> index 05c4a0e..b532dc9 100644
> --- a/debian/patches/use_debian_packaged_libs.patch
> +++ b/debian/patches/use_debian_packaged_libs.patch
> @@ -1,7 +1,15 @@
> -Author: Andreas Tille <tille at debian.org>
> +From: Andreas Tille <tille at debian.org>
> +Date: Thu, 28 May 2020 21:21:02 +0200
> +Subject: Use Debian packaged libraries
> +
>  Last-Update: Thu, 28 May 2020 17:21:42 +0200
> -Description: Use Debian packaged libraries
> +---
> + CMakeLists.txt     | 15 ++++++++++++---
> + src/CMakeLists.txt |  8 ++++----
> + 2 files changed, 16 insertions(+), 7 deletions(-)
> 
> +diff --git a/CMakeLists.txt b/CMakeLists.txt
> +index 638c27c..6166143 100644
>  --- a/CMakeLists.txt
>  +++ b/CMakeLists.txt
>  @@ -8,8 +8,10 @@ endif()
> @@ -17,32 +25,52 @@ Description: Use Debian packaged libraries
> 
>   set(CMAKE_CXX_STANDARD 11)
>   set(CMAKE_CXX_STANDARD_REQUIRED ON)
> -@@ -23,7 +25,7 @@ add_definitions(-D_XOPEN_SOURCE=700)
> +@@ -23,7 +25,14 @@ add_definitions(-D_XOPEN_SOURCE=700)
> 
>   set(PRE_CONFIGURE_FILE "metabat_version.h.in")
>   set(POST_CONFIGURE_FILE "metabat_version.h")
>  -include(cmake/git-watcher.cmake)
>  +# include(cmake/git-watcher.cmake)
> ++set(GIT_RETRIEVED_STATE "")
> ++set(GIT_HEAD_SHA1 "nosha")
> ++set(GIT_IS_DIRTY 0)
> ++set(BUILD_TIMESTAMP 0)
> ++configure_file("${PRE_CONFIGURE_FILE}" "${POST_CONFIGURE_FILE}" @ONLY)
> ++include_directories("${CMAKE_CURRENT_BINARY_DIR}")
> ++
> 
>   if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
>     # using Clang
> +diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> +index d47843e..b8dc3b9 100644
>  --- a/src/CMakeLists.txt
>  +++ b/src/CMakeLists.txt
> -@@ -36,7 +36,7 @@ set(targets metabat1 metabat2)
> +@@ -4,6 +4,8 @@ set(Boost_USE_STATIC_LIBS   ON)
> + find_package(Boost 1.55.0 COMPONENTS program_options filesystem system graph serialization iostreams REQUIRED)
> + include_directories( ${Boost_INCLUDE_DIRS} )
> +
> ++include_directories( ${HTSlib_INCLUDE_DIRS} )
> ++
> + # find and use OpenMP
> + find_package(OpenMP)
> +
> +@@ -36,8 +38,7 @@ set(targets metabat1 metabat2)
>   foreach(target ${targets})
> 
>      add_executable(${target} ${target}.cpp)
>  -   add_dependencies(${target} htslib zlib check_git_repository)
> -+   add_dependencies(${target} htslib zlib)
> -    target_link_libraries(${target} ${zlib_LIB} ${Boost_LIBRARIES} ${EXTRALIBS} )
> +-   target_link_libraries(${target} ${zlib_LIB} ${Boost_LIBRARIES} ${EXTRALIBS} )
> ++   target_link_libraries(${target} ${HTSlib_LIBRARIES} ZLIB::ZLIB ${Boost_LIBRARIES} ${EXTRALIBS} )
> 
>   endforeach()
> -@@ -48,7 +48,7 @@ install(TARGETS ${targets}
> +
> +@@ -48,8 +49,7 @@ install(TARGETS ${targets}
>   set(targets jgi_summarize_bam_contig_depths contigOverlaps)
>   foreach(target ${targets})
>      add_executable(${target} ${target}.cpp)
>  -   add_dependencies(${target} htslib zlib check_git_repository)
> -+   add_dependencies(${target} htslib zlib)
> -    target_link_libraries(${target} ${htslib_LIB} ${zlib_LIB} ${Boost_LIBRARIES} )
> +-   target_link_libraries(${target} ${htslib_LIB} ${zlib_LIB} ${Boost_LIBRARIES} )
> ++   target_link_libraries(${target} ${HTSlib_LIBRARIES} ZLIB::ZLIB ${Boost_LIBRARIES} )
>   endforeach()
> 
> +


-- 
http://fam-tille.de