drop zlib from pacakges that don't need it

[adammoody]

Some packages include a FIND_PACKAGE(zlib) step that don't actually depend on zlib. We need to check each package and remove it where it is not actually used.

While at it, let's check each package for any other dependencies that aren't actually dependencies.

[adammoody]

Well, I took a pass at this, but it's trickier than I first thought. When building the components as static libs with -DBUILD_SHARED_LIBS=OFF, we still need an -lz in there for any component that also depends on -lkvtree, since -lkvtree itself requires -lz for a proper link.

Can cmake automate that kind of transitive dependency for static libs?

Maybe we could still drop the FIND_PACKAGE(zlib) if the component doesn't have a direct dependency, but include a -lkvtree -lz everywhere we have an -lkvtree now if using a static-only build?

[adammoody]

@gonsie , we could benefit from your cmake expertise on this one.

[rhaas80]

cmake can pass along the libraries required by dependencies (eg zlib for kvtree). I ran into issue like this eg on lassen with static building and a typical fix would look like this:

diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt
index 3c73316..272b50c 100644
--- a/examples/CMakeLists.txt
+++ b/examples/CMakeLists.txt
@@ -32,9 +32,9 @@ INSTALL(FILES ${CMAKE_CURRENT_BINARY_DIR}/makefile.examples DESTINATION ${CMAKE_

 ## Prefer static?
 IF(SCR_LINK_STATIC)
-        SET(SCR_LINK_TO scr-static)
+        SET(SCR_LINK_TO scr-static ${SCR_EXTERNAL_LIBS})
 ELSE(SCR_LINK_STATIC)
-        SET(SCR_LINK_TO scr)
+        SET(SCR_LINK_TO scr ${SCR_EXTERNAL_LIBS})
 ENDIF(SCR_LINK_STATIC)


@@ -86,9 +86,9 @@ IF(ENABLE_FORTRAN)
         IF(SCR_LINK_STATIC)
                 # needed for LANL cray
                 SET(CMAKE_SHARED_LIBRARY_LINK_Fortran_FLAGS)
-                TARGET_LINK_LIBRARIES(test_ckpt_F scrf-static ${MPI_Fortran_LIBRARIES})
+                TARGET_LINK_LIBRARIES(test_ckpt_F scrf ${SCR_EXTERNAL_LIBS} ${MPI_Fortran_LIBRARIES})
         ELSE(SCR_LINK_STATIC)
-                TARGET_LINK_LIBRARIES(test_ckpt_F scrf ${MPI_Fortran_LIBRARIES})
+                TARGET_LINK_LIBRARIES(test_ckpt_F scrf-static ${SCR_EXTERNAL_LIBS} ${MPI_Fortran_LIBRARIES})
         ENDIF(SCR_LINK_STATIC)

        SCR_ADD_TEST(test_ckpt_F "" "rank_00000.ckpt")

eg here one ads ${SCR_EXTERNAL_LIBS} to the executable. SCR_EXTERNAL_LIBS itself is filled via cmake code like so (already present_:

## KVTREE
FIND_PACKAGE(KVTREE REQUIRED)
IF(KVTREE_FOUND)
        INCLUDE_DIRECTORIES(${KVTREE_INCLUDE_DIRS})
        LIST(APPEND SCR_EXTERNAL_LIBS ${KVTREE_LIBRARIES})
        LIST(APPEND SCR_EXTERNAL_SERIAL_LIBS ${KVTREE_BASE_LIBRARIES})
        LIST(APPEND SCR_LINK_LINE "-lkvtree")
ENDIF(KVTREE_FOUND)

and basically KVTREE_LIBRARIES must be set correctly by kvtree.

[adammoody]

Thanks @rhaas80 .

Do we need to then update our FindKVTree.cmake script for SCR to include libz in here?

scr/cmake/FindKVTREE.cmake

Lines 11 to 14 in e4fa306

	FIND_LIBRARY(KVTREE_LIBRARIES
	NAMES kvtree
	HINTS ${WITH_KVTREE_PREFIX}/lib
	)

as in something like:

    NAMES kvtree z

[adammoody]

It looks like one option might be to modify the KVTree CMake set up to install a KVTreeConfig.cmake file, and in there we could define KVTREE_LIBRARIES to list both libkvtree and libz?

https://cmake.org/cmake/help/latest/module/CMakePackageConfigHelpers.html
https://cmake.org/cmake/help/latest/manual/cmake-packages.7.html
https://cmake.org/cmake/help/latest/manual/cmake-packages.7.html#package-configuration-file

If that works and we opt to go that route, I think we could then drop the cmake/FindKVTree.cmake script from SCR and just be sure that the SCR cmake can find the KVTreeConfig.cmake file that was installed by kvtree instead.

[rhaas80]

Thanks @rhaas80 .

Do we need to then update our FindKVTree.cmake script for SCR to include libz in here?

scr/cmake/FindKVTREE.cmake

Lines 11 to 14 in e4fa306

	FIND_LIBRARY(KVTREE_LIBRARIES
	NAMES kvtree
	HINTS ${WITH_KVTREE_PREFIX}/lib
	)

as in something like:

    NAMES kvtree z

I think so, but I am no cmake expert (by a very large stretch not so).

[gonsie]

I’m looking at this issue and I want to make sure I understand the problem.

These components need zlib (explicitly via #include <zlib.h>) : axl, er, kvtree, redset, scr, shuffile.

It looks like er’s CMake file is missing a a find_package(zlib) call.

Other than that, everything looks normal. My understanding is that SCR should explicitly look for zlib, separately from looking for kvtree. Are we trying to do something to ensure that SCR is linking to the same zlib that kvtree is? Or is there something else that I’m missing?

[gonsie]

Make sure that the building and linking is working correctly for static builds, particularly for components that don’t use zlib (but depend on components which do use zlib).

[gonsie]

see also:

• Remove ZLib dependency ECP-VeloC/spath#12
• spath: zlib is a link dependency spack/spack#23303

[gonsie]

From my testing, everything seems fine. I can link static with no issues (quartz both with intel/19.0.4 and gcc/4.9.3). The only issue I found was ECP-VeloC/er#25.

[adammoody]

The root problem is a bit more general, but zlib happens to demonstrate the issue. We're trying to find the proper way in CMake to express transitive dependencies. As an example, say we have a package foo that depends on kvtree, but foo does not directly depend on zlib. kvtree does depend on zlib, so it looks like:

foo --> kvtree --> zlib

In order to get a static link of foo, one currently has to have a find_package(zlib) statement in the CMakeLists.txt of foo, even though foo itself does not directly depend on zlib.

It seemed to me that the CMakePackageConfig files might provide a way out, see #319 (comment).

I'm wondering if we can have kvtree install a package config file, and in there kvtree can list all of the libs it depends on, so that foo would automatically pick up any libs kvtree depends on when it reads the kvtree config file. Then we could drop the find_package(zlib) from the CMakeLists.txt of foo.

[adammoody]

As a specific example, er does not actually depend on zlib. The #include <zlib.h> in er_util.c should be removed. If one does that and then one subsequently also drops the find_package(zlib) related code from the er CMakeLists.txt, one can no longer build er statically. It fails to add the necessary -lz to the link line.

ECP-VeloC/er#26

[mcfadden8]

Which module(s) under er depend upon zlib? Is it possible that the CMakeLists.txt for those modules are not calling find_pacakage(zlib)? I think this should work

[adammoody]

Which module(s) under er depend upon zlib? Is it possible that the CMakeLists.txt for those modules are not calling find_pacakage(zlib)? I think this should work

er depends on kvtree, and kvtree depends on zlib

[adammoody]

Oh, it may work to search for zlib in the cmake modules for kvtree, redset, shuffile, and then add the zlib library to the list of libraries provided by each of those components. I pushed a commit that does that here:

ECP-VeloC/er@cf86307

Is that what you are saying, @mcfadden8 ?

I also have a start on adding package config file to kvtree. That approach is here for people to look at:

ECP-VeloC/KVTree#60

[mcfadden8]

Yeah, I'm not sure if my thinking is completely correct or not. I was thinking that if each module were defined in a way that it could be built as a stand-alone entity, that might make it easier for that module to be plugged into other modules that depend on it. When I thought about it like that, it seemed like having each module explicitly find all things it is dependent upon might help. I am curious to see whether this helps or not.

[adammoody]

Some more pointers to use cmake targets to resolve the chained dependency problem:

https://gitlab.kitware.com/cmake/community/-/wikis/doc/tutorials/Exporting-and-Importing-Targets#exporting-targets
https://github.com/LLNL/Umpire/blob/develop/umpire-config.cmake.in#L26
https://github.com/LLNL/Umpire/blob/develop/src/umpire/CMakeLists.txt#L164-L169