Bug 74040

Summary: Fails to build QtWebKit on QNX
Product: WebKit Reporter: ssukhyun
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: andersca, benjamin, dbates, hausmann, laszlo.gombos, milian.wolff, rwlbuis, tonikitoo, vestbo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Other   
Bug Depends on: 93278, 93460, 93842, 93843, 93849, 95468, 98031, 98032, 98038, 98040, 102794, 102871    
Bug Blocks:    
Attachments:
Description Flags
patch
hausmann: review-, hausmann: commit-queue-
patch
none
Patch
none
Patch
none
Patch hausmann: review-

Description ssukhyun 2011-12-07 17:33:58 PST
When building QtWebkit on QNX, we get compilation failures.
Most of them are related to include files.
Comment 1 ssukhyun 2011-12-07 20:46:42 PST
Created attachment 118321 [details]
patch
Comment 2 WebKit Review Bot 2011-12-07 20:49:05 PST
Attachment 118321 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/wtf/MathExtras.h:61:  "math.h" already included at Source/JavaScriptCore/wtf/MathExtras.h:57  [build/include] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 ssukhyun 2011-12-07 21:00:16 PST
Created attachment 118322 [details]
patch
Comment 4 Simon Hausmann 2011-12-13 00:09:38 PST
Comment on attachment 118321 [details]
patch

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

> Source/JavaScriptCore/wtf/MathExtras.h:53
> +#if OS(QNX)
> +#include <math.h>
> +#endif

This was already fixed in r101041 by Daniel.

> Source/WebCore/platform/network/MIMESniffing.cpp:28
> +#if OS(QNX)
> +#include <string.h>
> +#endif

What about replacing #include <cstring> with #include <string.h> without any #ifdef?
Comment 5 Milian Wolff 2012-06-13 08:39:19 PDT
Created attachment 147326 [details]
Patch
Comment 6 Milian Wolff 2012-06-13 08:40:59 PDT
Hey there,

using the patch you see above I managed to built Qt5 QtWebKit for QNX6 using the bbndk 2.0.1.

This also requires a patch to use the new Qt5 plugin system but that is apparently in the pipeline according to bbandix in #qtwebkit on IRC.

I have not yet managed to test the functionality of qtwebkit on a QNX6 device yet, but will do that now.
Comment 7 Milian Wolff 2012-06-13 09:16:49 PDT
The above patch needs to be changed to take the discussion from bug 77013 into account.
Comment 8 Early Warning System Bot 2012-06-13 09:20:55 PDT
Comment on attachment 147326 [details]
Patch

Attachment 147326 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12947827
Comment 9 Milian Wolff 2012-06-13 09:25:59 PDT
Created attachment 147336 [details]
Patch
Comment 10 Milian Wolff 2012-06-13 09:55:38 PDT
Created attachment 147345 [details]
Patch
Comment 11 Kenneth Rohde Christiansen 2012-06-13 13:36:26 PDT
Comment on attachment 147345 [details]
Patch

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

> Source/WTF/wtf/DisallowCType.h:43
> +// this breaks compilation of <QFontDatabase>, at least, so turn it off for now
> +// Also generates errors on wx on Windows and QNX, because these functions
> +// are used from wx and QNX headers.
> +#if !PLATFORM(QT) && !PLATFORM(WX) && !OS(QNX)

Add bug number?
Comment 12 Milian Wolff 2012-06-14 03:36:51 PDT
Well as you can see I just moved this comment from one place where it was already used to the more central one in DisallowCType.h. If you want, I can of course link to this bug report here, is that what you had in mind? If so, I'll update the patch.

Cheers
Comment 13 Simon Hausmann 2012-06-19 01:52:19 PDT
Comment on attachment 147345 [details]
Patch

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

> Tools/ChangeLog:13
> +        * WebKitTestRunner/EventSenderProxy.h:
> +        * WebKitTestRunner/PlatformWebView.h:
> +        * WebKitTestRunner/TestController.cpp:
> +        * WebKitTestRunner/TestController.h:
> +        * WebKitTestRunner/TestInvocation.cpp:
> +        * WebKitTestRunner/qt/TestControllerQt.cpp:

For portability fixes it really helps if the changelog explains why certain things are done, especially for changes that don't come with a comment in the source code. It may seem obvious to you as you are currently working on this, but in three months it won't be obvious anymore.
Comment 14 Simon Hausmann 2012-07-27 01:53:49 PDT
Comment on attachment 147345 [details]
Patch

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

This is going to need another iteration, I'm missing explanations (either in this bug or in the ChangeLog) as to why certain changes were made.

>> Source/WTF/wtf/DisallowCType.h:43
>> +#if !PLATFORM(QT) && !PLATFORM(WX) && !OS(QNX)
> 
> Add bug number?

Can you explain why you're moving the code block from config.h into this file?

> Source/WTF/wtf/Platform.h:493
> +#if OS(QNX) || PLATFORM(BLACKBERRY)
> +#define USE_SYSTEM_MALLOC 1
> +#endif

what's the difference between OS(QNX) and PLATFORM(BLACKBERRY)? Does the latter also include the non-QNX based blackberry platforms?
Comment 15 Antonio Gomes 2012-07-30 06:14:29 PDT
Comment on attachment 147345 [details]
Patch

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

>> Source/WTF/wtf/Platform.h:493
>> +#endif
> 
> what's the difference between OS(QNX) and PLATFORM(BLACKBERRY)? Does the latter also include the non-QNX based blackberry platforms?

Not for now, Simon.
Comment 16 Milian Wolff 2012-08-06 10:04:32 PDT
I'll now try to rebase this patch against current master and add comments wherever needed and try to answer the questions here as well. Furthermore, I'll try to split the issues into smaller bug reports and use this one as a master bug, hope that is OK.
Comment 17 Milian Wolff 2012-08-17 07:17:32 PDT
QtWebKit for QNX should now build properly, thanks for the help everyone.

I'll now try to get it actually working :)
Comment 18 Milian Wolff 2012-10-01 07:45:24 PDT
Reopening, as QtWebKit is again failing to build without manual patches. I'll open more child bugs again to track each issue separately.