Bug 31670 - [Qt] Remove the definition of WTF_CHANGES guards from the build system
Summary: [Qt] Remove the definition of WTF_CHANGES guards from the build system
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: QtWebKit Unassigned
URL:
Keywords: Qt
Depends on:
Blocks: 35784
  Show dependency treegraph
 
Reported: 2009-11-19 08:56 PST by Laszlo Gombos
Modified: 2010-03-28 14:46 PDT (History)
3 users (show)

See Also:


Attachments
proposed patch (5.29 KB, patch)
2009-11-19 09:04 PST, Laszlo Gombos
no flags Details | Formatted Diff | Diff
fix typo - s/sett/set (5.29 KB, patch)
2009-11-19 09:05 PST, Laszlo Gombos
no flags Details | Formatted Diff | Diff
proposed patch (2.20 KB, patch)
2010-03-26 12:34 PDT, Laszlo Gombos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Gombos 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.
Comment 1 Laszlo Gombos 2009-11-19 09:04:10 PST
Created attachment 43510 [details]
proposed patch
Comment 2 Laszlo Gombos 2009-11-19 09:05:32 PST
Created attachment 43511 [details]
fix typo - s/sett/set
Comment 3 Darin Adler 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.
Comment 4 Laszlo Gombos 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.
Comment 5 Simon Hausmann 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.
Comment 6 Laszlo Gombos 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.
Comment 7 Laszlo Gombos 2010-03-26 12:34:18 PDT
Created attachment 51771 [details]
proposed patch
Comment 8 Simon Hausmann 2010-03-28 02:27:59 PDT
Removing dependency, as in ToT they're built separately now at least.
Comment 9 WebKit Commit Bot 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>
Comment 10 WebKit Commit Bot 2010-03-28 14:42:02 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Simon Hausmann 2010-03-28 14:46:24 PDT
Revision r56700 cherry-picked into qtwebkit-2.0 with commit 8f84e1115edba4a317c3a0d955fa94ba1824171d