Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

link to ecp components via cmake config targets #514

Merged
merged 1 commit into from
Jan 19, 2023
Merged

Conversation

adammoody
Copy link
Contributor

@adammoody adammoody commented Nov 10, 2022

This uses newly added cmake config files for specifying component dependencies.

As part of this PR, we'll also add a cmake config for SCR itself installed to share/scr/cmake/scrConfig.cmake, which will help with: #497. The goal is to enable an SCR application to compile and link by adding the following to the application CMakeLists.txt file:

FIND_PACKAGE(scr REQUIRED)
ADD_EXECUTABLE(testscr testscr.c)
TARGET_LINK_LIBRARIES(testscr scr::scr)

and then build with:

# include path to SCR install (and its dependencies)
# to pick up their cmake package config files
export CMAKE_PREFIX_PATH=/path/to/scr/install:/path/to/deps/install

cmake ...

For dependencies that are not cmake projects themselves, we can install cmake module files, like FindDTCMP.cmake. This will install a handful of Find*.cmake files along side the scrConfig.cmake file. We then need to add the module path to the user's CMAKE_MODULE_PATH in the scrConfig.cmake file, e.g.,:

https://stackoverflow.com/questions/59674070/should-i-modify-cmake-module-path-in-packageconfig-cmake

@adammoody adammoody added the WIP label Nov 10, 2022
@adammoody
Copy link
Contributor Author

@white238 , we're working to add a CMake config file for SCR. Once this is ready, it would be helpful if you can verify that it works.

I also have a question for you. Some of the libraries that SCR uses, like DTCMP, are not CMake projects.

Is there a standard practice for specifying those dependencies within our scrConfig.cmake file?

For example, could we do something like install our FindDTCMP.cmake script into our install/share/scr/cmake directory and then call find_dependency(DTCMP REQUIRED) from the scrConfig.cmake?

@adammoody adammoody removed the WIP label Jan 18, 2023
@adammoody
Copy link
Contributor Author

adammoody commented Jan 18, 2023

@mcfadden8 , here is the PR to add the scrConfig.cmake file. For now, I'm only bothering with the "non-dist" cmake setup.

https://github.com/LLNL/scr/blob/cmake-config/cmake/scrConfig.cmake.in

This seems to be working for a test application, but I don't know whether I'm following good practice. In particular, I'm not clear on the best way to specify dependencies that are not cmake packages, like DTCMP.

My current approach is to distribute (install) our module files like FindDTCMP.cmake

scr/CMakeLists.txt

Lines 101 to 107 in 8a922aa

# Install package config
INSTALL(FILES ${CMAKE_CURRENT_BINARY_DIR}/scrConfig.cmake ${CMAKE_CURRENT_BINARY_DIR}/scrConfigVersion.cmake DESTINATION share/scr/cmake)
INSTALL(FILES cmake/FindDTCMP.cmake DESTINATION share/scr/cmake)
INSTALL(FILES cmake/FindLWGRP.cmake DESTINATION share/scr/cmake)
INSTALL(FILES cmake/FindMySQL.cmake DESTINATION share/scr/cmake)
INSTALL(FILES cmake/FindPDSH.cmake DESTINATION share/scr/cmake)
INSTALL(FILES cmake/FindYOGRT.cmake DESTINATION share/scr/cmake)

and then invoke those from the scrConfig.cmake:

# The packages below do not have cmake package config files.
# Instead, we provide cmake find module files, like FindDTCMP.cmake.
# This way users who build with cmake don't have to write their own.
# The line below registers the current working directory with cmake
# so that it can find the Find*.cmake module files.
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")
find_dependency(DTCMP REQUIRED)
find_dependency(LWGRP REQUIRED)

However, I just realized that the installed scrTargets.cmake includes the full path to libdtcmp already. For example, in share/scr/cmake/scrTargets.cmake, I see a line like:

set_target_properties(scr::scr PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include"
  INTERFACE_LINK_LIBRARIES "axl::axl_mpi;er::er;shuffile::shuffile;redset::redset;spath::spath;rankstr::rankstr;kvtree::kvtree;/path/to/lib/libdtcmp.so;/path/to/lib/libmpi.so;/path/to/lib64/libyogrt.so;/usr/lib64/libz.so;-lpthread"
)

Note that the above line has the full path to libdtcmp.so.

So maybe I don't need to call find_dependency(DTCMP) from scrConfig.cmake at all?

@mcfadden8
Copy link
Collaborator

I think that leaving the find_dependency(DTCMP) (and others) could be helpful to others when/if those libraries go missing for whatever reason. I'm not sure why the absolute path for DTCMP is being included though. Do you know when that is happening?

@adammoody
Copy link
Contributor Author

I think that leaving the find_dependency(DTCMP) (and others) could be helpful to others when/if those libraries go missing for whatever reason.

Good point.

I'm not sure why the absolute path for DTCMP is being included though. Do you know when that is happening?

Not sure either. Perhaps coming through the sequence here:

FIND_LIBRARY(DTCMP_LIBRARIES
NAMES dtcmp
HINTS ${WITH_DTCMP_PREFIX}/lib
)

LIST(APPEND SCR_EXTERNAL_LIBS ${DTCMP_LIBRARIES})

TARGET_LINK_LIBRARIES(scr PUBLIC ${SCR_EXTERNAL_LIBS})

CMake always feels like smoke and mirrors to me.

@mcfadden8
Copy link
Collaborator

mcfadden8 commented Jan 18, 2023

Do we always need to set up TARGET_LINK_LIBRARIES by hand like this (ie. build a list of the libraries we have found and then append them? Doesn't cmake/Find_Library do this for you somehow? The absolute path being inserted in the installed scrTargets.cmake file. It is interesting that the full path for all of the other libraries does not seem to be placed there.

@mcfadden8
Copy link
Collaborator

mcfadden8 commented Jan 18, 2023

I think what we want to wind up with is to somehow have a dtcmp placed int he ";" separatrely list (or is that what ${SCR_EXTERNAL_LIBS} is providing?)

Then, we could have all of the find_dependency calls set things up for the building cmake application?

@adammoody
Copy link
Contributor Author

adammoody commented Jan 19, 2023

Do we always need to set up TARGET_LINK_LIBRARIES by hand like this (ie. build a list of the libraries we have found and then append them? Doesn't cmake/Find_Library do this for you somehow?

I think we probably need both. Find_library locates the library, but it doesn't tell cmake which targets actually depend the library. Then we spell out the dependencies of each target with target_link_libraries.

The absolute path being inserted in the installed scrTargets.cmake file. It is interesting that the full path for all of the other libraries does not seem to be placed there.

The ecp components are added with entries like er::er, which is the cmake target name. And there are full paths for other libraries at the end, like libmpi and libyogrt. And we have -lpthread at the end. I think these are all copied verbatim using the value we give when we call target_link_libraries.

Copy link
Collaborator

@mcfadden8 mcfadden8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the approach is reasonable. I approve.

Separately, I wonder if it would make sense to update the bootstrap script to check out the external libraries to an alternate location and use this functionality of cmake to perform the build of the scr library? It may offer another way of continually testing the packaging functionality.

@adammoody
Copy link
Contributor Author

Thanks, @mcfadden8 . I'll take a look at the bootstrap script. I'll also look at adding an scrConfig to the "dist" version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants