cmake : building libsc inside p4est with or without building zlib
Closed this issue · 4 comments
Description
This is a minor change.
Currently, when building both p4est and libsc with cmake, libsc is built inside p4est with the following cmake args (from file cmake/sc.cmake):
set(cmake_sc_args
-DCMAKE_INSTALL_PREFIX:PATH=${SC_ROOT}
-DBUILD_SHARED_LIBS:BOOL=${BUILD_SHARED_LIBS}
-DCMAKE_BUILD_TYPE=Release
-DBUILD_TESTING:BOOL=false
-Dmpi:BOOL=${mpi}
-Dopenmp:BOOL=${openmp}
)
At this stage, we do not control if libsc will itself build zlib.
It means, no matter what, libsc will actually always build zlib, since libsc cmake option zlib is defaulting to true.
But I feel, that if we chose to build libsc from p4est, we should also control if libsc should use zlib from system or build zlib.
While I agree, a given user can always build libsc separately (and chose if he/she wants to build also zlib), and then build p4est; for simplicity we should be able to chose to build or not zlib directly from p4est build.
The reason, behind this minor change, is that when building and installing p4est (with libsc, and thus zlib), it brings in the same installation path libz.so, available through LD_LIBRARY_PATH. If the final application, also link to e.g. HDF5, which comes with it own libz.so dependency, it may be a source of trouble.
Proposed solution
Add a cmake variable/option in p4est/cmake/config.cmake that can be passed to the ExternalProject_Add call that builds libsc to specifiy if we want or not libsc to build zlib.
Actually in libsc/cmake/config.cmake, current behavior is:
- if zlib=ON, we build zlib
- if zlib=OFF, we don't use zlib at all, but there is a problem :
currently libsc/master branch does not build if configured like this: cmake -Dmpi=ON -Dzlib=OFF ../
(I think this case is not covered by ci)
/data/pkestene/github/libsc_cb/src/sc_io.c:653:33: error: ‘Z_BEST_COMPRESSION’ undeclared (first use in this function)
653 | sc_io_encode_zlib (data, out, Z_BEST_COMPRESSION);
| ^~~~~~~~~~~~~~~~~~
/data/pkestene/github/libsc_cb/src/sc_io.c:653:33: note: each undeclared identifier is reported only once for each function it appears in
it appears that line 653 should be protected by #ifdef SC_HAVE_ZLIB
or perhaps the whole function should.
Finally, why not let the user chose if he/she want to build zlib inside libsc, doing something like that in sc/cmake/config.cmake
if(zlib)
message(STATUS "Using builtin zlib")
include(${CMAKE_CURRENT_LIST_DIR}/zlib.cmake)
set(SC_HAVE_ZLIB true)
else()
find_package(ZLIB)
if(ZLIB_FOUND)
message(STATUS "Using system zlib : ${ZLIB_VERSION_STRING}")
set(SC_HAVE_ZLIB true)
else()
message(STATUS "Zlib disabled (not found). Consider using cmake \"-Dzlib=ON\" to turn on builtin zlib.")
set(SC_HAVE_ZLIB false)
endif()
endif()
Just to add a comment regarding cmake / autotools build:
currently the source archive https://p4est.github.io/release/p4est-2.8.5.5-9ddbb.tar.gz is self-contained regarding the autotools build, not regarding the cmake build (it need to download zlib sources). This is another reason why we need a switch inside cmake to enable/disable at p4est level the build of zlib.
If you agree on the need, i can provide a small modification for this.
Yes this sounds good! Any comments @scivision ?
This sounds good to me, thanks