Skip to content

Commit

Permalink
Add CMake option Pism_PKG_CONFIG_STATIC (default: on) (see #529)
Browse files Browse the repository at this point in the history
Normally it is enough to set "-L${petsc_prefix}/lib -lpetsc" to be able to link to PETSc:
libpetsc.so should have names and locations of its dependencies recorded in its metadata.

This mechanism allowing a linker to automatically discover indirect library dependencies
appears to be broken on some systems, requiring explicitly listing both direct and
indirect dependencies.

To try to avoid this issue PISM's CMake scripts run pkg-config with the flag "--static" to
get more complete lists of linker flags (usually used for static linking since static
libraries are just collections of object files and don't contain metadata).

This may lead to "overlinking", i.e. listing some libraries as *direct* dependencies of
libpism.so even though PISM itself does not use them. They would have to be loaded (either
as direct dependencies or not) anyway, so this appears to be the only side effect. I think
we can live with that.

Set this option to "off" to use pkg-config without the "--static" flag.

Note that we had to require CMake >= 3.16 for this to work properly (see
Kitware/CMake@28cb86d).
  • Loading branch information
ckhroulev committed Dec 23, 2023
1 parent 982613f commit 02aa847
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 16 deletions.
4 changes: 2 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ workflows:
name: old-cmake
cc: mpicc -cc=clang
cxx: mpicxx -cxx=clang++
cmake_prefix: /home/builder/local/cmake-3.13.0
cmake_prefix: /home/builder/local/cmake-3.16.0
petsc_dir: /home/builder/local/petsc
python: "Yes"

Expand All @@ -331,7 +331,7 @@ workflows:
name: old-cmake-minimal
cc: mpicc -cc=clang
cxx: mpicxx -cxx=clang++
cmake_prefix: /home/builder/local/cmake-3.13.0
cmake_prefix: /home/builder/local/cmake-3.16.0

- manual-html
- manual-pdf
Expand Down
14 changes: 7 additions & 7 deletions CMake/FindUDUNITS2.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -65,23 +65,23 @@ include (CheckCSourceRuns)
set(CMAKE_REQUIRED_INCLUDES ${UDUNITS2_INCLUDE_DIRS})
set(CMAKE_REQUIRED_LIBRARIES ${UDUNITS2_LIBRARIES})
set(CMAKE_REQUIRED_QUIET TRUE)
check_c_source_runs("${UDUNITS2_TEST_SRC}" UDUNITS2_WORKS_WITHOUT_EXPAT)
check_c_source_runs("${UDUNITS2_TEST_SRC}" UDUNITS2_WORKS_WITHOUT_LEXPAT)

if(${UDUNITS2_WORKS_WITHOUT_EXPAT})
message(STATUS "UDUNITS-2 does not require expat")
if(${UDUNITS2_WORKS_WITHOUT_LEXPAT})
message(STATUS "Linking to UDUNITS-2 does not require '-lexpat'")
else()
find_package(EXPAT REQUIRED)

set(CMAKE_REQUIRED_INCLUDES ${UDUNITS2_INCLUDE_DIRS} ${EXPAT_INCLUDE_DIRS})
set(CMAKE_REQUIRED_LIBRARIES ${UDUNITS2_LIBRARIES} ${EXPAT_LIBRARIES})
set(CMAKE_REQUIRED_QUIET TRUE)
check_c_source_runs("${UDUNITS2_TEST_SRC}" UDUNITS2_WORKS_WITH_EXPAT)
check_c_source_runs("${UDUNITS2_TEST_SRC}" UDUNITS2_WORKS_WITH_LEXPAT)

if(NOT ${UDUNITS2_WORKS_WITH_EXPAT})
message(FATAL_ERROR "UDUNITS-2 does not seem to work with or without expat")
if(NOT ${UDUNITS2_WORKS_WITH_LEXPAT})
message(FATAL_ERROR "UDUNITS-2 does not seem to work with or without -lexpat")
endif()

message(STATUS "UDUNITS-2 requires EXPAT")
message(STATUS "Using '-lexpat' to link to UDUNITS-2")
set (UDUNITS2_LIBRARIES "${UDUNITS2_LIBRARIES};${EXPAT_LIBRARIES}" CACHE STRING "" FORCE)
endif()

Expand Down
10 changes: 6 additions & 4 deletions CMake/PISM_CMake_macros.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,12 @@ function(pism_find_library PREFIX SPEC)
# Trick pkg-config into using "Libs.private" instead of "Libs" to get the full list of
# libraries (e.g. not just PETSc, but PETSc and its dependencies). This is needed to
# build PISM on some systems. See also: https://gitlab.kitware.com/cmake/cmake/-/issues/21714
if (CMAKE_VERSION VERSION_LESS "3.22")
list(APPEND PKG_CONFIG_EXECUTABLE "--static")
else()
set(PKG_CONFIG_ARGN "--static" CACHE INTERNAL)
if (Pism_PKG_CONFIG_STATIC)
if (CMAKE_VERSION VERSION_LESS "3.22")
list(APPEND PKG_CONFIG_EXECUTABLE "--static")
else()
set(PKG_CONFIG_ARGN "--static" CACHE INTERNAL "command-line arguments for pkg-config")
endif()
endif()
pkg_search_module(${PREFIX} REQUIRED IMPORTED_TARGET ${SPEC})
if (${${PREFIX}_FOUND})
Expand Down
5 changes: 3 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ option (Pism_USE_JANSSON "Use Jansson to read configuration files." OFF)
option (Pism_TEST_USING_VALGRIND "Add extra regression tests using valgrind" OFF)
mark_as_advanced (Pism_TEST_USING_VALGRIND)

option (Pism_ADD_FPIC "Add -fPIC to C++ compiler flags (CMAKE_CXX_FLAGS). Try turning it off if it does not work." ON)
option (Pism_PKG_CONFIG_STATIC "Use the --static pkg-config flag to get linker flags needed by prerequisites" ON)
option (Pism_ADD_FPIC "Add -fPIC to C & C++ compiler flags. Try turning it off if it does not work." ON)
option (Pism_CODE_COVERAGE "Add compiler options for code coverage testing." OFF)
option (Pism_LINK_STATICALLY "Set CMake flags to try to ensure that everything is linked statically")
option (Pism_LOOK_FOR_LIBRARIES "Specifies whether PISM should look for libraries. (Disable this on Crays.)" ON)
Expand All @@ -92,7 +93,7 @@ endif ()

# Add -fPIC to C and CXX flags.
if (Pism_ADD_FPIC)
add_compile_options(-fPIC)
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
endif ()

if (Pism_CODE_COVERAGE)
Expand Down
2 changes: 1 addition & 1 deletion docker/ubuntu-ci/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ foreach(script petsc.sh hdf5.sh netcdf.sh pnetcdf.sh parallelio.sh)
configure_file(${Pism_SOURCE_DIR}/doc/sphinx/installation/code/${script} . COPYONLY)
endforeach()

configure_file(cmake-3_13_0.sh . COPYONLY)
configure_file(cmake-3_16_0.sh . COPYONLY)
configure_file(old-petsc.sh . COPYONLY)

set(PISM_DOCKER_UBUNTU_VERSION 0.1.8)
Expand Down

0 comments on commit 02aa847

Please sign in to comment.