Bug 80590 - [Qt] Use Qt's module system for install rules and depending on QtWebKit
Summary: [Qt] Use Qt's module system for install rules and depending on QtWebKit
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tor Arne Vestbø
URL:
Keywords:
: 77955 (view as bug list)
Depends on: 80846
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-08 05:22 PST by Tor Arne Vestbø
Modified: 2013-03-08 08:01 PST (History)
7 users (show)

See Also:


Attachments
patch (39.75 KB, patch)
2012-03-08 05:24 PST, Tor Arne Vestbø
no flags Details | Formatted Diff | Diff
rebased (40.32 KB, patch)
2012-03-08 09:41 PST, Tor Arne Vestbø
no flags Details | Formatted Diff | Diff
fix style (40.24 KB, patch)
2012-03-08 10:05 PST, Tor Arne Vestbø
no flags Details | Formatted Diff | Diff
qt4 still needs rpath.prf (40.85 KB, patch)
2012-03-08 10:11 PST, Tor Arne Vestbø
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
load module file for qt4 (41.08 KB, patch)
2012-03-08 10:31 PST, Tor Arne Vestbø
no flags Details | Formatted Diff | Diff
with changelog, ready for review (49.31 KB, patch)
2012-03-08 10:48 PST, Tor Arne Vestbø
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tor Arne Vestbø 2012-03-08 05:22:44 PST
[Qt] Use Qt's module system for install rules and depending on QtWebKit
Comment 1 Tor Arne Vestbø 2012-03-08 05:24:43 PST
Created attachment 130809 [details]
patch
Comment 2 Tor Arne Vestbø 2012-03-08 09:41:25 PST
Created attachment 130833 [details]
rebased
Comment 3 Tor Arne Vestbø 2012-03-08 09:48:24 PST
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
Comment 4 WebKit Review Bot 2012-03-08 09:55:38 PST
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.
Comment 5 Tor Arne Vestbø 2012-03-08 10:05:10 PST
Created attachment 130836 [details]
fix style
Comment 6 Tor Arne Vestbø 2012-03-08 10:11:40 PST
Created attachment 130839 [details]
qt4 still needs rpath.prf
Comment 7 Early Warning System Bot 2012-03-08 10:25:41 PST
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
Comment 8 Tor Arne Vestbø 2012-03-08 10:31:47 PST
Created attachment 130844 [details]
load module file for qt4
Comment 9 Tor Arne Vestbø 2012-03-08 10:48:31 PST
Created attachment 130850 [details]
with changelog, ready for review
Comment 10 WebKit Review Bot 2012-03-08 10:52:21 PST
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.
Comment 11 Tor Arne Vestbø 2012-03-08 10:57:13 PST
Gah, I'll add the bug number when landing.
Comment 12 Simon Hausmann 2012-03-08 10:59:47 PST
*** Bug 77955 has been marked as a duplicate of this bug. ***
Comment 13 Simon Hausmann 2012-03-08 11:15:24 PST
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
>> +2012-03-08  Tor Arne Vestbø  <tor.arne.vestbo@nokia.com>
> 
> 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 :)
Comment 14 Simon Hausmann 2012-03-08 11:40:49 PST
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.
Comment 15 Tor Arne Vestbø 2012-03-08 23:49:37 PST
Landed in r110272.