Re: review of uhexen2 package

[gustavo panizzo <gfa@zumbi.com.ar>]

On Fri, Aug 31, 2012 at 05:08:51PM +0100, Jon Dowland wrote:
> Hi Gustavo,
hi Jon
> 
> As requested on IRC - in future please build a package and put it somewhere e.g.
> mentors.debian.net, and add it to <http://wiki.debian.org/Games/Sponsors/Queue>.
i didn't feel like the package were ready to ask for
sponsorshipi/upload, now it is :)

> You could also CC the ITP bug #632376.  Oh, actually, do you intend to maintain
> this as part of the games team? I see games team is not in the Maintainer field.
fixed

> 
> It took me a while to remember where you'd pinged me to look at this :)
> 
>  • the upstream version has .dfsg suffix but I can't find an indication
>    of what needed to be changed. README.source appears to be the dh_make
>    default. Could you document what changes are made to the upstream
>    source here?
upstream version is 1.5.4, but source is not DFSG complaint, 
i decided to append .dfsg to version make that visible
README.source was updated to document it.

> 
>  • 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
> 
>    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 

> 
>  • 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

>  • 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! :)

> 
>  • 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'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

> 
>  • 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

> 
> 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 


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

-- 
1AE0 322E B8F7 4717 BDEA  BF1D 44BB 1BA7 9F6C 6333