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

Description Alberto Garcia 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
Comment 1 Zack Weinberg 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.
Comment 2 Michael Catanzaro 2015-08-23 12:24:30 PDT
FWIW we've been carrying this patch in Fedora for a while
Comment 3 Alberto Garcia 2015-08-23 23:14:59 PDT
I tested it with webkitgtk 2.8.5 in s390x and I confirm that it works. Thanks!
Comment 4 Carlos Garcia Campos 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
Comment 5 Zack Weinberg 2015-08-28 11:42:33 PDT
Created attachment 260169 [details]
Patch
Comment 6 Zack Weinberg 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).
Comment 7 WebKit Commit Bot 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.
Comment 8 Zack Weinberg 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 ;-)
Comment 9 Filip Pizlo 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.
Comment 10 Filip Pizlo 2015-08-28 11:52:21 PDT
Comment on attachment 260169 [details]
Patch

R- because you should fix the ChangeLog style.
Comment 11 Zack Weinberg 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.
Comment 12 Filip Pizlo 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.
Comment 13 WebKit Commit Bot 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>
Comment 14 WebKit Commit Bot 2015-08-28 16:43:59 PDT
All reviewed patches have been landed.  Closing bug.