Add CMake option to choose building SHARED vs STATIC about mimalloc HOT 13 CLOSED

Thanks -- yes, I'll close :-)

from mimalloc.

This is also important for gnzlbg/mimallocator#1 which uses cmake on all platforms, as it is an automated step.

from mimalloc.

Thanks for the feedback. For windows, the main build I envision was VS2017, not cmake, but I understand it is important for automated build tools or working with mingw for example.

I like building all variants as it makes those all available at install -- the only place where it breaks seems to be on windows. My feeling is to just rename the target name for the static library to include "-static" in the name; like libmimalloc-static-debug.a (or .lib). That would seem to fix the current issues and wouldn't need all kinds of platform depenedent defines as is now the case in #22.

from mimalloc.

Hi! The CMake build script also makes it easy to add mimalloc into vcpkg: microsoft/vcpkg#7011. In fact, in this PR, I patched these lines to choose between static and shared build (For vcpkg, there is no need to build both static and shared libs in one run):

diff --git a/CMakeLists.txt b/CMakeLists.txt
index f6968ed..1b07cbf 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -108,13 +108,16 @@ endif()
 
 
 # shared library
+if(BUILD_SHARED_LIBS)
 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_options(mimalloc PRIVATE ${mi_cflags})
 target_include_directories(mimalloc PRIVATE include PUBLIC $<INSTALL_INTERFACE:${mi_install_dir}/include>)
 target_link_libraries(mimalloc PUBLIC ${mi_libraries})
+endif()
 
+if(NOT BUILD_SHARED_LIBS)
 # static library
 add_library(mimalloc-static STATIC ${mi_sources})
 set_target_properties(mimalloc-static PROPERTIES OUTPUT_NAME ${mi_basename})
@@ -122,22 +125,20 @@ target_compile_definitions(mimalloc-static PRIVATE ${mi_defines} MI_STATIC_LIB)
 target_compile_options(mimalloc-static PRIVATE ${mi_cflags})
 target_include_directories(mimalloc-static PRIVATE include PUBLIC $<INSTALL_INTERFACE:${mi_install_dir}/include>)
 target_link_libraries(mimalloc-static PUBLIC ${mi_libraries})
+endif()
 
+if(BUILD_SHARED_LIBS)
 # install static and shared library, and the include files
-install(TARGETS mimalloc EXPORT mimalloc DESTINATION ${mi_install_dir} LIBRARY NAMELINK_SKIP)
-install(TARGETS mimalloc-static EXPORT mimalloc DESTINATION ${mi_install_dir})
+install(TARGETS mimalloc EXPORT mimalloc ARCHIVE DESTINATION lib RUNTIME DESTINATION bin LIBRARY DESTINATION lib NAMELINK_SKIP)
+else()
+install(TARGETS mimalloc-static EXPORT mimalloc DESTINATION lib)
+endif()
 install(FILES include/mimalloc.h DESTINATION ${mi_install_dir}/include)
 install(FILES cmake/mimalloc-config.cmake DESTINATION ${mi_install_dir}/cmake)
 install(FILES cmake/mimalloc-config-version.cmake DESTINATION ${mi_install_dir}/cmake)
 install(EXPORT mimalloc DESTINATION ${mi_install_dir}/cmake)
-install(FILES "$<TARGET_FILE:mimalloc>" DESTINATION lib)  # duplicate the .so in the lib directory (unversioned)
-
 # single object file for more predictable static overriding
 add_library(mimalloc-obj OBJECT src/static.c)
-target_compile_definitions(mimalloc-obj PRIVATE ${mi_defines} MI_MALLOC_OVERRIDE)
+target_compile_definitions(mimalloc-obj PRIVATE ${mi_defines})
 target_compile_options(mimalloc-obj PRIVATE ${mi_cflags})
 target_include_directories(mimalloc-obj PRIVATE include PUBLIC $<INSTALL_INTERFACE:include>)
