https://bugzilla.redhat.com/show_bug.cgi?id=1745846
Bug ID: 1745846 Summary: Review Request: f31-backgrounds - Fedora 31 default desktop background Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: luya_tfz@thefinalzone.net QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.0/f31-ba... SRPM URL: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.0/f31-ba... Description: This package contains desktop backgrounds for the Fedora 31 default theme. Pulls in themes for GNOME, KDE, Mate and Xfce desktops. Fedora Account System Username:luya
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
Luya Tshimbalanga luya_tfz@thefinalzone.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |1744266, 1644937 | |(BetaBlocker,F31BetaBlocker | |)
--- Comment #1 from Luya Tshimbalanga luya_tfz@thefinalzone.net --- COPR built available on https://copr.fedorainfracloud.org/coprs/luya/f31-backgrounds/build/1021674/
Added dependent and blocker report.
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1644937 [Bug 1644937] Fedora 31 Beta blocker bug tracker https://bugzilla.redhat.com/show_bug.cgi?id=1744266 [Bug 1744266] Fedora 31 still using Fedora 30 backgrounds
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
Miro Hrončok mhroncok@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |mhroncok@redhat.com Assignee|nobody@fedoraproject.org |mhroncok@redhat.com Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #2 from Miro Hrončok mhroncok@redhat.com --- 1) The License tag of the base subpackage is repeated redundantly.
2) Attribution-Extras says:
$ grep Licence Attribution-Extras | sort | uniq Licence: CC-BY-SA 4.0 Licence: CC-BY 4.0 Licence: CC0 1.0 Licence: Free Art 1.0
But the license tag of extras-base is:
License: CC-BY and CC-BY-SA
When in fact it should be:
License: CC-BY and CC-BY-SA and CC0 and Free Art
The license file also contain only the CC licenses, not Free Art:
%license CC-BY-SA-4.0 CC-BY-4.0 CC0-1.0 Attribution-Extras
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #3 from Miro Hrončok mhroncok@redhat.com --- Oh, there is Free Art 1.3 text in the package as well.
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #4 from Miro Hrončok mhroncok@redhat.com --- Some of the images in extras are quite small:
$ file extras/*.png extras/a-world-lightyears-from-home.png: PNG image data, 256 x 191, 8-bit/color RGB, non-interlaced extras/cabines-de-bains.png: PNG image data, 256 x 160, 8-bit/color RGB, non-interlaced extras/descent-to-loch-ericht.png: PNG image data, 4864 x 2736, 8-bit/color RGB, non-interlaced extras/green-leaf.png: PNG image data, 256 x 144, 8-bit/color RGBA, non-interlaced extras/icelandic-stone-beach.png: PNG image data, 3840 x 2160, 8-bit/color RGB, non-interlaced extras/last-light-on-antler-peak.png: PNG image data, 256 x 144, 8-bit/color RGB, non-interlaced extras/life-is-blue.png: PNG image data, 4032 x 2268, 8-bit/color RGBA, non-interlaced extras/our-world.png: PNG image data, 256 x 144, 8-bit/color RGB, non-interlaced
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
Lukas Ruzicka lruzicka@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |lruzicka@redhat.com
--- Comment #5 from Lukas Ruzicka lruzicka@redhat.com --- The wallpaper looks good to me. It is very suitable for screens with icons on them, because of the subtle colours. Thank you.
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
Luya Tshimbalanga luya_tfz@thefinalzone.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW
--- Comment #6 from Luya Tshimbalanga luya_tfz@thefinalzone.net --- (In reply to Miro Hrončok from comment #2)
- The License tag of the base subpackage is repeated redundantly.
Removed
- Attribution-Extras says:
$ grep Licence Attribution-Extras | sort | uniq Licence: CC-BY-SA 4.0 Licence: CC-BY 4.0 Licence: CC0 1.0 Licence: Free Art 1.0
But the license tag of extras-base is:
The license file also contain only the CC licenses, not Free Art:
%license CC-BY-SA-4.0 CC-BY-4.0 CC0-1.0 Attribution-Extras
Free Art is now included.
Here is the updated files with fixed size on some extra wallpapers
Spec URL: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.0/f31-ba... SRPM URL: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.0/f31-ba...
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
Miro Hrončok mhroncok@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
--- Comment #7 from Miro Hrončok mhroncok@redhat.com --- The specfile version hasn't changed, but the SRPM version did. Also, the SRPM link gives 404. Are the links correct?
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #8 from Luya Tshimbalanga luya_tfz@thefinalzone.net --- I forgot to change v31.0.0 to v31.0.1. Typing from a cellphone
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #9 from Luya Tshimbalanga luya_tfz@thefinalzone.net ---
Spec URL: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.1/f31-ba... SRPM URL: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.1/f31-ba...
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #10 from Miro Hrončok mhroncok@redhat.com --- Package should install a .desktop file using desktop-file-install or run desktop-file-validate.
Package must own all directories that it creates. Directories without known owners: /usr/share/wallpapers /usr/share/gnome-background-properties /usr/share/backgrounds/f31
License mismatch: The package installs Free-Art-1.3, but the files are under Free-Art-1.0 (according to Attribution-Extras).
Diff spec file in url and in SRPM --------------------------------- --- /home/churchyard/rpmbuild/FedoraReview/1745846-f31-backgrounds/srpm/f31-backgrounds.spec 2019-08-28 06:28:18.636493744 +0200 +++ /home/churchyard/rpmbuild/FedoraReview/1745846-f31-backgrounds/srpm-unpacked/f31-backgrounds.spec 2019-08-27 15:08:09.000000000 +0200 @@ -37,4 +37,5 @@ %package base Summary: Base images for Fedora %{relnum} default background +License: CC-BY-SA
%description base
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
benson_muite@emailplus.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |benson_muite@emailplus.org
--- Comment #11 from benson_muite@emailplus.org --- Wallpaper is nice. However, COPR repository seemed to have trouble updating using sudo dnf copr enable luya/f31-backgrounds sudo dnf install f31-backgrounds-kde gives an error Failed to download metadata for repo 'copr:copr.fedorainfracloud.org:luya:f31-backgrounds'
Was easier to rebuild rpm, have made a pull request to update readme with build and installation instructions: https://github.com/fedoradesign/backgrounds/pull/14 This follows Magazine article: https://fedoramagazine.org/how-rpm-packages-are-made-the-source-rpm/ not the more complete documentation at: https://docs.fedoraproject.org/en-US/quick-docs/creating-rpm-packages/
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #12 from Luya Tshimbalanga luya_tfz@thefinalzone.net --- (In reply to Miro Hrončok from comment #10)
Package should install a .desktop file using desktop-file-install or run desktop-file-validate.
The .desktop file is specific to KDE Plasma as provided by KDE SIG themselves.
Package must own all directories that it creates. Directories without known owners: /usr/share/wallpapers /usr/share/gnome-background-properties /usr/share/backgrounds/f31
Weird. Something is not right on the fedora-review. The spec file clearly lists:
%dir %{_datadir}/backgrounds/%{bgname} %dir %{_datadir}/gnome-background-properties/
which should set to the right owners. The ownership of "/usr/share/wallpapers" should be set to KDE using their macro %{_kde4_datadir}. It seems like a bug.
License mismatch: The package installs Free-Art-1.3, but the files are under Free-Art-1.0 (according to Attribution-Extras).
Fixed.
Diff spec file in url and in SRPM
/home/churchyard/rpmbuild/FedoraReview/1745846-f31-backgrounds/srpm/f31- backgrounds.spec 2019-08-28 06:28:18.636493744 +0200 +++ /home/churchyard/rpmbuild/FedoraReview/1745846-f31-backgrounds/srpm-unpacked/ f31-backgrounds.spec 2019-08-27 15:08:09.000000000 +0200 @@ -37,4 +37,5 @@ %package base Summary: Base images for Fedora %{relnum} default background +License: CC-BY-SA
%description base
Fixed.
SPECS: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.2/f31-ba... SRPMS: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.2/f31-ba...
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #13 from Miro Hrončok mhroncok@redhat.com --- (In reply to Luya Tshimbalanga from comment #12)
(In reply to Miro Hrončok from comment #10)
Package should install a .desktop file using desktop-file-install or run desktop-file-validate.
The .desktop file is specific to KDE Plasma as provided by KDE SIG themselves.
Does that mean it doesn't need to be validated? Why not?
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_desktop_files
"It is not simply enough to just include the .desktop file in the package, one MUST run desktop-file-install (in %install) OR desktop-file-validate (in %check or %install) and have BuildRequires: desktop-file-utils, to help ensure .desktop file safety and spec-compliance."
Package must own all directories that it creates. Directories without known owners: /usr/share/wallpapers /usr/share/gnome-background-properties /usr/share/backgrounds/f31
Weird. Something is not right on the fedora-review. The spec file clearly lists:
%dir %{_datadir}/backgrounds/%{bgname} %dir %{_datadir}/gnome-background-properties/
which should set to the right owners. The ownership of "/usr/share/wallpapers" should be set to KDE using their macro %{_kde4_datadir}. It seems like a bug.
OK, let me recheck:
/usr/share/backgrounds/f31 is owned by f31-backgrounds-base /usr/share/gnome-background-properties is owned by f31-backgrounds-gnome /usr/share/wallpapers is owned by kde-filesystem
Mea cupla, everything is indeed alright there.
Downloading SRPM to check the remaining two.
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #14 from Miro Hrončok mhroncok@redhat.com --- (In reply to Luya Tshimbalanga from comment #12)
License mismatch: The package installs Free-Art-1.3, but the files are under Free-Art-1.0 (according to Attribution-Extras).
Fixed.
Indeed. Not sure if relicensing the work like this is ok, but it was done upstream, so I don't really cere.
Diff spec file in url and in SRPM
...
Fixed.
Indeed.
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #15 from Miro Hrončok mhroncok@redhat.com --- Rpmlint -------
I've removed dangling-relative-symlink (too many) and invalid-url (my mock has no internet access):
Checking: f31-backgrounds-31.0.2-1.fc32.noarch.rpm f31-backgrounds-base-31.0.2-1.fc32.noarch.rpm f31-backgrounds-animated-31.0.2-1.fc32.noarch.rpm f31-backgrounds-kde-31.0.2-1.fc32.noarch.rpm f31-backgrounds-gnome-31.0.2-1.fc32.noarch.rpm f31-backgrounds-mate-31.0.2-1.fc32.noarch.rpm f31-backgrounds-xfce-31.0.2-1.fc32.noarch.rpm f31-backgrounds-extras-base-31.0.2-1.fc32.noarch.rpm f31-backgrounds-extras-gnome-31.0.2-1.fc32.noarch.rpm f31-backgrounds-extras-mate-31.0.2-1.fc32.noarch.rpm f31-backgrounds-extras-kde-31.0.2-1.fc32.noarch.rpm f31-backgrounds-extras-xfce-31.0.2-1.fc32.noarch.rpm f31-backgrounds-31.0.2-1.fc32.src.rpm f31-backgrounds.noarch: W: no-documentation f31-backgrounds-base.noarch: W: no-documentation f31-backgrounds-animated.noarch: W: no-documentation f31-backgrounds-kde.noarch: W: no-documentation f31-backgrounds-gnome.noarch: W: no-documentation f31-backgrounds-mate.noarch: W: no-documentation f31-backgrounds-xfce.noarch: W: no-documentation f31-backgrounds-extras-base.noarch: W: no-documentation f31-backgrounds-extras-gnome.noarch: W: no-documentation f31-backgrounds-extras-mate.noarch: W: no-documentation f31-backgrounds-extras-kde.noarch: W: no-documentation f31-backgrounds-extras-xfce.noarch: W: no-documentation 13 packages and 0 specfiles checked; 0 errors, 593 warnings.
Ok.
Rpmlint (installed packages) ----------------------------
f31-backgrounds-extras-xfce.noarch: W: no-documentation f31-backgrounds-gnome.noarch: W: no-documentation f31-backgrounds-animated.noarch: W: no-documentation f31-backgrounds-mate.noarch: W: no-documentation f31-backgrounds.noarch: W: no-documentation f31-backgrounds-extras-kde.noarch: W: no-documentation f31-backgrounds-extras-base.noarch: W: no-documentation f31-backgrounds-extras-mate.noarch: W: no-documentation f31-backgrounds-extras-gnome.noarch: W: no-documentation f31-backgrounds-kde.noarch: W: no-documentation f31-backgrounds-base.noarch: W: no-documentation f31-backgrounds-xfce.noarch: W: no-documentation 12 packages and 0 specfiles checked; 0 errors, 605 warnings.
Ok.
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #16 from Miro Hrončok mhroncok@redhat.com --- Not to get lost in the comments. Lint the desktop files in %check or install them via the install tool and I will approve the package.
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #17 from Luya Tshimbalanga luya_tfz@thefinalzone.net --- desktop-file-validate will complain about the missing "Type" which only provides application, link and directory.
desktop-file-validate rpmbuild/SOURCES/f31-metadata.desktop rpmbuild/SOURCES/f31-metadata.desktop: error: required key "Type" in group "Desktop Entry" is not present
KDE SIG provided that workaround removing Type parameter for displaying wallpapers on Plasma on Fedora 25. https://src.fedoraproject.org/rpms/f25-backgrounds/c/9beaa478698cb6a0657034b...
You can discuss with them.
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #18 from Luya Tshimbalanga luya_tfz@thefinalzone.net --- Updated files taking account of desktop-file-validate
SPECS: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.2/f31-ba... SRPMS: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.2/f31-ba...
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
Miro Hrončok mhroncok@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags|fedora-review? |fedora-review+
--- Comment #19 from Miro Hrončok mhroncok@redhat.com --- install -D -p -m644 %{SOURCE1} \ %{buildroot}%{_datadir}/plasma/desktoptheme/%{Bg_Name}/metadata.desktop
%check desktop-file-validate %{buildroot}%{_datadir}/plasma/desktoptheme/%{Bg_Name}/metadata.desktop
This can be probably replaced with one call to desktop-file-install.
Anyway, package APPROVED. Thanks
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #20 from Luya Tshimbalanga luya_tfz@thefinalzone.net --- Thank you Miro!
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #21 from Luya Tshimbalanga luya_tfz@thefinalzone.net --- (In reply to Lukas Ruzicka from comment #5)
The wallpaper looks good to me. It is very suitable for screens with icons on them, because of the subtle colours. Thank you.
Glad to hear that.
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #22 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/f31-backgrounds
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
Geoffrey Marr gmarr@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |gmarr@redhat.com Whiteboard| |AcceptedBlocker
--- Comment #23 from Geoffrey Marr gmarr@redhat.com --- Discussed during the 2019-09-03 blocker review meeting: [1]
The decision to classify this bug as an "AcceptedBlocker" was made as it violates the following criteria:
"The default desktop background must be different from that of the last two stable releases"
[1] https://meetbot.fedoraproject.org/fedora-blocker-review/2019-09-03/f31-block...
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |MODIFIED
--- Comment #24 from Fedora Update System updates@fedoraproject.org --- FEDORA-2019-8b2cccfabf has been submitted as an update to Fedora 31. https://bodhi.fedoraproject.org/updates/FEDORA-2019-8b2cccfabf
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #25 from Fedora Update System updates@fedoraproject.org --- desktop-backgrounds-31.0.0-1.fc31, f31-backgrounds-31.0.2-2.fc31 has been pushed to the Fedora 31 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-8b2cccfabf
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA Last Closed| |2019-09-04 20:54:42
--- Comment #26 from Fedora Update System updates@fedoraproject.org --- desktop-backgrounds-31.0.0-1.fc31, f31-backgrounds-31.0.2-2.fc31 has been pushed to the Fedora 31 stable repository. If problems still persist, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
Adam Williamson awilliam@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |awilliam@redhat.com
--- Comment #27 from Adam Williamson awilliam@redhat.com --- Miro: for the record, the rules about FDO desktop file spec apply only to .desktop files that are actually application launchers, by my reading. This file is not an application launcher and is not actually intended to comply with the FDO spec, and 'correcting' it to pass desktop-file-validate actually breaks it. So please don't require that for any future reviews of background packages :)
See https://bugs.kde.org/show_bug.cgi?id=411876
https://bugzilla.redhat.com/show_bug.cgi?id=1745846
--- Comment #28 from Miro Hrončok mhroncok@redhat.com --- I've asked why it should not pass the check, I have not demanded the desktop file is changed ;)
Anyway, here, a followup: https://pagure.io/packaging-committee/issue/925
package-review@lists.fedoraproject.org