[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFS: MiceAmaze video game



2012/12/8 Paul Wise <pabs@debian.org>
>
> On Sat, Dec 8, 2012 at 12:57 AM, Raphael Champeimont wrote:
>
> > By the way, should I leave the previous upload entry in the changelog file,
> > since it was never uploaded in Debian?
> > (In the file I provide there is just a single entry for this new upload)
> > If so, should the previous version be marked as unreleased or unstable?
>
> It doesn't particularly matter which you choose, different sponsors
> require different things anyway.


ok

>
>
> Here is a re-review:
>
> Have you considered joining the Debian games team and helping out with
> other games too?
>
> http://wiki.debian.org/Games/Team


Well, for the moment I'm just trying to package my own program. In
fact my reasoning was that it would be useful to users to have a
package to install (which I put on the project website), then I
thought it would be even better to have it included in Debian (for the
kind of reasons in http://wiki.debian.org/AdvantagesForUpstream )

>
>
>
> I would strongly suggest removing the embedded code copies from the
> upstream tarball. This includes the data/fonts and SOIL
> subdirectories.


The point is, I still think it is valuable for users to have a
smallest-effort-to-use version, for users who probably will even run
the software as a user and don't want to have to install many
dependencies. Debian is very good in terms of package availability
since you can even get SOIL and DejaVu.ttf as packages, but not all
OSs have that...
But anyway, if it's an issue to have dependencies included, I can
provide a separate version of the upstream tarball, without deps.

>
>
> I would suggest building the data from its source at build time and
> dropping the prebuilt data from the upstream tarball. For the SVG
> files this will mean a build-dep on librsvg2-bin for rsvg-convert, or
> maybe inkscape if rsvg does not render them correctly.


Actually what you see in the source really are my source files. I
created the simple graphics with GIMP and the original files are those
you see. For the eagle images, I downloaded the SVG initially but then
modified them manually in GIMP also (basically for recoloring).
For a future release I will probably create better graphics in SVG
format, and in this case I would be happy to provide them. Is it
necessary to recreate all the current files in SVG?

>
>
> I found a lot of issues in the upstream Makefile (see below) and
> decided it would be best if I rewrote it for you, I've attached the
> new Makefile, hopefully it is acceptable to you. It still doesn't do
> things like allowing cross-compilation like a standard build system
> such as automake would, but it is enough for most of Debian's
> purposes. You will need to change the SOIL include to this:
>
> #include <SOIL/SOIL.h>


Ok tank you very much.

>
>
> Please do not statically link soil, always use -l (-lSOIL) to link
> against libraries.


ok

>
> The package fails to build when built twice in a row. You can
> reproduce this by running this, I think you want convert -scale
> instead of convert -resize.
>
> debuild && debuild && debuild
>
> dpkg-source: error: cannot represent change to miceamaze.png: binary
> file contents changed
> dpkg-source: error: add miceamaze.png in
> debian/source/include-binaries if you want to store the modified
> binary in the debian tarball
> dpkg-source: error: unrepresentable changes to source
> dpkg-buildpackage: error: dpkg-source -b miceamaze-1.6.1 gave error
> exit status 2
>
> Your upstream Makefile has Windows line endings.
>
> Why do you patch the upstream Makefile? It would be best to let
> dh_auto_build do its job, which includes passing the right CFLAGS/etc
> from dpkg-buildflags to make and setting DESTDIR etc. Since you aren't
> using autoconf, you might have to do dh_auto_build -- PREFIX=/usr
> BINDIR=/usr/games. We have xdg-utils in Debian so you don't need to
> patch out xdg-desktop-menu/etc, just build-depend on xdg-utils, I
> guess that doesn't support DESTDIR though. Other distributions will
> want DESTDIR too. DESTDIR should not be part of the prefix compiled
> into your program, otherwise it will not work when it is installed.


Yes, the reason I patched the xdg stuff is it would install in /usr
and so on. On the other hand, I think they are necessary if installing
directly because there is some kind of indexation xdg tools do, which
is done at package install time in Debian (and not make install time).

>
>
> You should use $(shell ...) instead of backticks in your Makefile.
>
> Your debian/rules file should use the --parallel argument to dh.
>
> I would suggest manually wrapping debian/menu manually since
> wrap-and-sort doesn't do it yet. Do it one item per line and one space
> indentation.
>
> Delete the word "free" from the package description. Everything in
> Debian main is Free and free.
>
> Usually we edit debian/changelog with dch, which will leave a blank
> line between the changelog entry header and the text of the changelog
> entry.

Ok I'll fix all that.

>
> Automatic checks:
>
> http://wiki.debian.org/HowToPackageForDebian#Check_points_for_any_package
>
> lintian:
>
> P: miceamaze source: source-contains-prebuilt-windows-binary SOIL/testSOIL.exe
> I: miceamaze source: quilt-patch-missing-description Makefile.patch
> P: miceamaze source: unknown-copyright-format-uri
> http:www.debian.org/doc/packaging-manuals/copyright-format/1.0/
> P: miceamaze: no-upstream-changelog
> I: miceamaze: desktop-entry-contains-encoding-key
> usr/share/applications/miceamaze.desktop:2 Encoding

Btw why doesn't my lintian give me these warnings? Am I missing an
option you use?

>
> cppcheck:
>
> [SOIL/src/original/stb_image-1.16.c:2924]: (error) Memory leak: out
> [SOIL/src/stb_image_aug.c:2651]: (error) Memory leak: out
>
> desktop-file-validate:
>
> ./miceamaze.desktop: warning: key "Encoding" in group "Desktop Entry"
> is deprecated
>
> isutf8:
>
> ./data/mazes/maze12.txt: line 31, char 1, byte offset 1: invalid UTF-8 code
> ./data/mazes/maze05.txt: line 31, char 1, byte offset 1: invalid UTF-8 code
> ./data/mazes/maze08.txt: line 31, char 1, byte offset 1: invalid UTF-8 code
> ./data/mazes/maze04.txt: line 31, char 1, byte offset 1: invalid UTF-8 code
> ./data/mazes/maze03.txt: line 31, char 1, byte offset 1: invalid UTF-8 code
> ./data/mazes/maze11.txt: line 31, char 1, byte offset 1: invalid UTF-8 code
> ./data/mazes/maze10.txt: line 31, char 1, byte offset 1: invalid UTF-8 code
> ./data/mazes/maze09.txt: line 31, char 1, byte offset 1: invalid UTF-8 code
> ./data/mazes/maze02.txt: line 31, char 1, byte offset 1: invalid UTF-8 code
> ./data/mazes/maze06.txt: line 31, char 1, byte offset 1: invalid UTF-8 code
> ./data/mazes/maze07.txt: line 31, char 1, byte offset 1: invalid UTF-8 code
> ./data/mazes/maze01.txt: line 31, char 1, byte offset 19: invalid UTF-8 code

OK this is because they are encoded as latin1 (only the maze
description line in fact is non-ASCII). Currently font rendering is
done assuming strings are latin1. It the program is localized in the
future (no plans for that now) it could be changed, but it's
non-trivial since it must also work for Windows (as the maze format is
common). Is it a critial problem?

Bye,
Raphael


Reply to: