> On Feb. 10, 2011, 2:02 a.m., Boroondas Gupte wrote: > > indra/cmake/GoogleMock.cmake, lines 13-14 > > <http://codereview.secondlife.com/r/147/diff/1/?file=842#file842line13> > > > > I'd put the > > -Wl,--no-as-needed > > down on gtest's line, so that the "wrapping" is more obvious. Or maybe > > even use some creative indentation like: > > # ... > > if (LINUX) > > set(GOOGLEMOCK_LIBRARIES > > gmock > > -Wl,--no-as-needed # VWR-24366: gmock is underlinked, it needs > > gtest. > > gtest > > -Wl,--as-needed) # continue to use "as-needed" for other libs > > elseif(WINDOWS) > > # ...
I don't think you can add comments like that in the middle of a SET( ... # ... ) ? Never tried it though. If GOOGLEMOCK_LIBRARIES would end up like "gmock -Wl,--no-as-needed # VWR-24366: gmock is underlinked, it needs gtest. gtest -Wl,--as-needed" then that definitely will break things ;) @merov: https://codereview.secondlife.com/r/95/ is still open and present on this reviewboard. It's about this same patch? - Aleric ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/147/#review350 ----------------------------------------------------------- On Feb. 9, 2011, 10:20 p.m., Merov Linden wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/147/ > ----------------------------------------------------------- > > (Updated Feb. 9, 2011, 10:20 p.m.) > > > Review request for Viewer. > > > Summary > ------- > > From Aleric's patch: > > Setting CMAKE_EXE_LINKER_FLAGS to "" because the tests "need" that is pretty > hard measure. Not only is it not necessary to do so, it also changes how the > viewer is linked depending on a whether or not the tests are compiled and > that is not good. > > The reason that this was needed is that libgmock is underlinked (see > http://wiki.mandriva.com/en/Underlinking), which is not compatible with > -Wl,--as-needed that is being used on linux. libgmock.so.0 needs a symbol > that is defined in libgtest.so.o, but -lgtest was not passed to the linker > when creating libgmock.so.0: > > Underlinked (no libgtest.so.o): > $ objdump -p /usr/lib/libgmock.so.0 | grep NEEDED > NEEDED libstdc++.so.6 > NEEDED libm.so.6 > NEEDED libc.so.6 > NEEDED libgcc_s.so.1 > > The solution is to wrap between -Wl,--no-as-needed -lgtest -Wl,--as-needed > causing it to be added again. This is only needed on linux, since that the > only platform that we use -Wl,--as-needed on. Moreover, we can just set > GOOGLEMOCK_LIBRARIES to "gmock -Wl,--no-as-needed gtest -Wl,--as-needed" > since that is only passed to TARGET_LINK_LIBRARIES which only adds -l in > front of 'things' that don't start with '-', to allow you do pass special > flags like this. > > > This addresses bug STORM-981. > http://jira.secondlife.com/browse/STORM-981 > > > Diffs > ----- > > doc/contributions.txt 27dae7b01a81 > indra/cmake/GoogleMock.cmake 27dae7b01a81 > indra/cmake/LLAddBuildTest.cmake 27dae7b01a81 > > Diff: http://codereview.secondlife.com/r/147/diff > > > Testing > ------- > > > Thanks, > > Merov > >
_______________________________________________ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
