[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFS: dhcp-probe, another try to request with a lot of update



Hi Laurent,

[...]

I've started another review of your package. The following things jumped at me
immediately (just tried to build the package using dpkg-buildpackage):

=================================== dpkg-buildpackage snippet ==================
make: Circular distclean <- clean dependency dropped.
dh_testdir
dh_testroot
test ! -f config.cache || rm -f config.cache
test ! -f config.status || rm -f config.status
test ! -f config.log || rm -f config.log
=================================== dpkg-buildpackage snippet ==================

- Your debian/rules has a circular dependency distclean/clean, please fix this.
  Actually, clean alone should be all you need, distclean is usually what is
  provided by the source's Makefiles
- rm -f will do, no need to test first. Really.

Then, trying to build stuff again (that is, running dpkg-buildpackage twice in a
row) building immediately fails because some cleanup is not done properly, which
may be the result of the clean/distclean mess.

A further review of your debian/ files yields:
- Please cleanup your changelog. Run a spell checker, remove duplicate info (no
  need to have 4 lines to comment on the man-page fix), remove wrong info
  (buildconf), and 
- make sure you get consistent about the config directory -- names range from
  /etc/dhcp-probe to /etc/dhcp-probed to /etc/default/dhcp-probe (check also
  debian/dirs)
- Your init.d file feels fairly hacked, please take a look at, e.g., the init.d
  file of the arpwatch package (which also runs multiple daemons for multiple
  interfaces). Just looking at the code you use to kill daemons makes me shiver.
  start-stop-daemon will do. Really. Please don't keep all the stuff you
  commented out, either remove it or implement it. force-reload, however, must
  get implemented (see Debian Policy Sec. 9.3.2).
- Your postinst should not do the configuration file auto-build in all cases,
  rather make use of the case statement to only do it in specific cases (like
  configure, but not on abort-*).
- Your postrm has redundant (and strange!) calls to rm: You don't need to remove
  each and every file if you run rm -rf afterwards!
- debian/rules:
  * lines 11--18: These variables are never used, remove them
  * line 39: Remove that distclean stuff in here
  * line 44: No, don't remove config.log manually, instead do
    test ! -f Makefile || make distclean
    which will actually also fix some more problems, I guess.
  * lines 45, 46: What is that good for? dh_clean will do.
  * line 51: build depends on config.status already, no need to depends once
    again
  * line 56: Remove dead comment
  * lines 59,61: Only depend on install, you can't do build and install in
    parallel and install already depends on build
  * lines 62,63: dh_testdir is ok, no additional files required
  * line 69: I think defaults should be fine, why do you add 80 15; that
    contradicts "defaults"

You may have good reasons for all that stuff and have done that intentionally.
If that is the case, please comment on the specific parts.

Best,
Michael






Attachment: pgpZw991DORPW.pgp
Description: PGP signature


Reply to: