Bug 147815

Summary: [GTK] webkitgtk 2.8.5 fails to build in mips, powerpc and others using GCC 5
Product: WebKit Reporter: Alberto Garcia <berto>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, commit-queue, fpizlo, ggaren, mcatanzaro, ossy, zackw
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch (from QtWebkit bug tracker)
none
Patch
fpizlo: review-
patch v3 (tab character removed) none

Alberto Garcia
Reported 2015-08-09 03:27:14 PDT
Here's the error message: CMakeFiles/JavaScriptCore.dir/runtime/JSArray.cpp.o: In function `JSC::JSArray::push(JSC::ExecState*, JSC::JSValue)': JSArray.cpp:(.text+0x4170): undefined reference to `void JSC::JSObject::putByIndexBeyondVectorLengthWithoutAttributes<(unsigned char)20>(JSC::ExecState*, unsigned int, JSC::JSValue)' collect2: error: ld returned 1 exit status This may be related to the usage of GCC 5. Here's what appears to be the same error in QtWebKit: https://bugreports.qt.io/browse/QTBUG-44829
Attachments
patch (from QtWebkit bug tracker) (832 bytes, patch)
2015-08-23 10:21 PDT, Zack Weinberg
no flags
Patch (1.67 KB, patch)
2015-08-28 11:42 PDT, Zack Weinberg
fpizlo: review-
patch v3 (tab character removed) (1.68 KB, patch)
2015-08-28 15:29 PDT, Zack Weinberg
no flags
Zack Weinberg
Comment 1 2015-08-23 10:21:47 PDT
Created attachment 259741 [details] patch (from QtWebkit bug tracker) Patch, by Khem Raj, hoisted from https://codereview.qt-project.org/#/c/107921/ & https://bugreports.qt.io/browse/QTBUG-44829 . Line numbers may be off. This doesn't appear to have anything to do with GTK (or Qt) -- it's completely generic JSC code. I can confirm that this fixes a failure to link the 'jsc' executable, but I have not verified that there are no other such problems in the rest of the current Webkit development sources.
Michael Catanzaro
Comment 2 2015-08-23 12:24:30 PDT
FWIW we've been carrying this patch in Fedora for a while
Alberto Garcia
Comment 3 2015-08-23 23:14:59 PDT
I tested it with webkitgtk 2.8.5 in s390x and I confirm that it works. Thanks!
Carlos Garcia Campos
Comment 4 2015-08-24 01:18:52 PDT
Thanks for the patch, we need a proper patch that applies on current trunk and with a ChangeLog entry, though. See http://www.webkit.org/coding/contributing.html
Zack Weinberg
Comment 5 2015-08-28 11:42:33 PDT
Zack Weinberg
Comment 6 2015-08-28 11:45:15 PDT
(In reply to comment #4) > Thanks for the patch, we need a proper patch that applies on current trunk > and with a ChangeLog entry, though. See > http://www.webkit.org/coding/contributing.html Sorry for the delay, I don't seem to be getting bugmail from this tracker for some reason. Anyway, the current patch should apply cleanly to trunk, has a changelog entry, and passes 'webkit-patch upload''s linter. Note that I corrected the bug title in the changelog entry; I would do that here too but I don't have the necessary privileges. I also made the changelog entry reflect the original author of the fix (Khem Raj).
WebKit Commit Bot
Comment 7 2015-08-28 11:45:19 PDT
Attachment 260169 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Zack Weinberg
Comment 8 2015-08-28 11:47:51 PDT
(In reply to comment #7) > Attachment 260169 [details] did not pass style-queue: > ERROR: Source/JavaScriptCore/ChangeLog:9: Line contains tab character. > [whitespace/tab] [5] > Total errors found: 1 in 2 files This is the first project I have ever tripped over in which a GNU-style ChangeLog file was *not* to be indented with a single hard tab? Anyway I would ask whoever commits this patch to fix up this trivial error for me; it does not seem worth respinning the whole thing over. You're going to have to edit the changelog to put yourself on the "Reviewed by" line anyway ;-)
Filip Pizlo
Comment 9 2015-08-28 11:51:47 PDT
(In reply to comment #8) > (In reply to comment #7) > > Attachment 260169 [details] did not pass style-queue: > > ERROR: Source/JavaScriptCore/ChangeLog:9: Line contains tab character. > > [whitespace/tab] [5] > > Total errors found: 1 in 2 files > > This is the first project I have ever tripped over in which a GNU-style > ChangeLog file was *not* to be indented with a single hard tab? > > Anyway I would ask whoever commits this patch to fix up this trivial error > for me; it does not seem worth respinning the whole thing over. You're > going to have to edit the changelog to put yourself on the "Reviewed by" > line anyway ;-) If you would like this change landed, then you should submit a patch that obeys our style. Also, patches by non-committers are usually landed by the commit-queue after a committer grants the cq+ flag. The commit-queue does not automatically fix style errors for you. So, what you're asking is for is not only that some reviewer do some work for you, but that they land the patch for you rather than using the commit-queue. Most reviewers have more important things to do than fix other people's style errors for them.
Filip Pizlo
Comment 10 2015-08-28 11:52:21 PDT
Comment on attachment 260169 [details] Patch R- because you should fix the ChangeLog style.
Zack Weinberg
Comment 11 2015-08-28 15:29:40 PDT
Created attachment 260184 [details] patch v3 (tab character removed) Downloaded patch, edited out offending tab character, put it back.
Filip Pizlo
Comment 12 2015-08-28 15:43:10 PDT
Comment on attachment 260184 [details] patch v3 (tab character removed) Thanks! You can mark the cq flag to "?" if you want someone to land it for you. Otherwise, we assume that you will land it yourself.
WebKit Commit Bot
Comment 13 2015-08-28 16:43:54 PDT
Comment on attachment 260184 [details] patch v3 (tab character removed) Clearing flags on attachment: 260184 Committed r189133: <http://trac.webkit.org/changeset/189133>
WebKit Commit Bot
Comment 14 2015-08-28 16:43:59 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.