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: rgw: do not link against boost in a wholesale #15347

Merged
merged 1 commit into from Jun 3, 2017

Conversation

Projects
None yet
4 participants
@tchaikov
Contributor

tchaikov commented May 29, 2017

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

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 29, 2017

I guess it's better to keep the PRs separate :-/

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 29, 2017

@smithfarm yeah, either way =)

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 29, 2017

Inclusion of this patch in #15225 appeared to cause build to fail, that's why I dropped it.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 29, 2017

okay, we can get #15225 in first. then let me worry about this one.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented May 29, 2017

@tchaikov
Not quite sure on how this is going to fxi my -lpython problem completely.
I see:

./CMakeLists.txt:638:# target_link_libraries(common json_spirit common_utf8 erasure_code rt uuid ${LIB_RESOLV} ${CRYPTO_LIBS} ${Boost_LIBRARIES} ${BLKID_LIBRARIES} ${EXECINFO_LIBRARIES} ${BLKIN_LIBRARIES})
./rgw/CMakeLists.txt:147:  ${CURL_LIBRARIES} ${Boost_LIBRARIES}

This way this list still suffers from my problem: libboost_python is on the list and not again linked to -lpython generating linking errors.

Where as I'm trying to prevent all this by removing libboost_python from the list.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 30, 2017

@wjwithagen

./CMakeLists.txt:638:# target_link_libraries(common json_spirit common_utf8 erasure_code rt uuid ${LIB_RESOLV} ${CRYPTO_LIBS} ${Boost_LIBRARIES} ${BLKID_LIBRARIES} ${EXECINFO_LIBRARIES} ${BLKIN_LIBRARIES})
./rgw/CMakeLists.txt:147:  ${CURL_LIBRARIES} ${Boost_LIBRARIES}

is addressed by
https://github.com/ceph/ceph/pull/15225/files#diff-95e351a3805a1dafa85bf20b81d086e6L640

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 30, 2017

@tchaikov Please rebase; I will test in OBS.

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 30, 2017

@tchaikov Nevermind - I'll cherry-pick for the OBS test, but for Shaman test you should rebase.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 30, 2017

@smithfarm rebased and pushed to ci as wip-15347-kefu.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 1, 2017

@smithfarm the branch builds fine on shaman, what about OBS?

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 1, 2017

@tchaikov Sorry for the latency. On it now.

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 1, 2017

It takes quite some time for OBS to build for all the target platforms/architectures. Expect to hear the result no earlier than tomorrow.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 1, 2017

@smithfarm thanks for your time!

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 2, 2017

@tchaikov The SUSE build fails with the following error:

[ 2264s] [100%] Linking CXX executable ../../bin/radosgw
[ 2270s] ../../boost/lib/libboost_coroutine.a(coroutine_context.o): In function `boost::coroutines::detail::coroutine_context::coroutine_context(void (*)(boost::context::detail::transfer_t), boost::coroutines::detail::preallocated const&)':
[ 2270s] coroutine_context.cpp:(.text+0x59): undefined reference to `make_fcontext'
[ 2270s] ../../boost/lib/libboost_coroutine.a(coroutine_context.o): In function `boost::coroutines::detail::coroutine_context::jump(boost::coroutines::detail::coroutine_context&, void*)':
[ 2270s] coroutine_context.cpp:(.text+0xe8): undefined reference to `jump_fcontext'
[ 2270s] collect2: error: ld returned 1 exit status
[ 2270s] src/rgw/CMakeFiles/radosgw.dir/build.make:159: recipe for target 'bin/radosgw' failed
[ 2270s] make[2]: *** [bin/radosgw] Error 1
[ 2270s] CMakeFiles/Makefile2:7841: recipe for target 'src/rgw/CMakeFiles/radosgw.dir/all' failed
[ 2270s] make[1]: *** [src/rgw/CMakeFiles/radosgw.dir/all] Error 2
[ 2270s] make[1]: *** Waiting for unfinished jobs.... 

I notice that the SUSE verion of FindBoost.cmake does not appear to set Boost_<C> at all. Where are these variables Boost_COROUTINE and Boost_CONTEXT supposed to be getting set?

Could we use Boost_<C>_LIBRARY instead, and possibly guard it with Boost_<C>_FOUND?

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 2, 2017

i don't think we need to protect them with Boost_<C>_FOUND, as find_package(Boost 1.61 COMPONENTS ${BOOST_COMPONENTS} **REQUIRED**) has taken care of it.

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 2, 2017

@tchaikov Getting the same failure now with ceph_test_cephd_api_misc -> tchaikov#2

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 2, 2017

We verified that linking ceph_test_cephd_api_misc with ${Boost_LIBRARIES} fixes it, now testing this version.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 2, 2017

