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

Bug#765179: RFS: yubikey-neo-manager/0.2.2-1 [ITP] -- YubiKey NEO management graphical user interface



On Tue, Oct 14, 2014 at 8:50 AM, Harlan Lieberman-Berg wrote:
> On Mon, 2014-10-13 at 21:35 +0200, Simon Josefsson wrote:
>> Dear mentors,
>>
>> I am looking for a sponsor for our package "yubikey-neo-manager":
>
> Thank you for your help in packaging the yubikey neo manager for Debian!
> I've got a few things that might need fixing up for you to take a look
> at.

Good review Harlan.

There is one remaining blocker for this package entering Debian:

The libu2f-host0 package is not yet in Debian.

Re icons, larger non-XPM ones (including SVG) should be installed in
/usr/share/icons/hicolor/<width>x<height>/apps:

http://standards.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html#directory_layout

Some other things you might want to fix:

I like to wrap debian/*.menu files at one attribute per line to make
diffs more readable.

I like to run wrap-and-sort -sa to wrap the other debian/* files in
the same way for the same reason.

I think the Priority should be optional rather than extra.

I would recommend debian/compat 9 and debhelper 9.

The license for neoman/libloader.py seems to be BSD-3-clause not BSD-2-clause?

The multiarch handling in neoman/libloader.py is incorrect, it should
not hard-code the paths for amd64 and i386, Debian has other 64-bit
and 32-bit ports plus the non-Linux ports.

The neoman/libloader.py looks at /etc/ld.so.conf but that won't work
on multi-arch systems becase /etc/ld.so.conf just includes
/etc/ld.so.conf/*.conf and contains no dirs.

The manual page, desktop file and icon(s) should be installed by the
upstream build system. PNG might be a better choice for the icons
since it provides 8-bit transparency instead of 1-bit transparency.

These files look like they were generated from other files, I would
suggest removing them from the VCS and tarballs and creating them at
build time if possible:

resources/installer_bg.png
resources/neoman.icns
resources/neoman.ico
resources/neoman.xpm
debian/neoman.xpm
neoman/neoman.png

DEFAULT_KEY does not look like something that should be included?

Are the files listed in neoman/appletdb.json Free Software? Are they
required for operation of yubikey-neo-manager?

The upstream signing key uses SHA1 for the self-sig, it should be
re-signed with a SHA512 self-sig:

https://help.riseup.net/en/security/message-security/openpgp/best-practices#self-signatures-should-not-use-sha1

uscan doesn't like the upstream signing key unless I move it to
debian/upstream/signing-key.asc (see below).

The cowbuilder parts of debian/README.source do not look necessary,
lots of folks use sbuild and the cowbuilder docs cover what is
mentioned in debian/README.source.

os.system (in release.py) should be replaced by use of the subprocess
module (with shell=False).

Automated checks:

https://wiki.debian.org/HowToPackageForDebian#Check_points_for_any_package
https://anonscm.debian.org/cgit/collab-maint/check-all-the-things.git

$ lintian
W: yubikey-neo-manager: image-file-in-usr-lib
usr/lib/python2.7/dist-packages/neoman/icon-about.png
W: yubikey-neo-manager: image-file-in-usr-lib
usr/lib/python2.7/dist-packages/neoman/icon_installed.png
W: yubikey-neo-manager: image-file-in-usr-lib
usr/lib/python2.7/dist-packages/neoman/icon_not_installed.png
W: yubikey-neo-manager: image-file-in-usr-lib
usr/lib/python2.7/dist-packages/neoman/icon_some_installed.png
W: yubikey-neo-manager: image-file-in-usr-lib
usr/lib/python2.7/dist-packages/neoman/neoman.png
E: yubikey-neo-manager: menu-icon-too-big
usr/share/pixmaps/neoman.xpm: 128x128 > 32x32

$ uscan --download-current-version --verbose --destdir .
...
-- Verifying OpenPGP signature yubikey-neo-manager-0.2.2.tar.gz.pgp
for yubikey-neo-manager-0.2.2.tar.gz
gpgv: Signature made Fri 26 Sep 2014 19:52:37 AWST using RSA key ID 6FBA95E8
gpgv: [don't know]: invalid packet (ctb=2d)
gpgv: keydb_search failed: invalid packet
gpgv: Can't check signature: public key not found
uscan warning: OpenPGP signature did not verify.

$ codespell --quiet-level=3
./ChangeLog:758: persistant  ==> persistent

$ find -type f -iname '*.desktop' -exec desktop-file-validate {} \;
./resources/neoman.desktop: error: value "YubiKey;NEO;Manager" for
locale string list key "Keywords" in group "Desktop Entry" does not
have a semicolon (';') as trailing character
./debian/neoman.desktop: error: value "YubiKey;NEO;Manager" for locale
string list key "Keywords" in group "Desktop Entry" does not have a
semicolon (';') as trailing character

$ pep8 --ignore W191 .
./neoman/device_ccid.py:34:40: W601 .has_key() is deprecated, use 'in'
./neoman/device_ccid.py:103:34: E128 continuation line under-indented
for visual indent
./neoman/device_ccid.py:106:5: E265 block comment should start with '# '
./neoman/device_ccid.py:123:80: E501 line too long (80 > 79 characters)
./neoman/device_ccid.py:128:80: E501 line too long (82 > 79 characters)
./neoman/device_u2f.py:41:36: W601 .has_key() is deprecated, use 'in'
./neoman/device_u2f.py:84:9: E265 block comment should start with '# '
./neoman/device_u2f.py:122:17: E125 continuation line with same indent
as next logical line
./neoman/device_u2f.py:130:9: E265 block comment should start with '# '
./neoman/libloader.py:314:14: E241 multiple spaces after ':'
./neoman/libloader.py:315:14: E241 multiple spaces after ':'
./neoman/libloader.py:316:13: E241 multiple spaces after ':'
./neoman/ykpers.py:87:54: E127 continuation line over-indented for visual indent
./neoman/ykpers.py:88:54: E127 continuation line over-indented for visual indent
./neoman/model/applet.py:120:5: E303 too many blank lines (2)
./neoman/model/neo.py:69:9: E265 block comment should start with '# '
./neoman/model/neo.py:135:13: E265 block comment should start with '# '
./neoman/model/neo.py:148:5: E303 too many blank lines (2)
./neoman/model/neo.py:170:80: E501 line too long (80 > 79 characters)
./neoman/view/applet.py:91:9: E265 block comment should start with '# '
./neoman/view/neo.py:38:80: E501 line too long (88 > 79 characters)

$ pyflakes .
./neoman/u2fh.py:27: 'c_ubyte' imported but unused
./neoman/libloader.py:215: local variable 'ext_re' is assigned to but never used
./neoman/device_ccid.py:27: 'from neoman.ykneomgr import *' used;
unable to detect undefined names
./neoman/device_u2f.py:27: 'from neoman.u2fh import *' used; unable to
detect undefined names
./neoman/device_u2f.py:28: 'c_ubyte' imported but unused
./neoman/device_otp.py:27: 'from neoman.ykpers import *' used; unable
to detect undefined names

$ cme check dpkg
Warning in 'control source Build-Depends:0' value 'python-setuptools
(>= 0.6b3)': unnecessary versioned dependency: python-setuptools >=
0.6b3. Debian has squeeze -> 0.6.14-4; wheezy -> 0.6.24-1; jessie ->
5.5.1-1; sid -> 5.5.1-1;
Warning in 'control source Build-Depends:1' value 'python-all (>=
2.6.6-3)': unnecessary versioned dependency: python-all >= 2.6.6-3.
Debian has squeeze -> 2.6.6-3+squeeze7; wheezy -> 2.7.3-4+deb7u1;
jessie -> 2.7.8-1; sid -> 2.7.8-1; jessie -> 2.7.8-1+b1; sid ->
2.7.8-1+b1; jessie -> 2.7.8-1+b2; sid -> 2.7.8-1+b2;
Warning in 'control binary:"yubikey-neo-manager" Depends:3' value
'libykneomgr0 (>= 0.1.2)': unnecessary versioned dependency:
libykneomgr0 >= 0.1.2. Debian has jessie -> 0.1.6-1; sid -> 0.1.6-1;
Warning in 'control binary:"yubikey-neo-manager" Depends:4' value
'libu2f-host0 (>= 0.0)': package libu2f-host0 is unknown. Check for
typos if not a virtual package.
Element 'XB-Python-Version' of node 'control
binary:"yubikey-neo-manager"' is deprecated

$ grep -ri 'fixme' .
./neoman/libloader.py:            # FIXME / TODO return '.' and
os.path.dirname(__file__)

-- 
bye,
pabs

https://wiki.debian.org/PaulWise


Reply to: