Bug 93492 – [Qt] Make it possible to build without QtTest/QtPrintSuport

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

[Loïc Yhuel]

[Qt] Make it possible to build without QtTest/QtPrintSuport

[Loïc Yhuel]

Created attachment 157241 [details]
Patch

[Tor Arne Vestbø]

Comment on attachment 157241 [details]
Patch

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

> Tools/QtTestBrowser/launcherwindow.cpp:727
> +#if !defined(QT_NO_PRINTER) && defined(HAVE_QTPRINTSUPPORT) && HAVE_QTPRINTSUPPORT

Why not just HAVE(QPRINTSUPPORT)?

> Tools/Tools.pro:12
> +    contains(DEFINES, HAVE_QTTESTLIB=1): SUBDIRS += DumpRenderTree/qt/DumpRenderTree.pro

I'd like some sort of warning here, like in default_pre, that we're disabling DRT due to missing testlib.

[Loïc Yhuel]

(In reply to comment #2)
> (From update of attachment 157241 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=157241&action=review
> 
> > Tools/QtTestBrowser/launcherwindow.cpp:727
> > +#if !defined(QT_NO_PRINTER) && defined(HAVE_QTPRINTSUPPORT) && HAVE_QTPRINTSUPPORT
> 
> Why not just HAVE(QPRINTSUPPORT)?
It was a copy-paste of what I did in launcherwindow.h, where HAVE() macro is not defined. But in fact, the code in the header can be moved to the cpp.
> 
> > Tools/Tools.pro:12
> > +    contains(DEFINES, HAVE_QTTESTLIB=1): SUBDIRS += DumpRenderTree/qt/DumpRenderTree.pro
> 
> I'd like some sort of warning here, like in default_pre, that we're disabling DRT due to missing testlib.
Only for DRT, or WTR and tests too ?

[Simon Hausmann]

Comment on attachment 157241 [details]
Patch

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

> Source/WebKit/qt/Api/qwebframe.cpp:1468
> +#if HAVE(QTPRINTSUPPORT)

I think here and in the other function it might be good to have an

    #else
    UNUSED_PARAM(printer)
    #endif

or the like, to avoid a compiler warning.

[Loïc Yhuel]

(In reply to comment #4)
> (From update of attachment 157241 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=157241&action=review
> 
> > Source/WebKit/qt/Api/qwebframe.cpp:1468
> > +#if HAVE(QTPRINTSUPPORT)
> 
> I think here and in the other function it might be good to have an
> 
>     #else
>     UNUSED_PARAM(printer)
>     #endif
> 
> or the like, to avoid a compiler warning.

There is no warning since -Wno-unused-parameter is added to QMAKE_CXXFLAGS in default_post.prf.

[Simon Hausmann]

Comment on attachment 157241 [details]
Patch

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

>>> Source/WebKit/qt/Api/qwebframe.cpp:1468
>>> +#if HAVE(QTPRINTSUPPORT)
>> 
>> I think here and in the other function it might be good to have an
>> 
>>     #else
>>     UNUSED_PARAM(printer)
>>     #endif
>> 
>> or the like, to avoid a compiler warning.
> 
> There is no warning since -Wno-unused-parameter is added to QMAKE_CXXFLAGS in default_post.prf.

Ok

>>> Tools/Tools.pro:12
>>> +    contains(DEFINES, HAVE_QTTESTLIB=1): SUBDIRS += DumpRenderTree/qt/DumpRenderTree.pro
>> 
>> I'd like some sort of warning here, like in default_pre, that we're disabling DRT due to missing testlib.
> 
> Only for DRT, or WTR and tests too ?

All three of them I'd say, or more generally - if done in default_pre.prf like suggested - visible early when WebKit.pro is parsed.

[Loïc Yhuel]

(In reply to comment #6)
> >>> Tools/Tools.pro:12
> >>> +    contains(DEFINES, HAVE_QTTESTLIB=1): SUBDIRS += DumpRenderTree/qt/DumpRenderTree.pro
> >> 
> >> I'd like some sort of warning here, like in default_pre, that we're disabling DRT due to missing testlib.
> > 
> > Only for DRT, or WTR and tests too ?
> 
> All three of them I'd say, or more generally - if done in default_pre.prf like suggested - visible early when WebKit.pro is parsed.

I added it in features.prf, to have the message and test at the same place. It's printed in configure step, just after pkg-config tests. Tell me if you prefer to have it in default_pre.prf instead.

[Loïc Yhuel]

Created attachment 157900 [details]
Patch

[WebKit Review Bot]

Comment on attachment 157900 [details]
Patch

Clearing flags on attachment: 157900

Committed r125377: <http://trac.webkit.org/changeset/125377>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.