Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
At the moment, LAPACK sets the global compile flags via the
CMAKE_Fortran_FLAGS
cache variable. The problem with that is if someone wants to compile LAPACK as a sub-project inside a bigger CMake project, theCMAKE_Fortran_FLAGS
that LAPACK sets will automatically be applied to all targets of the parent project, which is definitely not ideal.There are two options to fix this:
CMAKE_Fortran_FLAGS
as a local variable, i.e. we set it without theCACHE
keyword. That way the flags will only be applied in the LAPACK directory, not changing the flags in the parent target.add_compile_options(...)
andadd_link_options(...)
. The latter needs CMake >= 3.13Since we are already using
target_compile_options(...)
for some libraries, I think we should use the second option and not change any variables directly.#994 already pointed out that we currently need at least CMake 3.12 anyway because we are linking object libraries. So, the jump to CMake 3.13 wouldn't be completely unreasonable. The issue also mentioned the use of
Fortran_PREPROCESS
, which is actually a CMake 3.18 feature. We can still use CMake < 3.18, because non-existing target properties are just ignored. The only place where it is necessary is ifBUILD_INDEX64_EXT_API
is enabled because we need to preprocess all Fortran files in that case.This MR changes all occurrences of
CMAKE_Fortran_FLAGS
andCMAKE_*_LINKER_FLAGS
to use the functions instead. I compared the compiler and linker flags of the targets before and after the changes. The compiler flags are identical (possibly in a different order). The linker flags are fewer than before becauseCMAKE_Fortran_FLAGS
was added to the compiler and linker flags, which isn't the case anymore. For most flags, this is ok because they are not needed in the linking phase. Those that are needed during compilation and linking are added explicitly viaadd_link_options(...)
(I hope I didn't miss any; please check).I bumped the CMake minimum version to 3.13 and 3.18 if
BUILD_INDEX64_EXT_API
is enabled. If the user uses a CMake < 3.18,BUILD_INDEX64_EXT_API
is now disabled by default so that the default config works without changing any variables.Additionally, I fixed the formatting of the CMake file.
Fixes #994
Checklist