Bug 858084

Summary: Review Request: mingw-qt5-qtwebkit - Qt5 for Windows - QtWebKit component
Product: [Fedora] Fedora Reporter: Erik van Pienbroek <erik-fedora>
Component: Package ReviewAssignee: Michael Cronenworth <mike>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: fedora-mingw, package-review, rdieter
Target Milestone: ---Flags: mike: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: mingw-qt5-qtwebkit-5.2.1-2.fc20 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-03-17 03:51:51 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 858058, 858064, 858069, 858070, 858077    
Bug Blocks:    

Description Erik van Pienbroek 2012-09-17 22:52:41 UTC
Spec URL: http://svn.openftd.org/svn/fedora_cross/mingw-qt5-qtwebkit/mingw-qt5-qtwebkit.spec
SRPM URL: http://ftd4linux.nl/contrib/mingw-qt5-qtwebkit-5.0.0-0.1.beta1.fc17.src.rpm
Fedora Account System Username: epienbro
Description: 
This package contains the Qt software toolkit for developing
cross-platform applications.

This is the Windows version of Qt, for use in conjunction with the
Fedora Windows cross-compiler.

Comment 1 Erik van Pienbroek 2012-11-13 00:54:03 UTC
Updated Spec URL: http://svn.openftd.org/svn/fedora_cross/mingw-qt5-qtwebkit/mingw-qt5-qtwebkit.spec
Updated SRPM URL: http://build1.openftd.org/fedora-cross/src/mingw-qt5-qtwebkit-5.0.0-0.2.beta1.git20121112.23037105.fc19.src.rpm

* Mon Nov 12 2012 Erik van Pienbroek <epienbro> - 5.0.0-0.2.beta1.git20121112.23037105
- Update to 20121112 snapshot (rev 23037105)
- Rebuild against latest mingw-qt5-qtbase
- Dropped pkg-config rename hack as it's unneeded now
- Dropped upstreamed patch
- Added BR: python
- Added BR: ruby

Comment 2 Erik van Pienbroek 2013-05-12 18:39:41 UTC
Updated Spec URL: http://svn.nntpgrab.nl/svn/fedora_cross/mingw-qt5-qtwebkit/mingw-qt5-qtwebkit.spec
Updated SRPM URL: http://koji.vanpienbroek.nl/kojifiles/packages/mingw-qt5-qtwebkit/5.0.2/1.fc20/src/mingw-qt5-qtwebkit-5.0.2-1.fc20.src.rpm
Koji scratch build: https://koji.vanpienbroek.nl/koji/buildinfo?buildID=146

* Fri May 10 2013 Erik van Pienbroek <epienbro> - 5.0.2-1
- Update to 5.0.2
- Added BR: flex perl-version perl-Digest-MD5

* Sun Jan 06 2013 Erik van Pienbroek <epienbro> - 5.0.0-1
- Update to QtWebKit 5.0.0 Final
- Workaround linker failure caused by recent QWebKit/QWebKitWidgets split
- Use the Qt4 unicode API as the mingw-qt5-qtbase currently doesn't use ICU
- Added BR: mingw32-pkg-config mingw64-pkg-config mingw32-sqlite mingw64-sqlite
- Use the bundled ANGLE libraries for now as qtwebkit depends on non-public symbols

Comment 3 Rex Dieter 2013-05-17 12:34:22 UTC
fyi, a bundled code exception was granted wrt mt19937ar.c :
 https://fedorahosted.org/fpc/ticket/291

so, you can add:
Provides: bundled(mt19937ar)


