Bug 130195 - [CMake] Failure to link with older installations of WebP
Summary: [CMake] Failure to link with older installations of WebP
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-13 08:23 PDT by Mario Sanchez Prada
Modified: 2014-03-14 03:09 PDT (History)
9 users (show)

See Also:


Attachments
Patch proposal (2.50 KB, patch)
2014-03-13 08:27 PDT, Mario Sanchez Prada
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mario Sanchez Prada 2014-03-13 08:23:32 PDT
I hit the following problem today while trying to build WebKitGTK+ in my Ubuntu 12.10 box, which comes with libwebp 0.1.3-3 installed:

[4676/5658] Linking CXX shared library lib/libwebkitgtk-3.0.so.0.22.0
FAILED: : && /usr/lib/ccache/c++  -fPIC  -std=gnu++0x -fno-omit-frame-pointer -fno-tree-dce -O3 -DNDEBUG  -Wl,--no-undefined  -L/home/SERILOCAL/mario.prada/work/WebKit/WebKitBuild/Dependencies/Root/lib64 -shared -Wl,-soname,libwebkitgtk-3.0.so.0 -o lib/libwebkitgtk-3.0.so.0.22.0 @CMakeFiles/WebKit.rsp  && :
lib/libWebCoreGTK.a(lib/../Source/WebCore/CMakeFiles/WebCore.dir/platform/image-decoders/webp/WEBPImageDecoder.cpp.o):WEBPImageDecoder.cpp:function WebCore::WEBPImageDecoder::clear(): error: undefined reference to 'WebPIDelete'
lib/libWebCoreGTK.a(lib/../Source/WebCore/CMakeFiles/WebCore.dir/platform/image-decoders/webp/WEBPImageDecoder.cpp.o):WEBPImageDecoder.cpp:function WebCore::WEBPImageDecoder::decode(bool): error: undefined reference to 'WebPIUpdate'
lib/libWebCoreGTK.a(lib/../Source/WebCore/CMakeFiles/WebCore.dir/platform/image-decoders/webp/WEBPImageDecoder.cpp.o):WEBPImageDecoder.cpp:function WebCore::WEBPImageDecoder::decode(bool): error: undefined reference to 'WebPGetInfo'
lib/libWebCoreGTK.a(lib/../Source/WebCore/CMakeFiles/WebCore.dir/platform/image-decoders/webp/WEBPImageDecoder.cpp.o):WEBPImageDecoder.cpp:function WebCore::WEBPImageDecoder::decode(bool): error: undefined reference to 'WebPINewRGB'
collect2: error: ld returned 1 exit status


Apparently, the check_include_files() in FindWebP.cmake file is not working as expected, since WEBP_FOUND is always defined to FALSE after calling it, no matter what you put as the header file to check (it even fails when searching for "stdio.h"). This is causing that the -lwebp flag is not being added, thus causing the trouble
Comment 1 Mario Sanchez Prada 2014-03-13 08:27:57 PDT
Created attachment 226595 [details]
Patch proposal

Please review, thanks!
Comment 2 Mario Sanchez Prada 2014-03-13 08:51:40 PDT
Thanks for the review, Gustavo. Still, I'm now adding Ben to CC, as per Martin's suggestion, to get extra feedback before landing the patch (no pun intended, Gustavo! :))
Comment 3 Ben Boeckel 2014-03-13 09:40:50 PDT
Looks good to me.
Comment 4 Mario Sanchez Prada 2014-03-13 10:06:46 PDT
Comment on attachment 226595 [details]
Patch proposal

(In reply to comment #3)
> Looks good to me.

Thanks Ben, now setting the cq+ flag then.

Btw, any theory on why this is needed? As per discussion on IRC, it seems it could be related to CMake's cache, that might not get updated while calling check_include_files(), thus the need of explicitly calling set(). Does that make sense?
Comment 5 Ben Boeckel 2014-03-13 11:01:11 PDT
Following check_include_files down, it turns into a try_compile which stores its result using the cache. It is using the type INTERNAL though, so it should just overwrite whatever was there before. Has there been testing to show WEBP_FOUND before and after the check_include_files versus WEBP_FOUND_HEADER?
Comment 6 WebKit Commit Bot 2014-03-13 18:52:13 PDT
Comment on attachment 226595 [details]
Patch proposal

Clearing flags on attachment: 226595

Committed r165588: <http://trac.webkit.org/changeset/165588>
Comment 7 WebKit Commit Bot 2014-03-13 18:52:18 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Mario Sanchez Prada 2014-03-14 02:58:18 PDT
(In reply to comment #5)
> Following check_include_files down, it turns into a try_compile which stores its result using the cache. It is using the type INTERNAL though, so it should just overwrite whatever was there before. Has there been testing to show WEBP_FOUND before and after the check_include_files versus WEBP_FOUND_HEADER?

Yes, I've tried exactly that and in my lousy experiments (using message()) I observed that WEBP_FOUND was always FALSE before and after calling check_include_files(), even when looking for things such as "stdio.h" or getopts.h".
Comment 9 Csaba Osztrogonác 2014-03-14 03:09:48 PDT
I ran into the same bug and I can confirm that the committed fix is the
proper fix.

The root of the problem is the implementation of check_include_files

MACRO(CHECK_INCLUDE_FILES INCLUDE VARIABLE)
  IF("${VARIABLE}" MATCHES "^${VARIABLE}$") -----------> BANG !!!!!!!
....
  ENDIF("${VARIABLE}" MATCHES "^${VARIABLE}$")
ENDMACRO(CHECK_INCLUDE_FILES)


It is true if and only if the actual VARIABLE argument is undefined,
but "pkg_check_modules(WEBP libwebp)" in Source/cmake/FindWebP.cmake
set WEBP_FOUND to empty string. I confirmed it on a small example.
(But this kind of check only works inside macros. Otherwise it is always
true for undefined variable, empty string, non-empty string too.)