Bug 93191 – [CMake] Rewrite FindLibSoup2.cmake.

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Raphael Kubo da Costa (:rakuco)]

[CMake] Rewrite FindLibSoup2.cmake.

[Raphael Kubo da Costa (:rakuco)]

Created attachment 156548 [details]
Patch

[Thiago Marcos P. Santos]

Comment on attachment 156548 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=156548&action=review

> ChangeLog:17
> +        installed by, say, jhbuild.

This is what I don't get. This *sometimes* never happened to me. Would be nice if you could describe an example.

If you are building with cmake + jhbuild, it always links with the libs downloaded by jhbuild. If you are build using cmake only, it links with the system deps, but that is what you want.

[Thiago Marcos P. Santos]

Comment on attachment 156548 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=156548&action=review

>> ChangeLog:17
>> +        For one, this meant "-lsoup-2.4" was passed to the linked instead
>> +        of "-L/path/to/libsoup-2.4.so", which would sometimes make a
>> +        system version of libsoup to be picked up instead of the one
>> +        installed by, say, jhbuild.
> 
> This is what I don't get. This *sometimes* never happened to me. Would be nice if you could describe an example.
> 
> If you are building with cmake + jhbuild, it always links with the libs downloaded by jhbuild. If you are build using cmake only, it links with the system deps, but that is what you want.

This is what I don't get. This *sometimes* never happened to me. Would be nice if you could describe an example.

If you are building with cmake + jhbuild, it always links with the libs downloaded by jhbuild. If you are build using cmake only, it links with the system deps, but that is what you want.

[Patrick R. Gansterer]

Comment on attachment 156548 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=156548&action=review

> Source/cmake/FindLibSoup.cmake:31
> +FIND_PACKAGE(PkgConfig)

isn't it possible to write this without PkConfig? I don't like this dependency.

> Source/cmake/FindLibSoup.cmake:32
> +PKG_CHECK_MODULES(PC_LIBSOUP libsoup-2.4) # FIXME: After we require CMake 2.8.2 we can pass QUIET to this call.

Which CMake version do you use?

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 156548 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=156548&action=review

>> Source/cmake/FindLibSoup.cmake:31
>> +FIND_PACKAGE(PkgConfig)
> 
> isn't it possible to write this without PkConfig? I don't like this dependency.

pkg-config is necessary to obtain the package's version from the .pc file. If that is not needed (ie. a version is not passed to the FIND_PACKAGE call), then pkg-config is not needed; see how its values are only used as HINTS to the FIND_PATH and FIND_LIBRARY calls.

>> Source/cmake/FindLibSoup.cmake:32
>> +PKG_CHECK_MODULES(PC_LIBSOUP libsoup-2.4) # FIXME: After we require CMake 2.8.2 we can pass QUIET to this call.
> 
> Which CMake version do you use?

I use a recent version, however the minimum we require in the top-level CMakeLists.txt is 2.8.0. We could bump that requirement, but I'd rather not depend on this to get this patch reviewed, as I'm not sure how this version requirement plays out with the BlackBerry folks, for example.

[Thiago Marcos P. Santos]

Comment on attachment 156548 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=156548&action=review

>>> Source/cmake/FindLibSoup.cmake:32
>>> +FIND_PACKAGE(PkgConfig)
>>> +PKG_CHECK_MODULES(PC_LIBSOUP libsoup-2.4) # FIXME: After we require CMake 2.8.2 we can pass QUIET to this call.
>> 
>> Which CMake version do you use?
> 
> I use a recent version, however the minimum we require in the top-level CMakeLists.txt is 2.8.0. We could bump that requirement, but I'd rather not depend on this to get this patch reviewed, as I'm not sure how this version requirement plays out with the BlackBerry folks, for example.

What type of system we are trying to support that doesn't support pkg-config? If the library provides a .pc, why not use?

[Patrick R. Gansterer]

Comment on attachment 156548 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=156548&action=review

