Address providing compilers to downstream CMake projects

bartlettroscoe commented 1 year ago

Description

Pull Request:

528

changed the standard TriBITS TPL CUDA to have the FindTPLCUDA.cmake file call find_package(CUDAToolkit) instead of find_package(CUDA) (to address the problem described in https://github.com/trilinos/Trilinos/issues/10954#issuecomment-1262363127). However, this results in not being able call find_package(<PkgName>) on TriBITS packages that depend on the TPL CUDA (or any of the downstream TPLs that depend on the CUDA TPL), before the compilers are define in a statement like project(<ProjectName> C CXX Fortran) or enable_langauge(<LANG>). The problem is the the TriBITS project-level and package-level package-config files also define compiler variables <prefix>_<LANG>_COMPILER that need to be read and set to CMAKE_<LANG>_COMPILER and then defining the compilers like shown in:

https://github.com/TriBITSPub/TriBITS/blob/559f1ecc0ef2bc1bb46c9a434f634f23e5c0cfbf/tribits/examples/TribitsSimpleExampleApp/CMakeLists.txt#L10-L28

When CUDA is enabled in the TriBITS project (e.g. Trilinos), the customers CMake configure now breaks with an error message like:

CMake Error at <cmake-install-dir>/cmake/3.23.2/x86_64/share/cmake-3.23/Modules/FindThreads.cmake:66 (message):
  FindThreads only works if either C or CXX language is enabled
Call Stack (most recent call first):
  <cmake-install-dir>/cmake/3.23.2/x86_64/share/cmake-3.23/Modules/FindCUDAToolkit.cmake:910 (find_package)
  <cmake-install-dir>/cmake/3.23.2/x86_64/share/cmake-3.23/Modules/CMakeFindDependencyMacro.cmake:47 (find_package)
  <trilinos-install-dir>/lib/external_packages/CUDA/CUDAConfig.cmake:2 (find_dependency)
  <trilinos-install-dir>/lib/cmake/KokkosCore/KokkosCoreConfig.cmake:156 (include)
  <trilinos-install-dir>/lib/cmake/Kokkos/KokkosConfig.cmake:155 (include)
  <trilinos-install-dir>/lib/cmake/Trilinos/TrilinosConfig.cmake:123 (include)

The old FindCUDA.cmake module did not have the requirement that the C or CXX language had to be enabled before calling find_package(CUDA).

Therefore, this is a break in backward compatibility of TriBITS, and therefore also Trilinos through PR:

https://github.com/trilinos/Trilinos/pull/11093

In addition, when switching to GNUInstallDirs.cmake, the find_package() command fails to search lib64 until the compilers are defined (creating a chicken and the egg problem, see https://gitlab.kitware.com/cmake/cmake/-/issues/25157#note_1418009).

Proposed Solution 1

The simplest solution is to have customer CMake projects stop getting the compiler variables <prefix>_<LANG>_COMPILER from the upstream installed TriBITS package or project and change the order of the enabling of the compilers and the calling of find_package(<TribitsPackageOrProject>). Therefore, rewrite the above example as:

project(<ProjectName> ... LANGUAGES C CXX Fortran )

find_package(<TribitsProject> ...)

One downside of this is that the downstream customer CMake project will not even be able to query the upstream TriBITS project to determine what compilers were enabled before it was forced to enable its own compilers.

Proposed Solution 2

To keep the same ability to get the compilers used to build the upstream TriBITS project/package would be to split off a new package config file for each TriBITS project/package called something like <ProjectOrPackageName>EnvConfig.cmake that would set the variables:

<ProjectOrPackageName>_<LANG>_COMPILER
<ProjectOrPackageName>_<LANG>_COMPILER_FLAGS

but nothing else. Then the downstream customer app CMake project's CMakeLists.txt file could be updated to call:

find_package(<ProjectName>Env)

set(CMAKE_CXX_COMPILER ${<ProjectName>_CXX_COMPILER} )
set(CMAKE_C_COMPILER ${<ProjectName>_C_COMPILER} )
set(CMAKE_Fortran_COMPILER ${<ProjectName>_Fortran_COMPILER} )

set(CMAKE_CXX_FLAGS "${<ProjectName>_CXX_COMPILER_FLAGS} ${CMAKE_CXX_FLAGS}")
set(CMAKE_C_FLAGS "${<ProjectName>_C_COMPILER_FLAGS} ${CMAKE_C_FLAGS}")
set(CMAKE_Fortran_FLAGS "${<ProjectName>_Fortran_COMPILER_FLAGS} ${CMAKE_Fortran_FLAGS}")

# Enable the compilers now that we have gotten them from <ProjectName>ProjConfig.cmake
enable_language(C)
enable_language(CXX)
if (CMAKE_Fortran_COMPILER)
  enable_language(Fortran)
endif()

# Get the library targets from <ProjectName> (which will search <prefix>/lib64 and works with CUDA)
find_package(<ProjectName> ...)

In that way, find_package(CUDAToolkit) would only be called through the find_package(<ProjectName> ...) AFTER the compilers are enabled above that.

And we can avoid the problem with GNUInstallDirs.cmake installing package-config files under <prefix>/lib64/ and find_package() not searching that directory (because the compilers are not yet defined) by having the TriBITS project install the <ProjectName>Env.cmake file under <prefix>/<ProjectName>Env/ instead of under <prefix>/lib64/ so that find_package(<ProjectName>Env ...) will find it without having to define the compilers first.

bartlettroscoe commented 1 year ago

FYI: It looks like SPARC has gone with Proposed Solution 1 above which is to just put the call to project( ... C CXX ... ) before the call to `find_package(Trilinos) (see COMPSIMHD-14726)

But I will leave this issue open in case some other user does not like that solution and would still like to be able to get the compilers used to install the upstream TriBITS package or project.

bartlettroscoe commented 11 months ago

NOTE: There are still customers like Albany and Nalu that want to get compilers from Trilinos:

If there are more Trilinos customers like this, it might be better to do Proposed Solution 2 so they can continue to get compilers from the Trilinos installation.

bartlettroscoe commented 11 months ago

FYI, as discussed in:

https://github.com/sandialabs/Albany/issues/982#issuecomment-1734425748

it might be a good idea for TriBITS to write the directories used by GNUInstallDirs.cmake into the package config files. For example:

set(<PackageName>_CMAKE_INSTALL_BINDIR "...")
set(<PackageName>_CMAKE_INSTALL_LIBDIR "...")
...

set(<PackageName>_CMAKE_INSTALL_FULL_BINDIR "...")
set(<PackageName>_CMAKE_INSTALL_FULL_LIBDIR "...")
...

for all of the CMake vars supported by GNUInstallDirs.cmake.

sebrowne commented 11 months ago

Isn't that slightly circular? Meaning, wouldn't a customer already have to know which libdir is correct to find the package config files in the first place? Or rather, wouldn't CMake already know which one should be used if find_package() was able to find the package configs?

bartlettroscoe commented 11 months ago

Isn't that slightly circular? Meaning, wouldn't a customer already have to know which libdir is correct to find the package config files in the first place? Or rather, wouldn't CMake already know which one should be used if find_package() was able to find the package configs?

@sebrowne, it depends. If the base Trilinos install directory is <trilinosInstallDir> (which which <trilinosInstallDir>/lib or <trilinosInstallDir>/lib64 exists), then the downstream CMake project can find Trilinos (or individual Trilinos packages) by adding <trilinosInstallDir> to CMAKE_PREFIX_PATH and then calling find_pacakge(Trilinos ..) will find the file <trilinosInstallDir>/lib64/cmake/Trilinos/TrilinosConfig.cmake (if the compilers have already been defined as per the updated documentation here). The downstream CMake project does not need to know a-priori if the libraries (and the package config files) are <trilinosInstallDir>/lib or <trilinosInstallDir>/lib64, but they may want to know that after Trilinos is found. Does that make sense? (But in practice, the IMPORTED targets know the library directory so there is no need to know that explicitly.)

But the main motivation for adding variables as per above is to tell the downstream CMake project where the binaries are installed (see https://github.com/sandialabs/Albany/issues/982#issuecomment-1734443403) since it may not be under <trilinosInstallDir>/bin (as the person who configured Trilinos could have used a different directory for CMAKE_INSTALL_BINDIR). So if you are going to publish <PackageName>_CMAKE_INSTALL_BINDIR, you might as well publish all of those GNUInstallDirs.cmake directories for downstream customers.

TriBITSPub / TriBITS