Cmake static or shared #1160
Cmake static or shared #1160
Conversation
CMake imported targets are more robust
# Conflicts: # CMakeLists.txt
|
This looks really good! |
|
@prince-chrismc Can you approve the ci again? :) |
|
I don't have permission i just would benefit from this PR ❤️ |
Co-authored-by: Bjorn Svensson <bjorn.a.svensson@est.tech>
|
@autoantwort @chayim Assuming there are no objections, I'll probably squash and merge this PR since there are so many commits. |
| @@ -45,50 +43,25 @@ IF(WIN32) | |||
| ADD_DEFINITIONS(-D_CRT_SECURE_NO_WARNINGS -DWIN32_LEAN_AND_MEAN) | |||
| ENDIF() | |||
|
|
|||
| ADD_LIBRARY(hiredis_static STATIC ${hiredis_sources}) | |||
| ADD_LIBRARY(hiredis::hiredis_static ALIAS hiredis_static) | |||
There was a problem hiding this comment.
After you have removed this you have broke the possibility of having both shared and static libraries installed and exported to hiredis-targets.cmake. Existing projects which specify hiredis::hiredis_static in TARGET_LINK_LIBRARIES would be broken and if the target will be changed to hiredis::hiredis they will be linked to shared library.
There was a problem hiding this comment.
Yeah that is true. I could add the following (behind an option)
set_target_properties(hiredis PROPERTIES EXPORT_NAME hiredis_static)then you have your hiredis::hiredis_static target back.
There was a problem hiding this comment.
JFYI, there is a brilliant article for libraries' authors about exporting shared/static configurations in CMake.
https://alexreinking.com/blog/building-a-dual-shared-and-static-library-with-cmake.html
There was a problem hiding this comment.
No rush @autoantwort but if you add the final change to allow building both I'll get this merged.
There was a problem hiding this comment.
https://alexreinking.com/blog/building-a-dual-shared-and-static-library-with-cmake.html
Nice read.
@michael-grunder Done. Thank you for the ping, forgot it.
|
Merged, thanks! |
Removed patch which is included in this release. Changelog: https://github.com/redis/hiredis/blob/master/CHANGELOG.md This release includes improvements for static-only builds: redis/hiredis#1160 Signed-off-by: Bernd Kuhls <bernd@kuhls.net> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Removed patch which is included in this release. Changelog: https://github.com/redis/hiredis/blob/master/CHANGELOG.md This release includes improvements for static-only builds: redis/hiredis#1160 Signed-off-by: Bernd Kuhls <bernd@kuhls.net> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Same as #951 but merged with master