Red Hat Bugzilla – Bug 239785
Review Request: rott - Rise of the Triad
Last modified: 2007-11-30 17:12:04 EST
Spec URL: http://people.atrpms.net/~hdegoede/rott.spec
SRPM URL: http://people.atrpms.net/~hdegoede/rott-1.0-1.fc7.src.rpm
This is the icculus.org Linux port of Apogee's classic 3d shooter Rise of the
Triad, which has been released under the GPL by Apogee. This version is
enhanced with the "high" resolution rendering from the winrott port.
I really want to review (and play!) this, but autodownloader keeps failing on
me, with imho an incorrect error message (the files get downloaded about
halfway, and it then jumps to the 2nd mirror, the same happens, and the
download fails with a message "File 1rott13.zip not found on server(s).").
I realize this issue might not be specific to this package (your xu4 package
works great, btw :-) ). I just need to find some time to debug AutoDL and
identify this properly... any pointers?
I think this is possibly due to an overzealous timeout setting in
autodownloader, can you try changing line 218 of: /usr/share/autodl/AutoDL.py from:
Or maybe an even higher value (in seconds) ?
Thanks! I would be much obliged if you could test this soon, so that hopefully I
can get this fixed before thursday's freeze.
This doesn't seem to come with any data files. It was my understanding that the
games should be "free-standing", ie. at least playable from the package itself.
Otherwise, I don't see any reasons we wouldn't add Quake3 to the list...
The freestanding rule was really invented for more troublesome stuff were there
is a reverse engineered (and thus not approved by the original copyright
holders) engine. If this type of engine doesn't come with any free data, one
could say that its only use is using it to circumvent the copy protection in the
original games, which is considered a legal liability, this is also know as the
emulator clause in _the_ guidelines.
Similar cases to rott have already been discussed and approved by FESco, see for
Which itself contains links to another package doing the same.
(In reply to comment #2)
Ok, it does seem to be a timeout issue combined with my questionable network
connection. Increasing the timeout threshold makes things a bit better, but it
might be a good idea to have some form of "download resume" option (like wget's
-c option) - maybe urllib.urlretrieve() can be replaced with a call to wget? But
this is a discussion for another bugzilla, I think. :-)
As far as reviewing the package goes, a few points:
* The program crashes on startup on my machine (FC6/i386 dual core) if the SRPM
is rebuilt as-is; will attach the output (backtrace) shortly. After removing
$RPM_OPT_FLAGS from the EXTRAFLAGS parameter to make, it builds and runs fine.
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
* It looks like you are missing some useful %doc entries:
rott/13todo.txt (highlights some bugs/missing features in port)
rott/cmdline.txt (provides command line options)
rott/hacker.txt (useful for creating custom maps)
rott/wad.txt (useful for creating custom content)
* I have my doubts about the "rott-registered" package: it doesn't feel
"polished", in that it is a standalone installable "game package", yet it does
not create a suitable menu entry, or provide some assistance with actually
getting the game to work (the way rott-shareware does). Also, I'm not too sure
about Fedora's legal view on this (rott-registered) - since you need
proprietary, non-freely downloadable content for it to work. This would be the
case with Quake 3 mentioned in comment #3 above, as well. My personal feeling on
rott-registered is to either polish it up to provide the same level of "user
friendliness" as rott-shareware (and maybe make it Conflict with rott-shareware
if needed), or drop it.
(In reply to comment #3)
As I said above though, I'm not sure about the "registered" stuff, though. Maybe
provide a "codec buddy"-like popup message with 3D Realms's "buy rott" URL?
(they actually still sell it, believe it or not)
Created attachment 154819 [details]
rott crash output with $RPM_OPT_FLAGS enabled
(In reply to comment #4)
> Similar cases to rott have already been discussed and approved by FESco, see for
> Which itself contains links to another package doing the same.
Didn't know that, thanks.
About autodownloader, how much must you increase the timeout to make it reliable
(in most cases atleast), then I can issue an updated version with a better
Also I'm sure upstream (http://sf.net/projects/autodownloader) would be happy to
take a patch for a resume button on the download gui.
About the crash problem, can you please:
1) install the -debuginfo package together with the rott-shareware package from
the same build
2) cd ~/.rott
3) gdb `which rott-shareware`
4) set args window
And then attach the output of the bt command? Thanks!
I know you already have quite a few packages, but maybe you can package quake3io
yourself? Or atleast do an initial srpm?
About the -registered package, I understand. But I think its only fair towards
both Apogee and to registered rott owners to provide this as service. Perhaps I
should add an URL to the %description of the apogee webshop? And/or a readme
with this info?
Created attachment 154827 [details]
gdb bt of rott startup crash
(In reply to comment #8)
> About autodownloader, how much must you increase the timeout to make it
> (in most cases atleast), then I can issue an updated version with a better
> timeout setting.
15 seconds, first mirror fails fairly reliable (3drealms), but the second one
> Also I'm sure upstream (http://sf.net/projects/autodownloader) would be happy
> take a patch for a resume button on the download gui.
Cool, I'll look into whipping up something.
> And then attach the output of the bt command? Thanks!
> I know you already have quite a few packages, but maybe you can package
> yourself? Or atleast do an initial srpm?
Matthias Saou already has a working Quake3 RPM in FreshRPMs, maybe just prod
him to include it in Fedora?
Created attachment 154830 [details]
Test patch fixing the backtrace
Francois, can you try again with this patch added to the build?
It looks like I just was lucky that my username "hans" is very short :)
Excellent job - the patch works great.
I've given some thought about the rott-registered package, and I agree with your
views in comment #9 about it - especially the "registered users" bit. But still,
I feel we should add a wrapper startup script for rott to present a simple popup
with content installation/purchase instructions if the data files are not
present... something simple like kdialog/zenity would work nicely. And it would
be necessary to add a "ROTT Registered" (or similar) menu entry, just so that
everything is nicely integrated.
And... I have to ask this question: do you think we should ask for spot's (or
even FESCo's) view on the -registered package? I'm leaning towards "no, let's
finish the review", but I don't want to step on any toes...
(In reply to comment #12)
> And... I have to ask this question: do you think we should ask for spot's (or
> even FESCo's) view on the -registered package? I'm leaning towards "no, let's
> finish the review", but I don't want to step on any toes...
Spot makes decisions on licensing, there is no licensing issue here (the code is
100% GPL, the data isn't shipped) as for FESco, I don't think its necessary for
them to get involved either. The package as a whole is ok, that we agree on. The
-registered sub package is just a special compiled version. I could take all the
ifdef's and change it to a runtime variable, with a cmdline switch like it is
already for example for doom (prboom), would I then need to back out the runtime
registered support too? Should we remove doom registered support from prboom?
Here is a new version, changes:
- Add desktop entry and userfriendly launcher script for registered package
- Add a patch fixing crashes with "long" usernames
Go get it here:
Spec URL: http://people.atrpms.net/~hdegoede/rott.spec
SRPM URL: http://people.atrpms.net/~hdegoede/rott-1.0-2.fc7.src.rpm
Assigning to myself; will do a formal review shortly...
On the -registered issue: Good point in comment #13, I agree, and its good to
have it in writing for future reference. Proceeding with review:
* rpmlint is silent on all binary and -debug packages
* rpmlint output for src.rpm:
W: rott patch-not-applied Patch99: rott-1.0-registered.patch
-- This patch is applied during %build for the special -registered package;
this is clearly commented in the spec
* package is named well
* spec file is named well
* package meets Packaging Guidelines
* package license is GPL, COPYING file included
* License field in spec file matches actual license
* license file is included in %doc
* spec file is written in American English and legible
* package source md5sum matches upstream source:
* package builds successfully on i386 and x86_64 (PPC not tested)
* BuildRequires are good
* Requires are good
* package handles locales properly (no locales)
* package has no need for %post and %postun sections
* package is not relocatable
* package owns directories it creates
* no duplicate entries in %files
* file permissions are good
* proper %clean section
* spec file macros are used consistently
* package contains only GPL'ed code, not content
* no -doc, -devel subpackages necessary
X- some docs are missing (see comment #5)
* contents in %doc not required for runtime functionality of application
* .desktop files present and properly handled
* package builds in mock (fc6/i386)
* package functions properly on i386 and x86_64
Worth special mention:
* the packager has taken every precaution and put a lot of effort into handling
the (non-shipped) content required for both binary packages in order to improve
Hans, please just add the missing documentation files before importing the package.
Other than that, everything looks fine.
This package is APPROVED.
(In reply to comment #16)
> Hans, please just add the missing documentation files before importing the
Will do, and many thanks for the review!
New Package CVS Request
Package Name: rott
Short Description: Rise of the Triad
Branches: FC-6 devel
Something went wrong with this (new) package causing it not to get an F-7 branch
Package Change Request
Package Name: rott
New Branches: F-7
build for future F-8 F-7 and FE-6, closing.
sorry for not opening a new bug, but I cannot find where to file for rott.
I think rott-shareware should have a Requires on autodownloader
I just installed rott (from koji):
$ rpm -q rott-shareware
/usr/bin/rott-shareware: line 10: /usr/share/autodl/AutoDL.py: No such file or
(In reply to comment #20)
> sorry for not opening a new bug, but I cannot find where to file for rott.
> I think rott-shareware should have a Requires on autodownloader
> I just installed rott (from koji):
> $ rpm -q rott-shareware
> $ rott-shareware
> /usr/bin/rott-shareware: line 10: /usr/share/autodl/AutoDL.py: No such file or
Good catch! Thanks, a new version with this fixed (1.0-3.fcX) is now building
for fc6 fc7 and fc8.
great, and thanks for all the fedora games ;)