Bug 1295549 (qt5-qtwebengine, qtwebengine)

Summary: Review Request: qt5-qtwebengine - Qt5 - QtWebEngine components
Product: [Fedora] Fedora Reporter: Kevin Kofler <kevin>
Component: Package ReviewAssignee: Rex Dieter <rdieter>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: helio, i, package-review
Target Milestone: ---Flags: rdieter: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-01-10 22:13:07 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 928937, 1303611    

Description Kevin Kofler 2016-01-04 19:46:26 UTC
Spec URL: http://copr-dist-git.fedorainfracloud.org/cgit/kkofler/qtwebengine/qt5-qtwebengine.git/plain/qt5-qtwebengine.spec?id=67f67e9630dcebb2b7438852b92ed63e230f0648
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kkofler/qtwebengine/fedora-23-x86_64/00151375-qt5-qtwebengine/qt5-qtwebengine-5.5.1-3.fc23.src.rpm
Description: Qt5 - QtWebEngine components

Fedora Account System Username: kkofler

This is a resubmission of stalled review bug #1244196 with a new submitter.

Known issues:
* This is currently a package of QtWebEngine 5.5.1 (up from 5.5.0 from the old review). I'll be looking at 5.6.0 next.
* The i686 build requires SSE2. I have a patch against 5.6.0 that should fix that (which I attached to the old review), but I haven't tested it at all yet, so it is not yet included here.
* There is no GStreamer support yet, so patent-encumbered codecs are just not supported at all. (We have to remove them from the bundled FFmpeg, which is what the spec/SRPM I'm submitting already does.)

Comment 1 Kevin Kofler 2016-01-04 19:48:05 UTC
*** Bug 1244196 has been marked as a duplicate of this bug. ***

Comment 2 Kevin Kofler 2016-01-04 20:46:06 UTC
* I noticed at least two more BRs that appears to be unused: pkgconfig(libexif) and pkgconfig(hunspell). I don't see these being included/linked anywhere (nor do I see a bundled versions being built, either; the strings "exif" and "hunspell" do not show up at all in my build.log).

Comment 3 Kevin Kofler 2016-01-04 20:49:22 UTC
pkgconfig(gio-2.0) is also not used, QtWebEngine explicitly passes -Duse_gio=0. I guess I need to go through each and every BuildRequire and check with build.log whether it's actually used. :-(

Comment 4 Kevin Kofler 2016-01-05 02:36:29 UTC
Spec URL: http://copr-dist-git.fedorainfracloud.org/cgit/kkofler/qtwebengine/qt5-qtwebengine.git/plain/qt5-qtwebengine.spec?id=a63544c8d0e9c07fbe3649f7183bff67114e0c61
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kkofler/qtwebengine/fedora-23-x86_64/00151506-qt5-qtwebengine/qt5-qtwebengine-5.5.1-4.fc23.src.rpm

* Tue Jan 05 2016 Kevin Kofler <Kevin.org> - 5.5.1-4
- Remove unused BRs flex, libgcrypt-devel, bzip2-devel, pkgconfig(gio-2.0),
  pkgconfig(hunspell), pkgconfig(libpcre), pkgconfig(libssl),
  pkgconfig(libcrypto), pkgconfig(jsoncpp), pkgconfig(libmtp),
  pkgconfig(libexif), pkgconfig(liblzma), pkgconfig(cairo), pkgconfig(libusb),
  perl(version), perl(Digest::MD5), perl(Text::ParseWords), ruby
- Add missing explicit BRs on pkgconfig(x11),  pkgconfig(xext),
  pkgconfig(xfixes), pkgconfig(xdamage), pkgconfig(egl)
- Fix BR pkgconfig(flac++) to pkgconfig(flac) (libFLAC++ not used, only libFLAC)
- Fix BR python-devel to python
- Remove unused -Duse_system_openssl=1 flag (QtWebEngine uses NSS instead)
- Remove unused -Duse_system_jsoncpp=1 and -Duse_system_libusb=1 flags

This is probably the last 5.5.x build I'm doing, now we need to get started on 5.6.

Comment 5 Kevin Kofler 2016-01-06 15:04:42 UTC
Spec URL: http://copr-dist-git.fedorainfracloud.org/cgit/kkofler/qtwebengine/qt5-qtwebengine.git/plain/qt5-qtwebengine.spec?id=849178c1d044af50e13552490c81801565aef547
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kkofler/qtwebengine/fedora-rawhide-x86_64/00151796-qt5-qtwebengine/qt5-qtwebengine-5.6.0-0.3.beta.fc24.src.rpm
Description: Qt5 - QtWebEngine components.

* Wed Jan 06 2016 Kevin Kofler <Kevin.org> - 5.6.0-0.3.beta1
- linux-pri patch: Add use_system_protobuf, went missing in the 5.6 rebase

* Wed Jan 06 2016 Kevin Kofler <Kevin.org> - 5.6.0-0.2.beta1
- linux-pri patch: Add missing newline at the end of the log line
- Use export for NINJA_PATH (fixes system ninja-build use)

* Wed Jan 06 2016 Kevin Kofler <Kevin.org> - 5.6.0-0.1.beta1
- Readd BR pkgconfig(jsoncpp) because linux.pri now checks for it
- BR yasm only on x86 (i686, x86_64)
- Add dot at the end of %%description
- Rebase no-format patch
- Replace unbundle-gyp.patch with new linux-pri.patch
- Use system ninja-build instead of the bundled one
- Run the unbundling script replace_gyp_files.py in linux.pri rather than here
- Update file list for 5.6.0-beta (no more libffmpegsumo since Chromium 45)

Comment 6 Kevin Kofler 2016-01-06 15:06:06 UTC
So, this one is now for Rawhide, as required by the review guidelines.

Successful Rawhide build (i686 (with SSE2), x86_64):

Comment 7 Kevin Kofler 2016-01-06 15:23:41 UTC
I've just noticed that the License tag is incomplete. Spot's chromium.spec has this:
License: BSD and LGPLv2+ and ASL 2.0 and IJG and MIT and GPLv2+ and ISC and OpenSSL and (MPLv1.1 or GPLv2 or LGPLv2)
which is closer to the truth. We don't build with OpenSSL though, and of course we have to add the License of Qt's parts, so the License should be something like:
License: (LGPLv2 with exceptions or GPLv3 with exceptions) and BSD and LGPLv2+ and ASL 2.0 and IJG and MIT and GPLv2+ and ISC and (MPLv1.1 or GPLv2 or LGPLv2)
I'll fix it in the next build.

Comment 8 Rex Dieter 2016-01-07 19:10:35 UTC
An initial course review...

rpmlint clean

naming: ok

macros: ok

sources: n/a
I'll take your word on the stripping process.  will verify more in a later review pass

1.  license: tag needs work (per comment #7)

macros: ok (mostly)

2. SHOULD probably use instead:

%files examples

scriptlets: ok

3.  SHOULD add appropriate Provides: tags for bundled code, 
probably includes a reasonable reference for a starting point.

Comment 9 Kevin Kofler 2016-01-07 20:06:33 UTC
I'll update the specfile ASAP, probably tonight: 1. is already done in my local copy, 2. is trivial, but for 3., I need to check which of the Provides from chromium.spec actually apply here: QtWebEngine compiles only a subset of Chromium, so I need to check in the build.log what third_party stuff is actually being built (and the "Provides: bundled(kitchensink) = 1" probably doesn't belong ;-) ).

