NESTCollaboration / nest

Noble Element Simulation Technique is used to simulate noble-element energy deposition microphysics.
http://nest.physics.ucdavis.edu
Other
23 stars 42 forks source link

Inconsistency in CMake Configuration Instructions #174

Closed gerritzen closed 1 year ago

gerritzen commented 1 year ago

Hello,

I am trying to include NEST in another CMake project, specifically the LXe example in Geant4.

The current instructions in the NEST project documentation suggest using the -DNEST_DIR variable when configuring with CMake to locate the NEST installation directory:

cmake .. -DNEST_DIR=[path to NEST install directory]

However, the NESTConfig.cmake file is located in a subdirectory of the installation directory: [path to NEST install directory]/lib64/cmake/NEST. As a result, cmake does not find the NESTConfig.cmake.

Unless I was doing something fundamentally wrong, I see two solutions to the problem:

  1. Update the instructions to specify the correct path including the lib64/cmake/NEST subdirectory:
cmake .. -DNEST_DIR=[path to NEST install directory]/lib64/cmake/NEST
  1. Suggest users to set the CMAKE_PREFIX_PATH variable instead, which will allow CMake to look for the configuration file in the correct subdirectory automatically:
cmake .. -DCMAKE_PREFIX_PATH=[path to NEST install directory]

The second option would also allow to find gcem, but I am not sure if this is intended.

My setup:

Thank you for your consideration.

gerritzen commented 1 year ago

I encountered a separate issue with cmake. The file cmake/NESTConfig.cmake.in has the following line

 23 set(NEST_LIBRARIES @NEST_LIBRARY_TARGETS@)

which expands to

set(NEST_LIBRARIES NEST::Core NEST::NESTG4)

However, the auto-generated NESTTargets.cmake does not define a library NEST::Core, but only NEST::NESTCore. That also makes the check in NESTConfig.cmake.in:19 pointless.

A quick fix would be to change line 76 in CMakeLists.txt to:

list(APPEND NEST_LIBRARY_TARGETS "NEST::NESTCore")

and change line 19 in NESTConfig.cmake.in (if the check is even needed).

My proposal for a better solution would be to use the exported targets directly rather than the variable NEST_LIBRARIES. That would also make all the aliasing in the cmake files unnecessary and the libraries can just be referred to by their names. I made some changes in a fork I'll create a PR in a few minutes in case the second solution is preferred.

mszydagis commented 1 year ago

I believe this is now fixed? @gerritzen?