Hi, On 05/25/2016 11:23 PM, James Cowgill wrote: > On Wed, 2016-05-25 at 21:10 +0200, Muri Nicanor wrote: >> I am looking for a sponsor for my package "usbguard" > > This looks like quite an interesting package, so here's a review. wow, thanks for the detailed review, i really appreciate it, i've learned a lot! > You do not own the wnpp bug for this package. You need to retitle the > bug from an RFP to ITP and set yourself as the owner. Do this before > trying to fix anything else. done > Since libusbguard.so is in a public libs directory, you must put it in > a separate package (probably called libusbguard0). You should then put > all the development files in libusbguard-dev. done, now there are usbguard, libusbguard0 and libusbguard-dev. usbguard depends on libusbguard0. > Please run wrap-and-sort so wrap the Build-Depends field in the control > file. done > You don't need the -2 changelog entry since your -1 version was never > uploaded. oke, i deleted the old packages and reuploaded to mentors with -1 > You add a group "usbguard" in postinst but didn't remove it in postrm. > You should probably do that during the purge step. fixed > The other things in postrm seem incorrect. Why do you need to remove > the service file manually? fixed. but, when uninstalling the package, a /etc/systemd/system/usbguard.service link to /dev/null is created. so when reinstalling the package one has to unmask that service. i didn't find any other way than that part of usbguard.postrm > "usbguard" must depend on adduser to use addgroup in your postinst. fixed > The *.install files should use a wildcard (*) instead of including the > multiarch directory manually. fixed > In rules, --with-bundled-spdlog=no doesn't seem to work. yes, i removed that now. i tried several times and it uses system spdlog whatever the configure switch says. and i'm removing the bundled spdlog before anything else, to be sure. > Enable parallel building (dh --parallel) if it works. done > You build-depend on dh-autoreconf, but don't actually run it. Use > something like "dh --with=autoreconf,systemd". done > copyright: > Upstream code is GPL-2+ (not GPL-2) > The license identifier for the Boost License is "BSL-1.0" > The license identifier for your "MIT-License" is "Expat" > https://spdx.org/licenses/ > Authors isn't a valid field name. You can use Comment or > Upstream-Contact instead. fixed > The default config doesn't allow the root user to use usbguard. This > doesn't offer ant additional security, but does add inconvenience. i added the group wheel to the list of groups allowed to control usbguard. > usbguard.service contains: > WantedBy=base.target > but base.target doesn't exist on my system. fixed to basic.target > The usbguard-rules.conf manpage uses "usbguard-daemon.conf" in the NAME > section (and other places) which is obviously a typo. fixed > Please submit the patch you added upstream when you get the chance. done > Finally, although you've fixed all the lintian warnings, please try and > fix some of the info tags. > > I: usbguard source: duplicate-short-description usbguard usbguard-dev > I: usbguard source: debian-watch-file-is-missing > I: usbguard: hardening-no-pie usr/bin/usbguard > I: usbguard: hardening-no-bindnow usr/bin/usbguard > I: usbguard: spelling-error-in-binary usr/lib/x86_64-linux-gnu/libusbguard.so.0.0.0 Uknown Unknown > I: usbguard: hardening-no-bindnow usr/lib/x86_64-linux-gnu/libusbguard.so.0.0.0 > I: usbguard: hardening-no-pie usr/sbin/usbguard-daemon > I: usbguard: hardening-no-bindnow usr/sbin/usbguard-daemon > I: usbguard: spelling-error-in-manpage usr/share/man/man5/usbguard-rules.conf.5.gz formated formatted > I: usbguard: no-symbols-control-file usr/lib/x86_64-linux-gnu/libusbguard.so.0.0.0 > I: usbguard: systemd-service-file-missing-documentation-key lib/systemd/system/usbguard.service all fixed ;) > Hopefully I've covered everything! thanks again, such thorough reviews really help to get an understanding for the packaging process! cheers, -- muri
Attachment:
signature.asc
Description: OpenPGP digital signature