COPR built available on https://copr.fedorainfracloud.org/coprs/luya/f31-backgrounds/build/1021674/

Added dependent and blocker report.

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

Oh, there is Free Art 1.3 text in the package as well.

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

The wallpaper looks good to me. It is very suitable for screens with icons on them, because of the subtle colours. Thank you.

(In reply to Miro Hrončok from comment #2)
> 1) The License tag of the base subpackage is repeated redundantly. 

Removed

> 
> 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:
> 
> > 
> 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-backgrounds.spec
SRPM URL: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.0/f31-backgrounds-31.0.1-1.fc30.src.rpm

The specfile version hasn't changed, but the SRPM version did. Also, the SRPM link gives 404. Are the links correct?

I forgot to change v31.0.0 to v31.0.1.
Typing from a cellphone

Spec URL: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.1/f31-backgrounds.spec
SRPM URL: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.1/f31-backgrounds-31.0.1-1.fc30.src.rpm

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

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/

(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-backgrounds.spec
SRPMS: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.2/f31-backgrounds-31.0.2-1.fc30.src.rpm

(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.

(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.

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.

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.

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/9beaa478698cb6a0657034b7eb1db4c835f78b0b?branch=master

You can discuss with them.

Updated files taking account of desktop-file-validate

SPECS: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.2/f31-backgrounds.spec
SRPMS: https://github.com/fedoradesign/backgrounds/releases/download/v31.0.2/f31-backgrounds-31.0.2-2.fc30.src.rpm

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

Thank you Miro!

(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.

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/f31-backgrounds

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-blocker-review.2019-09-03-16.01.txt

FEDORA-2019-8b2cccfabf has been submitted as an update to Fedora 31. https://bodhi.fedoraproject.org/updates/FEDORA-2019-8b2cccfabf

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

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.

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

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