You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2021/05/13 22:30:43 UTC

[GitHub] [qpid-proton] jiridanek opened a new pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

jiridanek opened a new pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312


   I did not try on 2.8.12 yet, but I checked I am not using anything that is not available. That generator expression had to go because it confused generating the ProtonTargets.cmake file. I did not check what happens if I have debug proton and try to build release dispatch (or vice versa). Anyways,
   
   ```
   setting -DProton_USE_STATIC_LIBS when compiling dispatch gives me
   
   % ldd router/qdrouterd
           linux-vdso.so.1 (0x00007fbec46b3000)
           libqpid-dispatch.so => /home/jdanek/repos/qpid/qpid-dispatch/build_with_exports/src/libqpid-dispatch.so (0x00007fbec4592000)
           libssl.so.1.1 => /nix/store/g9r6l6mymsknrg0mc394iiggih16jkzm-openssl-1.1.1i/lib/libssl.so.1.1 (0x00007fbec44fc000)
           libcrypto.so.1.1 => /nix/store/g9r6l6mymsknrg0mc394iiggih16jkzm-openssl-1.1.1i/lib/libcrypto.so.1.1 (0x00007fbec420e000)
           libsasl2.so.3 => /nix/store/i0incb1cjxqyz4lc9wi0kjwv2qmwxqdw-cyrus-sasl-2.1.27/lib/libsasl2.so.3 (0x00007fbec41ef000)
           libpthread.so.0 => /nix/store/q53f5birhik4dxg3q3r2g5f324n7r5mc-glibc-2.31-74/lib/libpthread.so.0 (0x00007fbec41cc000)
           librt.so.1 => /nix/store/q53f5birhik4dxg3q3r2g5f324n7r5mc-glibc-2.31-74/lib/librt.so.1 (0x00007fbec41c2000)
           libdl.so.2 => /nix/store/q53f5birhik4dxg3q3r2g5f324n7r5mc-glibc-2.31-74/lib/libdl.so.2 (0x00007fbec41bd000)
           libpython3.7m.so.1.0 => /nix/store/33abxyajzdaggfbcrxzzpslc4582r84l-python3-3.7.9/lib/libpython3.7m.so.1.0 (0x00007fbec3e5d000)
           libwebsockets.so.15 => /nix/store/yf2b40nmyhyyhlzjjniwg9a6g4w99k4s-libwebsockets-3.2.2/lib/libwebsockets.so.15 (0x00007fbec3e04000)
           libnghttp2.so.14 => /nix/store/sw6jhdc4bpkz40x7vpnm4bsjjgilawas-nghttp2-1.41.0-lib/lib/libnghttp2.so.14 (0x00007fbec3dd7000)
           libc.so.6 => /nix/store/q53f5birhik4dxg3q3r2g5f324n7r5mc-glibc-2.31-74/lib/libc.so.6 (0x00007fbec3c18000)
           /nix/store/q53f5birhik4dxg3q3r2g5f324n7r5mc-glibc-2.31-74/lib/ld-linux-x86-64.so.2 => /nix/store/q53f5birhik4dxg3q3r2g5f324n7r5mc-glibc-2.31-74/lib64/ld-linux-x86-64.so.2 (0x00007fbec46b4000)
           libresolv.so.2 => /nix/store/q53f5birhik4dxg3q3r2g5f324n7r5mc-glibc-2.31-74/lib/libresolv.so.2 (0x00007fbec3bff000)
           libgssapi_krb5.so.2 => /nix/store/k4szgjabmw2mrk5b4p0215k64s9qpd5k-libkrb5-1.18/lib/libgssapi_krb5.so.2 (0x00007fbec3bac000)
           libkrb5.so.3 => /nix/store/k4szgjabmw2mrk5b4p0215k64s9qpd5k-libkrb5-1.18/lib/libkrb5.so.3 (0x00007fbec3ad2000)
           libk5crypto.so.3 => /nix/store/k4szgjabmw2mrk5b4p0215k64s9qpd5k-libkrb5-1.18/lib/libk5crypto.so.3 (0x00007fbec3aa2000)
           libcom_err.so.3 => /nix/store/k4szgjabmw2mrk5b4p0215k64s9qpd5k-libkrb5-1.18/lib/libcom_err.so.3 (0x00007fbec3a9a000)
           libcrypt.so.1 => /nix/store/q53f5birhik4dxg3q3r2g5f324n7r5mc-glibc-2.31-74/lib/libcrypt.so.1 (0x00007fbec3a60000)
           libncursesw.so.6 => /nix/store/gcsh4a410g668r0rd34k3cb6gfcwdkxj-ncurses-6.2/lib/libncursesw.so.6 (0x00007fbec39ec000)
           libutil.so.1 => /nix/store/q53f5birhik4dxg3q3r2g5f324n7r5mc-glibc-2.31-74/lib/libutil.so.1 (0x00007fbec39e7000)
           libm.so.6 => /nix/store/q53f5birhik4dxg3q3r2g5f324n7r5mc-glibc-2.31-74/lib/libm.so.6 (0x00007fbec38a6000)
           libgcc_s.so.1 => /nix/store/q53f5birhik4dxg3q3r2g5f324n7r5mc-glibc-2.31-74/lib/libgcc_s.so.1 (0x00007fbec388a000)
           libuv.so.1 => /nix/store/khbz2xp7w1bv2lypyf4fchdkjxiygyy9-libuv-1.40.0/lib/libuv.so.1 (0x00007fbec3857000)
           libkrb5support.so.0 => /nix/store/k4szgjabmw2mrk5b4p0215k64s9qpd5k-libkrb5-1.18/lib/libkrb5support.so.0 (0x00007fbec3848000)
           libkeyutils.so.1 => /nix/store/q77bffwam01blnxpjp55hx34r3arqhpj-keyutils-1.6.1-lib/lib/libkeyutils.so.1 (0x00007fbec3841000)
   ```
   
   Now, maybe dispatch could do LTO, and some of the other linking tricks (CPython adopted some fancy option in a recent version of Fedora...)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632143887



