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

cmake: Allow tests to build without NSS #13315

Merged
merged 1 commit into from Apr 28, 2017

Conversation

Projects
None yet
4 participants
@dang
Contributor

dang commented Feb 8, 2017

Without NSS, ceph_test_cephd_api_misc needs SSL libs linked in. These
are pulled in by civetweb.

Signed-off-by: Daniel Gryniewicz dang@redhat.com

@dang dang added the build/ops label Feb 8, 2017

@@ -16,7 +16,7 @@ add_executable(ceph_test_cephd_api_misc
set_target_properties(ceph_test_cephd_api_misc PROPERTIES COMPILE_FLAGS
${UNITTEST_CXX_FLAGS})
target_link_libraries(ceph_test_cephd_api_misc
cephd global ${UNITTEST_LIBS} cephdtest z snappy ceph_zstd)
cephd global ${UNITTEST_LIBS} cephdtest z snappy ceph_zstd ${SSL_LIBRARIES})

This comment has been minimized.

@tchaikov

tchaikov Mar 2, 2017

Contributor

maybe we should add this linkage to radosgw_a instead. because it is radosgw_a which includes civetweb_common_objs, and the later has civetweb.o. so i'd recommend

diff --git a/src/rgw/CMakeLists.txt b/src/rgw/CMakeLists.txt
index 4a06cf3..d9a2ca7 100644
--- a/src/rgw/CMakeLists.txt
+++ b/src/rgw/CMakeLists.txt
@@ -139,7 +139,7 @@ endif (WITH_RADOSGW_ASIO_FRONTEND)

 add_library(radosgw_a STATIC ${radosgw_srcs}
   $<TARGET_OBJECTS:civetweb_common_objs>)
-target_link_libraries(radosgw_a rgw_a)
+target_link_libraries(radosgw_a rgw_a ${SSL_LIBRARIES})

 add_executable(radosgw rgw_main.cc)
 target_link_libraries(radosgw radosgw_a librados
@@ -147,7 +147,7 @@ target_link_libraries(radosgw radosgw_a librados
   cls_log_client cls_statelog_client cls_timeindex_client
   cls_version_client cls_replica_log_client cls_user_client
   global ${FCGI_LIBRARY} ${LIB_RESOLV}
-  ${CURL_LIBRARIES} ${EXPAT_LIBRARIES} ${SSL_LIBRARIES} ${BLKID_LIBRARIES}
+  ${CURL_LIBRARIES} ${EXPAT_LIBRARIES} ${BLKID_LIBRARIES}
   ${ALLOC_LIBS})
 # radosgw depends on cls libraries at runtime, but not as link dependencies
 add_dependencies(radosgw cls_rgw cls_lock cls_refcount

.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Mar 2, 2017

and could you prefix the title of your commit message with the subcomponent your are changing ? see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#3-describe-your-changes, in this case, it would be "cmake: "

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Mar 2, 2017

and

Linking CXX executable ../../../bin/ceph_test_cephd_api_misc
../../../lib/libcephd.a(civetweb.c.o): In function `set_ssl_option':
/slow/kchai/ceph/src/civetweb/src/civetweb.c:11165: undefined reference to `SSL_CTX_set_ecdh_auto'
collect2: error: ld returned 1 exit status

as newer openssl does not offer this function anymore. maybe we should update civetweb submodule to address this? @yehudasa @mdw-at-linuxbox what do you think?

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Apr 10, 2017

@dang ping?

@yehudasa ping?

cmake - Allow tests to build without NSS
Without NSS, things that link radowsgw_a (including
ceph_test_cephd_api_misc) needs SSL libs linked in.  These are pulled in
by civetweb.

Signed-off-by: Daniel Gryniewicz <dang@redhat.com>

@tchaikov tchaikov changed the title from Allow tests to build without NSS to cmake: Allow tests to build without NSS Apr 26, 2017

@liewegas liewegas merged commit a07cacb into ceph:master Apr 28, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
arm build successfully built on arm
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment