Spec URL: http://zanoni.jcomserv.net/fedora/armacycles-ad/armacycles-ad.spec SRPM URL: http://zanoni.jcomserv.net/fedora/armacycles-ad/armacycles-ad-0.2.8.2.1-1.final.1.fc7.src.rpm Description: In this game you ride a lightcycle; that is a sort of motorbike that cannot be stopped and leaves a wall behind it. The main goal of the game is to make your opponents' lightcycles crash into a wall while avoiding the same fate. The focus of the game lies on the multiplayer mode, but it provides challanging AI opponents for a quick training match. This is Armagetron Advanced, rebranded as approved by upstream and spot.
Ugh, you are using the "Tron" trademark in the Summary, please don't, thats considered user visible. Also since this is going in I think it would be wise to not use the word Armagetron on the Games SIG page to refer to this review either.
Corrected both. Oops! :)
Spec URL: http://zanoni.jcomserv.net/fedora/armacycles-ad/armacycles-ad.spec SRPM URL: http://zanoni.jcomserv.net/fedora/armacycles-ad/armacycles-ad-0.2.8.2.1-2.final.1.fc7.src.rpm New build with .desktop and icon, as well as fixed summary.
I tried to do a review on this, I really did, but ... UGH, specfile makes eyes hurt. Can you please clean it up considerably and then resubmit? Here are some things to fix: * Don't write: " # inform automake of the rpm build directory DESTDIR=$RPM_BUILD_ROOT export DESTDIR pushd bindist-dedicated make install popd pushd bindist make install popd " instead write (as all other specs do) : " pushd bindist-dedicated make install DESTDIR=$RPM_BUILD_ROOT popd pushd bindist make install DESTDIR=$RPM_BUILD_ROOT popd " * There is no need to activate the armacycles default sdl audiodriver hack, our SDL already defaults to alsa. So no need to pass CXXFLAGS="-DDEFAULT_SDL_AUDIODRIVER=alsa" * This is a gnu autoconf configure script, so call it using %configure instead of passing all the dir options and CXXFLAGS yourself * please check all the non dir configure options if they are really necessary, for example atleast --disable-restoreold is useless as you don't also pass --enable-multiver (and you don't want to do that either). * even more bogus are the configure options (esp the combination): "--enable-useradd --disable-useradd" ?? * also please do not make (configure) lines wider then 80 chars, please put a \ at the end and continue on the next line * this also is bogus: --enable-automakedefaults --localstatedir=/var Quoting from ./configure --help: --enable-automakedefaults enforce the default installation directories as set by automake. localstatedir=prefix/var violates the FHS, so this is off by default. * looking even more at the configure flags, I notice that they both have: --disable-sysinstall Yet also both specify: --enable-etc --enable-initscript --enable-useradd Which (according to ./configure --help) have no influence when --disable-sysinstall is passed * long story short, it would seem that this is all thats needed for configure: %configure --disable-sysinstall --disable-uninstall --disable-glout resp: %configure --disable-sysinstall --disable-uninstall * these 2 shell variables are not used, please remove them: CLIENTPATH=%{_tmppath}/%{name}-%{version}-root/share/games/armagetronad/ SERVERPATH=%{_tmppath}/%{name}-%{version}-root/share/games/armagetronad-dedicat * Please use macros where ever possible for example do not write: mkdir $RPM_BUILD_ROOT/etc but write: mkdir -p $RPM_BUILD_ROOT/%{_sysconfdir} * please put the description and tags of subpackages at the top of the spec directly after the main package, not between the %files * please use %defattr instead of %attr for each file * please do not use /usr/share/games/armagetronad but /usr/share/armagetronad (hint try using the --enable-games configure argument) * I think you will want to buildrequire libxml2-devel not libxml2 * why patch progtitle into configure, but not progname? why still have progtitle bits in the .spec if its patched in why not just write: export progtitle="Armacycles Advanced" export progname=armacyclesad before the 2 calls to %configure, then the patching is no longer needed
erm correction, make the hint about not using /usr/share games: hint try using the --disable-games configure argument
More stuff: * files / dirs owned by both the main and dedicated packages: /etc/armagetronad /usr/share/games/armagetronad/language /usr/share/games/armagetronad/resource /usr/share/games/armagetronad/scripts Please put these in a -common packae and make both packages require the -common package * dirs which are bogus and must not be shipped: /usr/etc /usr/share/games/armagetronad/desktop * the -dedicated package should have a /etc/rc.d/init.d script to stop / start it as a server, and run as its own user. See batch/rcd_startstop for inspiration for writing a service start / stop script. Or alternatively you could completely forgo the -dedicated sub package, I doubt anyone will use it, then you also only have to call configure and make once, can forget about the common package, all in all then this would become a pretty normal package. (you could then also start over with /etc/rpmdevtools/spectemplate-minimal as basis to make a much better spec).
Started working in issues in #4. Spec URL: http://zanoni.jcomserv.net/fedora/armacycles-ad/armacycles-ad.spec No SRPM, because it won't build. It seems to make a difference which directory I called the configure script from. Sorry, I'm an autoconf n00b. And --disable-games seems not to work. Might I need to patch the Makefile?
Ooh, made some progress, hang on. . .
Ok, I've addressed most of #4-#5, and part of #6. Not sure how to rid my self of /usr/etc. Also, for the language, resource and script bits that are duplicated, should I just put them in /usr/share/armacycles-ad-common/language||resource||scripts and put symlinks in the main and -dedicated packages? Still need to work on the init script. Spec URL: http://zanoni.jcomserv.net/fedora/armacycles-ad/armacycles-ad.spec SRPM URL: http://zanoni.jcomserv.net/fedora/armacycles-ad/armacycles-ad-0.2.8.2.1-3.final.1.fc7.src.rpm
Created attachment 161672 [details] Updated specfile (In reply to comment #9) > Ok, I've addressed most of #4-#5, and part of #6. Not sure how to rid my self > of /usr/etc. > Attached is a specfile which fixes /usr/etc and a couple of other things. I hope you don't mind me doing it this way, thats easier then typing a long list of Must Fix items. Notice that rpmlint still complains about some none executable scripts / scripts without shebang, these need to be fixed too, I wonder if we need to ship these scripts at all, they seam maintainer oriented and are probably not needed to play the game. > Also, for the language, resource and script bits that are duplicated, should I > just put them in /usr/share/armacycles-ad-common/language||resource||scripts and > put symlinks in the main and -dedicated packages? > Ah, I thought the 2 packages used the same dir under /usr/share, as it turns out they both have there own dir, its ok to leave things as as. Last, please remove Version=1.0 from the .desktop, .desktop files should not contain a Version= field. An also please remove "Applcation;" from the Categories field in the .desktop file, thats wrong too. p.s. Great job in getting the trademark issue cleared, it will be good to have this in Fedora!
Thanks, I agree completely. :) Addressed the above, rpmlint is now clean. Sorry for the delay, life has been complicated lately. Spec URL: http://zanoni.jcomserv.net/fedora/armacycles-ad/armacycles-ad.spec SRPM URL: http://zanoni.jcomserv.net/fedora/armacycles-ad/armacycles-ad-0.2.8.2.1-5.fc7.src.rpm
The latest version looks fine, approved!
Even with no initscripts for -dedicated, or should I add them later? I suppose it is usable without them.
New Package CVS Request ======================= Package Name: armcycles-ad Short Description: A lightcycle game in 3D Owners: limb Branches: FC-6 F-7 InitialCC: Cvsextras Commits:
Whoops, bad package name, corrected: New Package CVS Request ======================= Package Name: armacycles-ad Short Description: A lightcycle game in 3D Owners: limb Branches: FC-6 F-7 InitialCC: Cvsextras Commits:
(In reply to comment #13) > Even with no initscripts for -dedicated, or should I add them later? I suppose > it is usable without them. Yes, I was thinking that we actually already have quite a few game servers without initscripts, and that making fixing that a must is a bit harsh. Still something worth doing though, but you might want to wait till the final form of initscripts for F-8 is clear.
Please use your FAS name for Owner fields for any further requests. cvs done.
armacycles-ad-0.2.8.2.1-5.fc7 has been pushed to the Fedora 7 stable repository. If problems still persist, please make note of it in this bug report.
Package Change Request ====================== Package Name: armacycles-ad Updated Fedora CC: z-man.net
pkgdb doesn't allow us to add arbitrary email addresses to initialCC. The person would need to sign up for a fedora account and then we can add them.. I don't see them in the account system off hand, if they are in there, list their fedora account name.