##########
File path: c/CMakeLists.txt
##########
@@ -487,16 +492,18 @@ if (BUILD_STATIC_LIBS)
   target_compile_definitions(qpid-proton-static PUBLIC PROTON_DECLARE_STATIC)
   target_link_libraries (qpid-proton-static
     qpid-proton-core-static
-    $<$<BOOL:${HAS_PROACTOR}>:qpid-proton-proactor-static>
+    ${TARTET_qpid-proton-proactor-static}
     ${SSL_LIB} ${SASL_LIB} ${TIME_LIB} ${PLATFORM_LIBS} ${PROACTOR_LIBS})
+  set_target_properties(qpid-proton-static PROPERTIES
+    POSITION_INDEPENDENT_CODE ON)
 endif(BUILD_STATIC_LIBS)
 
 # Install executables and libraries
 if (BUILD_STATIC_LIBS)
   set(STATIC_LIBS qpid-proton-static qpid-proton-core-static)
 endif()
 install(TARGETS qpid-proton qpid-proton-core ${STATIC_LIBS}
-  EXPORT  proton
+  EXPORT  ProtonTargets

Review comment:
       Just naming pattern that the CMake docs used in its example. I don't think this changes anything regarding compatibility.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632670303



##########
File path: c/CMakeLists.txt
##########
@@ -487,16 +492,18 @@ if (BUILD_STATIC_LIBS)
   target_compile_definitions(qpid-proton-static PUBLIC PROTON_DECLARE_STATIC)
   target_link_libraries (qpid-proton-static
     qpid-proton-core-static
-    $<$<BOOL:${HAS_PROACTOR}>:qpid-proton-proactor-static>
+    ${TARTET_qpid-proton-proactor-static}

Review comment:
       The generator expression bubbled through the ProtonTargets.cmake file (list of libraries to link to) but it was malformed there. I don't exactly remember how and i did not try to solve it. Since the patch is now being seriously considered (something beyond my immediate hopes yesterday), I will revisit this.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#issuecomment-848732752


   See #317 instead.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632660869



##########
File path: c/src/ProtonConfig.cmake.in
##########
@@ -22,50 +22,56 @@
 # Version: @PN_VERSION@
 # URL: http://qpid.apache.org/proton/
 
-set (Proton_VERSION       @PN_VERSION@)
+@PACKAGE_INIT@
+include("${CMAKE_CURRENT_LIST_DIR}/ProtonTargets.cmake")
 
-set (Proton_INCLUDE_DIRS  @INCLUDEDIR@)
-set (Proton_LIBRARIES     optimized @LIBDIR@/@PROTONLIB@ debug @LIBDIR@/@PROTONLIBDEBUG@)
-set (Proton_FOUND True)
+set(Proton_VERSION @PN_VERSION@)
+
+# find dependencies, because static libs don't transitively pull them
+if (Proton_USE_STATIC_LIBS)
+    set(CMAKE_MODULE_PATH_OLD ${CMAKE_MODULE_PATH})
+    set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")
+
+    set(CyrusSASL_FOUND @CyrusSASL_FOUND@)
+    if (CyrusSASL_FOUND)
+        find_package (CyrusSASL REQUIRED)
+    endif()
+
+    set(OpenSSL_FOUND @OpenSSL_FOUND@)
+    if (OpenSSL_FOUND)
+        find_package (OpenSSL REQUIRED)
+    endif()
 
-set (Proton_Core_INCLUDE_DIRS  @INCLUDEDIR@)
-set (Proton_Core_LIBRARIES     optimized @LIBDIR@/@PROTONCORELIB@ debug @LIBDIR@/@PROTONCORELIBDEBUG@)
+    set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH_OLD})
+endif()
 
