Bug 225400 - Review Request: tetrinetx - The GNU TetriNET server
Review Request: tetrinetx - The GNU TetriNET server
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: John Mahowald
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2007-01-30 08:06 EST by Francois Aucamp
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-16 06:19:52 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
jpmahowald: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Francois Aucamp 2007-01-30 08:06:48 EST
Spec URL: http://www.snoekie.com/rpm/tetrinetx.spec
SRPM URL: http://www.snoekie.com/rpm/tetrinetx-1.13.16-1.src.rpm
Description:
Tetrinetx is the GNU TetriNET server written in C. It includes IRC and
Spectator supports. As many other tetrinet servers, it uses IP independent
decryption which allows the server to run behind a router.

TetriNET is a network-based, multiplayer tetris game. This package contains
a server for hosting TetriNET games over a public or private network.


Package builds in mock (fc6/i386) and on x86_64.

rpmlint output:
===============
E: tetrinetx non-standard-uid xxxx 
-- quite a few times; this is due to the use of a "tetrinetx" user for the server (and its run-time files); maybe this should be changed to using the generic "games" account?

E: tetrinetx non-readable /etc/tetrinetx/game.secure 0600
-- this file contains security configuration for TetriNET, and should not be readable by anyone other than the tetrinet server
Comment 1 John Mahowald 2007-03-10 21:17:34 EST
Does build in devel x86_64

rpmlint:
E: tetrinetx non-standard-uid /var/games/tetrinetx tetrinetx
E: tetrinetx non-standard-gid /var/games/tetrinetx tetrinetx
E: tetrinetx non-standard-uid /etc/tetrinetx/game.pmotd tetrinetx
E: tetrinetx non-standard-gid /etc/tetrinetx/game.pmotd tetrinetx
E: tetrinetx non-standard-uid /etc/tetrinetx/game.conf tetrinetx
E: tetrinetx non-standard-gid /etc/tetrinetx/game.conf tetrinetx
E: tetrinetx zero-length /etc/tetrinetx/game.conf
E: tetrinetx non-standard-uid /var/run/tetrinetx tetrinetx
E: tetrinetx non-standard-gid /var/run/tetrinetx tetrinetx
E: tetrinetx non-standard-uid /var/log/tetrinetx tetrinetx
E: tetrinetx non-standard-gid /var/log/tetrinetx tetrinetx
E: tetrinetx non-standard-uid /etc/tetrinetx/game.motd tetrinetx
E: tetrinetx non-standard-gid /etc/tetrinetx/game.motd tetrinetx
E: tetrinetx non-standard-uid /etc/tetrinetx/game.secure tetrinetx
E: tetrinetx non-standard-gid /etc/tetrinetx/game.secure tetrinetx
E: tetrinetx non-readable /etc/tetrinetx/game.secure 0600


Originally I thought the games user might be preferable. Games SIG recommends
scorebord files setuid/setgid games if necessary. However they also want daemons
to run as a seperate user, so I'd leave it owned by tetrinetx.
http://fedoraproject.org/wiki/Extras/SIGs/Games


I'm not sure of the legality of including the word "tetris". I don't see it in
the tetrinetx docs.

Good:
+ license GPL, COPYING included
+ init script
+ %clean
+ macro usage throughout
+ spec commented
+ source matches upstream
+ proper BuildRoot
+ follows naming guidelines
Comment 2 Francois Aucamp 2007-03-12 04:09:59 EDT
(In reply to comment #1)
Thanks for the comments!

> E: tetrinetx zero-length /etc/tetrinetx/game.conf

Weird - this is not the case on my systems (FC5 on multi-CPU x86_64, FC6 on
i386). I have a game.conf file of about 4KB on both. We possibly have some form
of race condition (although I fail to see how) - could you please verify if
"game.conf" exists and is populated in the "bin" subdir of the package's build
directory after doing a rpmbuild -bc ?

> I'm not sure of the legality of including the word "tetris". I don't see it in
> the tetrinetx docs.

Thanks for pointing this out. The only other mention of this word I can find in
Fedora with a yum search is tetris-bsd in the bsd-games package. Shall we change
it to something like "falling bricks game" instead? Although I like using
"tetris" _somewhere_ simply because it makes it easier to search for the package...
Comment 3 John Mahowald 2007-03-13 03:37:33 EDT
I had built it in mock. However rpmbuild also leaves an empty game.conf in
~/rpmbuild/BUILD/tetrinetx-1.13.16+qirc-1.40c/bin    It exists in the source
tarball. It seems that your cat/sed command in the spec is screwing it up. Use
sed's (well, GNU sed's --in-place option instead.


In the build log I get:
warning: File listed twice: /etc/tetrinetx/game.conf
warning: File listed twice: /etc/tetrinetx/game.motd
warning: File listed twice: /etc/tetrinetx/game.pmotd
warning: File listed twice: /etc/tetrinetx/game.secure

Which is a result of listing it twice. Use the %dir directive if you only want
the directory. http://www.rpm.org/max-rpm/s1-rpm-inside-files-list-directives.html

Speaking of config files, do note that the config_dir patch hardcodes the
location. There at least needs to be a comment in the spec, if someone moved
%{_sysconfdir} they would want to know why it didn't go with.

As much as I also like searching on tetris, I want to avoid the trademark. You
can get a second opinion on this, but I'd rather not use that term.
Comment 4 Francois Aucamp 2007-03-13 05:58:03 EDT
New build:
Spec URL: http://www.snoekie.com/rpm/tetrinetx.spec
SRPM URL: http://www.snoekie.com/rpm/tetrinetx-1.13.16-2.src.rpm

Changes:
- Cleaned up sed scripts in %prep
- Replaced config.h patch with sed script in order to support RPM macros
- Removed trademarked names from %%description

(In reply to comment #3)
> tarball. It seems that your cat/sed command in the spec is screwing it up. Use

Fixed, thanks for the pointer.

> In the build log I get:
> warning: File listed twice: /etc/tetrinetx/game.conf

Oops! Fixed. :-)

> Speaking of config files, do note that the config_dir patch hardcodes the
> location. There at least needs to be a comment in the spec, if someone moved
> %{_sysconfdir} they would want to know why it didn't go with.

I decided to remove the patch and rather do this in the spec file itself using
macros, solving this potential issue.

> As much as I also like searching on tetris, I want to avoid the trademark. You
> can get a second opinion on this, but I'd rather not use that term.

Agreed. Fixed.
Comment 5 John Mahowald 2007-03-13 23:46:31 EDT
Builds on FC6 x86_64

Good things from comment 1 apply.

+ no "tetris" in spec
+ empty game.conf fixed
+ summary, Group  fine
+ installed, can play a game

APPROVED
Comment 6 Francois Aucamp 2007-03-14 02:55:52 EDT
Thanks for the review!

New Package CVS Request
=======================
Package Name: tetrinetx
Short Description: The GNU TetriNET server
Owners: faucamp@csir.co.za
Branches: FC-5 FC-6
InitialCC:

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