Bug 528096

A few quick comments:

gcc-c++ is part of the minimal build environment.
http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRequires
cmake is even in F10 >= 2.6.0 and that's the oldest supported release.
You can remove both.

Your >= versioning is unneeded since we ship versions greater than those the program needs in all supported fedora versions.
(BuildRequires:  qt4-devel >= 4.4.0 kdelibs4-devel >= 4.1 kdegames-devel >= 4.4.0
Requires:       kdelibs4 >= 4.1 qt4 >= 4.4.0)

kdelibs requires qt anyways, so no need for the qt4 in Requires

Same in buildrequires, kdelibs4-devel requires qt4-devel. And kdegames-devel requires kdelibs4-devel. So it's enough to have kdegames-devel in BuildRequires.

No need for VERBOSE=1 with make.

Dont use full path in % files section. Use macros.

%{_kde4_docdir}/HTML/en/%{name}/*

%doc is empty. Fill it with AUTHORS ChangeLog README COPYING TODO

You have to run desktop-file-validate for your .desktop file.

You're missing icon code snippets:

%post 
touch --no-create %{_datadir}/icons/hicolor &>/dev/null || :

%postun
if [ $1 -eq 0 ] ; then
   touch --no-create %{_kde4_iconsdir}/hicolor &>/dev/null
   gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &>/dev/null || :
   update-desktop-database -q &> /dev/null
   update-mime-database %{_kde4_datadir}/mime &> /dev/null
fi

%posttrans
gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null || :
update-desktop-database -q &> /dev/null
update-mime-database %{_kde4_datadir}/mime &> /dev/null

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

I'm taking this as the sponsor and official reviewer. Thanks to Thomas Janssen for the prereviewing. Ryan, can you please update your specfile to address his comments?

> No need for VERBOSE=1 with make.

We've been asked to add this to get build commands into build.log, so this has probably made its way into various templates, but these days the %cmake and %cmake_kde4 macros already set verbose mode (for the same reason), so it is indeed unnecessary to set it again in the make line.

>You have to run desktop-file-validate for your .desktop file.

Addressed.

>So it's enough to have kdegames-devel in BuildRequires.

[rrix@TheSwan rpmbuild]$ rpmlint RPMS/i586/kmagnet-0.02-2.fc11.i586.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

I assume that because rpmlint errs on Requires: kdelibs4, that the soname dependencies autogenerate that Requires. That leaves me with only a BuildRequires on kdegames-devel. Is this correct?


* Fri Oct 9 2009 Ryan Rix < phrkonaleash [AT] gmail dot com > - 0.02-2
- Fixed Requires and BuildRequires
- Removed VERBOSE=1 from Makefile
- Sane use of macros in files section
- Added documentation
- Added icon code snippets

[rrix@TheSwan rpmbuild]$ rpmlint RPMS/i586/kmagnet-0.02-2.fc11.i586.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

http://koji.fedoraproject.org/koji/taskinfo?taskID=1739167

Updated SPEC and SRPM:
http://rrix.fedorapeople.org/kmagnet/kmagnet.spec
http://rrix.fedorapeople.org/kmagnet/kmagnet-0.02-2.fc11.src.rpm

Taking review.

I haven't run the full checklist yet, so I may find more issues, but at a first glance:
* Please remove the redundant desktop-file-install call above your desktop-file-validate call. They both do the same thing (as the file is already installed) and desktop-file-validate is the simpler solution there.
* You're still trying to use %{_desktopdir} which is not defined anywhere.
* Please use %{_kde4_appsdir} instead of %{_kde4_datadir}/kde4/apps.

* Sat Oct 10 2009 Ryan Rix < phrkonaleash [AT] gmail dot com > - 0.02-3
- Removed redundant call to desktop-file-install
- Changed one %files entry to use %{_kde4_appsdir}

http://rrix.fedorapeople.org/kmagnet/kmagnet.spec
http://rrix.fedorapeople.org/kmagnet/kmagnet-0.02-3.fc11.src.rpm

* Fri Oct 16 2009 Ryan Rix < phrkonaleash [AT] gmail dot com > - 0.03-4
- Update to 0.03
- Upstream changed name to kMagnet
- Added a patch to fix .desktop file

http://rrix.fedorapeople.org/kmagnet/kmagnet.spec
http://rrix.fedorapeople.org/kmagnet/kmagnet-0.03-4.fc11.src.rpm

http://koji.fedoraproject.org/koji/taskinfo?taskID=1751216

I had a cursory look, looks good so far, more details after sleeping. :-)

First step: rpmlint run:
kmagnet.src:86: W: macro-in-%changelog files
(no output for the binary and debuginfo RPMs).

You have this in the changelog:
- Changed one %files entry to use %{_kde4_appsdir}
You should escape those % signs, as in:
- Changed one %%files entry to use %%{_kde4_appsdir}

MUST Items:
! rpmlint output:
  macros in changelog need fixing
+ named and versioned according to the Package Naming Guidelines
+ spec file name matches base package name
+ Packaging Guidelines:
  + License: GPLv3+ valid, matches actual license
  + No known patent problems
  + No emulator, no firmware, no binary-only or prebuilt components
  + Complies with the FHS
  + proper changelog, tags, BuildRoot, BuildRequires, Summary, Description
  + no non-UTF-8 characters
  + all relevant documentation included as %doc
  + RPM_OPT_FLAGS are used (%cmake_kde4 macro)
  + debuginfo package is valid
  + no static libraries nor .la files
  + no duplicated system libraries
  + no rpaths
  + no configuration files, so %config guideline doesn't apply
  + no init scripts, so init script guideline doesn't apply
  + .desktop file for the GUI app kmagnet present
  + desktop-file-validate is used in %install and the .desktop file passes validation
  + no timestamp-clobbering file commands
  + _smp_mflags used
  ! scriptlets are sane, but contain unnecessary lines (see SHOULD items)
  + not a web application, so web application guideline doesn't apply
  + no conflicts
+ complies with all the legal guidelines
+ COPYING packaged as %doc (but it's the wrong one, see SHOULD items)
+ source matches upstream:
  MD5: 1e996f149ea0a9fe11c73191a582088b
  SHA1: f6a24f5065802a5aebaf6f1fc6d8054ceef2705f
  SHA256: 0681cf46e31b96faaaeec80779faf0f7570609708eabbca2f12f0a5d6f41ae72
+ builds on at least one arch (F13 Koji scratch build)
+ no known non-working arches, so no ExcludeArch needed
+ no missing BuildRequires (builds in mock)
+ no translations, so translation/locale guidelines don't apply
+ no shared libraries, so no ldconfig calls neeed
+ no duplicated system libraries
+ package not relocatable
! ownership not correct:
  + doesn't own directories owned by another package, but...
  ! doesn't own 2 package-specific directories it should own:
    %{_kde4_docdir}/HTML/en/%{name}/
    %{_kde4_appsdir}/kMagnet/
    please remove the * at the end of those lines, you want to own the whole directories, not just their contents!
+ no duplicate files in %files
+ permissions correct, defattr used correctly
+ %clean section present and correct
+ macros used where possible
+ no non-code content
+ no large documentation files, so no -doc package needed
+ no %doc files required at runtime
+ no header files which would need to be in a -devel subpackage
+ no static libraries, so no -static package needed
+ no .pc files, so no Requires: pkgconfig needed
+ no devel symlinks which would need to be in a -devel subpackage
+ no -devel package, so "In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}" is irrelevant
+ no .la files
+ .desktop file for the GUI app kmagnet present
+ desktop-file-validate is used in %install and the .desktop file passes validation
+ buildroot is deleted at the beginning of %install
+ all filenames are valid UTF-8

SHOULD Items:
! license file: upstream is including a copy of the GPLv2 when the license headers actually reference the GPLv3+! So the COPYING upstream is shipping does not actually apply. Please ask upstream to fix this, as per SHOULD item: "If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it." But, as per "MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc.", it is OK to ship a package without a copy of the license if upstream doesn't include it, so this is not a review blocker.
+ no translations for description and summary provided by upstream
+ package builds in mock (F13 Koji scratch build)
+ submitter reports having successfully tested the package functionality
! scriptlets contain unnecessary:
  update-desktop-database -q &> /dev/null
  update-mime-database %{_kde4_datadir}/mime &> /dev/null
  lines. There is no MIME type being registered (so update-mime-database is unnecessary) nor being referenced in the .desktop file either (so update-desktop-database is not needed either). And where did you get the -q from? It's not in: https://fedoraproject.org/wiki/Packaging:ScriptletSnippets
  (The scriptlets are otherwise valid.)
+ no subpackages other than -devel (in fact no subpackages at all), so "Usually, subpackages other than devel should require the base package using a fully versioned dependency." is irrelevant
+ no .pc files, so "placement of .pc files" is irrelevant
+ no file dependencies

Please fix the following items:
* escape (double) the % signs in %changelog (MUST)
* own the following 2 directories:
    %{_kde4_docdir}/HTML/en/%{name}/
    %{_kde4_appsdir}/kMagnet/
  not just the files they contain (i.e. remove the * at the end of those lines) (MUST)
* remove the unnecessary lines from the %postun and %posttrans scriptlets:
  update-desktop-database -q &> /dev/null
  update-mime-database %{_kde4_datadir}/mime &> /dev/null
  (SHOULD, and trivial to fix, so please fix this while you are at fixing things unless you have a good reason not to)
* ask upstream to include a copy of the GPLv3 to match their license headers (SHOULD, but you don't have to wait for an answer to get the package approved)

Once you addressed these issues, I'll have another quick look to make sure it's all right and then I'll approve your package and sponsor you.

I did some more testing:
successful scratch build for F10 on all 4 arches:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1752638
also tried to play the game and it works fine indeed if you know how to play (the documentation is poor; the idea is that you have to open one of the predefined puzzles (currently only 3), then move with the cursor keys).

> * escape (double) the % signs in %changelog (MUST)
Done

> * own the following 2 directories:
>    %{_kde4_docdir}/HTML/en/%{name}/
>    %{_kde4_appsdir}/kMagnet/
>  not just the files they contain (i.e. remove the * at the end of those lines)
>(MUST)
Done

>* remove the unnecessary lines from the %postun and %posttrans scriptlets:
>  update-desktop-database -q &> /dev/null
>  update-mime-database %{_kde4_datadir}/mime &> /dev/null
>  (SHOULD, and trivial to fix, so please fix this while you are at fixing
>things unless you have a good reason not to)
Done

>* ask upstream to include a copy of the GPLv3 to match their license headers
>(SHOULD, but you don't have to wait for an answer to get the package approved)
Sent a mail upstream with this and other issues you and I discussed in IRC.

=======================

[rrix@TheSwan rpmbuild]$ rpmlint RPMS/i586/kmagnet-0.03-5.fc11.i586.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[rrix@TheSwan rpmbuild]$ rpmlint SRPMS/kmagnet-0.03-5.fc11.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[rrix@TheSwan rpmbuild]$ rpmlint RPMS/i586/kmagnet-debuginfo-0.03-5.fc11.i586.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

http://koji.fedoraproject.org/koji/taskinfo?taskID=1752652

http://rrix.fedorapeople.org/kmagnet/kmagnet-0.03-5.fc11.src.rpm
http://rrix.fedorapeople.org/kmagnet/kmagnet.spec

All issues addressed, the package is APPROVED.

You are now sponsored.

New Package CVS Request
=======================
Package Name: kmagnet
Short Description: KDE puzzle game with built in editor
Owners: rrix
Branches: F-10 F-11 F-12
InitialCC:

Small pet-peave, I'd suggest leaving "KDE" out of the pkg summary/description.  (Unless pkg is truly kde-only).

cvs done.

@Rex: What should I do to change it if it's already been completed in CVS?

First of all, you MUST NOT readd FE-NEEDSPONSOR nor reset fedora-cvs to ?. Please refresh the page before posting!

To make changes to already imported packages, you follow the regular packaging procedure: edit the specfile in CVS (bumping Release and adding a changelog entry), commit the edit (with the changelog entry as the commit message), tag and build.

If the package is not imported yet, the easiest solution is to import it first, then do the edits as above.

commited and built in devel

kmagnet-0.03-5.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/kmagnet-0.03-5.fc10

kmagnet-0.03-5.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/kmagnet-0.03-5.fc11

kmagnet-0.03-5.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

kmagnet-0.03-5.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.