-# Add modular target in a way compatible with cmake 2.8.12
-if (NOT TARGET Proton::core)
-  add_library(Proton::core UNKNOWN IMPORTED)
-  set_target_properties(Proton::core
-    PROPERTIES
-      IMPORTED_LOCATION "@LIBDIR@/@PROTONCORELIB@"
-      IMPORTED_LOCATION_DEBUG "@LIBDIR@/@PROTONCORELIBDEBUG@"
-      INTERFACE_INCLUDE_DIRECTORIES "${Proton_Core_INCLUDE_DIRS}")
+set(Proton_INCLUDE_DIRS @INCLUDEDIR@)
+if (NOT Proton_USE_STATIC_LIBS)
+    set (Proton_LIBRARIES Proton::qpid-proton)
+else()
+    set (Proton_LIBRARIES Proton::qpid-proton-static)
 endif()
+set (Proton_FOUND True)
 
+if (NOT Proton_USE_STATIC_LIBS)
+    add_library(Proton::core ALIAS Proton::qpid-proton-core)

Review comment:
       This ALIAS thing does not work on CMake 3.11. I get
   
   ```
   -- Found CyrusSASL: /usr/lib64/libsasl2.so (found version "2.1.27") 
   CMake Error at /qpid-proton/build/install/lib64/cmake/Proton/ProtonConfig.cmake:83 (add_library):
     add_library cannot create ALIAS target "Proton::core" because target
     "Proton::qpid-proton-core-static" is imported but not globally visible.
   ```
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632677600



##########
File path: c/CMakeLists.txt
##########
@@ -419,6 +419,8 @@ if (BUILD_STATIC_LIBS)
   add_library (qpid-proton-core-static STATIC ${qpid-proton-core-src})
   target_compile_definitions(qpid-proton-core-static PUBLIC PROTON_DECLARE_STATIC)
   target_link_libraries (qpid-proton-core-static ${SSL_LIB} ${SASL_LIB} ${PLATFORM_LIBS})
+  set_target_properties(qpid-proton-core-static PROPERTIES
+    POSITION_INDEPENDENT_CODE ON)

Review comment:
       Otherwise I cant build libqpid-dispatch.so




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek closed pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek closed pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] astitcher commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
astitcher commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632580725



