Exclude the mimalloc files from the Binaryen install #7386
Conversation
Since mimalloc is linked statically into the Binaryen tools, none of its files need to be installed with Binaryen.
|
|
||
| if(MIMALLOC_STATIC) | ||
| if(NOT(LINUX AND BUILD_STATIC_LIB) OR EMSCRIPTEN) | ||
| if(NOT(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND BUILD_STATIC_LIB) OR EMSCRIPTEN) |
There was a problem hiding this comment.
When using the emscripten-releases build script I got a very strange behavior; namely that the LINUX variable was empty, but CMAKE_SYSTEM_NAME was set to Linux as expected. I couldn't figure out what was causing it. This change works around that.
There was a problem hiding this comment.
OK, I figure it out; the LINUX variable was introduced in CMake 3.25 but we support 3.16 and use 3.21 on our bots. This is the right way to do it on those versions, so I'll just update the commit message.
| add_compile_definitions(MI_DEBUG=0) | ||
|
|
||
| add_subdirectory(mimalloc) | ||
| add_subdirectory(mimalloc EXCLUDE_FROM_ALL) |
There was a problem hiding this comment.
From briefly looking up EXCLUDE_FROM_ALL, it seems that it means that this directory won't be built by default? Is that what we want, or is there a more specific way to exclude it from being installed?
There was a problem hiding this comment.
Right, I think what's happening here is that the target_link_libraries(binaryen mimalloc-static) causes a dependence from libbinaryen on libmimalloc-static which causes it to be built when libbinaryen does. And then the tools link it in, but it doesn't get installed, which is exactly what we want.
There was a problem hiding this comment.
I couldn't really find another way to prevent the installation (see also https://discourse.cmake.org/t/hot-to-make-add-subdirectory-prevent-install-but-still-build-as-part-of-all/8698)
tlively
left a comment
tlively
left a comment
There was a problem hiding this comment.
Cool, makes (enough) sense to me.
Since mimalloc is linked statically into the Binaryen tools, none of its
files need to be installed with Binaryen.
Also use CMAKE_SYSTEM_NAME instead of LINUX, as the latter was introduced in CMake 3.25