@smithfarm i am fixing it in a slightly different way. does it make sense to you? and i also added your "Signed-off-by" line. is it fine with you?

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 2, 2017

@tchaikov You did what I was trying to do - I trust it's OK. We're testing it now.

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 2, 2017

[100%] Linking CXX executable ../../bin/radosgw-admin
../../boost/lib/libboost_coroutine.a(coroutine_context.o): In function `boost::coroutines::detail::coroutine_context::coroutine_context(void (*)(boost::context::detail::transfer_t), boost::coroutines::detail::preallocated const&)':
coroutine_context.cpp:(.text+0x59): undefined reference to `make_fcontext'
../../boost/lib/libboost_coroutine.a(coroutine_context.o): In function `boost::coroutines::detail::coroutine_context::jump(boost::coroutines::detail::coroutine_context&, void*)':
coroutine_context.cpp:(.text+0xe5): undefined reference to `jump_fcontext'
collect2: error: ld returned 1 exit status
src/rgw/CMakeFiles/radosgw.dir/build.make:161: recipe for target 'bin/radosgw' failed
make[2]: *** [bin/radosgw] Error 1
CMakeFiles/Makefile2:21995: recipe for target 'src/rgw/CMakeFiles/radosgw.dir/all' failed
make[1]: *** [src/rgw/CMakeFiles/radosgw.dir/all] Error 2
@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 2, 2017

So rgw_admin.cc does use the RGWCoroutinesManager class -> https://github.com/ceph/ceph/blob/master/src/rgw/rgw_admin.cc#L6240

@cbodley

This comment has been minimized.

Contributor

cbodley commented Jun 2, 2017

@smithfarm RGWCoroutinesManager and friends in rgw_coroutine.h are based on boost::asio::coroutine from boost/asio.hpp, which is completely separate from the boost coroutines library

the only thing that should depend on the coroutines/context libs is the boost/asio/spawn.hpp header used by rgw_asio_frontend.cc

(edit: in your output above, it was the radosgw target failing to link, not radosgw-admin)

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 3, 2017

@cbodley Thanks for the hints!

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 3, 2017

@smithfarm thanks for your efforts! squashed and repushed.

target_link_libraries(cephd_rgw LINK_PRIVATE
${Boost_COROUTINE_LIBRARY}
${Boost_CONTEXT_LIBRARY})
endif()

This comment has been minimized.

@smithfarm

smithfarm Jun 3, 2017

Contributor

I suspect this stanza is unnecessary. Running a -DWITH_EMBEDDED=ON test build without the stanza.

This comment has been minimized.

@tchaikov

tchaikov Jun 3, 2017

Contributor

you mean the whole

    target_link_libraries(cephd_rgw LINK_PRIVATE
      ${Boost_COROUTINE_LIBRARY}
      ${Boost_CONTEXT_LIBRARY})

block?

This comment has been minimized.

@smithfarm

smithfarm Jun 3, 2017

Contributor

Yup. I built with ./do_cmake.sh -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_INSTALL_LIBDIR=/usr/lib64 -DCMAKE_INSTALL_LIBEXECDIR=/usr/lib -DCMAKE_INSTALL_LOCALSTATEDIR=/var -DCMAKE_INSTALL_SYSCONFDIR=/etc -DCMAKE_INSTALL_MANDIR=/usr/share/man -DCMAKE_INSTALL_DOCDIR=/usr/share/doc/packages/ceph -DWITH_EMBEDDED=ON -DWITH_MANPAGE=ON -DWITH_PYTHON3=ON -DWITH_SYSTEMD=ON -DWITH_TESTS=OFF -DWITH_LTTNG=ON -DHAVE_BABELTRACE=ON -DWITH_OCF=ON -DWITH_RADOSGW_BEAST_FRONTEND=ON -DBOOST_J=8 and it succeeds without this block.

This comment has been minimized.

@smithfarm

smithfarm Jun 3, 2017

Contributor

Oh, wait - need to include -DWITH_TESTS=ON . . . let me do another build.

This comment has been minimized.

@tchaikov

tchaikov Jun 3, 2017

Contributor

yes, need to make sure the ceph_test_cephd_api_misc links.

This comment has been minimized.

@smithfarm

smithfarm Jun 3, 2017

Contributor

@tchaikov: Confirmed, see tchaikov#4

@smithfarm

Yay!

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 3, 2017

pushed to ceph-ci as wip-15347-kefu

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 3, 2017

Nit: maybe change title to cmake: rgw: do not link against boost in a wholesale