##########
File path: c/CMakeLists.txt
##########
@@ -487,16 +492,18 @@ if (BUILD_STATIC_LIBS)
   target_compile_definitions(qpid-proton-static PUBLIC PROTON_DECLARE_STATIC)
   target_link_libraries (qpid-proton-static
     qpid-proton-core-static
-    $<$<BOOL:${HAS_PROACTOR}>:qpid-proton-proactor-static>
+    ${TARTET_qpid-proton-proactor-static}

Review comment:
       What problem does this change solve? The generator works well with 2.8.12 already AFAIK.
   
   Also spelling as above.

##########
File path: c/CMakeLists.txt
##########
@@ -456,6 +458,9 @@ if (qpid-proton-proactor)
     add_library (qpid-proton-proactor-static STATIC ${qpid-proton-proactor})
     target_compile_definitions(qpid-proton-proactor-static PUBLIC PROTON_DECLARE_STATIC)
     target_link_libraries (qpid-proton-proactor-static ${PLATFORM_LIBS} ${PROACTOR_LIBS})
+    set_target_properties(qpid-proton-proactor-static PROPERTIES
+      POSITION_INDEPENDENT_CODE ON)
+    set(TARTET_qpid-proton-proactor-static qpid-proton-proactor-static)

Review comment:
       Spelling TARTET->TARGET?
   

##########
File path: c/CMakeLists.txt
##########
@@ -456,6 +458,9 @@ if (qpid-proton-proactor)
     add_library (qpid-proton-proactor-static STATIC ${qpid-proton-proactor})
     target_compile_definitions(qpid-proton-proactor-static PUBLIC PROTON_DECLARE_STATIC)
     target_link_libraries (qpid-proton-proactor-static ${PLATFORM_LIBS} ${PROACTOR_LIBS})
+    set_target_properties(qpid-proton-proactor-static PROPERTIES
+      POSITION_INDEPENDENT_CODE ON)

Review comment:
       As above I don't think this should be PIC for a static library.

##########
File path: c/CMakeLists.txt
##########
@@ -557,11 +564,25 @@ if(HAS_PROACTOR)
   configure_lib(PROTONPROACTORLIB qpid-proton-proactor)
 endif(HAS_PROACTOR)
 
+# CMake is happy to generate a lot of the *config.cmake file for us, see
+#  https://gitlab.kitware.com/cmake/cmake/-/issues/19560
+#  https://cmake.org/cmake/help/latest/manual/cmake-packages.7.html#creating-packages
+#  https://cmake.org/cmake/help/latest/module/CMakePackageConfigHelpers.html#example-generating-package-files

Review comment:
       No need for this comment here - put it in the commit message.
   Just make sure that the generated files are substitutable for the current one in all cases. I think they should be the only reason they are not use is because cmake earlier than 2.8.12 didn't support this as well.

##########
File path: c/CMakeLists.txt
##########
@@ -487,16 +492,18 @@ if (BUILD_STATIC_LIBS)
   target_compile_definitions(qpid-proton-static PUBLIC PROTON_DECLARE_STATIC)
   target_link_libraries (qpid-proton-static
     qpid-proton-core-static
-    $<$<BOOL:${HAS_PROACTOR}>:qpid-proton-proactor-static>
+    ${TARTET_qpid-proton-proactor-static}
     ${SSL_LIB} ${SASL_LIB} ${TIME_LIB} ${PLATFORM_LIBS} ${PROACTOR_LIBS})
+  set_target_properties(qpid-proton-static PROPERTIES
+    POSITION_INDEPENDENT_CODE ON)
 endif(BUILD_STATIC_LIBS)
 
 # Install executables and libraries
 if (BUILD_STATIC_LIBS)
   set(STATIC_LIBS qpid-proton-static qpid-proton-core-static)
 endif()
 install(TARGETS qpid-proton qpid-proton-core ${STATIC_LIBS}
-  EXPORT  proton
+  EXPORT  ProtonTargets

Review comment:
       No objection, I just can't remember what this is for - can you say what the EXPORT directive does?

##########
File path: c/CMakeLists.txt
##########
@@ -419,6 +419,8 @@ if (BUILD_STATIC_LIBS)
   add_library (qpid-proton-core-static STATIC ${qpid-proton-core-src})
   target_compile_definitions(qpid-proton-core-static PUBLIC PROTON_DECLARE_STATIC)
   target_link_libraries (qpid-proton-core-static ${SSL_LIB} ${SASL_LIB} ${PLATFORM_LIBS})
+  set_target_properties(qpid-proton-core-static PROPERTIES
+    POSITION_INDEPENDENT_CODE ON)

Review comment:
       I think this is probably wrong. You should *not* be building a dynamic library from static library parts.
   
   This implies that the final dispatch router executable should be statically linked into qdrouter.
   Ie: static qpid-proton-core static qpid-proton-proactor -> static qpid-dispatch -> qdrouter
   
   Mixing static and dynamic libs to build a library is IMO a really bad idea just waiting for build problems. Also if you statically link all the way you've a much better chance to get something like LTO working for a much bigger part of the final executable.

##########
File path: c/CMakeLists.txt
##########
@@ -487,16 +492,18 @@ if (BUILD_STATIC_LIBS)
   target_compile_definitions(qpid-proton-static PUBLIC PROTON_DECLARE_STATIC)
   target_link_libraries (qpid-proton-static
     qpid-proton-core-static
-    $<$<BOOL:${HAS_PROACTOR}>:qpid-proton-proactor-static>
+    ${TARTET_qpid-proton-proactor-static}
     ${SSL_LIB} ${SASL_LIB} ${TIME_LIB} ${PLATFORM_LIBS} ${PROACTOR_LIBS})
+  set_target_properties(qpid-proton-static PROPERTIES
+    POSITION_INDEPENDENT_CODE ON)

Review comment:
       PIC as above




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r639684453



##########
File path: c/CMakeLists.txt
##########
@@ -419,6 +419,8 @@ if (BUILD_STATIC_LIBS)
   add_library (qpid-proton-core-static STATIC ${qpid-proton-core-src})
   target_compile_definitions(qpid-proton-core-static PUBLIC PROTON_DECLARE_STATIC)
   target_link_libraries (qpid-proton-core-static ${SSL_LIB} ${SASL_LIB} ${PLATFORM_LIBS})
+  set_target_properties(qpid-proton-core-static PROPERTIES
+    POSITION_INDEPENDENT_CODE ON)

Review comment:
       -fPIC can be forced onto CMake from commandline when needed, so it is not necessary to put it into CMakeLists.txt




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632145960



##########
File path: c/src/ProtonConfig.cmake.in
##########
@@ -22,50 +22,56 @@
 # Version: @PN_VERSION@
 # URL: http://qpid.apache.org/proton/
 
-set (Proton_VERSION       @PN_VERSION@)
+@PACKAGE_INIT@
+include("${CMAKE_CURRENT_LIST_DIR}/ProtonTargets.cmake")
 
-set (Proton_INCLUDE_DIRS  @INCLUDEDIR@)
-set (Proton_LIBRARIES     optimized @LIBDIR@/@PROTONLIB@ debug @LIBDIR@/@PROTONLIBDEBUG@)
-set (Proton_FOUND True)
+set(Proton_VERSION @PN_VERSION@)
+
+# find dependencies, because static libs don't transitively pull them
+if (Proton_USE_STATIC_LIBS)
+    set(CMAKE_MODULE_PATH_OLD ${CMAKE_MODULE_PATH})
+    set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")
+
+    set(CyrusSASL_FOUND @CyrusSASL_FOUND@)
+    if (CyrusSASL_FOUND)
+        find_package (CyrusSASL REQUIRED)
+    endif()
+
+    set(OpenSSL_FOUND @OpenSSL_FOUND@)
+    if (OpenSSL_FOUND)
+        find_package (OpenSSL REQUIRED)
+    endif()
 
-set (Proton_Core_INCLUDE_DIRS  @INCLUDEDIR@)
-set (Proton_Core_LIBRARIES     optimized @LIBDIR@/@PROTONCORELIB@ debug @LIBDIR@/@PROTONCORELIBDEBUG@)
+    set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH_OLD})
+endif()
 
