Skip to content
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: fix the build of tests #7523

Merged
merged 4 commits into from Feb 11, 2016
Merged

Conversation

tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Feb 4, 2016

perfglue/heap_profiler.cc is referncing boost-system somehow, and
this file is compiled when we are using tcmalloc, so added it to
ALLOC_LIBS along with tcmalloc.

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

@tchaikov
Copy link
Contributor Author

tchaikov commented Feb 4, 2016

@cbodley again, could you help me on this? thanks =)

i think this change was introduced by the graylog changes in #7190, i failed to reproduce the compilation error spotted by @irq0 . but it popped up in our gitbuilder at last.

@cbodley
Copy link
Contributor

cbodley commented Feb 4, 2016

@tchaikov do you have a link to the gitbuilder log?

@tchaikov
Copy link
Contributor Author

tchaikov commented Feb 4, 2016

@cbodley http://gitbuilder.sepia.ceph.com/gitbuilder-ceph-tarball-trusty-amd64-cmake/log.cgi?log=995a6029bbaf6bdf1e5614bb07a2182187e11884 .

[ 80%] /usr/bin/ld: ../CMakeFiles/heap_profiler_objs.dir/perfglue/heap_profiler.cc.o: undefined reference to symbol '_ZN5boost6system15system_categoryEv'
//usr/lib/x86_64-linux-gnu/libboost_system.so.1.54.0: error adding symbols: DSO missing from command line
error: collect2: ld returned 1 exit status
Building CXX object src/CMakeFiles/librbd.dir/librbd/AsyncOperation.cc.o
make[2]: *** [src/test/test_rados_api_c_read_operations] Error 1

@cbodley
Copy link
Contributor

cbodley commented Feb 4, 2016

thanks @tchaikov. i also can't figure out why boost_system is getting pulled in - a grep for 'system_category' on our source tree came up empty.

i get the feeling the cmake build is getting this heap_profiler stuff horribly wrong. i read through the automake files, and all they do is build a libperfglue.la and add it to LIBOSD/LIBMON/LIBMDS. we should eventually do an audit on this stuff in cmake to make sure we get it right, but i'm fine with merging your fix in the meantime

@tchaikov
Copy link
Contributor Author

tchaikov commented Feb 5, 2016

@cbodley root caused and fixed it.

@tchaikov tchaikov changed the title cmake: add boost-system to ALLOC_LIBS cmake: fix the build of tests Feb 5, 2016
@tchaikov
Copy link
Contributor Author

tchaikov commented Feb 6, 2016

hey @alimaredia could you help review it?

@alimaredia
Copy link
Contributor

@mattbenjamin didn't you say we needed to do the two points Kefu mentioned in his previous post?

@tchaikov
Copy link
Contributor Author

tchaikov commented Feb 8, 2016

@cbodley
Copy link
Contributor

cbodley commented Feb 8, 2016

@tchaikov good catch!

@alimaredia those two changes are no longer needed after this fix

@@ -76,8 +76,7 @@ else()
endif()


set(EXTRALIBS uuid rt dl
${Boost_LIBRARIES} ${Boost_SYSTEM_LIBRARY} ${ATOMIC_OPS_LIBRARIES})
set(EXTRALIBS uuid rt dl ${Boost_LIBS} ${ATOMIC_OPS_LIBRARIES})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tchaikov ${Boost_LIBS} neither in /usr/share/cmake/modules/FindBoost.cmake nor anywhere else in the source tree. It should be changed to ${Boost_LIBRARIES}

This reverts commit 21438a6.

Signed-off-by: Kefu Chai <kchai@redhat.com>
Signed-off-by: Kefu Chai <kchai@redhat.com>
graylog uses boost/asio.hpp which introduces the link-time dependency on
libboost_system to the compilation units which includes Log.h and
LogClient.h. so it appears that perfglue/heap_profiler.cc is referencing
libboost_system.so, and fails the cmake build of all tests which links
against tcmalloc.

in this change, we:

* remove unnecessary #includes from Graylog.h
* forward declare Graylog class, so that "Graylog.h" is not included in
  any header files to avoid the link-time dependency pollution

Signed-off-by: Kefu Chai <kchai@redhat.com>
* and remove ${Boost_SYSTEM_LIBRARY} from test_rados_api_tier

Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov
Copy link
Contributor Author

@alimaredia could you take a look again? I just removed ${Boost_LIBS} .

@tchaikov
Copy link
Contributor Author

alimaredia added a commit that referenced this pull request Feb 11, 2016
@alimaredia alimaredia merged commit aa6d0a9 into ceph:master Feb 11, 2016
@tchaikov tchaikov deleted the wip-fix-cmake branch February 11, 2016 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants