https://bugzilla.redhat.com/show_bug.cgi?id=1295549
Bug ID: 1295549 Summary: Review Request: qt5-qtwebengine - Qt5 - QtWebEngine components Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: kevin@tigcc.ticalc.org QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: http://copr-dist-git.fedorainfracloud.org/cgit/kkofler/qtwebengine/qt5-qtweb... SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kkofler/qtwebengine/fedora-2... 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.)
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
Kevin Kofler kevin@tigcc.ticalc.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |928937 (qt-reviews) Assignee|nobody@fedoraproject.org |rdieter@math.unl.edu Alias| |qt5-qtwebengine, | |qtwebengine
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=928937 [Bug 928937] Qt-related package review tracker
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
Kevin Kofler kevin@tigcc.ticalc.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |helio@kde.org
--- Comment #1 from Kevin Kofler kevin@tigcc.ticalc.org --- *** Bug 1244196 has been marked as a duplicate of this bug. ***
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
--- Comment #2 from Kevin Kofler kevin@tigcc.ticalc.org --- * 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).
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
--- Comment #3 from Kevin Kofler kevin@tigcc.ticalc.org --- 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. :-(
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
--- Comment #4 from Kevin Kofler kevin@tigcc.ticalc.org --- Spec URL: http://copr-dist-git.fedorainfracloud.org/cgit/kkofler/qtwebengine/qt5-qtweb... SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kkofler/qtwebengine/fedora-2...
* Tue Jan 05 2016 Kevin Kofler Kevin@tigcc.ticalc.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.
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
--- Comment #5 from Kevin Kofler kevin@tigcc.ticalc.org --- Spec URL: http://copr-dist-git.fedorainfracloud.org/cgit/kkofler/qtwebengine/qt5-qtweb... SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kkofler/qtwebengine/fedora-r... Description: Qt5 - QtWebEngine components.
* Wed Jan 06 2016 Kevin Kofler Kevin@tigcc.ticalc.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@tigcc.ticalc.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@tigcc.ticalc.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)
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
--- Comment #6 from Kevin Kofler kevin@tigcc.ticalc.org --- So, this one is now for Rawhide, as required by the review guidelines.
Successful Rawhide build (i686 (with SSE2), x86_64): https://copr.fedoraproject.org/coprs/kkofler/qtwebengine/build/151796/
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
--- Comment #7 from Kevin Kofler kevin@tigcc.ticalc.org --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
Rex Dieter rdieter@math.unl.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
--- Comment #8 from Rex Dieter rdieter@math.unl.edu --- 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 %{_qt5_examplesdir}/
scriptlets: ok
3. SHOULD add appropriate Provides: tags for bundled code, https://spot.fedorapeople.org/chromium.spec probably includes a reasonable reference for a starting point.
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
--- Comment #9 from Kevin Kofler kevin@tigcc.ticalc.org --- 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 ;-) ).
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
--- Comment #10 from Kevin Kofler kevin@tigcc.ticalc.org --- Oh, and some of the Provides: bundled(*) in chromium.spec are things I am actually unbundling, so those also don't belong.
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
--- Comment #11 from Kevin Kofler kevin@tigcc.ticalc.org --- Spec URL: http://copr-dist-git.fedorainfracloud.org/cgit/kkofler/qtwebengine/qt5-qtweb... SRPM URL: https://copr-be.cloud.fedoraproject.org/results/kkofler/qtwebengine/fedora-r...
* Fri Jan 08 2016 Kevin Kofler Kevin@tigcc.ticalc.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. :-(
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
Rex Dieter rdieter@math.unl.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Flags|fedora-review? |fedora-review+
--- Comment #12 from Rex Dieter rdieter@math.unl.edu --- Thanks for the heroic effort here, approved.
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
--- Comment #14 from Jon Ciesla limburgher@gmail.com --- Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/qt5-qtwebengine
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
--- Comment #13 from Kevin Kofler kevin@tigcc.ticalc.org --- Wow, thanks! I requested the package creation through pkgdb.
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
Kevin Kofler kevin@tigcc.ticalc.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |NEXTRELEASE Last Closed| |2016-01-10 17:13:07
--- Comment #15 from Kevin Kofler kevin@tigcc.ticalc.org --- qt5-qtwebengine-5.6.0-0.6.beta.fc24 now successfully built in Rawhide.
https://bugzilla.redhat.com/show_bug.cgi?id=1295549
Kevin Kofler kevin@tigcc.ticalc.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |1303611
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1303611 [Bug 1303611] QtWebEngine
package-review@lists.fedoraproject.org