Bug 121089

Summary: [Qt] Fix build with Qt 5.2 QtPositioning module.
Product: WebKit Reporter: Allan Sandfeld Jensen <allan.jensen>
Component: Tools / TestsAssignee: Allan Sandfeld Jensen <allan.jensen>
Status: RESOLVED FIXED    
Severity: Normal CC: hausmann, jturcotte, kadam, michael.bruning, zarvai
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 121090    
Bug Blocks:    
Attachments:
Description Flags
Patch hausmann: review+

Description Allan Sandfeld Jensen 2013-09-10 04:53:39 PDT
There is no QtLocation module in Qt5 anymore, and there was one released. Part of it will be released with Qt 5.2 as QtPosition. We need to update the build to make that work with QtWebKit.
Comment 1 Allan Sandfeld Jensen 2013-09-10 05:18:11 PDT
(In reply to comment #0)
> There is no QtLocation module in Qt5 anymore, and there was one released. Part of it will be released with Qt 5.2 as QtPosition. We need to update the build to make that work with QtWebKit.

Should read: There was never such a module released.
Comment 2 Allan Sandfeld Jensen 2013-09-10 05:31:44 PDT
Created attachment 211191 [details]
Patch
Comment 3 Allan Sandfeld Jensen 2013-09-25 05:08:33 PDT
Comment on attachment 211191 [details]
Patch

The naming seems to be final now, putting back for review.
Comment 4 Michael Brüning 2013-09-25 05:19:48 PDT
Comment on attachment 211191 [details]
Patch

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

LGTM except for the small comment in defaul_pre.prf

> Tools/qmake/mkspecs/features/default_pre.prf:72
> +

I think this might have leaked from another change...
Comment 5 Michael Brüning 2013-09-25 05:26:40 PDT
Comment on attachment 211191 [details]
Patch

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

>> Tools/qmake/mkspecs/features/default_pre.prf:72
>> +# We use private_tests to detect developer build, since the destdir will
>> +# always be our webkit build dir. This might change as configure changes.
>> +contains(QT_CONFIG, private_tests): CONFIG += qt_developer_build
>> +
>> +# By default we enable "production build", and build-webkit, which is
>> +# used by bots and developers, will disable it, to enable warnings etc.
>> +CONFIG += production_build
>> +
> 
> I think this might have leaked from another change...

I meant the whle section of course...
Comment 6 Allan Sandfeld Jensen 2013-09-25 05:43:04 PDT
(In reply to comment #5)
> (From update of attachment 211191 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=211191&action=review
> 
> >> Tools/qmake/mkspecs/features/default_pre.prf:72
> >> +# We use private_tests to detect developer build, since the destdir will
> >> +# always be our webkit build dir. This might change as configure changes.
> >> +contains(QT_CONFIG, private_tests): CONFIG += qt_developer_build
> >> +
> >> +# By default we enable "production build", and build-webkit, which is
> >> +# used by bots and developers, will disable it, to enable warnings etc.
> >> +CONFIG += production_build
> >> +
> > 
> > I think this might have leaked from another change...
> 
> I meant the whle section of course...

No, but it probably should be mentioned in the Changelog. The problem is the production_build variable was not set correctly in features.prf, so that code needed to be moved above it to work correctly.
Comment 7 Allan Sandfeld Jensen 2013-09-25 07:56:11 PDT
Committed r156395: <http://trac.webkit.org/changeset/156395>
Comment 8 Ádám Kallai 2013-09-26 00:58:13 PDT
Something is wrong with this change. The geolocation, devicemotion, deviceorientation tests are failing because the geolacation, device orientation, orientation events are disabled after this patch. The bots use Qt 5.1.1 with qtjsbackend, qtdeclarative modules.
Comment 9 Allan Sandfeld Jensen 2013-09-26 03:35:59 PDT
(In reply to comment #8)
> Something is wrong with this change. The geolocation, devicemotion, deviceorientation tests are failing because the geolacation, device orientation, orientation events are disabled after this patch. The bots use Qt 5.1.1 with qtjsbackend, qtdeclarative modules.

Are you building with qmake with production_build set? After the patch we now properly disable the mocked geolocation when production_build is set
Comment 10 Zoltan Arvai 2013-09-26 07:03:49 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > Something is wrong with this change. The geolocation, devicemotion, deviceorientation tests are failing because the geolacation, device orientation, orientation events are disabled after this patch. The bots use Qt 5.1.1 with qtjsbackend, qtdeclarative modules.
> 
> Are you building with qmake with production_build set? After the patch we now properly disable the mocked geolocation when production_build is set

Qt is buildt with
./configure -opensource -confirm-license -no-pch -nomake examples -nomake tests -no-gtkstyle -qt-zlib -qt-sql-sqlite -release -prefix /usr/local/Trolltech/Qt5/Qt-5.1.1

WebKit buildt with
perl ./Tools/Scripts/build-webkit --release --qt

Production build .............. no

Have .......................... qtquick qtprintsupport qstyle qttestlib xcomposite xrender glx fontconfig sqlite3 qttestsupport

More logs
http://build.webkit.org/builders/Qt%20Linux%20Release/builds/63252/steps/compile-webkit/logs/stdio

In Tools/qmake/mkspec/features/features.prf we have:

    # Geolocation support if QtPositioning exists or if we're doing a developer build (Mock implementation exists
    # and used for layout tests)
    have?(qtpositioning)|!production_build: WEBKIT_CONFIG += geolocation

    # Orientation support if QtSensors exists or if we're doing a developer build (Mock implementation exists
    # and used for layout tests)
    have?(qtsensors)|!production_build: WEBKIT_CONFIG += orientation_events device_orientation
Comment 11 Allan Sandfeld Jensen 2013-09-26 07:21:46 PDT
Yeah I know what the problem is. These configure tests are run from default_pre which is before command line arguments to qmake have been processed. So at this point we can not actually tell if it is a production_build or not.