Bug 31670

Summary: [Qt] Remove the definition of WTF_CHANGES guards from the build system
Product: WebKit Reporter: Laszlo Gombos <laszlo.gombos>
Component: PlatformAssignee: QtWebKit Unassigned <webkit-qt-unassigned>
Status: CLOSED FIXED    
Severity: Normal CC: commit-queue, hausmann, kent.hansen
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on:    
Bug Blocks: 35784    
Attachments:
Description Flags
proposed patch
none
fix typo - s/sett/set
none
proposed patch none

Laszlo Gombos
Reported 2009-11-19 08:56:57 PST
Both HAVE(STDINT_H) and WTF_CHANGES are only used within WTF and WTF (code under JavaScriptCode/wtf) expect these to be defined, yet these guards are defined in WebCore and JavaScriptCore config.h files and in a few cases port specific makefiles. As WTF does not build without these flags and these flags are only used (at the moment) within WTF I think it make sense to move them to wtf/Platfrom.h. Patch will follow.
Attachments
proposed patch (5.29 KB, patch)
2009-11-19 09:04 PST, Laszlo Gombos 		no flags
DetailsFormatted DiffDiff
fix typo - s/sett/set (5.29 KB, patch)
2009-11-19 09:05 PST, Laszlo Gombos 		no flags
DetailsFormatted DiffDiff
proposed patch (2.20 KB, patch)
2010-03-26 12:34 PDT, Laszlo Gombos 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Laszlo Gombos
Comment 1 2009-11-19 09:04:10 PST
Created attachment 43510 [details] proposed patch
Laszlo Gombos
Comment 2 2009-11-19 09:05:32 PST
Created attachment 43511 [details] fix typo - s/sett/set
Darin Adler
Comment 3 2009-11-19 11:15:34 PST
Comment on attachment 43511 [details] fix typo - s/sett/set It's good to get rid of all the duplication. But this is a step in the wrong direction as far as what Platform.h is supposed to be. It's clearly wrong to define a platform-independent thing such as WTF_CHANGES in there.
Laszlo Gombos
Comment 4 2009-11-20 20:49:19 PST
Comment on attachment 43511 [details] fix typo - s/sett/set Cancel review based on Darin's comment. I might make this into a QtWebkit port-only patch. As I was working on this, noticed that when JSC is compiled for QtWebkit config.h is coming from WebCore and config.h from JavaScriptCore does not get used to compile the JSC part. This seems wrong and I will try to address it in my next patch.
Simon Hausmann
Comment 5 2009-11-21 00:44:45 PST
I believe that is an inherent problem from compiling WebCore and JavaScriptCore at the same time into one shared library. A while ago we had the two separated for a while, with JSC built as static library (PIC) and linked into QtWebKit. That however created the problem that QtWebKit's .prl file included a link reference to libJavaScriptCore.a, which made it impossible to link any application against QtWebKit as it always tried to drag in libJavaScriptCore.a. Now with the recently introduced LIBS_PRIVATE feature in qmake we _might_ now be able to try that earlier approach again. I believe however that the right thing to do - also with the other ports in mind - is to move towards building JavaScriptCore as a shared library. See also bug #27551 for some work in that area. If you agree about the latter approach, then we should probably make this bug depend on #27551.
Laszlo Gombos
Comment 6 2009-11-22 14:44:10 PST
Thanks Simon for the comments - making this bug dependent on 27551. In fact bug 27551 would probably solve the problem mentioned in the title entirely, but we can use this bug to clean up WebCore.pro (as it is proposed in the patch) after 27551 is fixed. in theory we could do some if the clean-up right now, but I consider that risky.
Laszlo Gombos
Comment 7 2010-03-26 12:34:18 PDT
Created attachment 51771 [details] proposed patch
Simon Hausmann
Comment 8 2010-03-28 02:27:59 PDT
Removing dependency, as in ToT they're built separately now at least.
WebKit Commit Bot
Comment 9 2010-03-28 14:41:57 PDT
Comment on attachment 51771 [details] proposed patch Clearing flags on attachment: 51771 Committed r56700: <http://trac.webkit.org/changeset/56700>
WebKit Commit Bot
Comment 10 2010-03-28 14:42:02 PDT
All reviewed patches have been landed. Closing bug.
Simon Hausmann
Comment 11 2010-03-28 14:46:24 PDT
Revision r56700 cherry-picked into qtwebkit-2.0 with commit 8f84e1115edba4a317c3a0d955fa94ba1824171d
Note You need to log in before you can comment on or make changes to this bug.