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 -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