cmake: rgw: do not link against boost in a wholesale
With the new Beast frontend, RGW now has a small Boost dependency [1] which was
being addressed by statically (and unconditionally) linking *all* the Boost
libraries. This patch ensures that only the necessary Boost components are
linked.

We use the target_link_libraries(<target> <item>...) [2] syntax to ensure that the
library dependencies are transitive: i.e. "when this target is linked into
another target then the libraries linked to this target will appear on the link
line for the other target too."

[1] The boost/asio/spawn.hpp header used by rgw_asio_frontend.cc depends on
    boost::coroutine/boost::context

[2] https://cmake.org/cmake/help/v3.3/command/target_link_libraries.html#libraries-for-both-a-target-and-its-dependents

Signed-off-by: Nathan Cutler <ncutler@suse.com>
Signed-off-by: Kefu Chai <kchai@redhat.com>

@tchaikov tchaikov changed the title from cmake: do not link against boost in a wholesale to cmake: rgw: do not link against boost in a wholesale Jun 3, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 3, 2017

done!

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 3, 2017

make: *** [all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.HV6F3b (%build)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.HV6F3b (%build)

https://shaman.ceph.com/builds/ceph/wip-15347-kefu/801b06fa3f35d851de525343effcc2df4101dca9/notcmalloc/45824/

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 3, 2017

@tchaikov Yeah, that failure is very strange. See further up:

[ 44%] Building CXX object src/mds/CMakeFiles/mds.dir/Migrator.cc.o
{standard input}: Assembler messages:
{standard input}:406703: Warning: partial line at end of file ignored
{standard input}:406649: Error: invalid operands (*UND* and .gcc_except_table sections) for `-'
c++: internal compiler error: Killed (program cc1plus)
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://bugzilla.redhat.com/bugzilla> for instructions.
make[2]: *** [src/mds/CMakeFiles/mds.dir/MDCache.cc.o] Error 4
make[2]: *** Waiting for unfinished jobs....
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 3, 2017

https://shaman.ceph.com/builds/ceph/wip-15347-kefu/ce789f6f1229836d8f1b23071df4e417d2e757c9/default/45837/

[ 86%] Building CXX object src/tools/cephfs/CMakeFiles/cephfs-journal-tool.dir/JournalFilter.cc.o
{standard input}: Assembler messages:
{standard input}:206621: Warning: end of file not at end of a line; newline inserted
{standard input}:207482: Error: character following name is not '#'
c++: internal compiler error: Killed (program cc1plus)
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://bugzilla.redhat.com/bugzilla> for instructions.
make[2]: *** [src/test/rbd_mirror/CMakeFiles/unittest_rbd_mirror.dir/image_replayer/test_mock_BootstrapRequest.cc.o] Error 4

again, mysterious error. i am inclined to believe this is an env issue.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 3, 2017

and this also happens with wip-bluestore branch

[ 40%] Built target common-objs
make: *** [all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.TNtJsM (%build)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.TNtJsM (%build)
+ rm -fr /tmp/install-deps.20824
Build step 'Execute shell' marked build as failure

https://shaman.ceph.com/builds/ceph/wip-bluestore/ad9bd66ea3873c91ad12e99b75f3c9408167ce31/default/45828/

@yuriw i vaguely remember that you were mumbling about the same problem on IRC last week.

@alfredodeza is it possible it's an issue in the jenkins builder?

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 3, 2017

Yup, other branches are suffering from the same problem - in addition to the one @tchaikov just posted, there are plenty of others - e.g. https://shaman.ceph.com/builds/ceph/wip-mgolub-testing/afdb09bc1603b64b73aeb2598703743f51794194/notcmalloc/45816/

@tchaikov tchaikov merged commit 97d6e15 into ceph:master Jun 3, 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 Jun 3, 2017

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Jun 3, 2017

Looks like it started 14 hours ago (about 6:00 p.m. EST Friday June 2) with this build failure: https://shaman.ceph.com/builds/ceph/wip-sage-testing2/f93ad23a8fec219667a03695136842edb0cceace/default/45729/

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 3, 2017

[ 14%] Building CXX object src/cls/CMakeFiles/cls_cephfs.dir/cephfs/cls_cephfs.cc.o
{standard input}: Assembler messages:
{standard input}:186778: Warning: end of file not at end of a line; newline inserted
{standard input}: Error: open CFI at the end of file; missing .cfi_endproc directive
c++: internal compiler error: Killed (program cc1plus)
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://bugzilla.redhat.com/bugzilla> for instructions.
make[2]: *** [src/mon/CMakeFiles/osd_mon_objs.dir/Monitor.cc.o] Error 4
make[1]: *** [src/mon/CMakeFiles/osd_mon_objs.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....

yeah, it does not look right. and we need to fix it.

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