Bug 1895833

Summary: Review Request: mingw-adwaita-qt - Adwaita theme for Qt-based applications
Product: [Fedora] Fedora Reporter: Jan Grulich <jgrulich>
Component: Package ReviewAssignee: Sandro Mani <manisandro>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: manisandro, package-review
Target Milestone: ---Flags: manisandro: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-11-12 14:56:59 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Jan Grulich 2020-11-09 08:37:32 UTC
Spec URL: https://jgrulich.fedorapeople.org/mingw-adwaita-qt/mingw-adwaita-qt.spec
SRPM URL: https://jgrulich.fedorapeople.org/mingw-adwaita-qt/mingw-adwaita-qt-1.1.91-1.fc33.src.rpm
Description: Theme to let Qt applications fit nicely into Fedora Workstation
Fedora Account System Username: jgrulich

Comment 1 Jan Grulich 2020-11-09 08:39:33 UTC
Hi Sandro, may I ask you one more time for a package review?

Comment 2 Sandro Mani 2020-11-09 21:39:31 UTC
- Please document the license breakdown
- You don't need noarch in the subpackages as the main package is noarch
- There are unowned directories - include the directories directly, not the files within
- You need to BR cmake and pretty soon also make explicitly

Proposed changes:

--- a/mingw-adwaita-qt.spec
+++ b/mingw-adwaita-qt.spec
@@ -13,6 +13,9 @@ Source0:        https://github.com/FedoraQt/adwaita-qt/archive/%{version}/adwait
 
 BuildArch:      noarch
 
+BuildRequires:  cmake
+BuildRequires:  make
+
 BuildRequires:  mingw32-filesystem >= 95
 BuildRequires:  mingw32-gcc
 BuildRequires:  mingw32-gcc-c++
@@ -30,7 +33,6 @@ Theme to let Qt applications fit nicely into Fedora Workstation
 %package -n mingw32-adwaita-qt5
 Summary:        Adwaita Qt5 theme
 Requires:       mingw32-libadwaita-qt5 = %{version}-%{release}
-BuildArch:      noarch
 
 %description -n mingw32-adwaita-qt5
 Adwaita theme variant for applications utilizing Qt5.
@@ -52,7 +54,6 @@ Static version of the mingw32-libadwaita-qt5 library.
 %package -n mingw64-adwaita-qt5
 Summary:        Adwaita Qt5 theme
 Requires:       mingw64-libadwaita-qt5 = %{version}-%{release}
-BuildArch:      noarch
 
 %description -n mingw64-adwaita-qt5
 Adwaita theme variant for applications utilizing Qt5.
@@ -92,8 +93,8 @@ Static version of the mingw64-libadwaita-qt5 library.
 %files -n mingw32-libadwaita-qt5
 %{mingw32_bindir}/libadwaitaqt-1.dll
 %{mingw32_bindir}/libadwaitaqtpriv-1.dll
