CMake: build either static or shared + don't force PIC + add ALIAS targets

[SpaceIm]

This PR tries to improve CMakeLists.txt:

• still allow static introduced in https://github.com/redis/hiredis/pull/851 (/cc @masariello), but don't build both. CMake has BUILD_SHARED_LIBS to control SHARED or STATIC, it's good practice to use this instead of building both flavors in the same build:
  • less tedious for downstream projects (static & shared have the same CMake imported target, it avoids to contaminate downstream build files with target name tests)
  • reduce build time, because most downstream don't want both shared & static
  • finally Makefile & CMakeLists.txt were not consistent
  • package managers: conan & vcpkg don't like CMakeLists building both shared & static.
• optional PIC for static libs (since https://github.com/redis/hiredis/pull/874 PIC was forced unconditionally), PIC is needed on Unix platform only if you link hiredis into a shared lib
• add ALIAS targets so that downstream projects can consume namespaced targets with find_package or add_subdirectory in a consistent way.
• link to OpenSSL::SSL imported target, it's far more robust. This imported target is available since CMake 3.4 in built-in FindOpenSSL.cmake module, and fortunatly it's the min CMake version in hiredis.

I did not remove it, but this file -commited in https://github.com/redis/hiredis/pull/851 - seems to be useless (some generated file by Visual Studio?): https://github.com/redis/hiredis/blob/master/hiredis.targets

[michael-grunder]

Thanks for the PR, the updated CMakeLists.txt is much cleaner.

I'm not very familiar with CMake, but is this change likely to cause pain for anyone if we were to release it in v1.0.1?

-ADD_LIBRARY(hiredis_static STATIC ${hiredis_sources})
+ADD_LIBRARY(hiredis::hiredis ALIAS hiredis

[SpaceIm]

Since commits in #851 don't land in any release for the moment, I guess that this change is harmless. It could eventually break people who were using master branch.

[michael-grunder]

Since commits in #851 don't land in any release for the moment

For some reason, I thought #851 was in v1.0.0.

I'll give it a quick test and then get it merged today.

Thanks again!

[SpaceIm]

I forgot: do be consistent with 1.0.0, BUILD_SHARED_LIBS could be turned to an option set to ON by default.

For some reason, I thought #851 was in v1.0.0.

I remember when I've packaged hiredis 1.0.0 for conan, and I'm pretty sure that there was only hardcoded shared libs (and I had to patch CMakeLists to allow static).

[masariello]

This file is used by cpack when building a nuget package.

https://github.com/redis/hiredis/blob/master/hiredis.targets

Will the shared and static packages have different IDs?

[clemensg]

Hi @SpaceIm , any chance of getting this rebased on current master? Thanks

[SpaceIm]

This file is used by cpack when building a nuget package.

https://github.com/redis/hiredis/blob/master/hiredis.targets

Will the shared and static packages have different IDs?

I don't know nuget, I guess they manage this detail themself.

[autoantwort]

Hi @SpaceIm , any chance of getting this rebased on current master? Thanks

I have created #1160 for that.

[chayim]

Closing based on Jan 28 comment since this is in.