Hi Hugo, thanks a lot for looking at this. I have made some changes to the package based on your feedback, pushed them to my git repository and uploaded the new version to mentors: https://mentors.debian.net/package/arpwatch
Some further questions and comments follow inline below. On Tue, 4 Apr 2017 12:29:09 +0200 Hugo Lefeuvre <h...@debian.org> wrote: > https://git.somlen.de/arpwatch.git/ returns 403 Forbidden :) I do not have a web based git viewer installed on my server, however git clone https://git.somlen.de/arpwatch.git should work. Remember to switch to the `staged-changes` branch. (I should probably create a page explaining that this is a git only url instead of displaying a 403…) > Quick review: > > * lintian reports > > P: arpwatch source: source-contains-data-from-ieee-data-oui-db > ethercodes.dat > > but it looks like you already fixed it. If this warning is not > relevant anymore please override it. Well, I did not really fix it. ethercodes.dat is still part of the source package, it's just no longer put into the binary package. But, as it is small and it does not violate the DFSG, Christian Seiler suggested on debian-mentors not to repack [1]. I have not previously overridden that tag because I was not sure if it is best practice to override lintian pediantic tags at all (only quite few packages seem to do it) as they also don't show up on the tracker.debian.org page. Anyways, I've now overridden the following two pedantic tags, together with a justification as comment: * source-contains-data-from-ieee-data-oui-db * debian-watch-may-check-gpg-signature > * There's no copyright entry for you Fixed. > Nitpicking: > > in debian/changelog: why "remove dmassagevendor" ? This changelog > entry could be more verbose. Right, I have a longer justification in the git history; I have expanded on the changelog entry. > $ cme check dpkg > [...] > Warning in 'dirs:0' value 'usr/sbin': Make sure that this directory is > actually needed. See > L<http://www.debian.org/doc/manuals/maint-guide/dother.en.html#dirs> for > details > Warning in 'dirs:1' value 'var/lib/arpwatch': Make sure that this > directory is actually needed. See > L<http://www.debian.org/doc/manuals/maint-guide/dother.en.html#dirs> > for details If I remove `usr/sbin` from dirs, buildpackage fails complaining that the directory does not exist (so something in the build system is slightly broken). `var/lib/arpwatch` is an empty directory created by the package that is populated with ethercodes.db during postinst (and then with interface specific database files once arpwatch is started). Should I create the directory during postinst instead? Creating the empty directory as part of the package seems nicer since dpkg will take care of the `rmdir` and warn the user if the directory is not empty on uninstall. > [...] > Warning in 'control source Vcs-Git' value > 'git://anonscm.debian.org/collab-maint/arpwatch.git': An unencrypted > transport protocol is used for this URI. It is recommended to use a > secure transport such as HTTPS for anonymous read-only access. > > Warning in 'control source Vcs-Git' value > 'git://anonscm.debian.org/collab-maint/arpwatch.git': URL is not the > canonical one for repositories hosted on Alioth. I had that on my TODO list but decided to wait and see how I get the package sponsored before changing the Url. I've now pointed it to what I expect to become its new home in case you are willing to sponsor the package: https://anonscm.debian.org/cgit/pkg-security/arpwatch.git https://anonscm.debian.org/git/pkg-security/arpwatch.git I've also adjusted debian/control to what I think it should be for team maintenance (maintainer is pkg-security-team, added myself as uploader). > Warning in 'control binary:arpwatch Pre-Depends:0' value 'dpkg (>= 1.16.1)': > unnecessary versioned dependency: dpkg (>= 1.16.1). > Debian has oldstable -> 1.16.18; stable -> 1.17.27; unstable -> 1.18.23; > testing -> 1.18.23; Ok, I removed the pre-dependenciy. In order to setup the file based trigger I followed man deb-triggers(5) from dpkg-dev version 1.18.23 (most recent version in unstable) which states: > The “-noawait” variants are only supported since dpkg 1.16.1, and will > lead to errors if used with an older dpkg. It is thus recommended to > add a “Pre-Depends: dpkg (>= 1.16.1)” to any package that wish to use > those directives. Should I file a bug against the dpkg-dev package that this recommendation should be dropped? > Warning in 'copyright Format' value > 'http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/': > Format uses insecure http protocol instead of https Fixed. > $ codespell * > aclocal.m4:784: seperate ==> separate > aclocal.m4:787: independantly ==> independently > aclocal.m4:788: dependancies ==> dependencies > arp2ethers:8: occurance ==> occurrence > config.sub:1161: nto ==> not | disable due to \n > debian/changelog:129: wont ==> won't, wont > dns.c:140: cannonical ==> canonical > WARNING: Decoding file ethercodes.dat > WARNING: using encoding=utf-8 failed. > WARNING: Trying next encoding: iso-8859-1 > ethercodes.dat:785: Intruments ==> Instruments > ethercodes.dat:838: Aircaft ==> Aircraft > ethercodes.dat:1180: Engeneering ==> Engineering > ethercodes.dat:2083: Internation ==> International > ethercodes.dat:7447: MANAGMENT ==> MANAGEMENT Except for debian/changelog all of these refer to files from upstream. From these files, the only thing that will end up being in the binary package is the typo in a comment of the arp2ethers /bin/sh script. I you prefer that I create a patch to fix that I will do so (I personally wouldn't bother). The spelling mistake in debian/changelog is from an old entry. Should I rewrite the changelog.Debian history to fix that spelling mistake? Thanks also for including the commands you used to find problems with the package, that's really helpful. Regards Lukas [1] https://lists.debian.org/debian-mentors/2017/03/msg00244.html
pgpwjS1NJqCHx.pgp
Description: OpenPGP digital signature