-# Add modular target in a way compatible with cmake 2.8.12
-if (NOT TARGET Proton::core)
-  add_library(Proton::core UNKNOWN IMPORTED)
-  set_target_properties(Proton::core
-    PROPERTIES
-      IMPORTED_LOCATION "@LIBDIR@/@PROTONCORELIB@"
-      IMPORTED_LOCATION_DEBUG "@LIBDIR@/@PROTONCORELIBDEBUG@"
-      INTERFACE_INCLUDE_DIRECTORIES "${Proton_Core_INCLUDE_DIRS}")
+set(Proton_INCLUDE_DIRS @INCLUDEDIR@)

Review comment:
       but not too newfangled, to keep it working on 2.8.12, of course




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632143486



##########
File path: c/CMakeLists.txt
##########
@@ -419,6 +419,8 @@ if (BUILD_STATIC_LIBS)
   add_library (qpid-proton-core-static STATIC ${qpid-proton-core-src})
   target_compile_definitions(qpid-proton-core-static PUBLIC PROTON_DECLARE_STATIC)
   target_link_libraries (qpid-proton-core-static ${SSL_LIB} ${SASL_LIB} ${PLATFORM_LIBS})
+  set_target_properties(qpid-proton-core-static PROPERTIES
+    POSITION_INDEPENDENT_CODE ON)

Review comment:
       these static libs will not later link into final binaries in Dispatch together with other dynlibs without this




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632677600



##########
File path: c/CMakeLists.txt
##########
@@ -419,6 +419,8 @@ if (BUILD_STATIC_LIBS)
   add_library (qpid-proton-core-static STATIC ${qpid-proton-core-src})
   target_compile_definitions(qpid-proton-core-static PUBLIC PROTON_DECLARE_STATIC)
   target_link_libraries (qpid-proton-core-static ${SSL_LIB} ${SASL_LIB} ${PLATFORM_LIBS})
+  set_target_properties(qpid-proton-core-static PROPERTIES
+    POSITION_INDEPENDENT_CODE ON)

Review comment:
       Otherwise I cant build libqpid-dispatch.so, because all that goes into a .so needs to be pic




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r639683797



##########
File path: c/src/ProtonConfig.cmake.in
##########
@@ -22,50 +22,56 @@
 # Version: @PN_VERSION@
 # URL: http://qpid.apache.org/proton/
 
-set (Proton_VERSION       @PN_VERSION@)
+@PACKAGE_INIT@
+include("${CMAKE_CURRENT_LIST_DIR}/ProtonTargets.cmake")
 
-set (Proton_INCLUDE_DIRS  @INCLUDEDIR@)
-set (Proton_LIBRARIES     optimized @LIBDIR@/@PROTONLIB@ debug @LIBDIR@/@PROTONLIBDEBUG@)
-set (Proton_FOUND True)
+set(Proton_VERSION @PN_VERSION@)
+
+# find dependencies, because static libs don't transitively pull them
+if (Proton_USE_STATIC_LIBS)
+    set(CMAKE_MODULE_PATH_OLD ${CMAKE_MODULE_PATH})
+    set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")
+
+    set(CyrusSASL_FOUND @CyrusSASL_FOUND@)
+    if (CyrusSASL_FOUND)
+        find_package (CyrusSASL REQUIRED)
+    endif()
+
+    set(OpenSSL_FOUND @OpenSSL_FOUND@)
+    if (OpenSSL_FOUND)
+        find_package (OpenSSL REQUIRED)
+    endif()
 
-set (Proton_Core_INCLUDE_DIRS  @INCLUDEDIR@)
-set (Proton_Core_LIBRARIES     optimized @LIBDIR@/@PROTONCORELIB@ debug @LIBDIR@/@PROTONCORELIBDEBUG@)
+    set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH_OLD})
+endif()
 
