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

Jan Cholasta jcholast at redhat.com
Tue Nov 13 08:40:54 UTC 2012


On 12.11.2012 15:19, Jakub Hrozek wrote:
> 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.

Updated patch attached.

Also created a new patch with fixes for errors reported by rpmlint.

Honza

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Use-systemd-by-default-on-Fedora-16.patch
Type: text/x-patch
Size: 3558 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20121113/eacf1054/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Fix-errors-reported-by-rpmlint.patch
Type: text/x-patch
Size: 8006 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20121113/eacf1054/attachment-0001.bin>


More information about the sssd-devel mailing list