RFS: arpwatch [ITA]
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 <hle at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-security-team/attachments/20170404/1c363de6/attachment.sig>
Reply to: