130195 2014-03-13 08:23:32 -0700 [CMake] Failure to link with older installations of WebP 2014-03-14 03:09:48 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 mario webkit-unassigned bunhere commit-queue gustavo gyuyoung.kim mathstuf ossy rakuco sergio webkit-ews oldest_to_newest 990010 0 mario 2014-03-13 08:23:32 -0700 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 990011 1 226595 mario 2014-03-13 08:27:57 -0700 Created attachment 226595 Patch proposal Please review, thanks! 990015 2 mario 2014-03-13 08:51:40 -0700 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! :)) 990036 3 mathstuf 2014-03-13 09:40:50 -0700 Looks good to me. 990047 4 226595 mario 2014-03-13 10:06:46 -0700 Comment on attachment 226595 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? 990068 5 mathstuf 2014-03-13 11:01:11 -0700 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? 990234 6 226595 commit-queue 2014-03-13 18:52:13 -0700 Comment on attachment 226595 Patch proposal Clearing flags on attachment: 226595 Committed r165588: <http://trac.webkit.org/changeset/165588> 990235 7 commit-queue 2014-03-13 18:52:18 -0700 All reviewed patches have been landed. Closing bug. 990373 8 mario 2014-03-14 02:58:18 -0700 (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". 990374 9 ossy 2014-03-14 03:09:48 -0700 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.) 226595 2014-03-13 08:27:57 -0700 2014-03-13 18:52:12 -0700 Patch proposal 0001-2014-03-13-Mario-Sanchez-Prada-mario.prada-samsung.c.patch text/plain 2557 mario RnJvbSA1ZGQxZTkwNDNlYmQxMGJmNDg2M2Q4MWQzMDkzYTM1N2RhYmI2ZmM0IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBNYXJpbyBTYW5jaGV6IFByYWRhIDxtYXJpby5wcmFkYUBzYW1z dW5nLmNvbT4KRGF0ZTogVGh1LCAxMyBNYXIgMjAxNCAxNToyNTo1OCArMDAwMApTdWJqZWN0OiBb UEFUQ0hdIDIwMTQtMDMtMTMgIE1hcmlvIFNhbmNoZXogUHJhZGEgIDxtYXJpby5wcmFkYUBzYW1z dW5nLmNvbT4KCiAgICAgICAgW0NNYWtlXSBGYWlsdXJlIHRvIGxpbmsgd2l0aCBvbGRlciBpbnN0 YWxsYXRpb25zIG9mIFdlYlAKICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MTMwMTk1CgogICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgoKICAg ICAgICBGaXggdGhlIGlzc3VlIGJ5IHVzaW5nIGEgdGVtcG9yYXJ5IHZhcmlhYmxlIGFzIHRoZSBv dXRwdXQgcGFyYW1ldGVyCiAgICAgICAgdG8gY2hlY2tfaW5jbHVkZV9maWxlcygpLCBzZXR0aW5n IFdFQlBfRk9VTkQgYWNjb3JkaW5nbHkgbGF0ZXIuCgogICAgICAgICogU291cmNlL2NtYWtlL0Zp bmRXZWJQLmNtYWtlOiBBdm9pZCByZXVzaW5nIHRoZSBXRUJQX0ZPVU5ECiAgICAgICAgdmFyaWFi bGUgaW4gY2hlY2tfaW5jbHVkZV9maWxlcygpLCBhcyBpdCB3aWxsIGxlYWQgdG8gbWlzbGVhZGlu ZwogICAgICAgIHJlc3VsdHMgKHdpbGwgYWx3YXlzIGJlIEZBTFNFKS4gVXNlIGEgdGVtcG9yYXJ5 IHZhcmlhYmxlIGluc3RlYWQKICAgICAgICBhbmQgZXhwbGljaXRseSBzZXQgV0VCUF9GT1VORCB0 byBUUlVFIGxhdGVyIGlmIG5lZWRlZC4KLS0tCiBDaGFuZ2VMb2cgICAgICAgICAgICAgICAgICAg fCAgIDE1ICsrKysrKysrKysrKysrKwogU291cmNlL2NtYWtlL0ZpbmRXZWJQLmNtYWtlIHwgICAg NSArKystLQogMiBmaWxlcyBjaGFuZ2VkLCAxOCBpbnNlcnRpb25zKCspLCAyIGRlbGV0aW9ucygt KQoKZGlmZiAtLWdpdCBhL0NoYW5nZUxvZyBiL0NoYW5nZUxvZwppbmRleCA1ZjhkYjhjLi4zOWRj NTZiIDEwMDY0NAotLS0gYS9DaGFuZ2VMb2cKKysrIGIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTgg QEAKKzIwMTQtMDMtMTMgIE1hcmlvIFNhbmNoZXogUHJhZGEgIDxtYXJpby5wcmFkYUBzYW1zdW5n LmNvbT4KKworICAgICAgICBbQ01ha2VdIEZhaWx1cmUgdG8gbGluayB3aXRoIG9sZGVyIGluc3Rh bGxhdGlvbnMgb2YgV2ViUAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MTMwMTk1CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgRml4IHRoZSBpc3N1ZSBieSB1c2luZyBhIHRlbXBvcmFyeSB2YXJpYWJsZSBhcyB0 aGUgb3V0cHV0IHBhcmFtZXRlcgorICAgICAgICB0byBjaGVja19pbmNsdWRlX2ZpbGVzKCksIHNl dHRpbmcgV0VCUF9GT1VORCBhY2NvcmRpbmdseSBsYXRlci4KKworICAgICAgICAqIFNvdXJjZS9j bWFrZS9GaW5kV2ViUC5jbWFrZTogQXZvaWQgcmV1c2luZyB0aGUgV0VCUF9GT1VORAorICAgICAg ICB2YXJpYWJsZSBpbiBjaGVja19pbmNsdWRlX2ZpbGVzKCksIGFzIGl0IHdpbGwgbGVhZCB0byBt aXNsZWFkaW5nCisgICAgICAgIHJlc3VsdHMgKHdpbGwgYWx3YXlzIGJlIEZBTFNFKS4gVXNlIGEg dGVtcG9yYXJ5IHZhcmlhYmxlIGluc3RlYWQKKyAgICAgICAgYW5kIGV4cGxpY2l0bHkgc2V0IFdF QlBfRk9VTkQgdG8gVFJVRSBsYXRlciBpZiBuZWVkZWQuCisKIDIwMTQtMDMtMTIgIE1hcnRpbiBS b2JpbnNvbiAgPG1yb2JpbnNvbkBpZ2FsaWEuY29tPgogCiAgICAgICAgIFtDTWFrZV0gQ2hhbmdl cyB0byB0aGUgYmluZGluZ3MgZ2VuZXJhdG9yIFBlcmwgbW9kdWxlcyBkbyBub3QgdHJpZ2dlciBy ZWdlbmVyYXRpb24gb2YgYmluZGluZ3MKZGlmZiAtLWdpdCBhL1NvdXJjZS9jbWFrZS9GaW5kV2Vi UC5jbWFrZSBiL1NvdXJjZS9jbWFrZS9GaW5kV2ViUC5jbWFrZQppbmRleCAzZWYxMWQ5Li5kZmVk ZDJkIDEwMDY0NAotLS0gYS9Tb3VyY2UvY21ha2UvRmluZFdlYlAuY21ha2UKKysrIGIvU291cmNl L2NtYWtlL0ZpbmRXZWJQLmNtYWtlCkBAIC0zNSw5ICszNSwxMCBAQCBwa2dfY2hlY2tfbW9kdWxl cyhXRUJQIGxpYndlYnApCiBpZiAoTk9UKFdFQlBfRk9VTkQpKQogICAgICMgT2xkZXIgaW5zdGFs bGF0aW9ucyBvZiBsaWJ3ZWJwIGRvIG5vdCBpbnN0YWxsIGEgcGtnY29uZmlnIGZpbGUsIHNvIHdl IGZhbGwKICAgICAjIGJhY2sgdG8gYSBtYW51YWwgc2VhcmNoIGZvciBhIGxpYndlYnAgaGVhZGVy IGZpbGUuCi0gICAgY2hlY2tfaW5jbHVkZV9maWxlcygid2VicC9kZWNvZGUuaCIgV0VCUF9GT1VO RCkKLSAgICBpZiAoV0VCUF9GT1VORCkKKyAgICBjaGVja19pbmNsdWRlX2ZpbGVzKCJ3ZWJwL2Rl Y29kZS5oIiBXRUJQX0ZPVU5EX0hFQURFUikKKyAgICBpZiAoV0VCUF9GT1VORF9IRUFERVIpCiAg ICAgICAgIHNldChXRUJQX0xJQlJBUklFUyAiLWx3ZWJwIikKKyAgICAgICAgc2V0KFdFQlBfRk9V TkQgVFJVRSkKICAgICBlbmRpZiAoKQogZW5kaWYgKCkKIAotLSAKMS43LjEwLjQKCg==