Re: Please review my patch for sysklogd
Patrick Winnertz <patrick.winnertz@skolelinux.org> writes:
> Hello!
>
> I tried this weekend on the BSP in Jülich, to remove some of the
> blocking bugs for our RC Bug.
>
> So If you please can review my patch for sysklogd (and maybe improve it)
> I added a debconf question, which only appears with a debconf prio of
> "low": With this debconf question it is possible to preseed sysklogd to
> log to a remote server. (The debconf question will
> edit /etc/default/syslogd to specify some options.
I'll do some comments myself that I think would make it easy to
merge..., read bellow:
[...]
> diff -Nru /tmp/VrD1HUUixW/sysklogd-1.4.1/debian/config /tmp/PdZaIpjxOf/sysklogd-1.4.1/debian/config
> --- /tmp/VrD1HUUixW/sysklogd-1.4.1/debian/config 1970-01-01 00:00:00.000000000 +0000
> +++ /tmp/PdZaIpjxOf/sysklogd-1.4.1/debian/config 2006-09-17 16:15:44.000000000 +0000
> @@ -0,0 +1,9 @@
> +#!/bin/sh
> +
> +. /usr/share/debconf/confmodule
> +
> +##Fix for debian-edu-config:
I think you can justify it, on bug report, saying that your motivation
was to solve a Debian-EDU bug but I think the solution shouldn't cite
it on code.
> +#make sysklogd options preseedable
# ask about default sysklogd options
This question isn't only suitable for preseeding but also for advanced
users and LTSP users too.
[...]
> diff -Nru /tmp/VrD1HUUixW/sysklogd-1.4.1/debian/control /tmp/PdZaIpjxOf/sysklogd-1.4.1/debian/control
> --- /tmp/VrD1HUUixW/sysklogd-1.4.1/debian/control 2006-09-17 16:15:44.000000000 +0000
> +++ /tmp/PdZaIpjxOf/sysklogd-1.4.1/debian/control 2006-09-17 16:15:44.000000000 +0000
> @@ -7,7 +7,7 @@
> Package: sysklogd
> Architecture: any
> Section: admin
> -Depends: ${shlibs:Depends}, klogd | linux-kernel-log-daemon
> +Depends: ${shlibs:Depends}, klogd | linux-kernel-log-daemon, debconf ( >=0.5 ) | debconf-2.0
> Conflicts: syslogd
> Provides: syslogd, system-log-daemon
> Replaces: syslogd
This isn't the best way of doing that. Would be better to use
dh_installdebconf capability to provide the misc:Depends variable for
dpkg-gencontrol and that would also makes the new migrations for
debconf far easier to deal then with hardcoded defaults.
+Depends: ${shlibs:Depends}, ${misc:Depends}, klogd | linux-kernel-log-daemon
Should work.
> diff -Nru /tmp/VrD1HUUixW/sysklogd-1.4.1/debian/postinst /tmp/PdZaIpjxOf/sysklogd-1.4.1/debian/postinst
> --- /tmp/VrD1HUUixW/sysklogd-1.4.1/debian/postinst 2006-09-17 16:15:44.000000000 +0000
> +++ /tmp/PdZaIpjxOf/sysklogd-1.4.1/debian/postinst 2006-09-17 16:15:44.000000000 +0000
> @@ -2,6 +2,8 @@
>
> set -e
>
> +. /usr/share/debconf/confmodule
> +
> if [ ! -d /var/log/news ] \
> && grep -q /var/log/news/ /etc/syslog.conf \
> && grep -q ^news: /etc/passwd \
> @@ -74,6 +76,19 @@
> fi
> set -e
> fi
> + ##Fix for debian-edu-config:
> + #Make sysklogd options preseedable.
Same comments then config file, see above.
[...]
> diff -Nru /tmp/VrD1HUUixW/sysklogd-1.4.1/debian/rules /tmp/PdZaIpjxOf/sysklogd-1.4.1/debian/rules
> --- /tmp/VrD1HUUixW/sysklogd-1.4.1/debian/rules 2006-09-17 16:15:44.000000000 +0000
> +++ /tmp/PdZaIpjxOf/sysklogd-1.4.1/debian/rules 2006-09-17 16:15:44.000000000 +0000
> @@ -74,8 +74,8 @@
> chown -R root:root debian/tmp.sysklogd
> chmod -R g-ws debian/tmp.sysklogd
> $(installbin) -d debian/tmp.sysklogd/usr/share/doc/$(package)
> - $(installbin) debian/{preinst,postinst,postrm,prerm} debian/tmp.sysklogd/DEBIAN/
> - $(installdoc) debian/conffiles debian/tmp.sysklogd/DEBIAN/
> + $(installbin) debian/{preinst,postinst,postrm,prerm,config} debian/tmp.sysklogd/DEBIAN/
> + $(installdoc) debian/{conffiles,templates} debian/tmp.sysklogd/DEBIAN/
For patching propose this is OK but I would suggest to use
dh_installdebconf and the rest of debhelper script bug it's outside of
your patch scope. The general packaging refactoring would be another
bug report. Keep in mind that current rules, used by the package, has
bashism and would break with dash and others shells.
[...]
> diff -Nru /tmp/VrD1HUUixW/sysklogd-1.4.1/debian/templates /tmp/PdZaIpjxOf/sysklogd-1.4.1/debian/templates
> --- /tmp/VrD1HUUixW/sysklogd-1.4.1/debian/templates 1970-01-01 00:00:00.000000000 +0000
> +++ /tmp/PdZaIpjxOf/sysklogd-1.4.1/debian/templates 2006-09-17 16:15:44.000000000 +0000
> @@ -0,0 +1,8 @@
> +Template: sysklogd/sysklogd-default-options
> +Type: string
> +Description: Enter the options you want sysklogd to use
> + Enter only the options like "-r" or "-m 0" without anything else before
> + or behind.
> +Description-de.UTF-8: Geben Sie die Optionen an, die sysklogd benutzen soll
> + Geben Sie lediglich die Optionen an, ohne zum Beispiel den Programmnamen davor
> + oder dahinter. Ähnlich wie dieses Beispiel: -r
Use po-debconf for translation, please. Add it also in depends line
;-)
Hopefully you'll appreciate the commends but, as I said, this are my
personal POV and you might disagree.
TIA,
--
O T A V I O S A L V A D O R
---------------------------------------------
E-mail: otavio@debian.org UIN: 5906116
GNU/Linux User: 239058 GPG ID: 49A5F855
Home Page: http://www.freedom.ind.br/otavio
---------------------------------------------
"Microsoft gives you Windows ... Linux gives
you the whole house."
Reply to: