[Bug 371411] Review Request: alienarena - 3d Deathmatch game

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: alienarena - 3d Deathmatch game


https://bugzilla.redhat.com/show_bug.cgi?id=371411





------- Additional Comments From j.w.r.degoede at hhs.nl  2007-11-17 08:30 EST -------
Full review results (Thanks to Mamoru for some of these):

Must Fix:
---------
* rpmlint: W: unstripped-binary-or-object /usr/lib/alienarena/game.so
  (chmod +x ?)

* Make the Requires: alienarena-data versioned, so that if a newer version of
  the data is needed by a newer release, an someone does yum update alienarena,
  the data will get updated too.

  Usually I use a = version requirement in the engine and a >= version req on
  the engine in the data.

* '--vendor ""' argument to desktop-file-install must be '--vendor fedora'

* Add "ActionGame" to the .desktop file Categories, so that alienarena will not
  end up on the main games sub-menu when the games menu has been further 
  sub-menud through install of the games-menus package (as the live dvd does for
  example)

* Remove these non RPM_OPTFLAGS from the flags used for the compile:
  -O2 -fomit-frame-pointer -ftree-vectorize -fexpensive-optimizations -march=k8
  Notice that upstream also adds this to CFLAGS:
  -fno-strict-aliasing -fmerge-constants
  But those are ok.

* You install symlinks to %{_libdir}/%{name}/game.so under /usr/share, but
  %{_libdir} is arch dependent and files under /usr/share may not be arch 
  dependent.

  Suggestion, instead use this in %prep:
  sed -i 's|"game.so"|"%{_libdir}/%{name}/game.so"|g' unix/sys_unix.c

  Together with a patch commenting the stupid code where it first tries to
  fopen the .so in a number of different places totally irrelevant to dlopen's
  search path (will attach the patch next).


Should Fix:
-----------
* defattr
  - We now recommend %defattr(-,root,root,-)
* Timestamps
  - Please use "-p" option to keep timestamps when using
    "cp" or "install" commands (for SOURCE3, SOURCE1)
* "-n %{name}-%{version}" to %setup is redundant, remove please
* Make .desktop file use aa as Icon, not /full/path/Aa.ico
* Use just alienarena as Exec in the .desktop file not a full path
* The symlinks to the binaries under /usr/share/alienarena do not appear to be
  needed
* Since crx.sdl may not be directly called, it should be moved to %_libexecdir
* Change wrapper script from:
---
#!/bin/bash

cd /usr/share/alienarena/
./crx.sdl
---
   To:
---
#!/bin/bash

cd /usr/share/alienarena/
exec /usr/libexec/crx.sdl "$@"
---
So that bash doesn't linger in memory and cmd line parameters can be passed.


-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.