Spec URL: http://repo.calcforge.org/f8/kdegames3.spec SRPM URL: http://repo.calcforge.org/f8/kdegames3-3.5.7-1.fc7.src.rpm Description: Games and gaming libraries for the K Desktop Environment which have not been ported to KDE 4 yet. Included with this package are: atlantik, kasteroids, kenolaba, kfouleggs, klickety, kpoker, ksmiletris, ksnake, ksokoban, ktron. NOTE: This package is only targeted for F8 and above.
Reviewing as promised
Here we go, starting directly wiht a full review. MUST ---- 0 rpmlint output is: W: kdegames3 macro-in-%changelog _bindir This mist be fixed, always use %% in the changelog W: kdegames3 dangling-relative-symlink /usr/share/doc/HTML/en/ktron/common ../common W: kdegames3 dangling-relative-symlink /usr/share/doc/HTML/en/kpoker/common ../common W: kdegames3 dangling-relative-symlink /usr/share/doc/HTML/en/ksmiletris/common ../common W: kdegames3 dangling-relative-symlink /usr/share/doc/HTML/en/ksokoban/common ../common W: kdegames3 dangling-relative-symlink /usr/share/doc/HTML/en/kenolaba/common ../common W: kdegames3 dangling-relative-symlink /usr/share/doc/HTML/en/kfouleggs/common ../common W: kdegames3 dangling-relative-symlink /usr/share/doc/HTML/en/atlantik/common ../common W: kdegames3 dangling-relative-symlink /usr/share/doc/HTML/en/kasteroids/common ../common W: kdegames3 dangling-relative-symlink /usr/share/doc/HTML/en/ksnake/common ../common W: kdegames3 dangling-relative-symlink /usr/share/doc/HTML/en/klickety/common ../common These are normal for KDE apps. E: kdegames3 file-in-usr-marked-as-conffile /usr/share/config.kcfg/kasteroids.kcfg W: kdegames3 conffile-without-noreplace-flag /usr/share/config.kcfg/kasteroids.kcfg E: kdegames3 file-in-usr-marked-as-conffile /usr/share/config.kcfg/kfouleggs.kcfg W: kdegames3 conffile-without-noreplace-flag /usr/share/config.kcfg/kfouleggs.kcfg E: kdegames3 file-in-usr-marked-as-conffile /usr/share/config.kcfg/ksnake.kcfg W: kdegames3 conffile-without-noreplace-flag /usr/share/config.kcfg/ksnake.kcfg E: kdegames3 file-in-usr-marked-as-conffile /usr/share/config.kcfg/ktron.kcfg W: kdegames3 conffile-without-noreplace-flag /usr/share/config.kcfg/ktron.kcfg Either these are config files and should be moved to /etc/kde and be made %config(noreplace) or they are merely defaults / template files and should not be %config at all. kdebase puts files under /usr/share/config.kcfg and doesn't mark them %config. E: kdegames3 invalid-desktopfile /usr/share/applications/kde/kpoker.desktop This is caused by the faulty / obsolete "Miniicon=kpoker" line, remove this and all is fine. * Package and spec file named appropriately * Packaged according to packaging guidelines * License ok * spec file is legible and in Am. English. * Source matches upstream * Compiles and builds on devel x86_64 * BR: ok * locales handled properly * Shared libraries, ldconfig run as required * Not relocatable 0 Package owns or requires all dirs * No duplicate files & Permissions ok * %clean & macro usage OK * Contains code only * %doc does not affect runtime, and isn't large enough to warrent a sub package * -devel package as needed * .desktop file as required MUST _really_ FIX ----------------- * this one is so important I just invented a new catagory for it :) Currently the specfile contains: %if 0%{?rhel} > 1 %define patch_name -patched %else %define patch_name %{nil} %endif And: Source: ftp://ftp.kde.org/pub/kde/stable/%{version}/src/kdegames-%{version}%{patch_name}.tar.bz2 However there is no kdegames-3.5.7-patch.tar.bz2 on the kde site, so RHEL uses a different tarbal, which doesn't come from upstream, which leads to the following: 1) Why? 2) Please do this with patches and or binary (small tarbal) overlays, since its fine for Fedora, there isn't a legal problem so no reason to use a modified tarbal 3) If you must use a modified tarbal, then a) Add a comment above the Source tag explaining in detail what was changed b) make the Source tag just the filename instead of a non existing URL MUST fix -------- * Various rpmlint errors, see above * Add Requires: hicolor-icon-theme for /usr/share/icons/hicolor/*/* dir ownership * unowned directory /usr/share/config.kcfg (own, or maybe better file a bug against kdelibs3 that it should own it). Should fix ---------- * This would seem like a good moment to prune the changelog a bit * Are the debug final and cvs defines + conditional code still needed?
AFAIK, the strange "-patched" tarball is because RH lawyers historically had a problem with KSokoban, probably because it used to contain the levels ported from the original game (arguably a copyright violation). This is no longer the case (it only comes with its own original (in the other sense of this confusing word) levels now), so IMHO this point is moot. The only remaining possible issue is a trademark issue, which if it is really an issue, can be solved by rebranding like you did with Gnometris. So I'm going to zap the strange RHEL conditionals.
> * unowned directory /usr/share/config.kcfg (own, or maybe better file a bug > against kdelibs3 that it should own it). This is owned by kde-filesystem now. I'm going to add a Requires on it.
> * Are the debug final and cvs defines + conditional code still needed? Yes, KDE 3 needs these. Disabling final speeds up compilation and can work around compiler bugs where the compiler just crashes on the big source file from --enable-final on some arches (one of the 3.5.7 packages triggered that on ppc64).
Updated (should fix all the MUST issues you found): Spec URL: http://repo.calcforge.org/f8/kdegames3.spec SRPM URL: http://repo.calcforge.org/f8/kdegames3-3.5.7-2.fc7.src.rpm - drop RHEL conditional using patched tarball - fix macro in changelog - don't mark %%{_datadir}/config.kcfg/* as %%config - add Requires on hicolor-icon-theme and kde-filesystem for dir ownership - patch out obsolete Miniicon= line from kpoker.desktop (Note: Don't get confused by the SRPM disttag, this is still targeted at F8+ only, possibly even F9+ only if we decide not to ship kdegames 4 in F8 yet.)
I haven't looked at your new version yet, but I just hit a snag with your old version, I tried to actual install it and I got: file /usr/bin/ksokoban from install of kdegames3-3.5.7-1.fc8 conflicts with file from package kdegames-3.5.7-1.fc8 And many more of these. I think this needs a "Conflicts: kdegames < 4" adding a "Obsoletes: kdegames < 4" would solve my problem, but might result in people updating only getting kdegames3-3.5.7 and not kdegames-4.0.0, depending on the order things get done. So a conflict is probably the best solution.
That should be: Conflicts: kdegames < 6:3.80 actually. The kdegames package has Epoch 6 and the first KDE 4 alpha was numbered 3.80.1. I'm adding it in that form.
Updated (adds the Conflicts): Spec URL: http://repo.calcforge.org/f8/kdegames3.spec SRPM URL: http://repo.calcforge.org/f8/kdegames3-3.5.7-3.fc7.src.rpm - Conflicts with pre-KDE-4 kdegames
Approved!
New Package CVS Request ======================= Package Name: kdegames3 Short Description: KDE 3 games not ported to KDE 4 Owners: kkofler,rdieter,than Branches: F-8 F-7 InitialCC: Cvsextras Commits: no
Ping? Still waiting on CVS directory creation. Meanwhile, I updated my SRPM (it has been a while since the successful review): Spec URL: http://repo.calcforge.org/f9/kdegames3.spec SRPM URL: http://repo.calcforge.org/f9/kdegames3-3.5.8-1.fc7.src.rpm * Fri Oct 26 2007 Kevin Kofler <...> - 3.5.8-1 - update to 3.5.8 - fix/update trademarks patch (Rex Dieter) - License: GPLv2, libs/devel License: LGPLv2 - libs subpkg (more multilib friendly) - include ksirtet (has been omitted from KDE 4.0) - drop kpoker-desktop patch (fixed upstream) * Wed Aug 8 2007 Kevin Kofler <...> - 3.5.7-4 - patch out the trademarked names of a certain box pushing game and a certain snake-like duel game