Bug 57017

Summary:

[Qt] Properly detect phonon include, and avoid double qtLibraryTarget() call

Product:

WebKit

Reporter:

Kristian Amlie <kristian.amlie>

Component:

WebKit Qt

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Critical

CC:

benjamin, commit-queue

Priority:

Keywords:

Qt, QtTriaged

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Properly-detect-phonon-include-and-avoid-double-qtLibraryTarget-call	none
Properly-detect-phonon-include-and-avoid-double-qtLibraryTarget-call-v2	menard: review-, menard: commit-queue-
Changelog fix.	none

Kristian Amlie

Reported 2011-03-24 08:19:57 PDT

This is needed for Qt Modularization.

Attachments
Properly-detect-phonon-include-and-avoid-double-qtLibraryTarget-call (2.88 KB, patch) 2011-03-24 08:23 PDT, Kristian Amlie	no flags	Details Formatted Diff Diff
Properly-detect-phonon-include-and-avoid-double-qtLibraryTarget-call-v2 (2.71 KB, patch) 2011-03-25 01:25 PDT, Kristian Amlie	menard: review- menard: commit-queue-	Details Formatted Diff Diff
Changelog fix. (2.22 KB, patch) 2011-03-31 06:19 PDT, Alexis Menard (darktears)	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Kristian Amlie

Comment 1 2011-03-24 08:23:58 PDT

Created attachment 86776 [details] Properly-detect-phonon-include-and-avoid-double-qtLibraryTarget-call

Benjamin Poulain

Comment 2 2011-03-24 10:27:45 PDT

Comment on attachment 86776 [details] Properly-detect-phonon-include-and-avoid-double-qtLibraryTarget-call View in context: https://bugs.webkit.org/attachment.cgi?id=86776&action=review Why do you add the isEmpty(QT_SOURCE_TREE)? > Source/WebCore/ChangeLog:5 > + Properly detect phonon include, and avoid double qtLibraryTarget() call Unecessary, you just copied the bug title. > Source/WebKit/qt/ChangeLog:3 > + Reviewed by NOBODY (OOPS!). Unecessary, you just copied the bug title.

Kristian Amlie

Comment 3 2011-03-25 01:20:47 PDT

(In reply to comment #2) > (From update of attachment 86776 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=86776&action=review > > Why do you add the isEmpty(QT_SOURCE_TREE)? Because if the QtKernel sources are available (as indicated by the existence of QT_SOURCE_TREE), then qtLibraryTarget will already have been called from within qbase.pri. This can happen in the case where you build the modularized Qt modules in development mode, where the modules can see each others sources. If building for release, then this will not be the case, and the qtLibraryTarget needs to be called. Long term, the reliance on qbase.pri in QT_SOURCE_TREE should be removed, but this will be some additional work.

Kristian Amlie

Comment 4 2011-03-25 01:25:08 PDT

Created attachment 86908 [details] Properly-detect-phonon-include-and-avoid-double-qtLibraryTarget-call-v2 Fixed ChangeLogs.

Alexis Menard (darktears)

Comment 5 2011-03-29 10:27:20 PDT

Comment on attachment 86908 [details] Properly-detect-phonon-include-and-avoid-double-qtLibraryTarget-call-v2 Sorry to be picky Kristian but the changelog is not properly filled :(. http://trac.webkit.org/wiki/QtWebKitContrib Tells you how to do it. In your case : Precede the short description with "[Qt]" if your patch is specific to the Qt port of WebKit. The bug link should be right under the description and then after the explanation comes with a . at the end of the sentence. Thanks for the patch though.

Alexis Menard (darktears)

Comment 6 2011-03-31 06:19:37 PDT

Created attachment 87712 [details] Changelog fix.

Kristian Amlie

Comment 7 2011-03-31 07:53:15 PDT

Hey, sorry for the late response. I only just now got back to actual coding. :-P (In reply to comment #5) > (From update of attachment 86908 [details]) > Sorry to be picky Kristian but the changelog is not properly filled :(. > > http://trac.webkit.org/wiki/QtWebKitContrib > > Tells you how to do it. > > In your case : > Precede the short description with "[Qt]" if your patch is specific to the Qt port of WebKit. > The bug link should be right under the description and then after the explanation comes with a . at the end of the sentence. > > Thanks for the patch though. I guess I misinterpreted Benjamin's comment to mean "remove the bug description, and leave a standalone description". Thanks for fixing that though!

WebKit Commit Bot

Comment 8 2011-03-31 12:27:17 PDT

The commit-queue encountered the following flaky tests while processing attachment 87712 [details]: fast/workers/storage/use-same-database-in-page-and-workers.html bug 50995 (author: dumi@chromium.org) The commit-queue is continuing to process your patch.

WebKit Commit Bot

Comment 9 2011-03-31 12:30:46 PDT

Comment on attachment 87712 [details] Changelog fix. Clearing flags on attachment: 87712 Committed r82605: <http://trac.webkit.org/changeset/82605>

WebKit Commit Bot

Comment 10 2011-03-31 12:30:50 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.