Spec URL: http://people.atrpms.net/~hdegoede/rott.spec SRPM URL: http://people.atrpms.net/~hdegoede/rott-1.0-1.fc7.src.rpm Description: 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?
Hmm, 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: socket.setdefaulttimeout(5) to socket.setdefaulttimeout(10) 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 example: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=238367 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. my RPM_OPT_FLAGS: -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables * 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) http://fedoraproject.org/wiki/Extras/SteeringCommittee/Meeting-20070510 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) <snip> > Similar cases to rott have already been discussed and approved by FESco, see for > example: > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=238367 > Which itself contains links to another package doing the same. Didn't know that, thanks.
Francois, 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 timeout setting. 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 5) run 6) bt And then attach the output of the bt command? Thanks! Bastien, I know you already have quite a few packages, but maybe you can package quake3io yourself? Or atleast do an initial srpm?
p.s. 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) > Francois, > > 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 > timeout setting. 15 seconds, first mirror fails fairly reliable (3drealms), but the second one works fine. > > 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. Cool, I'll look into whipping up something. > > And then attach the output of the bt command? Thanks! > Done. > > Bastien, > > I know you already have quite a few packages, but maybe you can package quake3io > 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? http://ftp.freshrpms.net/pub/freshrpms/fedora/linux/6/quake3/
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: MUST items: * 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: c1c6cbecf00f2229cf2e0053334dcfc1 rott-1.0.tar.gz * 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 SHOULD items: * 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 end-user experience 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 package. > Will do, and many thanks for the review! New Package CVS Request ======================= Package Name: rott Short Description: Rise of the Triad Owners: j.w.r.degoede Branches: FC-6 devel InitialCC: <empty>
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 rott-shareware-1.0-2.fc7.1 $ rott-shareware /usr/bin/rott-shareware: line 10: /usr/share/autodl/AutoDL.py: No such file or directory
(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-1.0-2.fc7.1 > $ rott-shareware > /usr/bin/rott-shareware: line 10: /usr/share/autodl/AutoDL.py: No such file or > directory > 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 ;)