I started working on packaging angleproject too for qt5-qtwebkit review (bug #915006), then noticed yours here and the recent comment about about having to use the bundled ANGLE lib.

As you seem to understand the background and use of ANGLE here (better than me at least), would you submitting a bundled code exception request per
https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Exceptions
?  
Or lacking that, give some detail/justification about why we can't unbundle angleproject here?

Comment 4 Erik van Pienbroek 2013-05-17 14:23:22 UTC
Hi Rex,

Thanks for the info about mt19937ar.c. I'll add the Provides tag you mentioned.

Regarding ANGLE: The 'ANGLE project' library is a Windows-specific library which allows using the OpenGL ES 2.0 (GLESv2) API on Windows environments (there is no native support for GLESv2 on Windows at the moment). This is done by translating GLESv2 API calls to DirectX 9 API calls.

It is correct that the qtwebkit source tarball contains a bundled copy of the 'ANGLE project' library. As it is a Windows-specific library it shouldn't be used when compiling qtwebkit for Linux. Whether to manually remove all bundled ANGLE pieces during %prep in the native qt5-qtwebkit package is a choice I leave up to you.

Side-note: multiple other packages also contain a bundled version of the ANGLE libraries in their source tarballs. Basically everything which is based on webkit, thus also webkitgtk and chromium. The qt5-qtbase source tarball also contains a bundled copy of the ANGLE libraries.

Regarding the use of ANGLE in this package: Initially I've tried to get ANGLE un-bundled from both qt5-qtbase and qt5-qtwebkit. For qt5-qtbase I managed to get it un-bundled successfully (which resulted in the mingw-angleproject package). Unfortunately un-bundling ANGLE from the qt5-qtwebkit package turned out to be much harder than expected. 

The main problem is that the ANGLE libraries are forked multiple times. Right now I'm aware of 4 separate sources which each have their own version of ANGLE:
* Upstream ANGLE project: http://code.google.com/p/angleproject/
* qt5-qtbase: http://qt.gitorious.org/qt/qtbase/trees/dev/src/3rdparty/angle
* qt5-qtwebkit: http://qt.gitorious.org/qt/qtwebkit/trees/dev/Source/ThirdParty/ANGLE
* webkit/webkitgtk: http://trac.webkit.org/browser/trunk/Source/ThirdParty/ANGLE

This has resulted in the fact that the ANGLE library as used by qt5-qtwebkit has a different set of functions and features than the upstream version of ANGLE.

Another major issue is that qt5-qtwebkit relies on internal symbols which are only visible when ANGLE is built as a static library.
Normally ANGLE is always built as a static library. We had to do some major hacking to get a proper shared library built in the mingw-angleproject package.
When trying to link qt5-qtwebkit against the shared library from this mingw-angleproject package this results in build failures due to unresolved symbols.

I've already spent multiple days trying to get the ANGLE pieces properly un-bundled from qt5-qtwebkit (as can be seen by patch 3 in the mingw-qt5-qtwebkit spec file), but
eventually I gave up.. So therefore we're using the bundled copy of ANGLE in the mingw-qt5-qtwebkit package for now. The .spec file also contains various comments about this subject

Comment 5 Erik van Pienbroek 2013-05-17 14:34:13 UTC
On second thought, mt19937ar.c isn't being used during compilation (for both Linux and Windows):

[erik@erik qtwebkit-opensource-src-5.0.2]$ grep -Hr mt19937ar *
ChangeLog-2012-05-22:        Move wince/mt19937ar.c to ThirdParty and make it a policy choice
ChangeLog-2012-05-22:        * Source/ThirdParty/mt19937ar.c: Copied from Source/JavaScriptCore/wtf/wince/mt19937ar.c.
Source/WTF/WTF.pro:    # for mt19937ar.c
Source/WTF/wtf/RandomNumber.cpp:#include "mt19937ar.c"
Source/WTF/ChangeLog:        Also include path for mt19937ar.c
Source/JavaScriptCore/ChangeLog-2011-02-16:        Move wince/mt19937ar.c to ThirdParty and make it a policy choice
Source/JavaScriptCore/ChangeLog-2010-05-24:        * wtf/wince/mt19937ar.c: Added.

--> mt19937ar.c is only used during the compilation of Source/WTF/wtf/RandomNumber.cpp, taking a closer look at this file:

#if USE(MERSENNE_TWISTER_19937)
extern "C" {
#include "mt19937ar.c"
}
#endif

--> mt19937ar.c is only used when MERSENNE_TWISTER_19937 is set, searching for places where it is set:

[erik@erik qtwebkit-opensource-src-5.0.2]$ grep -Hr MERSENNE_TWISTER_19937 *
Source/WTF/wtf/RandomNumberSeed.h:#if USE(MERSENNE_TWISTER_19937)
Source/WTF/wtf/RandomNumberSeed.h:#if USE(MERSENNE_TWISTER_19937)
Source/WTF/wtf/RandomNumber.cpp:#if USE(MERSENNE_TWISTER_19937)
Source/WTF/wtf/RandomNumber.cpp:#if USE(MERSENNE_TWISTER_19937)
Source/WTF/wtf/Platform.h:#define WTF_USE_MERSENNE_TWISTER_19937 1
Source/WTF/wtf/Platform.h:#define WTF_USE_MERSENNE_TWISTER_19937 1
Source/WebKit/blackberry/WebCoreSupport/AboutDataUseFeatures.in:MERSENNE_TWISTER_19937
Source/JavaScriptCore/ChangeLog-2011-02-16:        Modify RandomNumberSeed.h to use USE(MERSENNE_TWISTER_19937)
Source/JavaScriptCore/ChangeLog-2011-02-16:        * wtf/Platform.h: Defined WTF_USE_MERSENNE_TWISTER_19937 when
Source/JavaScriptCore/ChangeLog-2011-02-16:        (WTF::randomNumber): Substituted USE(MERSENNE_TWISTER_19937) for OS(WINCE).

--> The flag is only set in Source/WTF/wtf/Platform.h, taking a closer look at this:

#if PLATFORM(BLACKBERRY)
#define WTF_USE_MERSENNE_TWISTER_19937 1
<snip>
#endif

..

#if OS(WINCE)
#define WTF_USE_MERSENNE_TWISTER_19937 1
#endif

--> Conclusion: the code from mt19937ar.c is only being used for the BlackBerry and WindowsCE targets

Comment 6 Rex Dieter 2013-05-17 15:24:06 UTC
Heh, thanks for the detective-work wrt mt19937ar.c

Wrt angleproject, I wish it were that simple.  It is actually used in qtwebkit for all platforms, something to do with webGL support.

So, if I understand you correctly, angleproject may be possible to be unbundled if the default static linking is used?

Comment 7 Erik van Pienbroek 2013-05-17 16:22:57 UTC
(In reply to comment #6)
> Wrt angleproject, I wish it were that simple.  It is actually used in
> qtwebkit for all platforms, something to do with webGL support.

I read about this in your qt5-qtwebkit review ticket. Did you already try just removing the bundled ANGLE code and see what fails during the build? Perhaps the additional pieces from ANGLE aren't actually used by qt5-qtwebkit

> So, if I understand you correctly, angleproject may be possible to be
> unbundled if the default static linking is used?

I would have to try that out, but theoretically it should be possible. 

Unfortunately this still doesn't resolve the fact that each fork of ANGLE has its own set of features and internal symbols. Perhaps things have improved in the last couple of months (I haven't done investigations in this area recently) but I'm afraid it will still end up in a messy situation..

This also makes me wonder which a better situation: linking against a static ANGLE library which is bundled with qt5-qtwebkit, or linking against a static ANGLE library which is part of a separate mingw-angleproject-static package. Both aren't really ideal..

Comment 8 Rex Dieter 2013-05-17 16:26:57 UTC
For what it's worth... yes, I tried removing it.  As I said, it's included/built conditionally whether webgl feature is enabled.

Comment 9 Rex Dieter 2013-05-18 14:05:31 UTC
angle bundling exception submitted,
https://fedorahosted.org/fpc/ticket/293

feel free to comment there if you have anything to add (or correct).

Comment 10 Erik van Pienbroek 2013-05-18 16:21:51 UTC
Thanks for the heads up. I just subscribed myself to that ticket

Earlier today I managed to get ANGLE successfully un-bundled from mingw-qt5-qtwebkit. Luckily the webkit developers managed to get more stuff upstreamed and reduced their amount of local patches. After updating mingw-angleproject to the latest snapshot the 'missing features' issue was resolved.

The other major blocker was the use of symbols which are only exported in the one of the ANGLE static libraries. To workaround this I adjusted the list of symbols which are exported in the libGLESv2.dll library: http://pkgs.fedoraproject.org/cgit/mingw-angleproject.git/commit/?id=b353fa9cfc3fb5995340ce2242a1be08d3cd05ab

With these changes and http://svn.nntpgrab.nl/svn/fedora_cross/mingw-qt5-qtwebkit/qtwebkit-dont-use-bundled-angle-libraries.patch I managed to get mingw-qt5-qtwebkit built using the ANGLE shared library from the mingw-angleproject package.

I'm currently doing some more tests to be more certain that everything works but things look good so far.

Comment 11 Erik van Pienbroek 2013-05-20 11:26:43 UTC
By un-bundling ANGLE and using the shared library from the mingw-angleproject package (which was already used by mingw-qt5-qtbase) we managed to get the size of the Qt5WebKit.dll library reduced with 2,7MB (from 23,7MB to 21MB)! The build logs are now also much easier to read due to the backported patches which resolve a flood of compiler warnings when using gcc 4.8


Updated Spec URL: http://svn.nntpgrab.nl/svn/fedora_cross/mingw-qt5-qtwebkit/mingw-qt5-qtwebkit.spec
Updated SRPM URL: http://koji.vanpienbroek.nl/kojifiles/packages/mingw-qt5-qtwebkit/5.0.2/2.fc20/src/mingw-qt5-qtwebkit-5.0.2-2.fc20.src.rpm
Koji scratch build: http://koji.vanpienbroek.nl/koji/buildinfo?buildID=147

* Sat May 18 2013 Erik van Pienbroek <epienbro> - 5.0.2-2
- Bumped the BR: mingw{32,64}-angleproject to >= 0-0.5.svn2215.20130517
- Don't use the bundled ANGLE libraries any more
- Applied some upstream patches to prevent flooding the logs with
  compiler warnings when using gcc 4.8
- Added BR: mingw32-qt5-qtmultimedia mingw64-qt5-qtmultimedia
- Added BR: mingw32-libwebp mingw64-libwebp

Comment 12 Erik van Pienbroek 2013-08-01 15:52:38 UTC
Updated Spec URL: http://svn.nntpgrab.nl/svn/fedora_cross/mingw-qt5-qtwebkit/mingw-qt5-qtwebkit.spec
Updated SRPM URL: http://koji.vanpienbroek.nl/kojifiles/packages/mingw-qt5-qtwebkit/5.1.0/2.fc20/src/mingw-qt5-qtwebkit-5.1.0-2.fc20.src.rpm
Koji scratch build: http://koji.vanpienbroek.nl/koji/buildinfo?buildID=161


* Wed Jul 24 2013 Erik van Pienbroek <epienbro> - 5.1.0-2
- Fix detection of native tools which started to fail as of QtWebkit 5.1.0
- Avoid 'too many sections' build failure

* Sun Jul 14 2013 Erik van Pienbroek <epienbro> - 5.1.0-1
- Update to 5.1.0

Comment 13 Michael Cronenworth 2013-08-15 05:38:58 UTC
$ rpmlint Downloads/mingw-qt5-qtwebkit-5.1.0-2.fc20.src.rpm 
mingw-qt5-qtwebkit.src:30: W: macro-in-comment %{snapshot_rev}
mingw-qt5-qtwebkit.src:30: W: macro-in-comment %{snapshot_rev}
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

$ rpmlint Downloads/mingw-qt5-qtwebkit.spec 
Downloads/mingw-qt5-qtwebkit.spec:30: W: macro-in-comment %{snapshot_rev}
Downloads/mingw-qt5-qtwebkit.spec:30: W: macro-in-comment %{snapshot_rev}
0 packages and 1 specfiles checked; 0 errors, 2 warnings.

+ OK
! needs attention

+ The package is named according to Fedora MinGW packaging guidelines
+ The spec file name matches the base package name.
+ The package meets the Packaging Guidelines
+ The package is licensed with a Fedora approved license and meets the
  Licensing Guidelines.
+ The license field in the spec file matches the actual license
+ The stated license is the same as the one for the corresponding
  native Fedora package
! The package contains the license file (COPYING.lib)
+ Spec file is written in American English
+ Spec file is legible
+ Upstream sources match sources in the srpm. md5sum:
  fad7e7d6637719c2be101f9b3957d451  Downloads/qtwebkit-opensource-src-5.1.0.tar.xz
  fad7e7d6637719c2be101f9b3957d451  rpmbuild/SOURCES/qtwebkit-opensource-src-5.1.0.tar.xz
+ The package builds in koji
n/a ExcludeArch bugs filed
+ BuildRequires look sane
n/a locale handling
n/a ldconfig in %post and %postun
+ Package does not bundle copies of system libraries
n/a Package isn't relocatable
+ Package owns all directories it creates
+ No duplicate files in %files
+ Permissions are properly set
+ Consistent use of macros
+ The package must contain code or permissible content
n/a Large documentation files should go in -doc subpackage
+ Files marked %doc should not affect package
n/a Header files should be in -devel
Not applicable to MinGW packages.
n/a Static libraries should be in -static
n/a Library files that end in .so must go in a -devel package
n/a -devel must require the fully versioned base
+ Packages must not contain libtool .la files
n/a Packages containing GUI apps must include %{name}.desktop file
+ Directory ownership sane
+ Filenames are valid UTF-8

-----
Summary:
 - License files are missing. See native package for how to handle this.
 - The rest looks OK.

Comment 14 Erik van Pienbroek 2013-11-17 16:04:04 UTC
Updated Spec URL: http://svn.nntpgrab.nl/svn/fedora_cross/mingw-qt5-qtwebkit/mingw-qt5-qtwebkit.spec
Updated SRPM URL: http://koji.vanpienbroek.nl/kojifiles/packages/mingw-qt5-qtwebkit/5.1.1/1.fc21/src/mingw-qt5-qtwebkit-5.1.1-1.fc21.src.rpm
Koji scratch build: http://koji.vanpienbroek.nl/koji/buildinfo?buildID=170

* Sun Sep 22 2013 Erik van Pienbroek <epienbro> - 5.1.1-1
- Update to 5.1.1
- Added license files

Comment 15 Erik van Pienbroek 2013-12-12 21:45:43 UTC
Updated Spec URL: http://svn.nntpgrab.nl/svn/fedora_cross/mingw-qt5-qtwebkit/mingw-qt5-qtwebkit.spec
Updated SRPM URL: http://koji.vanpienbroek.nl/kojifiles/packages/mingw-qt5-qtwebkit/5.2.0/0.1.rc1.fc21/src/mingw-qt5-qtwebkit-5.2.0-0.1.rc1.fc21.src.rpm
Koji scratch build: http://koji.vanpienbroek.nl/koji/buildinfo?buildID=175

* Fri Nov 29 2013 Erik van Pienbroek <epienbro> - 5.2.0-0.1.rc1
- Update to 5.2.0 RC1

Comment 16 Michael Cronenworth 2014-01-06 21:08:48 UTC
$ rpmlint mingw-qt5-qtwebkit.spec
mingw-qt5-qtwebkit.spec:30: W: macro-in-comment %{snapshot_rev}
mingw-qt5-qtwebkit.spec:30: W: macro-in-comment %{snapshot_rev}
0 packages and 1 specfiles checked; 0 errors, 3 warnings.

$ rpmlint mingw-qt5-qtwebkit-5.2.0-0.1.rc1.fc21.src.rpm
mingw-qt5-qtwebkit.src: E: unknown-key GPG#9b393ae8
mingw-qt5-qtwebkit.src:30: W: macro-in-comment %{snapshot_rev}
mingw-qt5-qtwebkit.src:30: W: macro-in-comment %{snapshot_rev}
1 packages and 0 specfiles checked; 1 errors, 4 warnings.

+ OK
! needs attention

+ The package is named according to Fedora MinGW packaging guidelines
+ The spec file name matches the base package name.
+ The package meets the Packaging Guidelines
+ The package is licensed with a Fedora approved license and meets the
  Licensing Guidelines.
+ The license field in the spec file matches the actual license
+ The stated license is the same as the one for the corresponding
  native Fedora package
+ The package contains the license file (COPYING.lib)
+ Spec file is written in American English
+ Spec file is legible
+ Upstream sources match sources in the srpm. md5sum:
  0236ee173b2ba63a6a98974ffd791261  Downloads/qtwebkit-opensource-src-5.2.0-rc1.tar.xz
  0236ee173b2ba63a6a98974ffd791261  rpmbuild/SOURCES/qtwebkit-opensource-src-5.2.0-rc1.tar.xz
+ The package builds in koji
n/a ExcludeArch bugs filed
+ BuildRequires look sane
n/a locale handling
n/a ldconfig in %post and %postun
+ Package does not bundle copies of system libraries
n/a Package isn't relocatable
+ Package owns all directories it creates
+ No duplicate files in %files
+ Permissions are properly set
+ Consistent use of macros
+ The package must contain code or permissible content
n/a Large documentation files should go in -doc subpackage
+ Files marked %doc should not affect package
n/a Header files should be in -devel
Not applicable to MinGW packages.
n/a Static libraries should be in -static
n/a Library files that end in .so must go in a -devel package
n/a -devel must require the fully versioned base
+ Packages must not contain libtool .la files
n/a Packages containing GUI apps must include %{name}.desktop file
+ Directory ownership sane
+ Filenames are valid UTF-8

-----

APPROVED

Comment 17 Erik van Pienbroek 2014-01-06 22:14:40 UTC
Thanks for the review!

New Package SCM Request
=======================
Package Name: mingw-qt5-qtwebkit
Short Description: Qt5 for Windows - QtWebKit component
Owners: epienbro
Branches: f19 f20
InitialCC:

Comment 18 Gwyn Ciesla 2014-01-07 13:03:00 UTC
Git done (by process-git-requests).

Comment 19 Fedora Update System 2014-03-06 17:35:32 UTC
mingw-qt5-qtwebkit-5.2.1-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/mingw-qt5-qtwebkit-5.2.1-2.fc19

Comment 20 Fedora Update System 2014-03-06 17:36:32 UTC
mingw-qt5-qtwebkit-5.2.1-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/mingw-qt5-qtwebkit-5.2.1-2.fc20

Comment 21 Fedora Update System 2014-03-07 06:25:20 UTC
mingw-qt5-qtwebkit-5.2.1-2.fc19 has been pushed to the Fedora 19 testing repository.

Comment 22 Fedora Update System 2014-03-17 03:51:51 UTC
mingw-qt5-qtwebkit-5.2.1-2.fc19 has been pushed to the Fedora 19 stable repository.

Comment 23 Fedora Update System 2014-03-17 03:56:48 UTC
mingw-qt5-qtwebkit-5.2.1-2.fc20 has been pushed to the Fedora 20 stable repository.