search

TriBITSPub / TriBITS

TriBITS: Tribal Build, Integrate, and Test System,
http://tribits.org
Other
36 stars 46 forks source link

Address some issues for initial implementation for treating internal packages as external (#63) #562

Open bartlettroscoe opened 1 year ago

bartlettroscoe commented 1 year ago

Parent Issue:

Description

While the basic implementation for treating internally defined packages as external package in PR #560 is is complete (part of #63), it could use some more checking and smooth out some use cases. The following should be done following the merge of #560 when we have some time:

bartlettroscoe commented 1 year ago

CC: @ibaned, @crtrott, @jwillenbring, @rppawlo

@dalg24 and @masterleinad, following up from our meeting on Wednesday, I think there is a way to address special TPLs upstream from Kokkos like CUDA where you don't want to have to provide upstream <UpstreamPkg>::all_libs targets like CUDA::all_libs targets or TriBITS-compliant external package config files like CUDAConfig.cmake. As mentioned above, the idea is:

The idea is that when TriBITS calls find_package(Kokkos) when processing Kokkos as a TriBITS-compliant external package (which sets Kokkos_ENABLE_CUDA=ON) and KokkosConfig.cmake does not provide CUDA::all_libs target, then TriBITS will go ahead and process the TPL (e.g. CUDA) as usual TriBITS TPL that loads the file:

which is just two lines:

find_package(CUDAToolkit REQUIRED)  # Will abort if not found!
tribits_extpkg_create_imported_all_libs_target_and_config_file( CUDA
  INNER_FIND_PACKAGE_NAME  CUDAToolkit
  IMPORTED_TARGETS_FOR_ALL_LIBS  CUDA::cufft  CUDA::cublas  CUDA::cudart )

So if KokkosConfig.cmake called find_package(CUDAToolkit), then find_package(CUDAToolkit REQUIRED) would hopefully process the same FindCUDAToolkit.cmake find module, producing the same targets (actually skipping those because they are already defined) and the command tribits_extpkg_create_imported_all_libs_target_and_config_file() would create the CUDA::all_libs target and generate the CUDAConfig.cmake file as needed by downstream TriBITS packages that depend on CUDA.

I think that solves the problem you expressed (with the disadvantage that a different TPL could be found by Trilinos that what is found by Kokkos).

The constraint is that if a TriBITS-compliant external package does provide the <UpstreamPkg>::all_libs target from one of its upstream TPLs (known by the downstream TriBITS project), then it must provide them for all of that TPLs downstream TPLs as well. For example, if KokkosKernelsConfig.cmake provided CUBLAS::all_libs but did not provide CUSPARSE::all_libs, then that would be an error (because CUSPARSE --> CUBLAS --> CUDA as shown by FindTPLCUBLASDependencies.cmake and FindTPLCUBLASDependencies.cmake).

Does that make sense?

So we would be back in the situation that it may be possible for the configuration of Trilinos to find different TPLs that what Kokkos found when it was configured. That is not great but those are the breaks for packages that are not fully TriBITS compliant. (Fully TriBITS-compliant external packages that provide <UpstreamPkg>::all_libs targets and <UpstreamPkg>Config.cmake files for all of their TriBITS-known upstream TPLs would result in never having the same TPL searched for twice.)

This makes TriBITS more complex but it is what it is.

bartlettroscoe commented 1 year ago

CC: @ibaned, @crtrott, @jwillenbring, @rppawlo

@dalg24 and @masterleinad,

The other option is to treat Kokkos as an non-TriBITS-compliant external package with imported targets as described in:

This would allow the creation of a FindTPLKokkos.cmake file to map from whatever KokkosConfig.cmake is providing to what is needed by downstream TriBITS Trilinos packages. And we would need to write a wrapper CMakeLists.txt file under Trilinos/packages/kokkos that did similar mappings. So if you turn off all of the Kokkos tests, that could make it so that the native Kokkos raw CMake build system would not have to be TriBITS compliant at all (other than using modern CMake targets for everything).

Support for this type package that that is listed both as a TriBITS external package/TPL (in the TPLsList.cmake file) and as a TriBITS internal package (in the PackagesLists.cmake file) is not there yet but it may not be that hard to add such support. That would give the maximum flexibility for how existing CMake-based software can be pulled into a TriBITS project (either as external or internal packages or both with the decision made at runtime).

jwillenbring commented 1 year ago

@bartlettroscoe For the first option, in the case where everything was being built within Spack, wouldn't it be the case that the Spack dependencies could be used to help make sure we are using consistent versions of TPLs? This obviously does not fully solve the problem, but it would mitigate the impact of the possibility of finding different versions of TPLs if that was not possible for the case where we are building with Spack.

bartlettroscoe commented 1 year ago

CC: @ibaned, @crtrott, @jwillenbring, @rppawlo, @dalg24, @masterleinad

@bartlettroscoe For the first option, in the case where everything was being built within Spack, wouldn't it be the case that the Spack dependencies could be used to help make sure we are using consistent versions of TPLs?

@jwillenbring, for dependencies upstream from Kokkos that are found with find_package(<UpstreamDep> ...), then an overlord like Spack should make sure that the same dependency if found by find_package(<UpstreamDep> ...) whether called by Kokkos or any downstream CMake project. But this can really only be guaranteed as long as only one version/configuration of <UpstreamDep> is is findable. If more than one version/configuration of <UpstreamDep> is findable, then this is not guaranteed, even with the setting of the <UpstreamDep>_ROOT environment or cache variable because of case sensitivity as described in:

And note the fundamental problem with finding different external dependencies with multiple calls to find_package() in even the same CMake project described in:

This problem has no answer with raw CMake other than to tell users to make sure that does not happen (and modify the calls to find_package() in some cases).

This new TriBITS implementation and the way it handles external dependencies should fully resolve that problem by making sure that an external dependency is ever only found once, no matter how many individual CMake projects are stringed together and may have the same dependency with no need to add anything to the environment in downstream CMake projects. And by providing these more robust <Package>Config.cmake files, systems like Spack benefit from that as well and be more robust going forward. (There is a win-win relationship here between TriBITS and Spack.)