-# Add modular target in a way compatible with cmake 2.8.12
-if (NOT TARGET Proton::core)
-  add_library(Proton::core UNKNOWN IMPORTED)
-  set_target_properties(Proton::core
-    PROPERTIES
-      IMPORTED_LOCATION "@LIBDIR@/@PROTONCORELIB@"
-      IMPORTED_LOCATION_DEBUG "@LIBDIR@/@PROTONCORELIBDEBUG@"
-      INTERFACE_INCLUDE_DIRECTORIES "${Proton_Core_INCLUDE_DIRS}")
+set(Proton_INCLUDE_DIRS @INCLUDEDIR@)

Review comment:
       I'll leave this the way it was. Too many 'improvements' in too little time is not a good thing.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#issuecomment-843612523


   I'll redo this without the `POSITION_INDEPENDENT_CODE` in static libs and I will figure out how to create aliased imported target in CMake 2.8.12.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632145705



##########
File path: c/src/ProtonConfig.cmake.in
##########
@@ -22,50 +22,56 @@
 # Version: @PN_VERSION@
 # URL: http://qpid.apache.org/proton/
 
-set (Proton_VERSION       @PN_VERSION@)
+@PACKAGE_INIT@
+include("${CMAKE_CURRENT_LIST_DIR}/ProtonTargets.cmake")
 
-set (Proton_INCLUDE_DIRS  @INCLUDEDIR@)
-set (Proton_LIBRARIES     optimized @LIBDIR@/@PROTONLIB@ debug @LIBDIR@/@PROTONLIBDEBUG@)
-set (Proton_FOUND True)
+set(Proton_VERSION @PN_VERSION@)
+
+# find dependencies, because static libs don't transitively pull them
+if (Proton_USE_STATIC_LIBS)
+    set(CMAKE_MODULE_PATH_OLD ${CMAKE_MODULE_PATH})
+    set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")
+
+    set(CyrusSASL_FOUND @CyrusSASL_FOUND@)
+    if (CyrusSASL_FOUND)
+        find_package (CyrusSASL REQUIRED)
+    endif()
+
+    set(OpenSSL_FOUND @OpenSSL_FOUND@)
+    if (OpenSSL_FOUND)
+        find_package (OpenSSL REQUIRED)
+    endif()
 
-set (Proton_Core_INCLUDE_DIRS  @INCLUDEDIR@)
-set (Proton_Core_LIBRARIES     optimized @LIBDIR@/@PROTONCORELIB@ debug @LIBDIR@/@PROTONCORELIBDEBUG@)
+    set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH_OLD})
+endif()
 
-# Add modular target in a way compatible with cmake 2.8.12
-if (NOT TARGET Proton::core)
-  add_library(Proton::core UNKNOWN IMPORTED)
-  set_target_properties(Proton::core
-    PROPERTIES
-      IMPORTED_LOCATION "@LIBDIR@/@PROTONCORELIB@"
-      IMPORTED_LOCATION_DEBUG "@LIBDIR@/@PROTONCORELIBDEBUG@"
-      INTERFACE_INCLUDE_DIRECTORIES "${Proton_Core_INCLUDE_DIRS}")
+set(Proton_INCLUDE_DIRS @INCLUDEDIR@)

Review comment:
       there is probably some newfangled way to handle the include headers too and keep them associated with the targets...




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632645287



##########
File path: c/CMakeLists.txt
##########
@@ -419,6 +419,8 @@ if (BUILD_STATIC_LIBS)
   add_library (qpid-proton-core-static STATIC ${qpid-proton-core-src})
   target_compile_definitions(qpid-proton-core-static PUBLIC PROTON_DECLARE_STATIC)
   target_link_libraries (qpid-proton-core-static ${SSL_LIB} ${SASL_LIB} ${PLATFORM_LIBS})
+  set_target_properties(qpid-proton-core-static PROPERTIES
+    POSITION_INDEPENDENT_CODE ON)

Review comment:
       @astitcher so the `POSITION_INDEPENDENT_CODE ON` is so far the only thing that is not upstreamable?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632144705



##########
File path: c/src/ProtonConfig.cmake.in
##########
@@ -22,50 +22,56 @@
 # Version: @PN_VERSION@
 # URL: http://qpid.apache.org/proton/
 
-set (Proton_VERSION       @PN_VERSION@)
+@PACKAGE_INIT@
+include("${CMAKE_CURRENT_LIST_DIR}/ProtonTargets.cmake")
 
-set (Proton_INCLUDE_DIRS  @INCLUDEDIR@)
-set (Proton_LIBRARIES     optimized @LIBDIR@/@PROTONLIB@ debug @LIBDIR@/@PROTONLIBDEBUG@)
-set (Proton_FOUND True)
+set(Proton_VERSION @PN_VERSION@)
+
+# find dependencies, because static libs don't transitively pull them
+if (Proton_USE_STATIC_LIBS)
+    set(CMAKE_MODULE_PATH_OLD ${CMAKE_MODULE_PATH})
+    set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")
+
+    set(CyrusSASL_FOUND @CyrusSASL_FOUND@)
+    if (CyrusSASL_FOUND)
+        find_package (CyrusSASL REQUIRED)
+    endif()
+
+    set(OpenSSL_FOUND @OpenSSL_FOUND@)
+    if (OpenSSL_FOUND)
+        find_package (OpenSSL REQUIRED)
+    endif()
 
