Add CMAKE install target #988
Conversation
Add CMAKE install target (copied from a patch provided by PrimarchOfTheSpaceWolves in #957)
| else() | ||
| set(CMAKE_BUILD_TYPE Release) | ||
| set(CMAKE_BUILD_TYPE Release) | ||
| endif() |
There was a problem hiding this comment.
CMAKE_BUILD_TYPE should control debug/release builds, not the other way around, by the way this needs to be set in the cache before the project command if not already defined
There was a problem hiding this comment.
That is xianyi's code from 7ac7e14, the patch only changed the indentation here.
I take it this evaluation of BUILD_DEBUG should be removed completely ?
There was a problem hiding this comment.
More to the point, should this block become something like
if( NOT CMAKE_BUILD_TYPE )
set( CMAKE_BUILD_TYPE Release)
endif()
if (CMAKE_BUILD_TYPE = Debug)
set (DEBUG_POSTFIX "_d")
set(OpenBLAS_LIBNAME "${OpenBLAS_LIBNAME}_d")
else()
set (DEBUG_POSTFIX "")
endif()
which would also consolidate the definitions of DEBUG_POSTFIX as in your other comment ?
There was a problem hiding this comment.
I have reworked the PR now to query BUILD_DEBUG only if CMAKE_BUILD_TYPE is not set, and moved the conditional appending of a debug prefix to use the cmake-provided setting.
(Apologies again for the mess of incremental commits to the same file, I was working via the web interface and do not see a means of squashing the commit history there)
| set(DEBUG_POSTFIX "_d") | ||
| endif() | ||
| install (TARGETS ${OpenBLAS_LIBNAME}_static${DEBUG_POSTFIX} DESTINATION lib) | ||
| endif() |
There was a problem hiding this comment.
DEBUG_POSTFIX is already defined earlier, the install could be moved there to avoid duplicate code
Use BUILD_DEBUG option only if CMAKE_BUILD_TYPE is not set Consolidate debug postfixes in install target
|
I have reworked the PR now to query BUILD_DEBUG only if CMAKE_BUILD_TYPE is not set, and moved the conditional appending of a debug prefix to use the cmake-provided setting. |
Add CMAKE install target (copied from a patch provided by PrimarchOfTheSpaceWolves in #957)