Re: Request for review of pentobi packaging
On Wed, 21 Dec 2011 19:36:11 +0100
Martin Erik Werner <martinerikwerner@gmail.com> wrote:
> >
> > Please fill out the Vcs-* URLs (this is the location of the Debian
> > packaging). If you don't have it in a VCS yet, take a look at this:
> >
> > http://wiki.debian.org/Games/VCS
> This has been fixed, it would be nice if you pushed your packaging
> changes continuously to git in order for review though, I
> automatically started looking at git when you asked for re-review ;)
I will have the packaging on git.debian.org fully working when 1.0 is
packaged and released, part of the hesitation has been how I will handle
the repository and at the moment I think I may just have the debian/
folder in the repo. It has also been tricky given I've been using
upstreams development git version to get the upstream fixes for the
packaging.
>
> >
> > The package description could use some reformatting/cleanup. I'd
> > suggest asking on debian-l10n-english.
> >
> > No need for the extra blank line in debian/watch
> >
> > The upstream README doesn't look that useful, I would drop it from
> > the binary package.
> >
> > How about adding a longtitle to the debian/menu file?
All Fixed.
> >
> > If the SVG files in data/ are modified, the PNG files in that same
> > directory will not be re-rendered from the SVG files. I would
> > suggest that upstream should drop the PNG files and use either
> > Inksape or rsvg to create them at build time.
> This has not changed in the sources I'm assuming you have forwarded
> this upstream though?
Not entirely sure of the issue here. I believe upstream is not
concerned about this, or would rather not include inkscape/rsvg in the
build process.
>
> >
> > The Icon field in the pentobi.desktop file should not be an absolute
> > path, just the name of the icon file, without the extension
> > (Icon=pentobi). This is so that the user can override any icon and
> > also so that the deskop can use the SVG file when that is
> > appropriate.
Fixed.
> >
> > The regression tests require gogui-regress, it would be nice to
> > package that too so they could be run during the build.
> Have you looked into this? Might not happen for this release, but
> might good to note down as a to-do in Debian?
Explanation from upstream:
----
The files in regress are not really regression tests. These kind of
GTP-based tests are only historically called so in GNU Go (which
introduced GTP) but the tests in this folder will become probably a
mix of regression and performance tests in the future. It might be
possible to use a subset in the future for real regression tests but
since most of them they do full board searches, they might be too
slow. Also, installing gogui-regress is not a good idea because it is
oo Go-specific and has the big dependency on Java (there is a small
awk script in the GNU Go distribution that can also be used to run
them). A good candidate for regression tests is the unit tests that
can be compiled with PENTOBI_BUILD_TESTS=ON. These tests should be
fast and never fail.
----
> >
> > application-x-blokus-sgf.svg is public domain, not GPL-3.0+
> >
> > view-fullscreen.png is not copyright Markus Enzenberger but Jakub
> > Steiner. I would wager that other images in the package are similar.
> >
> > go-next.png go-previous.png go-down.png go-first.png go-last.png
> > go-up.png say they were produced in Inkscape, but there is no SVG
> > source file available for them. I'd wager go-home.png and lots of
> > other images were also produced in Inkscape even tho there is no
> > text saying they were. Similarly board_trigon.png pieces.png
> > pieces_trigon.png position_trigon.png view-fullscreen.png say they
> > were made in GIMP but there are no XCF files available. These files
> > are all allegedly GPL-3.0+, making them not redistributable without
> > source (probably SVG/XCF). Given that they probably also have a
> > different copyright holder, I'm not sure what the license actually
> > is.
All images are now either custom created, created by
modifying versions of the public domain tango icon theme icons, or are
unmodified tango icon theme icons. The unmodified icons have been noted
in the debian/copyright file, the others are GPL3. If there are no
matching svg files it is because they are low resolution images which
do not match the svg files as they have show signs of bitmap editing,
probably to make them look nicer at low resolution - the png's are the
source file.
> > It would be nice if upstream could put copyright holders and license
> > grant headers into each source code file.
Upstream would prefer to leave this as is.
>
> Where the icons are completely unmodified, you still need to state the
> public domain dedication in d/copyright. And likely include the svg
> equivalents for the sake of DFSG.
Explained above.
>
> >
> > desktop-file-validate warnings:
> >
> > pentobi.desktop: warning: key "Encoding" in group "Desktop Entry"
> > is deprecated pentobi.desktop: warning: value
> > "Application;Game;BoardGame;" for key "Categories" in group
> > "Desktop Entry" contains a deprecated value "Application"
> >
> > dpkg-shlibdeps warning:
> >
> > dpkg-shlibdeps: warning: dependency on libboost_thread.so.1.46.1
> > could be avoided if "debian/pentobi/usr/games/pentobi
> > debian/pentobi/usr/games/pentobi-thumbnailer" were not uselessly
> > linked against it (they use none of its symbols).
> >
> > lintian complaints (run lintian-info -t tagname for descriptions):
> >
> > P: pentobi source: unversioned-copyright-format-uri
> > http://dep.debian.net/deps/dep5
> > I: pentobi: spelling-error-in-binary usr/games/pentobi Programm
> > Program I: pentobi: spelling-error-in-binary usr/games/pentobi
> > Programm Program I: pentobi: spelling-error-in-binary
> > usr/games/pentobi Programms Programs I: pentobi:
> > spelling-error-in-binary usr/games/pentobi allows to allows one to
> The Programm spelling is from German, i.e. false positives, the
> "allows to" -> "allows one to" or "allowing" would still be something
> to forward upstream though.
>
> > P: pentobi: no-upstream-changelog
> I was thinking you could install upstream NEWS as a changelog, that is
> it's current use, isn't it?
>
> > I: pentobi: description-synopsis-might-not-be-phrased-properly
> > I: pentobi: desktop-entry-contains-encoding-key
> > usr/share/applications/pentobi.desktop:2 Encoding
All fixed.
>
> In addition I have a few more comments:
> * Installing an xpm 32x32 icon for usage by the debian menu
> in /usr/share/pixmaps/ would be nice, Imagemagick's convert is
> convenient for this purpose:
> convert foo.svg -resize 32x32 foo.xpm
> (You obviously need to build-depend on imagemagick in that case)
> * Specify above icon in the menu file
Both done.
> * Adding "Format: 1.0" to the .desktop file is, I think, nice in order
> to follow the specification.
But as noted by the specification it is not required. :)
I have uploaded the package containing all of the fixes to mentors.d.n:
http://mentors.debian.net/package/pentobi
Which can be grabbed via:
dget -x
http://mentors.debian.net/debian/pool/main/p/pentobi/pentobi_1.0~b1-1.dsc
If anyone has time to do one last look over the package, most
specifically making sure that upstream looks OK for uploading to NEW.
If everything does look good with the upstream part I will give them
the nod. Once 1.0 final is released I will package it then
begin the process for a sponsor.
Thanks,
Dean
Reply to: