Thank you very much for your previous comments, Kilian. I have uploaded a new package:
http://mentors.debian.net/ <http://mentors.debian.net/package/ndpmon> package/ndpmon <http://mentors.debian.net/package/ndpmon> > While reading into your package I'm wondering about: > > 1.) ndpmon.init: > - DAEMON and INIT must be defined in /etc/default/ndpmon. I doubt > that's a good default. You should define fall-back defaults in the init > script too > > - you background start-stop-daemon during starting yet try to > evaluate > its return value. I doubt that's a good combination. Moreover you have a > added sleep without any useful value IMHO as the return code is pulled > before the sleep anyway. > > - during stop you unconditionally cat a PIDFILE without checking it > exists and run kill on the outcome. A better solution would be something > like PID=$(ps -C $DAEMON -o pid=) and check if that's not empty and kill > that (or check if that's equal to the PIDFILE and kill it then). > > - status should check PIDFILE and/or something like the ps -C > $DAEMON > and report based on that. > > I'd recommend you check the manpage of ps for further options you > may > see fit. > > - exit $? at the end is very likely to not match what you intended > to > use as exit status. Maybe you should > I'm still not sure about the INIT script. Before, the INIT script that was in the original source code did not support status or force-reload. So I basically copied the script and added those sections. Now, I tried to incorporate the suggestions above, which results in an INIT script that is different from the one in the source code, but I'm not sure of how good of a job I did. Any more suggestions would be appreciated. 2.) Patches: > - there is no need to modify Makefile to delete config.log and > config.status unless you want to report this back to upstream (which is > not indicated in the header). From a Debian POV debian/clean and/or > debian/rules will do (and should be used preferred) > > - I'm not sure install.patch is needed. You can as well use > debian/tmp > as DESTDIR and move files from there using dh_install I guess. Reporting > the fixes back upstream seem to make sense yet there again is no > indication this was done. > I removed the patch to the Makefile and added a debian/clean file. Also, the installation is now handled by the files debian/ndpmon.dirs and debian/install. > - spelling errors - again look useful, but please make sure they > don't > remain in Debian's archive alone but are included upstream. > I plan to send the spelling patches upstream. I was holding off until I had a better idea of the status of getting the program into debian, and all of the changes that were necessary so I could just submit things once. > 3.) debian/rules template header can be omited. > Done. > as autotools-dev are already in Build-Depends (which is good) they > should also be activated in debian/rules (using --with autotools_dev) > Done. I also added -with autoreconf. > 4.) Fetching http://standards.ieee.org/regauth/oui/oui_public.txt. > > OUCH! There is no internet access guaranteed during building a > package. That means this is quite likely to fail. > > Moreover: > Fetching http://www.cavebear.com/CaveBear/Ethernet.txt > Error fetching http://www.cavebear.com/CaveBear/Ethernet/Ethernet.txt: > 404 Not Found > > N.b. there is an attempt to make a shared package for that. See e.g. > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=481299 for details and > more bugs numbers. Please try to use that package (once avail) or an > offline copy in the meantime. > The purpose of make-manuf is to provide an up-to-date copy of manuf, which the program uses to match MAC addresses with manufacturers. If Internet access is not available during the build, I thought it would be better to create an up-to-date copy of manuf offline, and simply produce a patch to bring the copy in the original source code up-to-date, and make sure make-manuf doesn't run. > 5.) Rebuilding your package twice in a row produces a Debian patch: > dpkg-source: info: local changes stored in > ndpmon-1.4.0/debian/patches/debian-changes-1.4.0-1, the modified files > are: > ndpmon-1.4.0/Makefile > ndpmon-1.4.0/config_ndpmon.xml > ndpmon-1.4.0/ndpmon.sh > ndpmon-1.4.0/ndpmon_defs.h > ndpmon-1.4.0/neighbor_list.xml > ndpmon-1.4.0/plugins/countermeasures/Makefile > ndpmon-1.4.0/plugins/mac_resolv/Makefile > dpkg-source: info: building ndpmon in ndpmon_1.4.0-1.debian.tar.gz > I believe this problem is solved with the use of the debian/clean file. > 6.) You still ship source files in your binary package like: > ./usr/lib/ndpmon/plugins/mac_resolv/mac_resolv.c > ./usr/lib/ndpmon/plugins/mac_resolv/mac_resolv.h > ./usr/lib/ndpmon/plugins/mac_resolv/Makefile.in > ./usr/lib/ndpmon/plugins/countermeasures/icmp_lib_nd.c > ./usr/lib/ndpmon/plugins/countermeasures/countermeasures.c > ./usr/lib/ndpmon/plugins/countermeasures/countermeasures.h > ./usr/lib/ndpmon/plugins/countermeasures/icmp_lib_nd.h > ./usr/lib/ndpmon/plugins/countermeasures/icmp_lib.c > ./usr/lib/ndpmon/plugins/countermeasures/Makefile.in > ./usr/lib/ndpmon/plugins/countermeasures/icmp_lib.h > ./usr/lib/ndpmon/plugins/countermeasures/countermeasures_on_link.h > ./usr/lib/ndpmon/plugins/countermeasures/countermeasures_guard.h > This problem is solved with the use of the debian/install file. John R. Baskwill, jr...@psu.edu Systems Analyst, Information Technology Services Penn State Harrisburg W303 Olmsted Building 777 West Harrisburg Pike Middletown, PA 17057-4898 Phone: 717-948-6268 Fax: 717-948-6535
