Re: review of uhexen2 package
CCing you gustavo - let me know if you're on-list and I'll stop.
On Sun, Sep 09, 2012 at 11:47:07PM -0300, gustavo panizzo wrote:
> On Fri, Aug 31, 2012 at 05:08:51PM +0100, Jon Dowland wrote:
> > • it would be nice to have a build target to automatically build a .dfsg
> > source tarball.
> > I notice that the installdocs override explicitly ignores
> > e.g. dos, win32 folders… they could be stripped from the upstream source
> yes, i started non installing dos and win32 docs, then i found more non-dfsg bits
> and source-cleaner.sh was born.
> i've removed those docs from the source.
>
> > too (since you are modifying it already) - aha! I've just found source-cleaner.sh.
> > Some explanation for what is being removed / why would be nice (LICENSE.txt?!)
> i moved the cleanup to source-cleaner.sh and it is documented on
> README.source
>
> i choose to remove LICENSE.txt (GPL v2 text) because is being ship as
> /usr/share/common-licenses/GPL-2
OK that makes sense. We should put these explanations into README.source if you
haven't already (haven't checked revised package)
> > There has been some interesting recent discussion on -devel about extending
> > uscan to support auto-stripping upstream sources according to some rules.
> > It would be worth keeping an eye on these developments.
> >
> > • the rules file is pretty big (for a dh rules file!) Some of those comments
> > could surely be removed. Are all of those 'cleans' needed in the build
> > target? I guess so, but that points at horrors in the upstream Makefile…
> > Could that be fixed?
> i removed some comments on the rules file, i leave others because i
> like when things have comments to clarify
Sure.
> > • parallel builds are not supported. Please consider appending --parallel to
> > dh build options, e.g. dh $@ --parallel, although since you override the
> > build target, more work might be required there.
> hmm how could i get how many processors should i use (make -j)
> i know you are a --parallel fan, but this game is small, it only takes
> 1'30" to compile on a 4 cores cpu
Yes it's not essential, merely nice. It's not a blocker for me to upload, at
least.
> > • A bunch of lintian errors relating to hardening flags. (hardening-no-relro
> > and hardening-no-fortify-functions). It might be worth reading the long
> > descriptions for these lintian errors. I see you've done some work to
> > support hardening flags already.
> yes, all binaries that talks over the network are hardened now, and
> the best part is they keep working! :)
Great!
> > • The package cannot be used without any game data, but there's no package
> > dependency on a data package. (Of course there is no data package presently).
> > We should extend game-data-package to support Hexen 2 data, and add Suggests:
> > game-data-packager to the uhexen2 package.
> i've added hexen2-data to Recommends, after we can teach
> game-data-package to make hexen2-data packages
> hexen2-data won't be free, but since is a contrib package we are ok
I started work on hexen2 for game-data-packager, I'll push my work to the git
repo soon.
> > I've started looking at providing hexen2 support in game-data-packager. There
> > was, apparently, a hexen2 demo, which uhexen2 supports, so that might be
> > something we can distribute in non-free. Although, sadly, as far as I can see,
> > there was no explicit demo license or distribution terms, and the bundled
> > "HEXEN II SUBLICENSE.doc" is the EULA for Hexen 2 proper, which makes it non-
> > redistributable amongst other things.
> :(
>
> i will take a look to sourceforge hosted binaries, i see 2 demo
> releases there
cool (game-data-packager could probably handle installing these)
> >
> > • because it can't be used without external data, it should be in section
> > contrib/games, rather than main/games (or 'games', main implied)
> i always inted to place uhexen2 on contrib, but i didn't know how to
> do it
Ah ok, simply prefix 'games' with 'contrib/' in the control file
> > OK! I will have more in the future. I haven't actually got the resulting thing to
> > run properly, need to fiddle with where to put my game data.
>
> thanks for the review, it was really extensive
You are welcome!
> i've uploaded a new version, only a few pieces are missing (md5sum for
> pak files and more testing upgrade-111.sh script)
>
> http://mentors.debian.net/debian/pool/contrib/u/uhexen2/uhexen2_1.5.4.dfsg-1.dsc
I will take a look on Tuesday.
Reply to: