Bug 179818 - Review Request: xpilot-ng - A mutiplayer space arcade game
Summary: Review Request: xpilot-ng - A mutiplayer space arcade game
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Hans de Goede
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-02-03 05:50 UTC by Wart
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-02-06 18:04:00 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Wart 2006-02-03 05:50:25 UTC
Spec Name or Url: http://www.kobold.org/~wart/fedora/xpilot-ng.spec
SRPM Name or Url: http://www.kobold.org/~wart/fedora/xpilot-ng-4.7.2-1.src.rpm
Description: 

A highly addictive, infinitely configurable multiplayer space
arcade game.  You pilot a spaceship around space, dodging
obstacles, shooting players and bots, collecting power-ups, and
causing general mayhem.

Comment 1 Wart 2006-02-03 06:04:58 UTC
There are two issues that I already know about with this package:

1) rpmlint on FC5 complains about non-utf-8 files:
W: xpilot-ng file-not-utf8 /usr/share/man/man6/xpilot-ng-x11.6.gz
W: xpilot-ng-server file-not-utf8 /usr/share/man/man6/xpilot-ng-server.6.gz
I'm not sure how to properly fix these.

2) Neither the base package nor the -server package own %{_datadir}/%{name}
Both packages can be installed independently, or both can be installed at the
same time.  In such a case, which package should own the directory?


Comment 2 Jeffrey C. Ollie 2006-02-03 14:12:00 UTC
(In reply to comment #1)
> There are two issues that I already know about with this package:
> 
> 1) rpmlint on FC5 complains about non-utf-8 files:
> W: xpilot-ng file-not-utf8 /usr/share/man/man6/xpilot-ng-x11.6.gz
> W: xpilot-ng-server file-not-utf8 /usr/share/man/man6/xpilot-ng-server.6.gz
> I'm not sure how to properly fix these.

In %prep:

pushd doc/man
iconv --from=ISO-8859-1 --to=UTF-8 xpilot-ng-server.man > xpilot-ng-server.man.new
mv xpilot-ng-server.man.new xpilot-ng-server.man
iconv --from=ISO-8859-1 --to=UTF-8 xpilot-ng-x11.man > xpilot-ng-x11.man.new
mv xpilot-ng-x11.man.new xpilot-ng-x11.man
popd

> 2) Neither the base package nor the -server package own %{_datadir}/%{name}
> Both packages can be installed independently, or both can be installed at the
> same time.  In such a case, which package should own the directory?

I would create a third package "xpilot-ng-common" that would own any files or
directories that were required by both the client and server packages.  Then add
a "Requires: xpilot-ng-common = %{version}-%{release}" to both the xpilot-ng and
xpilot-ng-server packages.

Comment 3 Hans de Goede 2006-02-03 15:32:34 UTC
I would only create the common package if there are indeed common files,
otherwise just own the dir in both packages, there is plenty of precedence for this.


Comment 4 Wart 2006-02-03 16:32:55 UTC
There are no common files between the two packages, but they both write to the
same directory.  However, both packages should probably have COPYING and README.
 But it would seem silly to make a "xpilog-ng-common" package that only included
the license file and README.

I created an updated package that uses iconv to utf8-ify the man pages and allow
both packages to own %{_datadir}/%{name}:

http://www.kobold.org/~wart/fedora/xpilot-ng-4.7.2-2.src.rpm
http://www.kobold.org/~wart/fedora/xpilot-ng.spec

Comment 5 Wart 2006-02-03 20:31:28 UTC
Yet another minor update:  I added the readme and license files to the server
subpackage.  I also added the missing %defattr() line for the server subpackage.

http://www.kobold.org/~wart/fedora/xpilot-ng-4.7.2-3.src.rpm
http://www.kobold.org/~wart/fedora/xpilot-ng.spec

Comment 6 Hans de Goede 2006-02-05 09:21:30 UTC
some first impression comments:
-use %{version} in Source0
-why the cp-ing of the doc files, you can have 2 identical %doc lines for
 (sub)packages.



