Hi,
On 26.10.2012 16:00, Pavel Březina wrote:
On 10/22/2012 03:46 PM, Jakub Hrozek wrote:
On Mon, Oct 22, 2012 at 02:00:29PM +0200, Pavel Březina wrote:
On 10/19/2012 12:43 PM, Jakub Hrozek wrote:
On Fri, Oct 19, 2012 at 10:19:25AM +0200, Pavel Březina wrote:
On 10/18/2012 03:24 PM, Simo Sorce wrote:
On Thu, 2012-10-18 at 09:46 +0200, Pavel Březina wrote:
https://fedorahosted.org/sssd/ticket/1437
Fedora F15 has been EOLed and will not get updates, why bother with
F15 ?
Also F15 systemd was quite immature, and at the time guidelines
mentioned you shouldn't change from sysv to systemd after GA.
Did you mean to put F16 there ?
Simo.
OK. New patch is attached.
Hi,
%if (0%{?enable_experimental} == 1)
@@ -57,6 +61,12 @@ Requires(post): initscripts chkconfig
Requires(preun): initscripts chkconfig
Requires(postun): initscripts chkconfig
+%if (0%{?use_systemd} == 1)
+Requires(post): systemd-units
+Requires(preun): systemd-units
+Requires(postun): systemd-units
+%endif
I think here ^^ should be an %else that would otherwise Require
initscripts. I don't think we need them in a pure systemd-enabled
package.
I'm no spec file expert, but everything is basically copy and paste
from our Fedora spec file so I assume it should be correct.
The Fedora spec file says:
Requires(post): systemd-units initscripts chkconfig
Right, I think the initscripts Requires is spurious,
I'm sending new patch that requires initscripts conditionally.
but we keep
chkconfig to migrate from sysv scripts (which Pavel just showed me is
forbidden by the Packaging Guidelines).
A few comments on the patch:
+Requires(post): chkconfig
+Requires(preun): chkconfig
+Requires(postun): chkconfig
Are you sure chkconfig is required in all of these? It is called only in
%triggerun, so Requires(post) should be enough AFAIK.
+%if (0%{?use_systemd} == 1)
+# Replace sysv init script with systemd unit file
+rm -f $RPM_BUILD_ROOT/%{_initrddir}/%{name}
+mkdir -p $RPM_BUILD_ROOT/%{_unitdir}/
+cp src/sysv/systemd/sssd.service $RPM_BUILD_ROOT/%{_unitdir}/
+%endif
I think this should be done by calling configure with the right options
instead:
%if (0%{?use_systemd} == 1)
%global with_initscript --with-initscript=systemd
--with-systemdunitdir=%{_unitdir}
%else
%global with_initscript --with-initscript=sysv
%endif
...
%configure \
...
%{with_initscript} \
...
+%preun
+if [ $1 = 0 ]; then
A nitpick: the test should read [ $1 -eq 0 ].
+%triggerun -- sssd < %{version}-%{release}
+if /sbin/chkconfig --level 3 sssd ; then
+ /bin/systemctl --no-reload enable sssd.service >/dev/null 2>&1 || :
+fi
+
+if /sbin/chkconfig --level 5 sssd ; then
+ /bin/systemctl --no-reload enable sssd.service >/dev/null 2>&1 || :
+fi
+
+/sbin/chkconfig --del sssd >/dev/null 2>&1 || :
I don't think this is right (both the sssd version and the body of the
scriptlet), read
<http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Packages_migrating_to_a_systemd_unit_file_from_a_SysV_initscript>.
+%postun
+/bin/systemctl daemon-reload >/dev/null 2>&1 || :
+if [ $1 -ge 1 ] ; then
+ # On upgrade, reload init system configuration if we changed unit files
+ /bin/systemctl daemon-reload >/dev/null 2>&1 || :
+ /bin/systemctl try-restart sssd.service >/dev/null 2>&1 || :
+fi
There is no reason to call systemctl daemon-reload again inside the if.
Honza
--
Jan Cholasta
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
