Skip to content

Commit

Permalink
cmake: force linkage of shared libraries (#1431)
Browse files Browse the repository at this point in the history
This is brought by an issue that plagued pid_lut builds inside the
container:

https://eicweb.phy.anl.gov/EIC/benchmarks/physics_benchmarks/-/jobs/3070278#L1611

Our build process produces two shared objects:
- A foo_plugin that incorporates src/foo/foo.cc (containing
JANA2-related enty point)
- A foo_library that contains all other *.cc files In order to have a
complete set of symbols we ask linker to kindly link foo_library to
foo_plugin. Unfortunately, it can ignore our request if it deems that
the foo_library symbols are not referenced anywhere. This commit
modifies the linking process to incorporate --no-as-needed option. The
fancy feature from the

https://gitlab.kitware.com/cmake/cmake/-/commit/42965799b4747ab1e0afa6546be13444f68c1987
makes it so that other libraries (e.g. external ones like podio) can
still be linked lazily.

At this point we can remove WITH_STATIC_LIBRARY per-plugin
configuration.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
veprbl and pre-commit-ci[bot] authored May 8, 2024
1 parent 8f66e97 commit 01ef680
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 19 deletions.
9 changes: 8 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#
# cmake-format: on

cmake_minimum_required(VERSION 3.18)
cmake_minimum_required(VERSION 3.24)

project(EICrecon)

Expand Down Expand Up @@ -65,6 +65,13 @@ endif()
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)

# Infrastructure to support use of --no-as-needed
include(CheckLinkerFlag)
check_linker_flag(CXX "LINKER:--no-as-needed" CXX_LINKER_HAS_no_as_needed)
set(CMAKE_CXX_LINK_LIBRARY_USING_NO_AS_NEEDED_SUPPORTED TRUE)
set(CMAKE_CXX_LINK_LIBRARY_USING_NO_AS_NEEDED
"LINKER:--push-state,--no-as-needed" "<LINK_ITEM>" "LINKER:--pop-state")

# Export compile commands as json for run-clang-tidy
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)

Expand Down
25 changes: 16 additions & 9 deletions cmake/jana_plugin.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ macro(plugin_add _name)
PUBLIC $<BUILD_INTERFACE:${EICRECON_SOURCE_DIR}/src>
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}/${PROJECT_NAME}>)
target_include_directories(${_name}_plugin SYSTEM
PUBLIC ${JANA_INCLUDE_DIR})
target_include_directories(${_name}_plugin SYSTEM
PUBLIC ${ROOT_INCLUDE_DIRS})
PUBLIC ${JANA_INCLUDE_DIR} ${ROOT_INCLUDE_DIRS})
set_target_properties(
${_name}_plugin
PROPERTIES PREFIX ""
Expand Down Expand Up @@ -91,11 +89,14 @@ macro(plugin_add _name)
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}/${PROJECT_NAME}>)
target_include_directories(${_name}_library SYSTEM
PUBLIC ${JANA_INCLUDE_DIR})
target_link_libraries(${_name}_library ${JANA_LIB} podio::podio
podio::podioRootIO spdlog::spdlog)
target_link_libraries(${_name}_library ${JANA_LIB} podio::podio
podio::podioRootIO fmt::fmt)
target_link_libraries(${_name}_library Microsoft.GSL::GSL)
target_link_libraries(
${_name}_library
${JANA_LIB}
podio::podio
podio::podioRootIO
spdlog::spdlog
fmt::fmt
Microsoft.GSL::GSL)

# Install library
install(
Expand All @@ -105,7 +106,13 @@ macro(plugin_add _name)
endif(${_name}_WITH_LIBRARY)

if(${_name}_WITH_LIBRARY AND ${_name}_WITH_PLUGIN)
target_link_libraries(${_name}_plugin ${_name}_library)
# Ensure that whenever a plugin is loaded its library is loaded as well
if(CXX_LINKER_HAS_no_as_needed)
target_link_libraries(${_name}_plugin
$<LINK_LIBRARY:NO_AS_NEEDED,${_name}_library>)
else()
target_link_libraries(${_name}_plugin ${_name}_library>)
endif()
endif()
endmacro()

Expand Down
2 changes: 1 addition & 1 deletion src/services/log/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ get_filename_component(PLUGIN_NAME ${CMAKE_CURRENT_LIST_DIR} NAME)

# Function creates ${PLUGIN_NAME}_plugin and ${PLUGIN_NAME}_library targets
# Setting default includes, libraries and installation paths
plugin_add(${PLUGIN_NAME} WITH_STATIC_LIBRARY)
plugin_add(${PLUGIN_NAME} WITH_SHARED_LIBRARY)

# The macro grabs sources as *.cc *.cpp *.c and headers as *.h *.hh *.hpp Then
# correctly sets sources for ${_name}_plugin and ${_name}_library targets Adds
Expand Down
8 changes: 0 additions & 8 deletions src/tests/algorithms_test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,6 @@ target_link_libraries(
podio::podio
podio::podioRootIO)

# As-needed fails due to absent podio symbols in libJANA.so
# (https://github.com/JeffersonLab/JANA2/pull/255)
include(CheckLinkerFlag)
check_linker_flag(CXX "-Wl,--no-as-needed" CXX_LINKER_HAS_no_as_needed)
if(CXX_LINKER_HAS_no_as_needed)
target_link_options(${TEST_NAME} PRIVATE "-Wl,--no-as-needed")
endif()

# Install executable
install(TARGETS ${TEST_NAME} DESTINATION bin)

Expand Down

0 comments on commit 01ef680

Please sign in to comment.