[SSSD] [PATCH] use systemd by default on fedora15+

Jakub Hrozek jhrozek at redhat.com
Mon Nov 12 14:19:04 UTC 2012


On Thu, Nov 08, 2012 at 01:17:28PM +0100, Jan Cholasta wrote:
> 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 agreed to take over this patch.



More information about the sssd-devel mailing list