Re: vavoom, updated version
Whoa! that is a review :)
On Wed, Feb 29, 2012 at 11:44:59AM +0800, Paul Wise wrote:
> Some issues you might want to look at for the next upload:
> If you contact upstream as a result of this review, please include
> these two links in your email:
i've contacted, in private, upstream about licensing, but they don't reply my emails.
i will cc d-d-g to see if i get a better response
> You might want to run wrap-and-sort -s to make diffs of debian/control
> and other files more readable.
i didn't know that tool, and many others you mention. thanks
> Your packaging isn't committed to the games team VCS repos:
this is my first public debian package, i didn't want to mess with VCS
at the same time.
i'm playing with VCS for my next pkg uhexen
> Please check if vavoom works with libpng1.5 as indicated by the PTS page:
> Please edit the debtags for vavoom:
will do both
> There are some build log issues:
> The package FTBFS on Hurd due to the use of PATH_MAX:
i was wrinting an email to d-d-g and d-mentors asking for advice.
i see 3 ways to fix it,
- disable vavoom on hurd
after all, most hurd installs are on VM, no much gain on running a
- define the macro PATH_MAX
easy, but is not really a fix
- replace the macro with a function, as is recomended by hurd folks
i'm not a C expert, so it might take a while and can be buggy
after some thinking, i choose the third option
> I don't think it is appropriate to recommend contrib/non-free deps,
> please move heretic-wad, game-data-packager to Suggests:
FTP master agrees with it. is not the first doom engine, and game i
guess, with non-free recommends. anyway non-free is not default so if
you are using defaults you won't get any of these stuff
> The patch headers are a bit weird, they include snippets from
> debian/changelog and duplicated DEP-3 headers. Please delete them all
> and write some DEP-3 headers from scratch:
> Please forward the patches, manual pages and desktop file upstream and
> add DEP-3 headers or comments indicating where they have been
> The upstream docs/vavoom.txt file includes installation instructions,
> which isn't useful for users of binary packages. Please ask upstream
> to split those out into README.install.
> You may want to make the copyright file machine-readable:
> Some of the upstream code contains the incorrect FSF address. I would
> suggest that upstream should point at the GNU website instead, as
> suggested by GNU:
> I note that upstream uses system() in one file. In this case the usage
> seems safe since it is only run during the build process, but you
> should ensure that future uses of this function are safe or switch to
> the exec*() family of functions.
> GCC warnings, these are repeated many times:
> utils/vlaunch/vlaunch.xpm:292:1: warning: deprecated conversion from
> string constant to 'char*' [-Wwrite-strings]
> source/gl_model.cpp:317:70: warning: cast to pointer from integer of
> different size [-Wint-to-pointer-cast]
> lintian complaints:
> W: vavoom source: out-of-date-standards-version 3.9.2 (current is 3.9.3)
that went out btw the upload and the build, i miss it
> W: vavoom: binary-without-manpage usr/games/vlaunch
> cppcheck warnings:
> [libs/core/str.h:355]: (error) Uninitialized variable: Len
> [libs/core/str.h:356]: (error) Uninitialized variable: Len
> [libs/core/stream.h:134]: (error) Uninitialized variable: Val
> [source/r_main.cpp:376]: (error) Throwing exception in destructor
> [source/r_main.cpp:392]: (error) Throwing exception in destructor
> Probably more cppcheck issues exist, I killed cppcheck at this point
> because it uses so much CPU.
> dpkg-shlibdeps warnings:
> The wxWidgets ones are probably the fault of that, not vavoom. The
> other ones should probably be fixed in the upstream build system
> dpkg-shlibdeps: warning: dependency on libwx_gtk2u_richtext-2.8.so.0
> could be avoided if "debian/vavoom/usr/games/vlaunch" were not
> uselessly linked against it (they use none of its symbols).
> There are some duplicate files. Not sure if unduplicating them is
> possible or useful to do upstream. The Debian manual pages should not
> be duplicate, instead they should use symlinks or the .SO mechanism.
i will fix the duplicate manpages, and the missing.
i don't like the idea to mess that much with upstream source
thanks again for you deep review, i appreciate it
1AE0 322E B8F7 4717 BDEA BF1D 44BB 1BA7 9F6C 6333