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

Bug#643595: snes9x review (current version from Debian Mentors)



Hello Michael,

here my review of your snes9x package that is currently on Debian Mentors.

Going through the files in the debian directory and doing a lintian check with "lintian -i -I -E --pedantic snes9x_1.53-3_amd64.changes" on the freshly built package without modifications:

==================================================================

debian/changelog:

* Please change the following line:

"  * debian/watch: Fixing watch file."

to

"  * debian/watch: Fix watch file."

* There are trailing spaces at the end of the following lines:

181, 215, 225, 246, 254, 260, 337.

Please remove them. You can edit the changelog in emacs to spot these spaces easily.

* Please fix the following spelling mistake:

"  * debian/rules: Fix a bug that emptys snes9x-x target."

It's spelled "empties", not "emptys".

* Please fix the capitalization of the "Closes" directive for the entries where it is written lower case. It should always be "(Closes: #nnnnnn), not "(closes: #nnnnnn)". (Yes, I am nitpicky, but it's a matter of consistency and you also want your package to pass NEW quickly ;)).

* Replace "New Upstream" or "New upstream version" always with "New upstream release." which is the canonical form for that entry.

If you see other obvious spelling, grammar mistakes, please correct them!

==================================================================

debian/compat:

Please bump the version of debhelper to version 9 (also adjust dependency in debian/control).

This should also enable automatic hardening and fix the lintian warnings mentioned further below.

==================================================================

debian/control:

* The standards version was bumped to 3.9.4, however this change was not documented in debian/changelog, please add that. Also, please make sure to include all changes that you are making now to the package (e.g. fixing the spelling in the changelog) to the Debian changelog :).

* There are no entries for Vcs-Git and Vcs-Browser. Is your source for the Debian packaging not somewhere available online?

* Capitalization: "Workstation" => "workstation"

==================================================================

debian/copyright:

* This should be updated to the new machine-readable copyright format, see:

> http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/

==================================================================

debian/patches/series and debian/patches/fix-typo.patch:

* Remove extra empty lines at the end of the files.

==================================================================

debian/rules:

* The rules file can probably further simplified taking advantage of the capabilities of recent versions of debhelper (version 9).

Many packages just require the following two lines:

%:
	dh $@

But snes9x might need some extra overrides. Please refer to the debhelper manual for more. You may also ask for help in the #debian-mentors IRC channel on OFTC or other resources.

==================================================================

debian/snes9x-x.README.debian:

The instructions about adding users to the groups "audio" (users are in these groups when logging in locally anyway) and "kmem" (is this really still required, please check!) are probably obsolete. The former is definitely redundant, but I am not sure about the latter. Please check and if it's no longer required, please remove these lines.

What about this statement, is this still up-to-date?

"The new Upstream does not support any of these Versions anymore. At least the OpenGL version is expected to come back soon."

If yes, please fix the capitalization of "Upstream" and "Versions."

Or you may just drop/rewrite this file altogether.

==================================================================

There are lintian problems that should be addressed:

glaubitz@z6:..debian/snes9x-1.53> lintian -i -E -I --pedantic /var/cache/pbuilder/result//snes9x_1.53-3_amd64.changes
W: snes9x-gtk: hardening-no-relro usr/games/snes9x-gtk
N:
N:    This package provides an ELF binary that lacks the "read-only
N:    relocation" link flag. This package was likely not built with the
N: default Debian compiler flags defined by dpkg-buildflags. If built using
N:    dpkg-buildflags directly, be sure to import LDFLAGS.
N:
N:    Refer to http://wiki.debian.org/Hardening for details.
N:
N:    Severity: normal, Certainty: certain
N:
N:    Check: binaries, Type: binary, udeb
N:
W: snes9x-gtk: hardening-no-fortify-functions usr/games/snes9x-gtk
N:
N: This package provides an ELF binary that lacks the use of fortified libc N: functions. Either there are no potentially unfortified functions called N: by any routines, all unfortified calls have already been fully validated
N:    at compile-time, or the package was not built with the default Debian
N:    compiler flags defined by dpkg-buildflags. If built using
N:    dpkg-buildflags directly, be sure to import CPPFLAGS.
N:
N:    NB: Due to false-positives, Lintian ignores some unprotected functions
N:    (e.g. memcpy).
N:
N:    Refer to http://wiki.debian.org/Hardening and
N:    http://bugs.debian.org/673112 for details.
N:
N:    Severity: normal, Certainty: possible
N:
N:    Check: binaries, Type: binary, udeb
N:
W: snes9x-x: hardening-no-relro usr/games/snes9x
W: snes9x-x: hardening-no-fortify-functions usr/games/snes9x
I: snes9x-x: possible-documentation-but-no-doc-base-registration
N:
N: The package ships a .html or .pdf file under /usr/share/doc/, which are
N:    usually documentation, but it does not register anything in doc-base.
N:    (Files under an examples directory are excluded.)
N:
N: Refer to Debian Policy Manual section 9.10 (Registering Documents using
N:    doc-base) for details.
N:
N:    Severity: wishlist, Certainty: possible
N:
N:    Check: menus, Type: binary
N:
glaubitz@z6:..debian/snes9x-1.53>

Bumping the debhelper version from 7 to 9 should fix all the issues with the hardening of the binary, you can ignore the warning about the doc-base registration so far. It's not that important in my opinion.

==================================================================

Please fix the above issues and re-upload the fixed package to Mentors for another review. Please do not bump the Debian revision for the fixed package, just overwrite the current version on Debian Mentors.

If you need help fixing the above issues, please let me know or ask the guys in #debian-mentors on the OFTC IRC network. They are very helpful and with their help you should be able to fix the issues in no time.

==================================================================

Cheers,

Adrian

--
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913


Reply to: