Re: Please review packaging of clanlib2
On Tue, Mar 6, 2012 at 3:44 AM, Andreas Moog wrote:
> Mentors doesn't accept my package for some reason, so I put it on my own
> webspace:
>
> dget -x http://www.warperbbs.de/packages/clanlib2_2.3.5-1.dsc
A review is below.
> Good question. I thought due to the non-trivial changes to the API/ABI
> it would be better to use a new source package name. But seeing that
> there is only 1 rdepends (trophy) to the old clanlib, it makes more
> sense to reuse the existing source package name. I'll write to the
> current maintainers/uploaders.
My thoughts exactly, thanks.
... and the review:
If you contact upstream, please mention these URLs:
http://wiki.debian.org/UpstreamGuide
http://www.freedesktop.org/wiki/Games/Upstream
The Vcs-* fields point at collab-maint, but your earlier mail pointed
at pkg-games. The collab-maint URLs give a 404 message.
libclanlib-2.3-1 contains the library, which will only be pulled in
automatically. I suggest that it should contain the least detailed
long description and the current description should move to the -dev
package since that and the doc package will be the ones that are
manually installed.
The -dev package should suggest the -doc package and the -doc package
should probably recommend the -dev package?
Do you need Multi-Arch: same fields on some of the packages?
The watch file doesn't need that blank line.
If you are taking over the clanlib source package name,
debian/changelog should contain the old changelog entries of clanlib
1. debian/control needs adjusting too.
Looking at the output of `cat debian/*symbols | c++filt`, I wonder if
it is a good idea to add symbols files and am reminded of these posts:
http://www.eyrie.org/~eagle/journal/2012-01/007.html
http://www.eyrie.org/~eagle/journal/2012-02/001.html
dpkg-gensymbols -c0 is not a good idea.
Upstream appears to be enabling SSE2 where it is available on the
build machine. On i386 this means clanlib2 based games will crash on
machines without SSE2. For Linux distros, clanlib should build with
the baseline CPU instructions supported by the port.
There are a lot of warnings from dpkg-shlibdeps.
The debian/copyright file misses some info for these, please recheck
that debian/copyright is complete:
Setup/CodeBlocks/iniparser.c
Setup/CodeBlocks/iniparser.h
There are a lot of duplicate files in the source package.
pngcheck error:
Tests/Display/Collision/images/outline_from_bitmap.png additional
data after IEND chunk
ERROR: Tests/Display/Collision/images/outline_from_bitmap.png
cppcheck warnings:
[Examples/Game/Pacman/sources/world.h:56]: (error) Memory leak: World::map
[Examples/Sound/Sound/Sources/gui.cpp:273]: (error) Same iterator is
used with both spin_slider_map_integer and spin_slider_map_float
[Sources/Core/Math/rect_packer_impl.h:73]: (error) Memory leak:
CL_RectPacker_Impl::active_root_node
[Sources/Display/2D/texture_group_impl.h:90]: (error) Memory leak:
CL_TextureGroup_Impl::active_root
[Sources/Display/Font/FontEngine/font_engine_freetype.cpp:70]: (error)
Throwing exception in destructor
[Sources/Sound/Win32/soundoutput_directsound.cpp:284]: (error)
Possible null pointer dereference: ptr1
[Sources/Sqlite/sqlite_command_provider.cpp:72]: (error) Throwing
exception in destructor
[Tests/Core/CSSTokenize/giflib/getarg.c:221]: (error) Invalid number
of character ({) when these macros are defined: ''.
[Tests/Core/CSSTokenize/giflib/getarg.c:221]: (error) Invalid number
of character ({) when these macros are defined: 'HAVE_CONFIG_H'.
[Tests/Core/CSSTokenize/giflib/dgif_lib.c:587]: (error) Possible null
pointer dereference: Private - otherwise it is redundant to check if
Private is null at line 599
[Tests/Core/CSSTokenize/giflib/dgif_lib.c:82]: (error) Resource leak: FileHandle
[Tests/Core/CSSTokenize/giflib/getarg.c:221]: (error) Invalid number
of character ({) when these macros are defined: '__MSDOS__'.
[Tests/Core/CSSTokenize/giflib/qprintf.c:63]: (error) Invalid number
of character ({) when these macros are defined: ''.
[Tests/Core/CSSTokenize/giflib/qprintf.c:63]: (error) Invalid number
of character ({) when these macros are defined: 'HAVE_CONFIG_H'.
[Tests/Core/CSSTokenize/giflib/qprintf.c:63]: (error) Invalid number
of character ({) when these macros are defined: 'SYSV'.
[Tests/Core/CSSTokenize/giflib/qprintf.c:63]: (error) Invalid number
of character ({) when these macros are defined: '_GBA_NO_FILEIO'.
[Tests/Core/CSSTokenize/giflib/qprintf.c:63]: (error) Invalid number
of character ({) when these macros are defined: '_GBA_OPTMEM'.
[Tests/Core/CSSTokenize/giflib/qprintf.c:63]: (error) Invalid number
of character ({) when these macros are defined: '_WIN32'.
[Tests/Core/CSSTokenize/giflib/qprintf.c:63]: (error) Invalid number
of character ({) when these macros are defined: '__MSDOS__'.
[Tests/Core/CSSTokenize/giflib/qprintf.c:63]: (error) Invalid number
of character ({) when these macros are defined: '__cplusplus'.
GCC warnings:
Crypto/big_int.cpp: In member function 'cl_bigint_size
CL_BigInt::trailing_zeros() const':
Crypto/big_int.cpp:1714:10: warning: right shift count >= width of
type [enabled by default]
Text/console.cpp: In static member function 'static void
CL_Console::write(const CL_StringRef&)':
Text/console.cpp:55:40: warning: ignoring return value of 'ssize_t
write(int, const void*, size_t)', declared with attribute
warn_unused_result [-Wunused-result]
Text/console_logger.cpp: In member function 'virtual void
CL_ConsoleLogger::log(const CL_StringRef&, const CL_StringRef&)':
Text/console_logger.cpp:111:46: warning: ignoring return value of
'ssize_t write(int, const void*, size_t)', declared with attribute
warn_unused_result [-Wunused-result]
Unix/soundoutput_oss.cpp: In member function 'virtual void
CL_SoundOutput_OSS::write_fragment(float*)':
Unix/soundoutput_oss.cpp:171:42: warning: ignoring return value of
'ssize_t write(int, const void*, size_t)', declared with attribute
warn_unused_result [-Wunused-result]
swr_display_window_provider.cpp:42:7: warning: extra tokens at end of
#else directive [enabled by default]
setupmikmod.cpp:101:1: warning: deprecated conversion from string
constant to 'CHAR* {aka char*}' [-Wwrite-strings]
setupmikmod.cpp:101:1: warning: deprecated conversion from string
constant to 'CHAR* {aka char*}' [-Wwrite-strings]
setupmikmod.cpp:101:1: warning: deprecated conversion from string
constant to 'CHAR* {aka char*}' [-Wwrite-strings]
setupmikmod.cpp: In static member function 'static void
CL_SetupMikMod::init(bool)':
setupmikmod.cpp:118:16: warning: deprecated conversion from string
constant to 'CHAR* {aka char*}' [-Wwrite-strings]
lintian complaints:
P: libclanlib-2.3-dbg: no-upstream-changelog
I: libclanlib-2.3-dbg: extended-description-is-probably-too-short
P: libclanlib-2.3-dev: no-upstream-changelog
I: libclanlib-2.3-dev: extended-description-is-probably-too-short
P: libclanlib-2.3-1: no-upstream-changelog
P: libclanlib-2.3-doc: no-upstream-changelog
I: libclanlib-2.3-doc: extended-description-is-probably-too-short
X: libclanlib-2.3-doc: duplicate-files
And vast numbers of more duplicate-files warnings.
--
bye,
pabs
http://wiki.debian.org/PaulWise
Reply to: