Bug 37867 – [Qt] Patches to upstream from Qt for 2.0

• Status: CLOSED
• Resolution: FIXED
• Component: Tools / Tests

[Jocelyn Turcotte]

Some patches that were committed directly in Qt that have to be upstreamed to ease next imports.

[Jocelyn Turcotte]

Created attachment 53838 [details]
Build fix when !ENABLED(NETSCAPE_PLUGIN_API)

[Jocelyn Turcotte]

Created attachment 53839 [details]
Build fix on Mac

[Jocelyn Turcotte]

Created attachment 53840 [details]
Workaround syncqt dependency mechanism

[Jocelyn Turcotte]

Created attachment 53841 [details]
Corrects JSC symbols visibility

[Jocelyn Turcotte]

Created attachment 53842 [details]
Corrects autotests resources compilation in QTDIR_build

[Kenneth Rohde Christiansen]

Comment on attachment 53838 [details]
Build fix when !ENABLED(NETSCAPE_PLUGIN_API)

The Changelog should have a reference to the bug report and it should not have [Qt] prefixed as this is not Qt specific. Please fix that and re-upload a new patch.

[Kenneth Rohde Christiansen]

Comment on attachment 53839 [details]
Build fix on Mac


> +        - Make sure JavaScriptCore's debug_and_release config follows WebCore's

...follow that of WebCore.

I would like either Laszlo or Simon to take a look at this one.

[Kenneth Rohde Christiansen]

Comment on attachment 53840 [details]
Workaround syncqt dependency mechanism


>  
> +        [Qt] Hide the OpenGL and QtMultimedia dependencies from syncqt.

What is syncqt? maybe the script used for synching... Is that script in WebKit?


> +            tobe|!tobe: QT += multimedia

I don't understand this? tobe or not tobe?

[Kenneth Rohde Christiansen]

Comment on attachment 53841 [details]
Corrects JSC symbols visibility


> +# Add these two lines both in QTDIR_build and out because we don't include qbase.pri

... which does that?

and out? what do you mean?

