Skip to content

Commit

Permalink
Support use with linkers/setups needing rpath-link (#8)
Browse files Browse the repository at this point in the history
Discovered in testing of integration of G4VG in apt-sim/AdePT#289,
targets linking to installed G4VG may fail to link due to linker
not having needed information on location of deps-of-deps of G4VG.
This happens in build environments where

1. The installed dependencies (here, Geant4, VecGeom) do not have
   their rpath set correctly.
2. They are otherwise in a location the linker is not aware of,
   e.g. via ldconfig of LD_LIBRARY_PATH etc.

The link error warns about this and suggests adding the path through,
e.g. `-rpath-link`. CMake will always add this flag/path correctly,
but only if it knows about the dependencies (even if private).

The G4VG library only depends on the static Celeritas::geocel target,
which does not forward its dependencies to Geant4/Vecgeom and so
it does not set `-rpath-link` for these.

Workaround this issue by explicitly making used Geant4 targets
PRIVATE deps of the g4vg target. Refind Geant4/VecGeom dependencies
when finding G4VG so these are resolved and available if the linker
requires it.

Add smoke test for linking that simply tries to link only the G4VG
target. Confirmed to reproduce problem observed in AdePT, with
the fixes above resolving it.
  • Loading branch information
drbenmorgan authored May 7, 2024
1 parent 804ea9c commit f372cdd
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 10 deletions.
17 changes: 8 additions & 9 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ g4vg_set_default(BUILD_TESTING ${G4VG_BUILD_TESTS})

g4vg_set_default(CMAKE_CXX_EXTENSIONS OFF)

#----------------------------------------------------------------------------#
# Dependencies
# Found here because we need the targets visible to both import Celeritas and
# to declare them as-needed in g4vg itself for correct downstream use

find_package(Geant4 REQUIRED)
find_package(VecGeom REQUIRED)

#----------------------------------------------------------------------------#
# Add code

Expand All @@ -45,15 +53,6 @@ if(G4VG_BUILD_TESTS)
if(NOT GTest_FOUND)
find_package(GTest 1.10 REQUIRED)
endif()
if(NOT VecGeom_FOUND)
# Note: using the same version as celeritas to silence cmake config
# messages
find_package(VecGeom 1.2.4 REQUIRED)
endif()
if(NOT Geant4_FOUND)
find_package(Geant4 REQUIRED)
endif()

add_subdirectory(test)
endif()

Expand Down
5 changes: 5 additions & 0 deletions cmake/G4VGConfig.cmake.in
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
@PACKAGE_INIT@

# Refind dependencies in case linker needs paths to resolve
include(CMakeFindDependencyMacro)
find_dependency(Geant4)
find_depencency(VecGeom)

include("${CMAKE_CURRENT_LIST_DIR}/G4VGTargets.cmake")

9 changes: 8 additions & 1 deletion src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,15 @@
cuda_rdc_add_library(g4vg SHARED
G4VG.cc
)
# Whilst geocel contains the primary Geant4/VecGeom dependency, we must also
# declare the geometry/gdml deps explicitly so that clients can pick them
# up correctly through LINK_DEPENDENT_LIBRARIES should the linker require it.
# See: https://discourse.cmake.org/t/use-cases-for-imported-link-dependent-libraries
cuda_rdc_target_link_libraries(g4vg
PRIVATE Celeritas::geocel
PRIVATE
Celeritas::geocel
Geant4::G4geometry
$<IF:$<TARGET_EXISTS:Geant4::G4gdml>,Geant4::G4gdml,Geant4::G4persistency>
)
cuda_rdc_target_include_directories(g4vg
PUBLIC
Expand Down
5 changes: 5 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
#-----------------------------------------------------------------------------#

# - Raw link test
add_executable(g4vg_link_test G4VG.link.test.cc)
target_link_libraries(g4vg_link_test PRIVATE G4VG::g4vg)

# - Primary code test
file(TO_CMAKE_PATH "${PROJECT_SOURCE_DIR}" G4VG_SOURCE_DIR)
configure_file(g4vg_test_config.h.in g4vg_test_config.h @ONLY)

Expand Down
11 changes: 11 additions & 0 deletions test/G4VG.link.test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//----------------------------------*-C++-*----------------------------------//
// Copyright 2024 UT-Battelle, LLC, and other Celeritas developers.
// See the top-level COPYRIGHT file for details.
// SPDX-License-Identifier: (Apache-2.0 OR MIT)
//---------------------------------------------------------------------------//
//! \file G4VG.link.test.cc
//---------------------------------------------------------------------------//

#include "G4VG.hh"

int main() {}

0 comments on commit f372cdd

Please sign in to comment.