Comment 10 Kevin Kofler 2016-01-07 20:11:12 UTC
Oh, and some of the Provides: bundled(*) in chromium.spec are things I am actually unbundling, so those also don't belong.

Comment 11 Kevin Kofler 2016-01-08 13:02:11 UTC
Spec URL: http://copr-dist-git.fedorainfracloud.org/cgit/kkofler/qtwebengine/qt5-qtwebengine.git/plain/qt5-qtwebengine.spec?id=560090c51468c04db69325987997b19ef3cffe0d
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kkofler/qtwebengine/fedora-rawhide-x86_64/00152190-qt5-qtwebengine/qt5-qtwebengine-5.6.0-0.4.beta.fc24.src.rpm

* Fri Jan 08 2016 Kevin Kofler <Kevin.org> - 5.6.0-0.4.beta1
- Fix License tag
- Use %%_qt5_examplesdir macro
- Add Provides: bundled(*) for all the bundled libraries that I found

It took me 2 hours to check (and document, so that we know how to update it in the future) all the bundled stuff. :-(

Comment 12 Rex Dieter 2016-01-08 21:47:18 UTC
Thanks for the heroic effort here, approved.

Comment 13 Kevin Kofler 2016-01-08 21:56:52 UTC
Wow, thanks! I requested the package creation through pkgdb.

Comment 14 Gwyn Ciesla 2016-01-09 01:07:49 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/qt5-qtwebengine

Comment 15 Kevin Kofler 2016-01-10 22:13:07 UTC
qt5-qtwebengine-5.6.0-0.6.beta.fc24 now successfully built in Rawhide.