> +contains(QT_CONFIG, reduce_exports):CONFIG += hide_symbols
> +unix:contains(QT_CONFIG, reduce_relocations):CONFIG += bsymbolic_functions
> +
>  CONFIG(QTDIR_build) {
>      # Make sure we compile both debug and release on mac when inside Qt.
>      # This line was extracted from qbase.pri instead of including the whole file

[Jocelyn Turcotte]

Created attachment 53852 [details]
Build fix when !ENABLED(NETSCAPE_PLUGIN_API) v2

[Jocelyn Turcotte]

Created attachment 53853 [details]
Build fix on Mac v2

[Jocelyn Turcotte]

Created attachment 53854 [details]
Corrects JSC symbols visibility v2

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 2
Last 3072 characters of output:
  any such category.  The filter input "-whitespace,+whitespace/braces" fails
  the category "whitespace/tab" and passes "whitespace/braces".

  Examples: --filter=-whitespace,+whitespace/braces
            --filter=-whitespace,-runtime/printf,+runtime/printf_format
            --filter=-,+build/include_what_you_use

Paths:
  Certain style-checking behavior depends on the paths relative to
  the WebKit source root of the files being checked.  For example,
  certain types of errors may be handled differently for files in
  WebKit/gtk/webkit/ (e.g. by suppressing "readability/naming" errors
  for files in this directory).

  Consequently, if the path relative to the source root cannot be
  determined for a file being checked, then style checking may not
  work correctly for that file.  This can occur, for example, if no
  WebKit checkout can be found, or if the source root can be detected,
  but one of the files being checked lies outside the source tree.

  If a WebKit checkout can be detected and all files being checked
  are in the source tree, then all paths will automatically be
  converted to paths relative to the source root prior to checking.
  This is also useful for display purposes.

  Currently, this command can detect the source root only if the
  command is run from within a WebKit checkout (i.e. if the current
  working directory is below the root of a checkout).  In particular,
  it is not recommended to run this script from a directory outside
  a checkout.

  Running this script from a top-level WebKit source directory and
  checking only files in the source tree will ensure that all style
  checking behaves correctly -- whether or not a checkout can be
  detected.  This is because all file paths will already be relative
  to the source root and so will not need to be converted.

Options:
  -h, --help            show this help message and exit
  -f RULES, --filter-rules=RULES
                        set a filter to control what categories of style
                        errors to report.  Specify a filter using a comma-
                        delimited list of boolean filter rules, for example "
                        --filter -whitespace,+whitespace/braces".  To display
                        all categories and which are enabled by default, pass
                        no value (e.g. '-f ""' or '--filter=').
  -g COMMIT, --git-commit=COMMIT, --git-diff=COMMIT, --git-since=COMMIT
                        check all changes after the given git commit.
  -m INT, --min-confidence=INT
                        set the minimum confidence of style errors to report.
                        Can be an integer 1-5, with 1 displaying all errors.
                        Defaults to 1.
  -o FORMAT, --output-format=FORMAT
                        set the output format, which can be "emacs" or "vs7"
                        (for Visual Studio).  Defaults to "emacs".
  -v, --verbose         enable verbose logging.

This script can miss errors and does not substitute for code review.

ERROR: no such option: --no-squash


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

[Tor Arne Vestbø]

Comment on attachment 53853 [details]
Build fix on Mac v2


> +# Output in JavaScriptCore/<config>
> +CONFIG(debug, debug|release): JAVASCRIPTCORE_DESTDIR = debug
> +CONFIG(release, debug|release): JAVASCRIPTCORE_DESTDIR = release
> +# Use different targets to prevent parallel builds file clashes on Mac
> +CONFIG(debug, debug|release): JAVASCRIPTCORE_TARGET = jscored
> +CONFIG(release, debug|release): JAVASCRIPTCORE_TARGET = jscore

Nit, please combine these into scoped blocks, let's try to avoid repeated CONFIG(...) all over the place

[Jocelyn Turcotte]

Created attachment 53929 [details]
Build fix on Mac v3

Here is your patch

[Jocelyn Turcotte]

Created attachment 53946 [details]
Remove dep of JSC to QtGui

[Jocelyn Turcotte]

Created attachment 53947 [details]
Remove autotest tr strings

[Jocelyn Turcotte]

Created attachment 53948 [details]
MSVC 64 build fix (see comment)

Part of this patch will break the patch in bug #35607
I don't know what we do in that case, up to you.

[Jocelyn Turcotte]

Created attachment 53949 [details]
Disable Cpp0x (see comment)

This patch is currently useful only if the developer modified its mkspec to build Qt using Cpp0x.

I don't know if this should go in trunk, maybe it should go in the staging area instead until there is actually a more official way to enable Cpp0x in qt (e.g. an option in configure).
What do you think?

[Jocelyn Turcotte]

Created attachment 53950 [details]
MSVC 2010 build fix

[Kenneth Rohde Christiansen]

Comment on attachment 53948 [details]
MSVC 64 build fix (see comment)


> +        [Qt] Fix the MSVC 64bit build.

The change is not in Qt-only code, so you need to remove the [Qt]. I would prefer you adding at least the not Qt specific patches in separate bug reports.

[Eric Seidel (no email)]

Comment on attachment 53838 [details]
Build fix when !ENABLED(NETSCAPE_PLUGIN_API)

Cleared Kenneth Rohde Christiansen's review+ from obsolete attachment 53838 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[Jocelyn Turcotte]

Comment on attachment 53948 [details]
MSVC 64 build fix (see comment)

Patch has been moved to bug #37980

[WebKit Commit Bot]

Comment on attachment 53840 [details]
Workaround syncqt dependency mechanism

Clearing flags on attachment: 53840

Committed r58092: <http://trac.webkit.org/changeset/58092>

[WebKit Commit Bot]

Comment on attachment 53842 [details]
Corrects autotests resources compilation in QTDIR_build

Clearing flags on attachment: 53842

Committed r58096: <http://trac.webkit.org/changeset/58096>

[WebKit Commit Bot]

Comment on attachment 53852 [details]
Build fix when !ENABLED(NETSCAPE_PLUGIN_API) v2

Rejecting patch 53852 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Simon Hausmann', u'--force']" exit_code: 1
patching file WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebCore/plugins/PluginView.cpp
Hunk #1 succeeded at 1297 with fuzz 2 (offset -1 lines).
Hunk #2 succeeded at 1308 (offset -1 lines).
Hunk #3 succeeded at 1318 with fuzz 1 (offset -1 lines).
patching file WebCore/plugins/PluginViewNone.cpp
Hunk #1 FAILED at 126.
1 out of 1 hunk FAILED -- saving rejects to file WebCore/plugins/PluginViewNone.cpp.rej

Full output: http://webkit-commit-queue.appspot.com/results/1863029

[WebKit Commit Bot]

Comment on attachment 53854 [details]
Corrects JSC symbols visibility v2

Clearing flags on attachment: 53854

Committed r58100: <http://trac.webkit.org/changeset/58100>

[WebKit Commit Bot]

Comment on attachment 53946 [details]
Remove dep of JSC to QtGui

Clearing flags on attachment: 53946

Committed r58104: <http://trac.webkit.org/changeset/58104>

[WebKit Commit Bot]

Comment on attachment 53947 [details]
Remove autotest tr strings

Clearing flags on attachment: 53947

Committed r58105: <http://trac.webkit.org/changeset/58105>

[Laszlo Gombos]

Comment on attachment 53852 [details]
Build fix when !ENABLED(NETSCAPE_PLUGIN_API) v2

Marking patch obsolete as this issue a DUP of bug 36621 and patch failed to apply as fix for bug 36621 is already landed.

Next time I think it would be better to open a separate bug for separate issues.

[Laszlo Gombos]

Comment on attachment 53950 [details]
MSVC 2010 build fix

JavaScriptCore/runtime/Structure.cpp:172
 +          else
Can we use c++ cast here instead of the C style cast ? 

JavaScriptCore/runtime/Structure.cpp:179
 +      }