-
-install(FILES ${CMAKE_CURRENT_BINARY_DIR}/CMakeFiles/mimalloc-obj.dir/src/static.c${CMAKE_C_OUTPUT_EXTENSION}
-        DESTINATION ${mi_install_dir}
-        RENAME ${mi_basename}${CMAKE_C_OUTPUT_EXTENSION} )

If we don't want to build both static and shared at the same time, then an extra BUILD_SHARED_LIBS is enough.

option(BUILD_SHARED_LIBS "Build shared library" ON)

if(BUILD_SHARED_LIBS)
else()
    # Build static library
endif()

If we want to build both static and shared at the same time, I think it will still be helpful to add some options to control over the building of static and shared libs:

option(BUILD_SHARED_LIBS "Build shared library" ON)
option(BUILD_STATIC_LIBS "Build static library" ON)

if(BUILD_SHARED_LIBS)
endif()
if(BUILD_STATIC_LIBS)
endif()

from mimalloc.

Thanks very much for the fast response @daanx . Obviously, we don't want to dictate, only make suggestions with supporting perspectives.

Indeed, if some sort of SHARED/STATIC options toggles aren't made available in the upstream project, we'll probably end up patching in a similar same way for our use case. It won't be the first project which doesn't let consumers choose the build type in the build system, we've patched others as well. I would say it's fairly unusual and is likely to cause confusion and ambiguity for other consumers. Obviously, everyone on this thread prefers a single build system to think about, especially when it comes to automation and integrating this into other projects and the wider OSS ecosystem as a dependency.

If you've never done "Open CMake Project" in Visual Studio, perhaps it's worth giving a shot. CMake has been a first-class project type for visual studio for over two years now, and the Visual Studio team is putting the vast majority of related resources into promoting and supporting creating and opening CMake Projects rather than Solution-based projects. Obviously, Solution based projects and MSBuild arent going anywhere, countless many companies are deeply entrenched in this project infrastructure. However, for new open-source projects with no dependencies, there are very few reasons to maintain the visual studio project as a separate entity.

Finally, CLion is on track to likely becoming one of the most popular cross-platform IDEs for C++ and it only supports CMake as it's native C++ project type for all platforms (including windows). It doesn't support the Solution/Project system.

from mimalloc.

Hi all, I just merged #22 but I do see the argument that it is a bit of an anti pattern as it will build the static variants without overriding... but for automated builds it is clearly annoying to get errors.

I am all for supporting the best common practice with regard to cmake on Windows/in Visual Studio.
If that means wrapping in BUILD_SHARED_LIBS and BUILD_STATIC_LIBS that is ok -- as long as by default we build all of them as that seems quite convenient to me.

from mimalloc.

A funny coincidence, but this popular C++ library project just tweeted about this topic today:

https://twitter.com/obiltschnig/status/1143082996884525058
"About 90 % of all the effort in a @pocoproject release goes into maintaining and fixing Visual Studio project files. That's why we'll say goodbye to them with next release and use CMake instead."

from mimalloc.

Excellent, thanks for being open to suggestion. I think @myd7349 's patches here in comments would make a suitable PR for our use case as well. Would you be willing to submit as a PR @myd7349 ?

from mimalloc.

Hi, all! A new PR is submitted.

from mimalloc.

What is the problem with providing both targets? I think it is really good! To avoid unnecessary compiling one can use add_subdirectory(path/to/dir EXCLUDE_FROM_ALL) Why do we need unnecessary additional switches?

from mimalloc.

I have done something similar with this PR zlib-ng/zlib-ng#380.

I have made a change so that if BUILD_SHARED_LIBS is not set it will output both SHARED and STATIC. If BUILD_SHARED_LIBS explicitly set to ON, then only SHARED library will be built. If BUILD_SHARED_LIBS explicitly set to OFF, then only STATIC library will be built.

from mimalloc.

@myd7349 I don't see a current PR with the new options. Am I missing something?

from mimalloc.

It seems like these options have been added, should this issue be closed now?

from mimalloc.