-%{mingw32_includedir}/AdwaitaQt/*.h
-%{mingw32_libdir}/cmake/AdwaitaQt/*cmake
+%{mingw32_includedir}/AdwaitaQt/
+%{mingw32_libdir}/cmake/AdwaitaQt/
 %{mingw32_libdir}/pkgconfig/adwaita-qt.pc
 
 %files -n mingw32-libadwaita-qt5-static
@@ -107,8 +108,8 @@ Static version of the mingw64-libadwaita-qt5 library.
 %files -n mingw64-libadwaita-qt5
 %{mingw64_bindir}/libadwaitaqt-1.dll
 %{mingw64_bindir}/libadwaitaqtpriv-1.dll
-%{mingw64_includedir}/AdwaitaQt/*.h
-%{mingw64_libdir}/cmake/AdwaitaQt/*cmake
+%{mingw64_includedir}/AdwaitaQt/
+%{mingw64_libdir}/cmake/AdwaitaQt/
 %{mingw64_libdir}/pkgconfig/adwaita-qt.pc
 
 %files -n mingw64-libadwaita-qt5-static


Scratch build with proposed changes: https://koji.fedoraproject.org/koji/taskinfo?taskID=55265362

Comment 3 Sandro Mani 2020-11-09 21:40:29 UTC
Could you take these two? They are pretty trivial mingw-python packages:

https://bugzilla.redhat.com/show_bug.cgi?id=1896174
https://bugzilla.redhat.com/show_bug.cgi?id=1896175

Thanks!

Comment 4 Jan Grulich 2020-11-11 07:44:21 UTC
Thanks for the review and suggestions.

Fixed package:
Spec URL: https://jgrulich.fedorapeople.org/mingw-adwaita-qt/mingw-adwaita-qt.spec
SRPM URL: https://jgrulich.fedorapeople.org/mingw-adwaita-qt/mingw-adwaita-qt-1.1.91-1.fc33.src.rpm
Description: Theme to let Qt applications fit nicely into Fedora Workstation
Fedora Account System Username: jgrulich

I will look into those two reviews during the day.

Comment 5 Sandro Mani 2020-11-11 10:25:04 UTC
The license still needs clarifying [1]. Licensecheck reports as below (for files without unknown license). The license rather appears to be GPLv2+ and MIT (aka Expat).

[1] https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_multiple_licensing_scenarios

---

./LICENSE.GPL2: GNU General Public License, Version 2
./LICENSE.LGPL2: GNU Lesser General Public License, Version 2.1
./tests/test.cpp: GNU General Public License v2.0 or later
./tests/test.h: GNU General Public License v2.0 or later
./src/demo/main.cpp: GNU General Public License v2.0 or later
./src/demo/widgetfactory.cpp: GNU General Public License v2.0 or later
./src/demo/widgetfactory.h: GNU General Public License v2.0 or later
./src/lib/adwaita.cpp: GNU General Public License v2.0 or later
./src/lib/adwaita.h: GNU General Public License v2.0 or later
./src/lib/adwaitaaddeventfilter.cpp: GNU General Public License v2.0 or later
./src/lib/adwaitaaddeventfilter.h: GNU General Public License v2.0 or later
./src/lib/adwaitacolors.cpp: GNU General Public License v2.0 or later
./src/lib/adwaitacolors.h: GNU General Public License v2.0 or later
./src/lib/adwaitadebug.h: GNU General Public License v2.0 or later
./src/lib/adwaitamnemonics.cpp: GNU General Public License v2.0 or later
./src/lib/adwaitamnemonics.h: GNU General Public License v2.0 or later
./src/lib/adwaitasplitterproxy.cpp: GNU General Public License v2.0 or later
./src/lib/adwaitasplitterproxy.h: GNU General Public License v2.0 or later
./src/lib/adwaitawidgetexplorer.cpp: GNU General Public License v2.0 or later
./src/lib/adwaitawidgetexplorer.h: GNU General Public License v2.0 or later
./src/lib/adwaitawindowmanager.cpp: Expat License GNU General Public License v2.0 or later
./src/lib/adwaitawindowmanager.h: GNU General Public License v2.0 or later
./src/lib/config-adwaita.h.cmake: GNU General Public License v2.0 or later
./src/style/adwaitahelper.cpp: GNU General Public License v2.0 or later
./src/style/adwaitahelper.h: GNU General Public License v2.0 or later
./src/style/adwaitastyle.cpp: GNU General Public License v2.0 or later
./src/style/adwaitastyle.h: GNU General Public License v2.0 or later
./src/style/adwaitastyleplugin.cpp: GNU General Public License v2.0 or later
./src/style/adwaitastyleplugin.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaanimation.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaanimation.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaanimationdata.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaanimationdata.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaanimations.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaanimations.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitabaseengine.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitabaseengine.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitabusyindicatordata.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitabusyindicatordata.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitabusyindicatorengine.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitabusyindicatorengine.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitadatamap.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitadialdata.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitadialdata.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitadialengine.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitadialengine.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaenabledata.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaenabledata.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitagenericdata.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitagenericdata.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaheaderviewdata.cpp: Expat License GNU General Public License v2.0 or later
./src/lib/animations/adwaitaheaderviewdata.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaheaderviewengine.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaheaderviewengine.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitascrollbardata.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitascrollbardata.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitascrollbarengine.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitascrollbarengine.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaspinboxdata.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaspinboxdata.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaspinboxengine.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitaspinboxengine.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitastackedwidgetdata.cpp: Expat License
./src/lib/animations/adwaitastackedwidgetdata.h: Expat License
./src/lib/animations/adwaitastackedwidgetengine.cpp: Expat License
./src/lib/animations/adwaitastackedwidgetengine.h: Expat License
./src/lib/animations/adwaitatabbardata.cpp: Expat License GNU General Public License v2.0 or later
./src/lib/animations/adwaitatabbardata.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitatabbarengine.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitatabbarengine.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitatoolboxengine.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitatoolboxengine.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitatransitiondata.cpp: Expat License
./src/lib/animations/adwaitatransitiondata.h: Expat License
./src/lib/animations/adwaitatransitionwidget.cpp: Expat License
./src/lib/animations/adwaitatransitionwidget.h: Expat License
./src/lib/animations/adwaitawidgetstatedata.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitawidgetstatedata.h: GNU General Public License v2.0 or later
./src/lib/animations/adwaitawidgetstateengine.cpp: GNU General Public License v2.0 or later
./src/lib/animations/adwaitawidgetstateengine.h: GNU General Public License v2.0 or later

Comment 6 Jan Grulich 2020-11-12 07:04:59 UTC
You are right, the license shouldn't be LGPLv2+. I copied the license from the original adwaita-qt package.

Fixed package:
Spec URL: https://jgrulich.fedorapeople.org/mingw-adwaita-qt/mingw-adwaita-qt.spec
SRPM URL: https://jgrulich.fedorapeople.org/mingw-adwaita-qt/mingw-adwaita-qt-1.1.91-1.fc33.src.rpm
Description: Theme to let Qt applications fit nicely into Fedora Workstation
Fedora Account System Username: jgrulich

Comment 7 Sandro Mani 2020-11-12 09:17:11 UTC
All good, approved!

Comment 8 Gwyn Ciesla 2020-11-12 14:33:47 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/mingw-adwaita-qt