oliora/ppconsul

Add option to build dynamic libs in CMake scripts (this will also remove ugly json11.a dependency)

Closed this issue · 6 comments

ik-wg commented
Add option to build dynamic libs in CMake scripts (this will also remove ugly json11.a dependency)
jgornowich commented

Can't the "STATIC" be taken away from the src/CMakeLists.txt and ext/CMakeLists.txt and then use the global cmake flag BUILD_SHARED_LIBS=ON to support this feature?

I can make a PR if this is the desired way to fix it.

oliora commented

@jgornowich I think a dedicated CMake option is needed since it affects building of Ppconsul in several ways.

  1. json11 should be built as static library always
  2. If Ppconsul is built as shared library (my preferable default option) then json11 should not be installed.
  3. If Ppconsul is built as static library then both Ppconsul and jso11 libraries need to be installed.

About the last option. I'm not sure that that Ppconsul should be installed as static library at all. Static libraries (with rare exclusion) should never leave the build phase. Only shared libraries should be installed since they have version and dependency information embedded thus can be consumed in kinda convenient way. Static libraries are kinda too fragile. I'd like to know more about @ik-wg scenario of using static Ppconsul/json11 libs.

oliora commented

I'll implement this some time soon.

jgornowich commented

@oliora I agree with your points. My use of the ppconsul library was always to build shared libraries. #14 caused my use of including this library to no longer work because it forces static libraries. Is there a work around in the meantime? I can try to take a shot at an update if you'd like.

oliora commented

Now Ppconsul is build as shared library by default. To build it as static library one need to run CMake with -DBUILD_STATIC_LIB=ON option.

  • 👍1
jgornowich commented

Thx! Looks like it builds and tests for me fine now.