Re: Quake packaging

[Simon McVittie <smcv@debian.org>]

On Tue, 28 Jun 2011 at 18:51:03 +0100, David Banks wrote:
> On 28/05/11 12:02, Simon McVittie wrote:
> > On Fri, 27 May 2011 at 22:36:44 +0100, David Banks wrote:
> <snip>
> 
> Sorry to continue this thread necromancy.  I've now done this wrapper
> package in the style of quake3.

(Advance warning: I'm quite a pedantic reviewer.)

I've had a look at quake and quakespasm, and in general they look good.
Thanks for working on these!

I've pushed some proposed changes to a branch called "smcv" in each
repository: feel free to merge or reject them. There are some other things
that should be addressed before upload, listed below.

Regards,
    S

Both packages
=============

In my branches I changed the target distribution to UNRELEASED, because if
I'm going to sponsor this, I'll upload directly from pkg-games git, and only
push the change from UNRELEASED to unstable when I have actually uploaded.

Do you intend these packages to be games-team-maintained? (I assume you do,
since they're in pkg-games git.) If so, the games team should be in Uploaders
to signal that (or if you don't want to be the primary maintainer, make
the games team the Maintainer and move yourself to Uploaders).

Feel free to add me to Uploaders too, if you want an "official" co-maintainer.

quake
=====

Fixed in my branch
------------------

In my branch I moved it to contrib/games, because this package is specifically
for the id Software game; if we packaged OpenQuartz (which, to be honest,
I probably wouldn't bother with), it should have its own menu entry and
launcher, live in /usr/share/games/openquartz, and not provide quake-data.
(It's not Quake, but a remarkably similar game called OpenQuartz, as far
as I'm concerned.)

I added g-d-p as an alternative to quake-data in the dependency; the
ftpmaster who reviewed quake3 (I forget who now) suggested that, as a
way for new users to "bootstrap" the data. Doing that requires that the
menu entry does something fairly sensible when the data is missing, but you
took my need-data.sh from quake3, so that's already covered.

The other changes in my branch are hopefully self-explanatory.

Unaddressed
-----------

I'm not sure whether the quake-data virtual package really makes sense
or not: I'm be tempted to depend on
quake-registered | quake-shareware | game-data-packager directly.

I don't have either of the mission packs myself, so I don't know how they
work. If you launch a game of vanilla Quake, can you "upgrade" to a
mission pack through the GUI (like you can click on Team Arena or use the
Mods menu in Quake 3), or do you have to run the engine with a command-line
option?

If you need a command-line option, the packages produced by g-d-p should
probably be called quake-armagon-data and quake-dissolution-data, and the
quake source package should produce quake-armagon and quake-dissolution
binary packages (similar to quake) with a .desktop and .menu that use
the appropriate option.

Is there any particular reason why you chose GPL-3+ for the packaging?
If you don't have a strong objection to the GPL-2, I'd prefer it if
the packaging was under a license at least as permissive as Quake itself
(i.e. GPL-2+): this package is conceptually Quake, even though it doesn't
contain anything under id copyright.

The "Files" stanza for your own copyright should have an explicit license
statement (something like the "Permission is granted..." blurb for
need-data.sh, or the FSF-recommended "This program is free software..." blurb
as used in openarena), even though the full license is included
by-reference anyway.

The "Files" stanza for quake.png needs a verbatim copy of the
license statement from wherever you got it (judging by its style and copyright
holder, presumably it's from some sort of "extra Tango icons" package?),
and preferably a URL or explanatory text for where you got it from. If
it's really GPLv2-only, that makes me sad, but there's nothing we can do
about that.

quakespasm
==========

Fixed in my branch
------------------

I changed the target distribution to UNRELEASED, because if I'm going to
sponsor this, I'll upload directly from pkg-games git, and only push the
change from UNRELEASED to unstable when I have actually uploaded.

I flattened the two changelog entries into one, since its history before
the initial release to Debian isn't interesting for Debian.

As in quake, stanzas in debian/copyright should have a verbatim copy of the
license statement. All files in quakespasm seem to be the same (modulo
whitespace), so I added one.

The other changes in my branch are hopefully self-explanatory.

Unaddressed
-----------

I notice that quakespasm uses ~/.quakespasm, not ~/.quake. This means that
if we ever decide to make quake use a different engine, everyone doing an
upgrade will "lose" their settings and savegames, which would be unfortunate.
Possibly we should add a way to override SYS_USERDIR via the command
line (can Q1 cvars be command-line-only like they can in Q3?) or an
environment variable (if in doubt, this is safer), and make quake
specifically use ~/.quake?

Are we considering quakespasm to be something that users should never run
directly? If so, it shouldn't be in $PATH. (This is probably not the case,
though, particularly if our justification for it being in main is the
existence of OpenQuartz.)

Judging by the comment at the top of snd_mp3.c, it's likely to have copyright
holders whose copyright notices haven't been preserved (the authors of
libid3tag). This is a GPL violation, strictly speaking... to be safe,
we should assume that every copyright holder of libid3tag might be a
copyright holder on that file, and add appropriate notices. Doing that
as a patch would be fine, I think.