Comment 7 Hans de Goede 2006-02-05 10:21:53 UTC
MUST items:
* rpmlint output is clean
* package and spec name matches upstream
* GPL license valid, matches upstream, license file included
* Meets packaging guidelines
* Spec file is legible and in Am. English.
* Source matches upstream (md5sum ok)
* Builds cleanly on FC5 x86_64
* Valid BR; none are redundant
* No lang files; no shlibs.
* Package not relocatable
* 0wns all directories that it creates
* File permissions ok
* %clean looks good
* code and content, both under GPL
* minimal doc files, do not affect runtime
* no -devel package necessary
* desktop file installed correctly

SHOULD items:
* Runs although I have yet to figure out the keys to use.

MUST FIX:
*use %{version} in Source0
*don't cp the doc-files to .server versions use just them 2 times in the %doc
 parts of the %files sections
*icons should be installed in /usr/share/icons/hicolor/48x48/apps/
 (freedekstop.org standard)
*and then should run gtk-update-icon-cache in %post(un), use the
 scriptlets found on: http://www.fedoraproject.org/wiki/ScriptletSnippets
 for this.


Comment 8 Wart 2006-02-06 00:31:09 UTC
(In reply to comment #7)

> MUST FIX:
> *use %{version} in Source0

Added.

> *don't cp the doc-files to .server versions use just them 2 times in the %doc
>  parts of the %files sections

For some misguided reason I thought that this would result in both packages
owning the files.  Fixed.

> *icons should be installed in /usr/share/icons/hicolor/48x48/apps/
>  (freedekstop.org standard)

Fixed.

> *and then should run gtk-update-icon-cache in %post(un), use the
>  scriptlets found on: http://www.fedoraproject.org/wiki/ScriptletSnippets
>  for this.

Fixed.

http://www.kobold.org/~wart/fedora/xpilot-ng-4.7.2-4.src.rpm
http://www.kobold.org/~wart/fedora/xpilot-ng.spec

Comment 9 Hans de Goede 2006-02-06 09:37:14 UTC
Checking ... Approved

Notes:
-I'm now behind my i386 machine and that freezes when running this probably xorg
 bug, it does run for you? (worked fine on my x86_64).
-It wouldn't build without disabling selinux because of the new execstack
 selinux stuff, I hope it will build on the buildsys, but I'm not sure. This 
 is a rawhide problem, not a problem in / with the src.rpm
-please pass -p (preserve timestamps) to install that way 2 builds (on say 2
 different archs) of the same build end up with the same timestamps for
 identical files. Having different timestamps for things like icons can cause
 problems for multilib systems. (Not really an issue in this case though).

Comment 10 Wart 2006-02-06 16:22:51 UTC
(In reply to comment #9)
> Notes:
> -I'm now behind my i386 machine and that freezes when running this probably xorg
>  bug, it does run for you? (worked fine on my x86_64).

I've run the client in classic "x11" mode and the server on devel-i386 with no
problem.  Though now I notice that the newer SDL client fails to run on devel
because of the execstack stuff.

> -It wouldn't build without disabling selinux because of the new execstack
>  selinux stuff, I hope it will build on the buildsys, but I'm not sure. This 
>  is a rawhide problem, not a problem in / with the src.rpm

I haven't seen any problems building on my devel box, just running the newer SDL
client because of the execstack stuff.

> -please pass -p (preserve timestamps) to install that way 2 builds (on say 2
>  different archs) of the same build end up with the same timestamps for
>  identical files. Having different timestamps for things like icons can cause
>  problems for multilib systems. (Not really an issue in this case though).

Done.  I'll add the -p to release -4 before importing to CVS.

Thanks for the review!

Comment 11 Wart 2006-02-06 18:04:00 UTC
devel build (job #3772) succeeded.


Note You need to log in before you can comment on or make changes to this bug.