Spec Name or Url: http://www.daughtersoftiresias.org/progs/vultures/nethack-vultures.spec SRPM Name or Url: http://www.daughtersoftiresias.org/progs/vultures/nethack-vultures-1.10.1-0.1.src.rpm Description: Vulture's Eye is a mouse-driven interface for NetHack that enhances the visuals, audio and accessibility of the game, yet retains all the original gameplay and game features. Vulture's Eye is based on Falcon's Eye, but is greatly extended. Also included is Vulture's Claw, which is based on the Slash'Em core.
Created attachment 121203 [details] Changes made after my last version of this package Having done most of the packaging work, I'm a bit biased reviewer, but will do early next week until someone picks it up until that. However, a few notes about the differences happened after the package this was based on (http://cachalot.mine.nu/4/SRPMS/nethack-vultures-1.10.1-0.1.src.rpm, diff attached). Please bump the release tag _always_ when you make changes to any package. Can be fixed by bumping to 1%{?dist} later after approved. Why was the log2stderr patch removed? Without it, the program will try to trash the harddisk with logfiles in the working dirs. I consider that a blocker. The "get rid of ncurses-devel build dep" was handled by just removing the build dep. The last time I tested, that broke the build. Does it build cleanly for you if you don't have ncurses-devel installed? The "move manual to doc dir" TODO was taken care of by adding _another_ copy of the manual (plus a lot of completely unneeded files) to the doc dir and removing the symlinking between Eye and Claw's manual, resulting in not moving the one manual to a proper place, but in a total of _three_ manuals included, two of which in places where they're not needed, and another in doc dir but with lots of unneeded cruft included. Removing the manuals included in %{_prefix}/games/vultures*/manual and changing %files to do vultures/win/jtp/gamedata/manual/ would be the right thing to do (unless the manuals are needed in the game dir(s)). Finally (not a change from my previous version, but anyway), SDL_mixer has been changed to pull in timidity++ so the dependency can be removed from here.
I assumed that the log2stderr patch was the one that you sent us to include in the source tree, which we did (we've applied all the patches that you've sent us). Although, now thinking back about it, that will only be an appropriate solution for future builds, not this one, even if that was the case. Putting it back in. Sorry about the revision number - you're right, I did forget to bump it! For some reason, I was thinking that your revision was .0. Upped. I didn't try building with ncurses-devel missing, but I grepped through the source, and nothing in there that should be building that looked like it would need ncurses-devel - only obsolete window managers. I suppose uninstalling ncurses-devel wouldn't be too much of a pain for me, so I'll try it out. Ah, I see what's happening - the version that we have a release for here *is* building the obsolete window managers. It's probably just easiest to put ncurses-devel in, rather than make another release. Done. When we make a release that doesn't build the other window managers, I'll take it back out. Remove the timidity++ dependency... Done. The manual thing confuses me, because that this is not the code that I entered. I think when I had backed out some broken code, I must have backed out my fixes (I had two rm statements earlier for the manuals before the "Save some space" comment, as well as removing them from the file list - that's why manual was removed from the for loop - and had narrower doc entries). Restoring the deleted code. Thanks, Ville. Sorry for the errors. http://www.daughtersoftiresias.org/progs/vultures/nethack-vultures.spec http://www.daughtersoftiresias.org/progs/vultures/nethack-vultures-1.10.1-0.2.src.rpm
Ok, thanks, the 0.2 release looks better. Yep, you got the log2stderr stuff right, the intention of the stderr patch I sent "upstream" to you was that in the future, the log2stderr patch in this package could be later reduced to just #undef'ing (or removing the definition of) JTP_LOG_FILE. However, release 0.2 of the package adds the log2stderr patch back to the source RPM, but forgets to apply it. Needs %patch3 in %prep. Regarding the ncurses-devel dependency, I vaguely remember that it had something to do with the -config patch enabling "#define LINUX", but I may be wrong. Anyway, I agree that just having the build dependency in is ok for now. In case you're not aware of them, for finding missing build dependencies you'll probably find the mock package and/or the fedora-rmdevelrpms script in fedora-rpmdevtools useful. So, this is approved, as long as the missing %patch3 line is put in before the first build (you may do that after importing to CVS, while bumping the release tag to 1%{?dist} ;)).
Done :) When I get a chance (an hour or two from now), I'll continue on with the process.
It seems you've added the 14MB binary tarball three times into CVS. It belongs in the lookaside cache where it already is. Please remove it from all branches in CVS. You know you can use eg. 1%{?dist} as the release in the specfile for all branches if you don't want to manually tinker with them between branches and ensure upgradeability, right? Also, non-pre-release release tags usually don't start with "0."; my specfile had that but it was meant only for the duration of the package review.
Ok, thanks - I didn't know those things. Where is this lookaside cache? And how can I access it to put new release tarballs in there?
cvs-import.sh has already taken care of this for you when you imported the package. For more info, see http://fedoraproject.org/wiki/Extras/UsingCvsFaq
Thanks! If it's ok with you, I'm going to go ahead and mark this release pending. What about regular Falcon's Eye? Should we revisit the issue of keeping/removing it in a month or so (after people have had a chance to use Vulture's Eye from RPM and we know that there are no significant issues that we didn't catch), or should we address keeping/removing it now?
If by "release pending" you mean that it has been built in the buildsys and is waiting to be pushed, sure. At this point new packages are usually just closed as NEXTRELEASE though. http://fedoraproject.org/wiki/Extras/NewPackageProcess I think we should proceed with removing Falcon's Eye already now; it has been obsoleted by the vanilla nethack package for a while already and I'm not aware of any plans to revive it now that Vulture's * is in. I'll take care of it.
Thanks!