>>> Source/cmake/FindLibSoup.cmake:31
>>> +FIND_PACKAGE(PkgConfig)
>> 
>> isn't it possible to write this without PkConfig? I don't like this dependency.
> 
> pkg-config is necessary to obtain the package's version from the .pc file. If that is not needed (ie. a version is not passed to the FIND_PACKAGE call), then pkg-config is not needed; see how its values are only used as HINTS to the FIND_PATH and FIND_LIBRARY calls.

If required then FIND_PACKGAGE() needs a REQUIRED.

>>>> Source/cmake/FindLibSoup.cmake:32
>>>> +PKG_CHECK_MODULES(PC_LIBSOUP libsoup-2.4) # FIXME: After we require CMake 2.8.2 we can pass QUIET to this call.
>>> 
>>> Which CMake version do you use?
>> 
>> I use a recent version, however the minimum we require in the top-level CMakeLists.txt is 2.8.0. We could bump that requirement, but I'd rather not depend on this to get this patch reviewed, as I'm not sure how this version requirement plays out with the BlackBerry folks, for example.
> 
> What type of system we are trying to support that doesn't support pkg-config? If the library provides a .pc, why not use?

I don't know the libsoup api. But most libraries i use provide a macro with the version, or at a function which returns the version.
pkg-config is an additional dependency, which I'd like to avoid if possible. But if the .pc file is the only place where the version is stored, it's ok to depend on it.

[Raphael Kubo da Costa (:rakuco)]

Created attachment 157858 [details]
New patch now that we require CMake 2.8.3

[Gyuyoung Kim]

Comment on attachment 157858 [details]
New patch now that we require CMake 2.8.3

Attachment 157858 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13478442

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #3)
> This is what I don't get. This *sometimes* never happened to me. Would be nice if you could describe an example.
>
> If you are building with cmake + jhbuild, it always links with the libs downloaded by jhbuild. If you are build using cmake only, it links with the system deps, but that is what you want.

