Bug#775445: Debian review
This mail is just to add a note that bitfighter was reviewed by 'pabs' on IRC. Attached is the log of the review (taken from my client). I am working on cleaning things up and resolving multiple issues brought forward in the review, mostly under these categores:
* Font dependency issues
* Source issues (UTF-8, unneeded junk)
* Copyright issues (music, graphics)
Thanks to 'pabs' again for his time.
Stay tuned,
David
[23:03] <pabs> raptor: around? doing a review of bitfighter
[23:03] <raptor> hello!
[23:03] <pabs> hey :)
[23:03] <pabs> debian/source/include-binaries is empty and can be removed
[23:03] <raptor> doing so now...
[23:04] <raptor> (and thank you!)
[23:04] <pabs> debian/patches/respect-link-flags looks like it should be pushed upstream
[23:05] <raptor> yes, i will do that - is there a convention for tagging patches as upstream?
[23:05] <pabs> check out http://dep.debian.net/deps/dep3/
[23:05] <pabs> some parts of this patch look wrong, for example you remove manual page and .desktop file installation debian/patches/initial-debianization
[23:06] <raptor> hmm...
[23:06] <pabs> I think disabling the updater should be dealt with by a cmake build option rather than a patch
[23:07] <raptor> ok, I can patch that
[23:07] <pabs> IIRC CMAKE_BUILD_TYPE is supposed to be set to None, not RelWithDebInfo
[23:08] <pabs> debian/README.source contains only the template, you should either remove it or fill it out
[23:08] <raptor> ah yes, it is set to both - i was curious about that because i see several projects in pkg-games that set that flag
[23:09] <pabs> personaly I would put the licenses at the bottom of debian/copyright rather than the top - easier to get an overview that way
[23:09] <raptor> ha, good point
[23:09] <raptor> swapping that now
[23:09] <pabs> Vcs-* in debian/control need to either be removed or uncommented
[23:10] <pabs> Depends in debian/control is hard-coded to some libraries. you shouldn't need to do that, the build process will automatically fill those out
[23:10] <pabs> fonts-* will need to remain though
[23:10] <pabs> I would recommend running `wrap-and-sort -sa` so that various fields are wrapped and easier to debdiff
[23:11] <raptor> i am unfamiliar with that utility
[23:11] <pabs> it is from the devscripts package
[23:11] <raptor> is it applied to the project as a whole?
[23:11] <pabs> just to the debian/ dir
[23:11] <pabs> I would suggest pushing the desktop file and manual page upstream
[23:11] <raptor> ok
[23:13] <pabs> brb 5 min
[23:13] <raptor> for the hardcoding in depends - you mean to do this?: libluajit-5.1-2 -> libluajit-5.1
[23:13] <raptor> OK
[23:13] <pabs> correct
[23:19] <pabs> raptor: what is dfsg in the version about? there don't appear to be any instructions for repacking the upstream tarball to the Debian orig.tar.gz?
[23:20] <pabs> the 'SEE ALSO' section of the manual page appears to be entirely incorrect
[23:20] <raptor> the original tarball had a lot of junk in it, like windows libraries, etc.
[23:21] <raptor> there is a pristine-tar branch in the git repo created for this
[23:21] <raptor> should I have written actual instructions somewhere?
[23:21] <pabs> ah, I see the manual page was generated by help2man. personally I would suggest running help2man at build time, unless of course you have modified it
[23:22] <pabs> usually the instructions either go upstream to fix their tarball, or in the get-orig-source target in debian/rules
[23:22] <raptor> hmm.. i'll have to research that a little
[23:23] <raptor> would i add the instructions to the debian directory or just send them upstream?
[23:24] <pabs> personally I would fix the script/process that upstream uses to generate their tarball. get-orig-source is a workaround for when upstream refuses that
[23:24] <raptor> ok.. in this case, with a released version, should I have upstream regenerate their tarball?
[23:27] <pabs> no, the existing tarballs should remain as-is, just when the next version comes out it would be good to have a Debian-suitable tarball
[23:27] <raptor> ok, we'll do that, too :)
[23:28] <pabs> you can generate two tarballs if needed: one with all the extra stuff for people who want that, one for Debian and other Linux distros
[23:29] <raptor> good idea
[23:30] <pabs> couple of lintian warnings: https://mentors.debian.net/package/bitfighter
[23:31] <raptor> the gpg and font ones?
[23:33] <raptor> upstream has never signed their tarball
[23:33] <raptor> also the fonts don't seem to exist in any other debian font package
[23:34] <raptor> one of the fonts could possible be ignore (webhostinghub-glyphs) as it's not critical to the UI of the game, but the other is needed
[23:34] <raptor> *ignored
[23:36] <raptor> wrap-and-sort is nice!
[23:38] <pabs> re the fonts, you could split them out into separate fonts-foo packages and then depend on those
[23:39] <pabs> raptor: here is a list of commands that produce some output that you might want to look at https://paste.debian.net/141694/
[23:39] <raptor> i admit that creating font packages isn't very appealing...
[23:40] <pabs> are these fonts created by bitfighter upstream or by other folks?
[23:40] <raptor> other folks
[23:40] <pabs> Modern-Vision.ttf is non-free so we can't distribute it in Debian
[23:40] <raptor> we lucked out with the other fonts being already in debian...
[23:41] <raptor> what!?
[23:41] <raptor> I'll have to check with the head developer on that one
[23:41] <pabs> the license file says cc-by-nc-nd, the nd part (no derivs) makes it non-free
[23:42] <raptor> oh interesting.. i did not know that
[23:42] <raptor> even if we have not altered the file?
[23:42] <raptor> oh, but debian needs the liberty to alter
[23:42] <pabs> right
[23:42] <raptor> ok, got it
[23:43] <pabs> btw, generally we like things that are maintained by separate folks to be packaged as separate Debian packages, from the output of those separate folks
[23:43] <raptor> ok
[23:43] <raptor> that makes sense
[23:43] <pabs> so I think webhostinghub-glyphs should be packaged separately based on the version from their site and then bitfighter can depend on it
[23:44] <raptor> I think it will be easier to remove it from the game
[23:44] <raptor> it's only use in one place, i think
[23:45] <raptor> is that list of commands above something you usually run for review against packages?
[23:46] <pabs> I run a tool called check-all-the-things, which runs those tools
[23:46] <pabs> https://anonscm.debian.org/cgit/collab-maint/check-all-the-things.git
[23:46] <raptor> haha, that's great
[23:47] <pabs> orbitron should be packaged separately too, here is the homepage: https://www.theleagueofmoveabletype.com/orbitron
[23:48] * pabs sighs at reserved font name crap
[23:48] <raptor> oh oops, that actually isn't used anymore in the game; i'll clean that up
[23:49] <pabs> what is resource/sfx/gofast.sfs about/from?
[23:49] <raptor> good question, let me check the repo history
[23:50] <pabs> looks like the copyright/license info for the music isn't present in the upstream tarball nor debian/copyright, looking on modarchive some of the .xm files are CC-BY-SA.
[23:51] <raptor> is that a non-free license?
[23:52] <raptor> ok, i'll add that to the copyright
[23:52] <pabs> it is a free license
[23:52] <raptor> ah right, just the ND part
[23:52] <raptor> ooo, i like check-all-the-things
[23:53] <raptor> ok, the gofast.sfs file is a source file used in some wave generator application that generates the output .wav
[23:54] <raptor> but unneeded for the game
[23:55] <pabs> which application? would be nice to render the wav file at build time
[23:55] <pabs> btw, an upstream guide for games devs http://www.freedesktop.org/wiki/Games/Upstream/
[23:55] <pabs> !ug
[23:55] <dpkg> ug is probably If you contact <upstream>, please point them at our upstream guide: https://wiki.debian.org/UpstreamGuide
[23:55] <pabs> and one for general stuff ^
[23:56] <raptor> ok, thanks
[23:56] <pabs> ok, so all of the .xm files are CC-BY-SA-3.0
[23:57] <raptor> i'll add that to the copyright
[23:57] <pabs> I can't find the license of the .ogg files though
[23:57] <raptor> oh... i converted those, i think they came from musopen
[23:57] <raptor> let me check
[23:57] * pabs also wonders how they were produced/recorded/mixed
[23:58] <raptor> it was a military band
[23:58] <raptor> for open source music
[00:01] <raptor> pabs: found it: http://musopen.org/music/piece/488
[00:01] <raptor> CC-PD public domain
[00:02] <pabs> where do you see CC-PD on that page?
[00:02] <raptor> i still cannot find the original app for the .sts file - i'll have to check with the lead developer
[00:03] <raptor> bottom-right of the page
[00:03] <pabs> ah, I see
[00:03] <pabs> what about the other menu ogg file?
[00:04] <raptor> ohhh... good question, i'm not sure I know that one. I'll take a note and ask the lead dev on that, too
[00:07] <pabs> I found two files that are not UTF-8 encoded: poly2tri/sweep/sweep.h resource/scripts/inspect.lua
[00:07] <pabs> hmm, few images in resource/ - how are the graphics for this done?
[00:07] <raptor> did you find those with check-all-the-things script?
[00:08] <raptor> let me check the images
[00:09] <pabs> the UTF-8 thing? no, by manually running isutf8 from moreutils, still on my todo to integrate that into check-all-the-things
[00:10] <pabs> looking at the upstream screenshots on the website, it looks like everything is rendered at runtime using vector rendering
[00:10] <raptor> yes, no internal textures except for the fonts
[00:10] <pabs> looks like a fun game :)
[00:11] <raptor> ok, the image in resource/, I made from a screenshot in the GIMP
[00:11] <pabs> I noticed the server stuff is very specific to the place where it is currently hosted. would be interesting to generalise it
[00:11] <raptor> that xpm was generated by a linux util... i think just for debian
[00:11] <pabs> I guess you mean redship64.png?
[00:12] <raptor> yes - and the bmp where generated the same way, but probably by the lead dev on his windows box
[00:12] <pabs> probably a good idea to generate the .xpm at build time in debian/rules if nothing else uses it
[00:12] <raptor> ok.. i vaguely remember a lintian warning about the xpm
[00:13] <raptor> before it was created, i mean
[00:13] <raptor> what do you mean by 'server stuff' ?
[00:15] <pabs> the 'master/master support files and scripts' dir
[00:15] <raptor> oh yes, that is built only on bitfighter.org
[00:15] <raptor> there is a masterInterface.cpp class that is used in the main game
[00:15] <raptor> to handle the RPC calls
[00:16] <raptor> much of that could probably be removed from the source
[00:16] <raptor> is that what you mean by 'generalize'?
[00:16] <pabs> I'd recommend keeping it, others might be interested in running their own private servers
[00:17] <pabs> well, for example, startmaster uses /home/eykamp/
[00:18] <pabs> if it were packaged as bitfighter-server that would definitely need to be changed
[00:18] <raptor> ha
[00:18] <raptor> that's funny
[00:18] <raptor> i didn't even know that was there...
[00:18] <pabs> :)
[00:19] <pabs> anyways, there is probably more to fix but I best do some work now
[00:19] <pabs> feel free to pastebin the full chat log to the other upstream devs if you want
[00:20] <pabs> or to the RFS bug report for that matter
[00:20] <raptor> ok, i'll go through the check-all-the-things, too
[00:20] <raptor> thank you very much for your time!
[00:20] <pabs> np :)
Reply to: