Bug 205030 - Review Request: atomix - Little mind game where you have to build molecules out of atoms lying around
Review Request: atomix - Little mind game where you have to build molecules o...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Paul F. Johnson
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-09-02 12:45 EDT by Thorsten Leemhuis
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-17 03:39:14 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 Thorsten Leemhuis 2006-09-02 12:45:43 EDT
Spec URL: 
  http://www.leemhuis.info/files/fedorarpms/SPECS.fdr/atomix.spec
SRPM URL: 
  http://www.leemhuis.info/files/fedorarpms/SRPMS.fdr/atomix-2.14.0-1.src.rpm

Description: 
  Atomix is yet another little mind game. You have to build molecules out of single atoms laying around. Of course there is a time limit and the handling is not as easy as you might expect ;-). This game is inspiried by the orignal Amiga game Atomix and uses the GNOME librarys.
Comment 1 Wart 2006-09-02 13:50:14 EDT
The game is setgid games, but doesn't appear to completely drop setgid
privileges after starting up, according to /proc/<pid>/status.  In fact, there
are two processes created when atomix runs, one without setgid privileges, and
one with.  This seems to use gnome functions to handle the scoreboard file, so
I'd be curious to learn how gnome deals with security for setgid + scoreboard files.
Comment 2 Paul F. Johnson 2006-09-02 14:14:57 EDT
You're missing find-langs from the spec file - there are a lot of translation
files in there.

rpmlint throws up two errors for the rpm package

zero-length /var/lib/games/atomix.scores
non-standard-executable-perm /usr/bin/atomix 02755
Comment 3 Paul F. Johnson 2006-09-02 15:50:03 EDT
builds fine in mock with no real hiccups. rpmlint obviously gives the same results.

As soon as you fix the two errors from rpmlint and address #1, I'll do the full
review.
Comment 4 Wart 2006-09-02 15:55:50 EDT
(In reply to comment #2)
> You're missing find-langs from the spec file - there are a lot of translation
> files in there.
> 
> rpmlint throws up two errors for the rpm package
> 
> zero-length /var/lib/games/atomix.scores

The directory /var/lib/games is not writable by the 'games' group.  In order for
the application to write to its own scoreboard file, it must create the
scoreboard file (and make it group writable) as part of the install process. 
This is acceptable.

> non-standard-executable-perm /usr/bin/atomix 02755

This is acceptable for a game that is setgid 'games' for writing to a shared
scoreboard file.
Comment 5 Paul F. Johnson 2006-09-02 16:27:33 EDT
Okay, give #4

Review
------

Good :

Spec file readable and in US English
No ownership problems with directories
No devel package (so that bit can be ignored!)
Upstream md5sum = package md5sum
Builds cleanly in mock (i386)
No dupes in rpms
Package includes documentation
Correctly creates it's own localdir instead of polluting
Consistent use of macros
Uses dist and smp_flags
Software actually works
No suprious permissions in the rpms

Needs work
Needs to include find-lang

Once the needs work has been attended to, I'm happy for this to go in.
Comment 6 Thorsten Leemhuis 2006-09-03 07:54:00 EDT
(In reply to comment #1)
> The game is setgid games, but doesn't appear to completely drop setgid
> privileges after starting up, according to /proc/<pid>/status. [...]

Yes, sorry, I wanted to comment on this. Citing
http://www.fedoraproject.org/wiki/Extras/SIGs/Games
"If necessary, a game can be made setgid 'games' in order to allow a shared
scoreboard file. But only if necessary, and care should be taken to drop setgid
privileges when not needed. The following example shows how to properly drop
setuid/setgid privileges. [...]"

I'm not a real C-Programmer (never found time for it :-/ and these days my work
around Fedora consumes all of my free time) so I can't integrate above or
comment closer on what Wart noticed in Comment #1. I'd be glad if someone from
the Games SIG (or someone else) could look closer into this whole stuff.

(In reply to comment #2)
> You're missing find-langs from the spec file - there are a lot of translation
> files in there.

/me looks closer -- yes, seems so, but they didn't get installed. (even with
gettext around on the system). Ahh, seems it's a upstream problem. See GNOME
bz#334319 . I worked around this bug with running
sed  -i 's!^SOURCES = !&\n'"$(grep "^CATALOGS" po/Makefile.in)"'!' po/Makefile
after configure. There are probably better ways do do that. Suggestion welcomed.

Latest package now atomix-2.14.0-2

Spec URL: 
  http://www.leemhuis.info/files/fedorarpms/SPECS.fdr/atomix.spec
Diff URL: 
  http://www.leemhuis.info/files/fedorarpms/SPECS.fdr/atomix.spec.diff
SRPM URL: 
  http://www.leemhuis.info/files/fedorarpms/SRPMS.fdr/atomix-2.14.0-2.src.rpm
Comment 7 Paul F. Johnson 2006-09-03 15:11:51 EDT
Okay, now that the langs are there, I can see no problem why this shouldn't be
in extras. The setgid is an issue, so before I FE-ACCEPT this, I'll seek guidance.
Comment 8 Paul F. Johnson 2006-09-10 10:52:04 EDT
Guidance has pretty much not occured :-(

I'll stick out my neck on this (as it's the same as other games I've checked
which do the same as this) and say

ACCEPTED
Comment 9 Thorsten Leemhuis 2006-09-17 03:39:14 EDT
Paul, thx for the review.

(In reply to comment #8)
> Guidance has pretty much not occured :-(

I waited another week and tried two times to ping the games SIG in #fedora-games
-- no reaction. So I went forward and imported atomix. We can fix this later if
we reqally want to.

Build for devel succeed, FC-5 branch requested.

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