Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.

Bug 179818

Summary: Review Request: xpilot-ng - A mutiplayer space arcade game
Product: [Fedora] Fedora Reporter: Wart <wart>
Component: Package ReviewAssignee: Hans de Goede <hdegoede>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: hdegoede
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-02-06 18:04:00 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 163779    

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.