-set (Proton_Core_INCLUDE_DIRS  @INCLUDEDIR@)
-set (Proton_Core_LIBRARIES     optimized @LIBDIR@/@PROTONCORELIB@ debug @LIBDIR@/@PROTONCORELIBDEBUG@)
+    set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH_OLD})
+endif()
 
-# Add modular target in a way compatible with cmake 2.8.12
-if (NOT TARGET Proton::core)
-  add_library(Proton::core UNKNOWN IMPORTED)
-  set_target_properties(Proton::core
-    PROPERTIES
-      IMPORTED_LOCATION "@LIBDIR@/@PROTONCORELIB@"
-      IMPORTED_LOCATION_DEBUG "@LIBDIR@/@PROTONCORELIBDEBUG@"
-      INTERFACE_INCLUDE_DIRECTORIES "${Proton_Core_INCLUDE_DIRS}")
+set(Proton_INCLUDE_DIRS @INCLUDEDIR@)
+if (NOT Proton_USE_STATIC_LIBS)
+    set (Proton_LIBRARIES Proton::qpid-proton)
+else()
+    set (Proton_LIBRARIES Proton::qpid-proton-static)
 endif()
+set (Proton_FOUND True)
 
+if (NOT Proton_USE_STATIC_LIBS)
+    add_library(Proton::core ALIAS Proton::qpid-proton-core)
+else ()
+    add_library(Proton::core ALIAS Proton::qpid-proton-core-static)
+endif ()
+set (Proton_Core_INCLUDE_DIRS @INCLUDEDIR@)
+set (Proton_Core_LIBRARIES Proton::core)
 set (Proton_Core_FOUND True)

Review comment:
       the idea is to be backwards compatible and allow easy api to switch between static/dynamic libs for cmake users; it is weird but so far it worked




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632677317



##########
File path: c/CMakeLists.txt
##########
@@ -419,6 +419,8 @@ if (BUILD_STATIC_LIBS)
   add_library (qpid-proton-core-static STATIC ${qpid-proton-core-src})
   target_compile_definitions(qpid-proton-core-static PUBLIC PROTON_DECLARE_STATIC)
   target_link_libraries (qpid-proton-core-static ${SSL_LIB} ${SASL_LIB} ${PLATFORM_LIBS})
+  set_target_properties(qpid-proton-core-static PROPERTIES
+    POSITION_INDEPENDENT_CODE ON)

Review comment:
       @astitcher ideally, I want a cmake object library which dispatch then sucks in; I am not actually interested in creating an artifact here, I just want to put the proton symbols into dispatch... 
   
   I got inspired here by
   
   > For this to happen, first create the static library with an option that makes the compiler generate “Position Independent Code” for every object that is archived as a part of the static library. This is required, because, a dynamic library, by its definition, is a “shared library” - which means it is relocatable. In order to have object files in the static library packed within the dynamic library, it is important these objects are also relocatable - i.e., made position independent.
   > https://blog.ramdoot.in/how-can-i-link-a-static-library-to-a-dynamic-library-e1f25c8095ef




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632678211



##########
File path: c/CMakeLists.txt
##########
@@ -456,6 +458,9 @@ if (qpid-proton-proactor)
     add_library (qpid-proton-proactor-static STATIC ${qpid-proton-proactor})
     target_compile_definitions(qpid-proton-proactor-static PUBLIC PROTON_DECLARE_STATIC)
     target_link_libraries (qpid-proton-proactor-static ${PLATFORM_LIBS} ${PROACTOR_LIBS})
+    set_target_properties(qpid-proton-proactor-static PROPERTIES
+      POSITION_INDEPENDENT_CODE ON)
+    set(TARTET_qpid-proton-proactor-static qpid-proton-proactor-static)

Review comment:
       ;P thanks, at least I am consistent




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r639686204



