Re: Bug#660519: RFS: manaplus/1.2.2.19
Here is another review of manaplus:
If you contact upstream as a result of this review, please point them
at these two pages:
http://wiki.debian.org/UpstreamGuide
http://www.freedesktop.org/wiki/Games/Upstream
Did you intend for this to be maintained as part of the Debian games team?
Do you intend to package the servers for manaplus too?
All the servers appear to require a login to play, are there any I
could use for 'testing' the game without registering?
Please consider wrapping debian/menu with one item per line.
description= is not valid in debian/menu, you want longtitle= IIRC.
xz compression is better than bzip2, have you considered switching?
Personally I don't think the package is large enough to bother
switching from gzip though.
Some of the images were created in Inkscape or GIMP but there are no
source SVG or XCF images, that might be a GPL/DFSG violation.
There is one pre-mixed, pre-encoded audio file, could you ask upstream
about how it was created?
In the upstream .desktop files, none of the language-specific Icon or
Name lines appear to be needed.
You are installing the upstream PNG icon into the wrong location, it
should be /usr/share/pixmaps since none of the
/usr/share/icons/hicolor/*/apps/ dirs are for the same resolution as
the image. Please also install manaplus.svg into
/usr/share/icons/hicolor/scalable/apps/ and use python-scour to strip
down the installed image.
The manaplus.svg file looks slightly different to manaplus.png and the
other files, it looks like that means the source SVG for manaplus.png
is missing and there is a GPL/DFSG violation.
There is no need to install manaplusttest at all IMO since it does
nothing useful to users, please get upstream to disable that. If they
are not willing to do that, please at least remove the desktop file
for it.
Please ask upstream to use fontconfig to find font files on Linux or
switch to a font rendering system that does that (such as SDL-Pango,
QuesoGLC). Then you will not need to use those symlinks, which are
fragile when font paths move around and are not portable between Linux
distros.
Some of the upstream code has the wrong address for the FSF in the
license grant, please let them know about that.
Some of the copyright/license information is missing from
debian/copyright. Please audit every file, licensecheck --copyright
helps to find missed stuff though.
You are missing a depends on x11-utils because the code uses xmessage
to display errors.
I'm not sure if any of the errors contain untrusted network/other
input, but it is not a good idea to use the system() function for
anything other than constant commands. Please send upstream a patch to
use the exec family of functions, they are the only ones that are
guaranteed to be safe from arbitrary code execution in the face of
untrusted input.
I note that the upstream help system uses plain text files for
translations, has upstream considered using standard po files for
that?
There are some duplicate files (I detect them using fdupes), you might
want to ask upstream to remove them from the source package and in the
meantime reduce the size of the package slightly using symlinks:
rdfind -outputname /dev/null -makesymlinks true debian/manaplus/
symlinks -r -s -c debian/manaplus/
cppcheck warnings (send upstream):
[src/graphicsvertexes.h:140]: (error) Memory leak: ImageVertexes::ogl
[src/game.cpp:1828]: (error) Possible null pointer dereference: newMap
- otherwise it is redundant to check if newMap is null at line 1823
Lintian complaints:
O: manaplus-data: package-contains-empty-directory
usr/share/manaplus/data/themes/classic/
I: manaplus: spelling-error-in-binary usr/games/manaplus dont don't
--
bye,
pabs
http://wiki.debian.org/PaulWise
Reply to: