I have uploaded a new version of the package to mentors incorporating
the changes mentioned below.
> There is one remaining blocker for this package entering Debian:
>
> The libu2f-host0 package is not yet in Debian.
I'd love for you two to put your review cycles into it:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=764262
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=764460
> 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
Thanks -- we are trying to sort this out, either we do something in the
Debian packaging or we do it upstream -- but the latter requires some
thinking since the package is cross-platform (Windows, Mac, GNU).
> 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.
Agreed, done.
> I like to run wrap-and-sort -sa to wrap the other debian/* files in
> the same way for the same reason.
Didn't know about that tool, thanks for pointer. Ran it without -s
though.
> I think the Priority should be optional rather than extra.
Done.
> I would recommend debian/compat 9 and debhelper 9.
Done.
> The license for neoman/libloader.py seems to be BSD-3-clause not
> BSD-2-clause?
Done.
> 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.
No idea how to resolve this -- I suppose libloader.py is some external
file we copie in, maybe there are Debian-ified versions of it. Dain?
Not sure why this package have to hardcode or read ld.so.conf at
all.
> 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.
They seem to be PNG's in 0.2.3. Dain, can you install them?
> 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
It is common to ship generated files in tarballs, to avoid forcing
users to have a lot of tools available. Agree with removing them from
git though, Dain?
> DEFAULT_KEY does not look like something that should be included?
Why not? Earlier NEOs had that key as the default (it is the common
Visa/Mastercard standard key), although modern NEOs have randomized
keys.
> Are the files listed in neoman/appletdb.json Free Software? Are they
> required for operation of yubikey-neo-manager?
ykneo-oath and ykneo-openpgp are free software, but they are not
required for operation and most people will not want or need them.
> 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
Leave this to Dain :-)
> uscan doesn't like the upstream signing key unless I move it to
> debian/upstream/signing-key.asc (see below).
Dain?
> 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.
Yeah, it is mostly an example and reminder for the people working with
the packaging.
> os.system (in release.py) should be replaced by use of the subprocess
> module (with shell=False).
Upstream issue -- Dain?
> Automated checks:
Nice, will take a further looks later.
/Simon
>
> 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__)
>
Attachment:
signature.asc
Description: PGP signature