Remove CMake < 3 CMP0042 workarounds #1040
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CMake only sets install names on darwin to `@rpath/<name>` (which is desired otherwise rpaths don't work at all) when CMP0042 is ON. That's the default when CMake 3.0 or higher is required. And lapack requires it already for years: as of v3.9.1 (8f004b3). So, delete the old workarounds that effectively set CMP0042 to ON. Further, delete the following three options that are redefinitions of builtin with values that are builtin defaults: - `CMAKE_MACOSX_RPATH` - `CMAKE_SKIP_BUILD_RPATH` - `CMAKE_BUILD_WITH_INSTALL_RPATH` Lastly, lapack automatically sets `CMAKE_INSTALL_RPATH_USE_LINK_PATH` to `ON` whenever installing to a non-system dir. The assumption is that whenever you install something to a non-system dir, you need rpaths to locate dependencies. But this is just an assumption which may or may not hold. The downside of it is that the option can be annoying when lapack is used as a sub-project as it affects a global CMake variable (for example OpenBLAS uses lapack as a sub-project). Instead, let users or packagers provide this on the command line if they really need it -- remove it from lapack as it's as helpful as it is harmful.
|
ping @langou / @ahnaf-tahmid-chowdhury (ref #1002) |
ahnaf-tahmid-chowdhury
approved these changes
Aug 11, 2024
langou
approved these changes
Aug 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
TL;DR: This deletes obsolete CMake code and improves upon #1002
by sticking to (sensible) CMake defaults, in accordance to the principle
of least astonishment ;)
CMake only sets install names on darwin to
@rpath/<name>(which isdesired otherwise rpaths don't work at all) when CMP0042 is ON. That's
the default when CMake 3.0 or higher is required. And lapack requires it
already for years: as of v3.9.1 (8f004b3).
So, delete the old workarounds that effectively set CMP0042 to ON.
Further, delete the following three options that are redefinitions of
builtins and have values that are default anyways:
CMAKE_MACOSX_RPATHCMAKE_SKIP_BUILD_RPATHCMAKE_BUILD_WITH_INSTALL_RPATHLastly, lapack automatically sets
CMAKE_INSTALL_RPATH_USE_LINK_PATHtoONwhenever installing to a non-system dir. The assumption is thatwhenever you install something to a non-system dir, you need rpaths to
locate dependencies. But this is just an assumption which may or may
not hold. The downside of it is that the option can be annoying when
lapack is used as a sub-project as it affects a global CMake variable
(for example OpenBLAS uses lapack as a sub-project). Instead, let users
or packagers provide this on the command line if they really need it --
remove it from lapack as it's as helpful as it is harmful.
Checklist