[Bug 517183] Review Request: mipv6-daemon - IPv6 Mobility Daemon

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=517183





--- Comment #2 from Jarod Wilson <jwilson at redhat.com>  2009-08-13 16:37:44 EDT ---
Okay, so a few issues to work out...

1) without solid justification, the initscript should not be enabled by
default, it should be up to the user to enable it.

2) in one place, the initscript says 'config: /etc/mip6d.conf', then later you
test for /etc/sysconfig/mip6, then you source /etc/sysconfig/mip6d. And the
package has no config file whatsoever mentioned anywhere. :)

To resolve the enabled-by-default and three different names for the config, I'd
suggest:

--- mip6d.init.orig 2009-08-13 16:09:39.219807916 -0400
+++ mip6d.init 2009-08-13 16:22:46.888931134 -0400
@@ -2,11 +2,11 @@
 #
 # mip6d  Start script for the Mobile IPv6 daemon
 #
-# chkconfig: 2345 55 25
+# chkconfig: - 55 25
 # description: The mobile IPv6 daemon allows nodes to remain \
 #  reachable while moving around in the IPv6 Internet.
 # processname: mip6d
-# config: /etc/mip6d.conf
+# config: /etc/sysconfig/mip6d
 #
 ### BEGIN INIT INFO
 # Provides: mipv6-daemon
@@ -14,7 +14,7 @@
 # Required-Stop: $local_fs $remote_fs $network
 # Should-Start: $syslog
 # Should-Stop: $network $syslog
-# Default-Start: 2 3 4 5
+# Default-Start:
 # Default-Stop: 0 1 6
 # Short-Description: Start and stop Mobile IPV6 daemon
 # Description: The mobile IPv6 daemon allows nodes to remain
@@ -24,7 +24,7 @@
 # Source function library.
 . /etc/rc.d/init.d/functions

-if [ -f /etc/sysconfig/mip6 ]; then
+if [ -f /etc/sysconfig/mip6d ]; then
  . /etc/sysconfig/mip6d
 fi



On to the spec file itself...

1) as previously mentioned, the config file isn't accounted for anywhere in the
spec, and it needs to be, complete with %config(noreplace) labelling.

2) missing a chkconfig --add %post call

3) BuildRequires: /etc/init.d looks bogus, just flex and bison should be
sufficient.

4) 'Requires: initscripts, chkconfig' should be sufficient for Requires.

5) Generally, you don't want to inclue INSTALL files in %doc, because they're
irrelevant for a packaged install. Also, traditionally, %doc is the first line
after %defattr in the %files section.

6) use %{_initrddir} instead of /etc/rc.d/init.d/

7) %attr shouldn't be necessary on the initscript, you already install -m755'd
it.

I'd suggest the following spec file changes:

--- mipv6-daemon-orig.spec 2009-08-13 11:36:05.068932552 -0400
+++ mipv6-daemon.spec 2009-08-13 16:26:20.352931018 -0400
@@ -11,9 +11,8 @@ Source1: mip6d.init
 Patch0:  mipv6-daemon-header-fix.patch
 BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)

-BuildRequires: flex bison /etc/init.d
-Requires: initscripts
-Requires(post): chkconfig >= 0.9, /sbin/service
+BuildRequires: flex bison
+Requires: initscripts, chkconfig

 %description
 The mobile IPv6 daemon allows nodes to remain
@@ -31,8 +30,10 @@ make %{?_smp_mflags}
 rm -rf $RPM_BUILD_ROOT
 make install DESTDIR=$RPM_BUILD_ROOT

-install -d $RPM_BUILD_ROOT/etc/rc.d/init.d
-install -m755 %{SOURCE1} $RPM_BUILD_ROOT/etc/rc.d/init.d/mip6d
+install -d $RPM_BUILD_ROOT%{_initrddir}
+install -m755 %{SOURCE1} $RPM_BUILD_ROOT%{_initrddir}/mip6d
+install -d $RPM_BUILD_ROOT%{_sysconfdir}/sysconfig
+touch $RPM_BUILD_ROOT%{_sysconfdir}/sysconfig/mip6d

 %clean
 rm -rf $RPM_BUILD_ROOT
@@ -44,6 +45,9 @@ then
  /sbin/chkconfig --del mip6d
 fi

+%post
+/sbin/chkconfig --add mip6d
+
 %postun
 if [ "$1" -ge "1" ]; then
  /sbin/service mip6d condrestart > /dev/null 2>&1 ||:
@@ -51,12 +55,13 @@ fi

 %files
 %defattr(-,root,root,-)
+%doc AUTHORS BUGS COPYING NEWS README README.IPsec THANKS extras
+%ghost %config(noreplace) %{_sysconfdir}/sysconfig/mip6d
+%{_initrddir}/mip6d
 %{_sbindir}/*
 %{_mandir}/man1/*
 %{_mandir}/man5/*
 %{_mandir}/man7/*
-%attr(0755,root,root) /etc/rc.d/init.d/mip6d
-%doc AUTHORS BUGS COPYING INSTALL INSTALL.kernel NEWS README README.IPsec
THANKS extras

 %changelog
 * Tue Aug 12 2009 Thomas Graf <tgraf at, redhat.com> 0.4-1


With these changes applied, the only noise I get from rpmlint is a complaint
about the initscript name not matching the package name, which we can ignore.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.