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: link ceph-{mgr,mon,mds,osd} against libcommon statically #12878

Merged
merged 6 commits into from Jan 26, 2017

Conversation

Projects
None yet
5 participants
@tchaikov
Contributor

tchaikov commented Jan 11, 2017

when packaging debian packages, dpkg-shlibdeps analyzes the linked
shared libraries of the binaries in given package by looking at their
names. but if the name does not include any useful versioning
information, it complains. we have no intention of versioning
libceph-common, as it is merely an internal shared library used by ceph
packages only. but there is no simple way to disable dpkg-shlibdeps'
warning of

dpkg-shlibdeps: warning: can't extract name and version from library
name 'libceph-common.so'

other than skipping the dpkg-shlibdeps for the whole package. so let's
just version it anyway.

Signed-off-by: Kefu Chai kchai@redhat.com

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jan 11, 2017

this silences tons of warnings in http://gitbuilder.sepia.ceph.com/gitbuilder-ceph-deb-trusty-amd64-notcmalloc/log.cgi?log=03db36f35478ee18b4a7cf4d71543faa8894508c , like

warning: dpkg-shlibdeps: can't extract name and version from library name 'libceph-common.so'

@tchaikov tchaikov changed the title from cmake: version libceph-common to appease dpkg-shlibdeps to cmake: link ceph-{mgr,mon,mds,osd} against libcommon statically Jan 13, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jan 13, 2017

@cbodley mind taking a look? thanks.

@ceph-jenkins

This comment has been minimized.

Collaborator

ceph-jenkins commented Jan 18, 2017

Build finished.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jan 18, 2017

retest this please (boost submodule sync failed)

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jan 18, 2017

@dmick and @cbodley could any of you guys take a look?

@cbodley

This comment has been minimized.

Contributor

cbodley commented Jan 18, 2017

@tchaikov doesn't this reintroduce the issue of duplicated static variables from libcommon?

if shared library versioning is the main issue, can we not just set it to 1.0 and leave it?

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jan 18, 2017

@cbodley the ceph-osd's dependency on libceph-common is the main issue here. and libcommon is only linked by executables not linked against libcephfs, librados, or librbd with the commit of 496d883, so the duplicated-static-variable issue is not reintroduced by this change.

if shared library versioning is the main issue, can we not just set it to 1.0 and leave it?

yeah, i changed it to 0.0.0.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jan 18, 2017

change log

  • rebased against master again.
