Bug 31670 – [Qt] Remove the definition of WTF_CHANGES guards from the build system

• Status: CLOSED
• Resolution: FIXED
• Component: Platform

[Laszlo Gombos]

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.

[Laszlo Gombos]

Created attachment 43510 [details]
proposed patch

[Laszlo Gombos]

Created attachment 43511 [details]
fix typo - s/sett/set

[Darin Adler]

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 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]

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]

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]

Created attachment 51771 [details]
proposed patch

[Simon Hausmann]

Removing dependency, as in ToT they're built separately now at least.

[WebKit Commit Bot]

Comment on attachment 51771 [details]
proposed patch

Clearing flags on attachment: 51771

Committed r56700: <http://trac.webkit.org/changeset/56700>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Hausmann]

Revision r56700 cherry-picked into qtwebkit-2.0 with commit 8f84e1115edba4a317c3a0d955fa94ba1824171d