-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide CMake option for static library #161
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for bringing this PR up. However, I think that it requires some more changes such that it reliably works in practice.
@@ -1,6 +1,6 @@ | |||
include(GNUInstallDirs) | |||
|
|||
add_library(mtkahypar SHARED libmtkahypar.cpp) | |||
add_library(mtkahypar libmtkahypar.cpp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think here is the place where you want to use the cmake option BUILD_SHARED_LIBS
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The BUILD_SHARED_LIBS
options applies to all add_library
instances, so in this case it should be a question of preference and I thought that it would be better to go with all the other option definitions.
Should I still move it to the subdir config?
lib/CMakeLists.txt
Outdated
@@ -1,6 +1,6 @@ | |||
include(GNUInstallDirs) | |||
|
|||
add_library(mtkahypar SHARED libmtkahypar.cpp) | |||
add_library(mtkahypar libmtkahypar.cpp) | |||
target_link_libraries(mtkahypar ${Boost_LIBRARIES} TBB::tbb TBB::tbbmalloc_proxy) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you build mt-kahypar as a static library. TBB must be also compiled as a static library:
https://stackoverflow.com/questions/638278/how-to-statically-link-to-tbb
Otherwise, this will not work.
# Building shared libs uses malloc_proxy, static uses malloc. | ||
if(${BUILD_SHARED_LIBS}) | ||
target_link_libraries(mtkahypar TBB::tbbmalloc_proxy) | ||
else() | ||
target_link_libraries(mtkahypar TBB::tbbmalloc) | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know much about TBB, is the assumption correct to use tbbmalloc
instead of tbbmalloc_proxy
for a static version? TBB only produces this one on a static build.
As mentioned in #159, a static build compiles, but does not actually work right now. It behaves differently from a dynamic build at run time. As my knowledge of this framework is very limited, I am not sure why.
This seems to originate from the |
Tagging @SebastianSchlag who wrote the factories. Is there a way to get the factories / registries to work with a static build? In the current version with hash-maps it is probably difficult to get static initialization / registration. |
That's interesting. I guess something along the lines of [1] or, to be more precise, in the blogpost [2] that's referenced there should work. I haven't found the time to look into this in detail though yet. [1] https://stackoverflow.com/questions/59779679/c-library-self-registering-classes-factory-map-empty-in-client-application |
This provides the
BUILD_SHARED_LIBS
option (by defaultON
) so thatlibmtkahypar
can be built statically by setting it toOFF
.