@@ -7,4 +7,4 @@ set(global_common_files
add_library(global_common_objs OBJECT ${global_common_files})
add_library(global STATIC ${libglobal_srcs}
$<TARGET_OBJECTS:global_common_objs>)
target_link_libraries(global ceph-common ${DPDK_LIBRARIES} ${EXTRALIBS})
target_link_libraries(global common ${DPDK_LIBRARIES} ${EXTRALIBS})

This comment has been minimized.

@cbodley

cbodley Jan 18, 2017

Contributor

linking statically into global is problematic for targets that link against both global and librados. in src/rgw/CMakeLists.txt, for example:

target_link_libraries(rgw_a librados cls_lock_client cls_rgw_client cls_refcount_client
  cls_log_client cls_statelog_client cls_timeindex_client cls_version_client
  cls_replica_log_client cls_user_client ceph-common common_utf8 global
...

This comment has been minimized.

@tchaikov

tchaikov Jan 18, 2017

Contributor

ah, thanks! right! maybe i should remove ceph-common or common from global's linked libraries. because quite a few executables link against global, this would introduce more code churn. but seems it's unavoidable..

@tchaikov tchaikov changed the title from cmake: link ceph-{mgr,mon,mds,osd} against libcommon statically to [DNM] cmake: link ceph-{mgr,mon,mds,osd} against libcommon statically Jan 18, 2017

@tchaikov tchaikov assigned tchaikov and unassigned dmick Jan 18, 2017

tchaikov added some commits Jan 11, 2017

cmake: version libceph-common to appease dpkg-shlibdeps
when packaging debian packages, dpkg-shlibdeps analyzes the linked
shared libraries of the binaries in given package by looking at their
names. but if the name does not include any useful versioning
information, it complains. we have no intention of versioning
libceph-common, as it is merely an internal shared library used by ceph
packages only. but there is no simple way to disable dpkg-shlibdeps'
warning of

  dpkg-shlibdeps: warning: can't extract name and version from library
  name 'libceph-common.so'

other than skipping the dpkg-shlibdeps for the whole package. so let's
just version it anyway.

Signed-off-by: Kefu Chai <kchai@redhat.com>
cmake: always build a static libceph-common
Signed-off-by: Kefu Chai <kchai@redhat.com>
cmake: link ceph-{mds,mgr,mon,osd} against libcommon
add a static library named global-static, which does not link with
libceph-common. so the executables which does not link against
lib{rados,cephfs,rbd} can be linked against global-static instead if
they want to access the symbols previously available from libglobal.
and libglobal is now linked against libceph-common. and it is supposed
to be used by executables packaged by ceph-test. these exectuables can
safely depend on libceph-common offered by package of "librados2".

Signed-off-by: Kefu Chai <kchai@redhat.com>
debian: remove xml2 from ceph-test's Depends
as we are not using any of the binaries offered by xml2. and ceph.spec
does not reference xml2 either.

Signed-off-by: Kefu Chai <kchai@redhat.com>
key_value_store: remove unneeded #include
Signed-off-by: Kefu Chai <kchai@redhat.com>
cmake: remove unnecessary linkages
Signed-off-by: Kefu Chai <kchai@redhat.com>

@tchaikov tchaikov changed the title from [DNM] cmake: link ceph-{mgr,mon,mds,osd} against libcommon statically to cmake: link ceph-{mgr,mon,mds,osd} against libcommon statically Jan 20, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jan 20, 2017

retest this please (jenkins agent issue)

@tchaikov

This comment has been minimized.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jan 20, 2017

@cbodley fixed and repushed.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jan 24, 2017

@cbodley do you mind taking another look? thanks!

@cbodley

This comment has been minimized.

Contributor

cbodley commented Jan 25, 2017

@tchaikov i understand the first patch that adds a version to libceph-common for dpkg-shlibdeps

could you please clarify what motivated the later changes to the linkage of ceph daemons? can they not just link against ceph-common to satisfy their dependency on libglobal?

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jan 25, 2017

@cbodley the motivation behind the the changes to the linkage of ceph-daemos is, i don't want the ceph-daemons to depend on librados2, which offers the libceph-common.

let's take ceph-osd's debian package as an example:

Package: ceph-osd
Architecture: linux-any
Depends: ceph-base (= ${binary:Version}),
         parted,
         ${misc:Depends},
         ${python:Depends},
         ${shlibs:Depends}

and ceph-base offers the plugins and rados classes.

currently, libceph-common is included by librados2. and librados2 is supposed to be used by a rados client. so it should not depend on any of the rados plugins or rados classes, see http://packages.ubuntu.com/yakkety/librados2. if we include libceph-common in a package depended by both client and server, we would need to put it into a dedicated package. because there is no existing ceph packages can fit in this role. ceph-common includes essential ceph tools and plugins which are not used by clients. so i decide to link the daemons with libceph-common statically.

Package: librados2
Conflicts: librados, librados1
Replaces: librados, librados1
Architecture: linux-any
Section: libs
Depends: ${misc:Depends}, ${shlibs:Depends}
Description: RADOS distributed object store client library
 RADOS is a reliable, autonomic distributed object storage cluster
 developed as part of the Ceph distributed storage system.  This is a
 shared library allowing applications to access the distributed object
 store using a simple file-like interface.

libcephfs2 depends on librados2 via ${shlibs:Depends}.

Package: libcephfs2
Conflicts: libceph, libceph1, libcephfs
Replaces: libceph, libceph1, libcephfs
Architecture: linux-any
Section: libs
Depends: ${misc:Depends}, ${shlibs:Depends}
Description: Ceph distributed file system client library
 Ceph is a massively scalable, open-source, distributed
 storage system that runs on commodity hardware and delivers object,
 block and file system storage.  This is a
 shared library allowing applications to access a Ceph distributed
 file system via a POSIX-like interface.

@tchaikov tchaikov merged commit 8a86a3f into ceph:master Jan 26, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@tchaikov tchaikov deleted the tchaikov:wip-cmake branch Jan 26, 2017

@tchaikov tchaikov assigned cbodley and unassigned tchaikov Jan 26, 2017

@jdurgin

This comment has been minimized.

Member

jdurgin commented Jan 27, 2017

this broke the centos package build: http://tracker.ceph.com/issues/18692

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jan 27, 2017

fix posted at #13148

Adirl pushed a commit to Adirl/ceph that referenced this pull request Jan 30, 2017

cmake: fix broken RDMA compilation after merge PR ceph#12878
issue: 965984

Change-Id: I2e401acf204069d3907001d276c58d9fb4cd0d58
Signed-off-by: Oren Duer <oren@mellanox.com>

tchaikov added a commit that referenced this pull request Jan 30, 2017

Merge pull request #13186 from Adirl/fix_rdma_compile
cmake: fix broken RDMA compilation after merge PR #12878

Reviewed-by: Kefu Chai <kchai@redhat.com>

chardan added a commit to chardan/ceph that referenced this pull request Feb 3, 2017

cmake: fix broken RDMA compilation after merge PR ceph#12878
issue: 965984

Change-Id: I2e401acf204069d3907001d276c58d9fb4cd0d58
Signed-off-by: Oren Duer <oren@mellanox.com>

jcsp pushed a commit to jcsp/ceph that referenced this pull request Feb 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment