Bug 90973

Created attachment 151680 [details] Patch

Comment on attachment 151680 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=151680&action=review I wonder if this is really needed -- gtest is only needed by unit tests, whose linking time isn't very important, and pthreads would need to be present anyway for gtest to be built. > Source/cmake/gtest/CMakeLists.txt:26 > +IF(CMAKE_USE_PTHREADS_INIT) You need FIND_PACKAGE(Threads) to use this. Plus, shouldn't you just always link to the thread library?

Comment on attachment 151680 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=151680&action=review >> Source/cmake/gtest/CMakeLists.txt:26 >> +IF(CMAKE_USE_PTHREADS_INIT) > > You need FIND_PACKAGE(Threads) to use this. Plus, shouldn't you just always link to the thread library? FIND_PACKAGE(Threads) is done at Options${Platform} like the other FIND_PACKAGEs. pthreads won't happen if you are building on Windows, remember that we are not the only customers of CMake build system and I want to make this reusable.

Comment on attachment 151680 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=151680&action=review >>> Source/cmake/gtest/CMakeLists.txt:26 >>> +IF(CMAKE_USE_PTHREADS_INIT) >> >> You need FIND_PACKAGE(Threads) to use this. Plus, shouldn't you just always link to the thread library? > > FIND_PACKAGE(Threads) is done at Options${Platform} like the other FIND_PACKAGEs. > > pthreads won't happen if you are building on Windows, remember that we are not the only customers of CMake build system and I want to make this reusable. Hmm, but don't you need to link to the equivalent thread library in other systems as well?

Created attachment 152071 [details] Patch Rebased.

(In reply to comment #4) > (From update of attachment 151680 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=151680&action=review > > >>> Source/cmake/gtest/CMakeLists.txt:26 > >>> +IF(CMAKE_USE_PTHREADS_INIT) > >> > >> You need FIND_PACKAGE(Threads) to use this. Plus, shouldn't you just always link to the thread library? > > > > FIND_PACKAGE(Threads) is done at Options${Platform} like the other FIND_PACKAGEs. > > > > pthreads won't happen if you are building on Windows, remember that we are not the only customers of CMake build system and I want to make this reusable. > > Hmm, but don't you need to link to the equivalent thread library in other systems as well? I don't think threading support is mandatory, but don't expect for example that thread-safe death tests will be available in this case.

Comment on attachment 152071 [details] Patch Attachment 152071 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13180986

Created attachment 152512 [details] Patch

Comment on attachment 152512 [details] Patch Attachment 152512 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13253265

(In reply to comment #9) > (From update of attachment 152512 [details]) > Attachment 152512 [details] did not pass efl-ews (efl): > Output: http://queues.webkit.org/results/13253265 [ 90%] /usr/bin/ld: ../../bin/test_ewk_view: hidden symbol `Building CXX object Source/WebKit2/CMakeFiles/ewebkit2.dir/UIProcess/WebGrammarDetail.cpp.o WTF::fastMalloc(unsigned long)' in ../../lib/libwtf_efl.a(FastMalloc.cpp.o) is referenced by DSO /usr/bin/ld: final link failed: Bad value Weird, I don't get this error locally.

Created attachment 152532 [details] Patch

Comment on attachment 152532 [details] Patch Attachment 152532 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13242970

Created attachment 152535 [details] Patch One more attempt to make EFL EWS happy. I really could not reproduce this linking error. :(

Comment on attachment 152535 [details] Patch Attachment 152535 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13243988

Created attachment 152555 [details] Patch One more attempt.

(In reply to comment #15) > Created an attachment (id=152555) [details] > Patch > > One more attempt. EWS is sane. Don't know why I was not having this linking errors on my desktop. The problem was gtest has now to link with WTF, since it uses it wrappers for memory allocation.

Comment on attachment 152555 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152555&action=review r=me > ChangeLog:10 > + dependencies like pthreads (and avoid linking errors when it is not Nit: "and avoid" => "which produces" OR "which causes"

Created attachment 152970 [details] Patch Updated patch. Thanks for reviewing.

Comment on attachment 152970 [details] Patch Clearing flags on attachment: 152970 Committed r122943: <http://trac.webkit.org/changeset/122943>

All reviewed patches have been landed. Closing bug.