Case in point I am facing right now (it's not related to libsoup itself, but the cause is the same): building the EFL port on Arch Linux with tests enabled fails because linking libgtest fails:

    Linking CXX shared library ../../../lib/libgtest.so
    cd /home/rakuco/dev/webkit/WebKit/WebKitBuild/Release/Source/cmake/gtest && /usr/bin/cmake -E cmake_link_script CMakeFiles/gtest.dir/link.txt --verbose=1
    /usr/lib/ccache/bin/g++  -fPIC -O3 -DNDEBUG -Wl,--as-needed -Wl,--no-copy-dt-needed-entries -shared -Wl,-soname,libgtest.so -o ../../../lib/libgtest.so CMakeFiles/gtest.dir/__/__/ThirdParty/gtest/src/gtest.cc.o CMakeFiles/gtest.dir/__/__/ThirdParty/gtest/src/gtest-death-test.cc.o CMakeFiles/gtest.dir/__/__/ThirdParty/gtest/src/gtest-filepath.cc.o CMakeFiles/gtest.dir/__/__/ThirdParty/gtest/src/gtest_main.cc.o CMakeFiles/gtest.dir/__/__/ThirdParty/gtest/src/gtest-port.cc.o CMakeFiles/gtest.dir/__/__/ThirdParty/gtest/src/gtest-test-part.cc.o CMakeFiles/gtest.dir/__/__/ThirdParty/gtest/src/gtest-typed-test.cc.o ../../../lib/libwtf_efl.a -lpthread -lpthread ../../../../Dependencies/Root/lib64/libglib-2.0.so ../../../../Dependencies/Root/lib64/libgobject-2.0.so -licuuc -licui18n -lecore -leina -lecore_evas -levas -leina -lecore_evas -levas -lrt -ldl -Wl,-rpath,/home/rakuco/dev/webkit/WebKit/WebKitBuild/Dependencies/Root/lib64
    /usr/bin/ld: cannot find -lecore
    /usr/bin/ld: cannot find -leina
    /usr/bin/ld: cannot find -lecore_evas
    /usr/bin/ld: cannot find -levas
    /usr/bin/ld: cannot find -leina
    /usr/bin/ld: cannot find -lecore_evas
    /usr/bin/ld: cannot find -levas

The glib and EFL dependencies come via the TARGET_LINK_LIBRARIES(gtest ${WTF_LIBRARY_NAME}) call in Source/cmake/gtest/CMakeLists, since library dependencies are transitive by default.

However, since the EFL libraries were only found via PKG_CHECK_MODULES(), ${EINA_LIBRARIES}, for example, instead of containing an absolute path, is set to simply "eina". Since $WEBKITOUTPUTDIR/Dependencies/Root/lib64 is not in the compiler's default path, the linking fails (this could be worked around by setting the LIBRARY_PATH environment variable, but this would just pile up another hack instead of fixing the problem).

LibSoup's case is similar, as only "-lsoup-2.4" is passed to the linker, but, contrary to what's common with the EFL, there's usually a libsoup package installed on the user's system in a directory that the linker searches by default, such as /usr/lib. This version is then picked up instead of the version we built with jhbuild.

On other thing to notice is that this problem is particularly prone to happen in these cases of transitive dependencies: in WTF, for example, the problem is avoided because we call SET_TARGET_PROPERTIES(${WTF_LIBRARY_NAME} LINK_FLAGS "${EINA_LDFLAGS}"). ${EINA_LDFLAGS}, on its turn, is another variable set via PKG_CHECK_MODULES() and contains something like "-L/path/to/the/directory/of/;-leina", which is a hackish way of avoiding these problems (even if there the order the -L and -l flags are passed does matter). However, this target property is not transitive and "-L/path/to/the/directory/of" is not passed when linking gtest against WTF.

Bottom line: this could all be avoided by using full paths obtained via FIND_LIBRARY().

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 156548 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=156548&action=review

>>>> Source/cmake/FindLibSoup.cmake:31
>>>> +FIND_PACKAGE(PkgConfig)
>>> 
>>> isn't it possible to write this without PkConfig? I don't like this dependency.
>> 
>> pkg-config is necessary to obtain the package's version from the .pc file. If that is not needed (ie. a version is not passed to the FIND_PACKAGE call), then pkg-config is not needed; see how its values are only used as HINTS to the FIND_PATH and FIND_LIBRARY calls.
> 
> If required then FIND_PACKGAGE() needs a REQUIRED.

PkgConfig is part of CMake's standard modules, so it is always present. I used REQUIRED in the PKG_CHECK_MODULES call in the next iteration of the patch, though.

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #9)
> (From update of attachment 157858 [details])
> Attachment 157858 [details] did not pass efl-ews (efl):
> Output: http://queues.webkit.org/results/13478442

This shows that libgio-2.0.so was being pulled in implicitly by LIBSOUP2_LDFLAGS. Since FindGIO.cmake was not in good shape and was not being used at all, I've rewritten the glib-related find modules in bug 93786.

[Raphael Kubo da Costa (:rakuco)]

Created attachment 158092 [details]
Same patch after r125443

[Gyuyoung Kim]

Comment on attachment 158092 [details]
Same patch after r125443

Attachment 158092 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13481947

[Raphael Kubo da Costa (:rakuco)]

Created attachment 158113 [details]
Now link directly to gobject in WebCore

[Rob Buis]

Comment on attachment 158113 [details]
Now link directly to gobject in WebCore

LGTM.

[Raphael Kubo da Costa (:rakuco)]

Committed r125467: <http://trac.webkit.org/changeset/125467>

[Raphael Kubo da Costa (:rakuco)]

I've landed two-followup fixes for the bots (which run with SHARED_CORE=ON) in <http://trac.webkit.org/changeset/125468> and <http://trac.webkit.org/changeset/125470>. I'm not entirely sure the first one is really needed, but I can investigate that later.