Bug 200665 - Review Request: ltris
Review Request: ltris
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Thorsten Leemhuis (ignored mailbox)
Fedora Package Reviews List
:
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2006-07-30 05:12 EDT by Michael J Knox
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-16 16:32:59 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Michael J Knox 2006-07-30 05:12:19 EDT
Spec URL: http://www.knox.net.nz/~michael/ltris.spec
SRPM URL: http://www.knox.net.nz/~michael/ltris-1.0.11-1.src.rpm

Description: 
LTris is a clone of the tetris game for Linux. It uses a SDL library.
Comment 1 cq92j9y+rlkr0w 2006-07-30 11:51:32 EDT
Hi Michael,

I haven't tried your package, but are you sure you need to run the
'gtk-update-icon-cache' scriptlet? LTris doesn't install icons into
%{_datadir}/icons/hicolor.

Please correct me if I'm mistaken ;)
Comment 2 Michael J Knox 2006-07-30 15:34:54 EDT
no, you are quiet right. 

fixed

Spec URL: http://www.knox.net.nz/~michael/ltris.spec
SRPM URL: http://www.knox.net.nz/~michael/ltris-1.0.11-2.src.rpm
Comment 3 Michał Bentkowski 2006-07-30 15:46:48 EDT
According to http://fedoraproject.org/wiki/Extras/SIGs/Games you should
package data files and game files separately. Please read whole site and
make necessary changes required by Games Packaging Guidelines.
Comment 4 Michael J Knox 2006-07-30 16:06:31 EDT
The SIG guide states "if possible" not "must". ltris is less than half a meg in
total size. The idea behind splitting the packages, to reduce size, is not
really a concern here IMHO. 
Comment 5 Michał Bentkowski 2006-07-30 16:51:51 EDT
SIG guide also states that this "should be done even if upstream uses one
tarball for game source and data". But for ltris you're probably right.
Comment 6 Wart 2006-07-31 01:27:20 EDT
Requires: SDL_mixer can be dropped.  The package already requires
libSDL_mixer-1.2.so.0()(64bit), which is provided by SDL_mixer.

ltris.png is a 48x48 color icon.  Why not put it in the
$RPM_BUILD_ROOT/%{_datadir}/icons/hicolor/48x48/apps/ directory?  I've seem
packages use both, and I'm not sure which is the preferred location for desktop
icons.

You define _localstatedir, but never use it.  Drop the %define.

World writable scoreboard files are bad.  I would suggest either of the following:
1) Make the game setgid 'games' and make the scoreboard file 'games' group
writable.  Be sure to audit and modify the code to make sure that it's setgid safe.
2) Move the scoreboard file to $HOME so that it's private for each user.  This
removes the need to use a setgid binary, but also disables having a shared
scoreboard file.

Package creates but doesn't own '/var/games/%{name}'.  Since there's only one
file in this directory, it's acceptable (but not required) to put the scoreboard
file directly in /var/games without the %{name} subdirectory.  I'll update the
Games packaging guidelines to reflect this.
Comment 7 Michael J Knox 2006-07-31 01:38:47 EDT
_localstatedir defines where the scoreboard gets put. I will see how much work
is need to make the scoreboard private to each user. 
Comment 8 Wart 2006-07-31 02:01:30 EDT
(In reply to comment #7)
> _localstatedir defines where the scoreboard gets put.

I see.  Instead of redefining _localstatedir, it would be better just to add
--localstatedir /var/games/%{name} at the end of %configure, instead of
redefining a standard rpm variable which might have unforseen side effects
(though it doesn't seem to in this case).

Comment 9 Michael J Knox 2006-09-16 16:32:59 EDT
Sorry. Due to my stepping out for a while, I am unable to complete this submission.

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