Bug 80590 – [Qt] Use Qt's module system for install rules and depending on QtWebKit

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tor Arne Vestbø]

[Qt] Use Qt's module system for install rules and depending on QtWebKit

[Tor Arne Vestbø]

Created attachment 130809 [details]
patch

[Tor Arne Vestbø]

Created attachment 130833 [details]
rebased

[Tor Arne Vestbø]

Comment on attachment 130833 [details]
rebased

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

> Source/api.pri:282
> +haveQt(5) {
> +    # Install rules handled by Qt's module system
> +} else {

This is just indenting, no code change

> Source/api.pri:-229
> -plugin_backend_xlib: PKGCONFIG += x11

Just moved this to the top

[WebKit Review Bot]

Attachment 130833 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/qt/declarative/experimental/..." exit_code: 1
Source/WebKit/qt/declarative/experimental/plugin.cpp:28:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit/qt/declarative/experimental/plugin.cpp:29:  "private/qwebdownloaditem_p.h" already included at Source/WebKit/qt/declarative/experimental/plugin.cpp:26  [build/include] [4]
Source/WebKit/qt/declarative/experimental/plugin.cpp:33:  "private/qwebviewportinfo_p.h" already included at Source/WebKit/qt/declarative/experimental/plugin.cpp:27  [build/include] [4]
Total errors found: 3 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tor Arne Vestbø]

Created attachment 130836 [details]
fix style

[Tor Arne Vestbø]

Created attachment 130839 [details]
qt4 still needs rpath.prf

[Early Warning System Bot]

Comment on attachment 130839 [details]
qt4 still needs rpath.prf

Attachment 130839 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11889002

[Tor Arne Vestbø]

Created attachment 130844 [details]
load module file for qt4

[Tor Arne Vestbø]

Created attachment 130850 [details]
with changelog, ready for review

[WebKit Review Bot]

Attachment 130850 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/WebKit/qt/ChangeLog',..." exit_code: 1
Tools/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Source/WebKit/qt/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Source/WebKit2/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 4 in 36 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tor Arne Vestbø]

Gah, I'll add the bug number when landing.

[Simon Hausmann]

*** Bug 77955 has been marked as a duplicate of this bug. ***

[Simon Hausmann]

Comment on attachment 130850 [details]
with changelog, ready for review

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

r=me with comments

>> ChangeLog:1
>> [phone]  Tor Arne Vestbø  <[e-mail]>
> 
> ChangeLog entry has no bug number  [changelog/bugnumber] [5]

I agree with the style queue here, would be nice to have the url to bugzilla here and in the other ChangeLogs when landing.

> Source/api.pri:38
> +    # to a bogus patch in default_pre.prf.

patch -> path

> Source/api.pri:58
> +        make_module_fwd_symlink.target = $$convenience_module_path/$$module_filename
> +        make_module_fwd_symlink.commands = $$QMAKE_MKDIR $$convenience_module_path \
> +            && $(SYMLINK) $$QMAKE_EXTRA_MODULE_FORWARDS/$$module_filename $$convenience_module_path
> +        make_module_fwd_symlink.depends = $$QMAKE_EXTRA_MODULE_FORWARDS/$$module_filename

I suppose this is going to require a copy instead of symlink on Windows?

A cross-platform alternative would be to put a .pri file there that includes the real .pri file - a poor man's symlink :)

> Tools/qmake/mkspecs/features/default_post.prf:147
> +contains(QT, webkit) {
> +    haveQt(4) {
> +        # Qt 4 still uses custom rules to depend on QtWebKit
> +        load(qtwebkit)
> +    }

Neat trick to make this work with the Qt 4 build :)

[Simon Hausmann]

If the fix for this bug lands before the fix for bug #72121, then the latter is going to need a fix (CONFIG += qtwebkit -> QT += webkit). If the latter lands first, this patch will need to fix Source/WebKit2/PluginProcess.pro when landing.

[Tor Arne Vestbø]

Landed in r110272.