Fix CMake configuration on Windows #22
Conversation
myd7349
commented
Jun 23, 2019
myd7349
commented
- Do not define MI_MALLOC_OVERRIDE when built as a static library
- Use different output names for shared lib and static lib
CMakeLists.txt
Outdated
| @@ -111,14 +110,28 @@ endif() | |||
| add_library(mimalloc SHARED ${mi_sources}) | |||
| set_target_properties(mimalloc PROPERTIES VERSION ${mi_version} NO_SONAME "YES" OUTPUT_NAME ${mi_basename} ) | |||
| target_compile_definitions(mimalloc PRIVATE ${mi_defines} MI_SHARED_LIB MI_SHARED_LIB_EXPORT) | |||
| target_compile_definitions(mimalloc PRIVATE $<$<BOOL:${OVERRIDE}>:MI_MALLOC_OVERRIDE>) | |||
There was a problem hiding this comment.
It might be easier to read for non-CMake experts (such as myself) if there was an if(OVERRIDE MATCHES "ON")...endif guard around a list(APPEND mi_defines MI_MALLOC_OVERRIDE here...
There was a problem hiding this comment.
Hey! @dscho Thanks for your kindly review. I have updated the PR. :-)
CMakeLists.txt
Outdated
| if(NOT WIN32) | ||
| # It is only possible to override malloc on Windows when building as a DLL. | ||
| # (src/alloc-override.c) | ||
| target_compile_definitions(mimalloc-static PRIVATE $<$<BOOL:${OVERRIDE}>:MI_MALLOC_OVERRIDE>) |
There was a problem hiding this comment.
Here, too, it might make sense to imitate the if(OVERRIDE MATCHES "ON") block that was edited, and why not just move that into the else() clause a couple lines above?
CMakeLists.txt
Outdated
| if(NOT WIN32) | ||
| # It is only possible to override malloc on Windows when building as a DLL. | ||
| # (src/alloc-override.c) | ||
| target_compile_definitions(mimalloc-obj PRIVATE $<$<BOOL:${OVERRIDE}>:MI_MALLOC_OVERRIDE>) |
There was a problem hiding this comment.
Here, too, I think it would make sense to just insert this block instead:
if(OVERRIDE MATCHES "ON")
list(APPEND mi_defines MI_MALLOC_OVERRIDE)
endif()It is easier to read, as it imitates the rather idiomatic (and simple) CMake style used in the rest of the file. This is particularly important when one keeps in mind that most developers do not really want to become CMake experts.
- Do not define MI_MALLOC_OVERRIDE when built as a static library - Use different output names for shared lib and static lib
myd7349
force-pushed
the
win32-cmake-patch
branch
from
fb3986e
to
b7c8d8f
Compare
|
Thanks so much! |