##########
File path: c/CMakeLists.txt
##########
@@ -487,16 +492,18 @@ if (BUILD_STATIC_LIBS)
   target_compile_definitions(qpid-proton-static PUBLIC PROTON_DECLARE_STATIC)
   target_link_libraries (qpid-proton-static
     qpid-proton-core-static
-    $<$<BOOL:${HAS_PROACTOR}>:qpid-proton-proactor-static>
+    ${TARTET_qpid-proton-proactor-static}

Review comment:
       I have to do it this way. I have to evaluate everything in the context of the Proton project and then see the result in Dispatch (or whatever dependent project).




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632672479



##########
File path: c/CMakeLists.txt
##########
@@ -487,16 +492,18 @@ if (BUILD_STATIC_LIBS)
   target_compile_definitions(qpid-proton-static PUBLIC PROTON_DECLARE_STATIC)
   target_link_libraries (qpid-proton-static
     qpid-proton-core-static
-    $<$<BOOL:${HAS_PROACTOR}>:qpid-proton-proactor-static>
+    ${TARTET_qpid-proton-proactor-static}
     ${SSL_LIB} ${SASL_LIB} ${TIME_LIB} ${PLATFORM_LIBS} ${PROACTOR_LIBS})
+  set_target_properties(qpid-proton-static PROPERTIES
+    POSITION_INDEPENDENT_CODE ON)
 endif(BUILD_STATIC_LIBS)
 
 # Install executables and libraries
 if (BUILD_STATIC_LIBS)
   set(STATIC_LIBS qpid-proton-static qpid-proton-core-static)
 endif()
 install(TARGETS qpid-proton qpid-proton-core ${STATIC_LIBS}
-  EXPORT  proton
+  EXPORT  ProtonTargets

Review comment:
       It causes CMake to generate ProtonTargets.cmake, which I then include from the ProtonConfig.cmake file. It does the heavy lifting of locating the library files relative to the ProtonConfig.cmake. I'll attach that file
   
   [ProtonTargets.txt](https://github.com/apache/qpid-proton/files/6480182/ProtonTargets.txt)
   [ProtonTargets-relwithdebinfo.txt](https://github.com/apache/qpid-proton/files/6480183/ProtonTargets-relwithdebinfo.txt)
   [ProtonConfig.txt](https://github.com/apache/qpid-proton/files/6480184/ProtonConfig.txt)
   [ProtonConfigVersion.txt](https://github.com/apache/qpid-proton/files/6480185/ProtonConfigVersion.txt)
   [FindCyrusSASL.txt](https://github.com/apache/qpid-proton/files/6480186/FindCyrusSASL.txt)
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org

[GitHub] [qpid-proton] jiridanek commented on a change in pull request #312: This seems to work to allow linking with static Proton from Dispatch from within CMake

Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #312:
URL: https://github.com/apache/qpid-proton/pull/312#discussion_r632662829



##########
File path: c/src/ProtonConfig.cmake.in
##########
@@ -22,50 +22,56 @@
 # Version: @PN_VERSION@
 # URL: http://qpid.apache.org/proton/
 
-set (Proton_VERSION       @PN_VERSION@)
+@PACKAGE_INIT@
+include("${CMAKE_CURRENT_LIST_DIR}/ProtonTargets.cmake")
 
-set (Proton_INCLUDE_DIRS  @INCLUDEDIR@)
-set (Proton_LIBRARIES     optimized @LIBDIR@/@PROTONLIB@ debug @LIBDIR@/@PROTONLIBDEBUG@)
-set (Proton_FOUND True)
+set(Proton_VERSION @PN_VERSION@)
+
+# find dependencies, because static libs don't transitively pull them
+if (Proton_USE_STATIC_LIBS)
+    set(CMAKE_MODULE_PATH_OLD ${CMAKE_MODULE_PATH})
+    set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")
+
+    set(CyrusSASL_FOUND @CyrusSASL_FOUND@)
+    if (CyrusSASL_FOUND)
+        find_package (CyrusSASL REQUIRED)
+    endif()
+
+    set(OpenSSL_FOUND @OpenSSL_FOUND@)
+    if (OpenSSL_FOUND)
+        find_package (OpenSSL REQUIRED)
+    endif()
 
-set (Proton_Core_INCLUDE_DIRS  @INCLUDEDIR@)
-set (Proton_Core_LIBRARIES     optimized @LIBDIR@/@PROTONCORELIB@ debug @LIBDIR@/@PROTONCORELIBDEBUG@)
+    set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH_OLD})
+endif()
 
-# Add modular target in a way compatible with cmake 2.8.12
-if (NOT TARGET Proton::core)
-  add_library(Proton::core UNKNOWN IMPORTED)
-  set_target_properties(Proton::core
-    PROPERTIES
-      IMPORTED_LOCATION "@LIBDIR@/@PROTONCORELIB@"
-      IMPORTED_LOCATION_DEBUG "@LIBDIR@/@PROTONCORELIBDEBUG@"
-      INTERFACE_INCLUDE_DIRECTORIES "${Proton_Core_INCLUDE_DIRS}")
+set(Proton_INCLUDE_DIRS @INCLUDEDIR@)
+if (NOT Proton_USE_STATIC_LIBS)
+    set (Proton_LIBRARIES Proton::qpid-proton)
+else()
+    set (Proton_LIBRARIES Proton::qpid-proton-static)
 endif()
+set (Proton_FOUND True)
 
+if (NOT Proton_USE_STATIC_LIBS)
+    add_library(Proton::core ALIAS Proton::qpid-proton-core)

Review comment:
       > New in version 3.18: An ALIAS can target a non-GLOBAL Imported Target.
   
   So i used 3.18 feature by mistake.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org