Same here.

[WebKit Commit Bot]

Comment on attachment 53929 [details]
Build fix on Mac v3

Rejecting patch 53929 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Tor Arne Vestb\xf8', u'--force']" exit_code: 1
patching file JavaScriptCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file JavaScriptCore/JavaScriptCore.pri
Hunk #2 succeeded at 72 (offset 2 lines).
patching file JavaScriptCore/JavaScriptCore.pro
Hunk #1 FAILED at 8.
1 out of 1 hunk FAILED -- saving rejects to file JavaScriptCore/JavaScriptCore.pro.rej

Full output: http://webkit-commit-queue.appspot.com/results/1796120

[Jocelyn Turcotte]

Comment on attachment 53929 [details]
Build fix on Mac v3

Clearing flags on attachment: 53929

Committed r58306: <http://trac.webkit.org/changeset/58306>

[Jocelyn Turcotte]

Comment on attachment 53949 [details]
Disable Cpp0x (see comment)

Clearing flags on attachment: 53949

Committed r58305: <http://trac.webkit.org/changeset/58305>

[Jocelyn Turcotte]

Comment on attachment 53950 [details]
MSVC 2010 build fix

Clearing flags on attachment: 53950

Committed r58308: <http://trac.webkit.org/changeset/58308>

[Jocelyn Turcotte]

Resolving bug after all patches have been landed

(In reply to comment #30)
> Next time I think it would be better to open a separate bug for separate
> issues.

Yep agreed, this bug became a bigger mess than I thought.

[WebKit Review Bot]

http://trac.webkit.org/changeset/58305 might have broken Qt Linux ARMv5 Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/58305
http://trac.webkit.org/changeset/58306
http://trac.webkit.org/changeset/58307
http://trac.webkit.org/changeset/58308

[WebKit Review Bot]

http://trac.webkit.org/changeset/58306 might have broken Qt Linux ARMv5 Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/58305
http://trac.webkit.org/changeset/58306
http://trac.webkit.org/changeset/58307
http://trac.webkit.org/changeset/58308

[WebKit Review Bot]

http://trac.webkit.org/changeset/58308 might have broken Qt Linux ARMv5 Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/58305
http://trac.webkit.org/changeset/58306
http://trac.webkit.org/changeset/58307
http://trac.webkit.org/changeset/58308

[Simon Hausmann]

<cherry-pick-for-backport: r58307>

[Simon Hausmann]

Revision r58092 cherry-picked into qtwebkit-2.0 with commit d5e8869917b882c4d2a6e449682bd12d3ca95e65
Revision r58096 cherry-picked into qtwebkit-2.0 with commit e765f06a9508bea1d49a883d811ddaca7c92b50a
Revision r58100 cherry-picked into qtwebkit-2.0 with commit f07a3c62f795c418e6656ef664b14ed50282aea0
Revision r58104 cherry-picked into qtwebkit-2.0 with commit 11d902dbdede03de1637ead81fac9bd3a69ebc3b
Revision r58105 cherry-picked into qtwebkit-2.0 with commit e4ff26f12a8ce50d12662487f2a1d8c1820eabb2
Revision r58305 cherry-picked into qtwebkit-2.0 with commit 70fd060abd5646593a64d8f17518f67fc3a058d6
Revision r58306 cherry-picked into qtwebkit-2.0 with commit ec1dbe948b373b65ea3727518da577a089158a46
Revision r58307 cherry-picked into qtwebkit-2.0 with commit 9f1b689350cc6aa46978a3c1eedfdda7199c2072
Revision r58308 cherry-picked into qtwebkit-2.0 with commit b3af4e186eda0106db92c90cc2adaa7f8d26f169