Hi Vincent, Thanks for taking the time to do another thorough review of another of my packaging efforts! :) On 2016-11-19.19:47, Vincent Bernat wrote: > ❦ 20 novembre 2016 01:43 +1100, Scott Leggett <scott@sl.id.au> : > > > * Rely on automatic -dbgsym package rather than deprecated -dbg. > > You need to make a migration for people having a -dbg. This is done with > --dbgsym-migration argument of dh_strip. > Done. > > * Add quagga user to quaggavty group in preinst. > > I suppose this was in the original packaging, but this is better to use > adduser/addgroup unconditionally. Both commands will check if the > user/group already exists with the appropriate parameters. Notably, if > the user/group is not a system one, it will trigger an error. > Thanks, that simplifies the script nicely. Done. > > * Use systemd .service files rather than init.d scripts (Closes: #678946, > > #805840, #839819, #412483). > > I am quite OK with that. You don't have time to sort out those > problems. However, this is somewhat controversial. > I'm open to accepting patches / co-maintainership from anyone who wishes to test and use traditional init scripts with quagga. Given the longevity of bugs like #678946, I'm not optimistic of such patches materialising. I personally cannot test such init scripts since all my systems now use systemd, and I can't in good faith include code that I can't test and which has known bugs. If there is another approach you think I could take here, please advise... > > * Split quagga package out into multiple packages (Closes: #705306). > > People upgrading will suddenly have all the daemons missing. You could > introduce a "base" or "minimal" package that wouldn't depend on the > daemons (and the daemons would depend on them). The "quagga" package > would still depend on all the daemons. However, the NEWS entry could be > sufficient too. > That is a much better solution, thank you for the suggestion! I have updated the package as you suggest: there is now a "quagga" metapackage, and a "quagga-core", which all the routing daemons depend on. > > Also : > > - the -dev package should be libquagga-dev > Fixed. > - default/quagga could be called "quagga.default" and would be > installed automatically as /etc/default/quagga. > As this is now part of "quagga-core", I still have to install this file manually as far as I can tell. Thanks for the tip on the debhelper way to install it though. > - the service files seem to use wrong paths. In Debian, chown is in > /bin, not /usr/bin. > This is interesting.. I ran `which chmod` on my unstable test box, and got "/usr/bin/chmod". /bin is a symlink to /usr/bin in unstable now, and /usr/bin is before /bin in $PATH. Either way, I've changed the paths in the .service files to /bin/*. > - if you think that maintaining all those lintian overrides is too much > effort, I think that people usually don't override "informational" > messages. > Okay, I've removed the trivial overrides. > - in maintainers script, I would drop the non-standard debug stuff at > the top of each of them > Done. > - in postinst, changing permissions in /etc is frowned upon since it > could undo what a user has done. You should use dpkg-statoverride > which gives the user a chance to implement its own policy. Thanks for introducing me to another interesting dpkg-* tool. Fixed. The new package is available again at https://mentors.debian.net/package/quagga -- Regards, Scott.
Attachment:
signature.asc
Description: Digital signature