On (24/06/16 13:51), Lukas Slebodnik wrote:
> On (24/06/16 13:49), Lukas Slebodnik wrote:
>> On (24/06/16 10:29), Michal Židek wrote:
>>> On 06/24/2016 09:57 AM, Jakub Hrozek wrote:
>>>> On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:
>>>>> On 06/23/2016 09:40 PM, Jakub Hrozek wrote:
>>>>>> On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote:
>>>>>>> On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik
wrote:
>>>>>>>> On (23/06/16 18:59), Michal Židek wrote:
>>>>>>>>> On 06/23/2016 06:52 PM, Lukas Slebodnik wrote:
>>>>>>>>>> On (23/06/16 18:24), Michal Židek wrote:
>>>>>>>>>>> On 06/23/2016 05:50 PM, Michal Židek wrote:
>>>>>>>>>>>> Lukas,
>>>>>>>>>>>>
>>>>>>>>>>>> did you have some local patches related
to
>>>>>>>>>>>> this patch, that you did not send here?
>>>>>>>>>>>>
>>>>>>>>>>>> Because I can not apply the patch on
current
>>>>>>>>>>>> master and the changes in the context of
>>>>>>>>>>>> your patch seem like you did something
with
>>>>>>>>>>>> default config file path?
>>>>>>>>>>>>
>>>>>>>>>>>> Slightly modified version that applies
on
>>>>>>>>>>>> current master is attached (the changes
>>>>>>>>>>>> are the same, but the context differs).
>>>>>>>>>>>>
>>>>>>>>>>>> Michal
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I added the missing access check for
>>>>>>>>>>> snippets. If the snippets does not
>>>>>>>>>>> pass the access check it is skipped
>>>>>>>>>>> and a loud debug message is logged.
>>>>>>>>>>>
>>>>>>>>>>> I attach it as separate patch for
>>>>>>>>>>> easier review, but it should be squashed
>>>>>>>>>>> before pushing. I also attach the first
>>>>>>>>>>> patch for convenience.
>>>>>>>>>>>
>>>>>>>>>>> Michal
>>>>>>>>>>
>>>>>>>>>>> From
6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001
>>>>>>>>>>> From: =?UTF-8?q?Michal=20=C5=BDidek?=
<mzidek(a)redhat.com>
>>>>>>>>>>> Date: Tue, 22 Mar 2016 14:09:34 +0100
>>>>>>>>>>> Subject: [PATCH] confdb: Make it possible to
use config snippets
>>>>>>>>>>>
>>>>>>>>>>> Resolves:
>>>>>>>>>>>
https://fedorahosted.org/sssd/ticket/2247
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Lukas Slebodnik
<lslebodn(a)redhat.com>
>>>>>>>>>>> ---
>>>>>>>> LGTM.
>>>>>>>>
>>>>>>>> It isn't ACK because I was involved in development
>>>>>>>> and I didn;t test last version but code is fine fore me.
>>>>>>>
>>>>>>> I just ran a quick test and the patch works for me and the
code reads OK
>>>>>>> as well. I will push after CI finishes.
>>>>>>>
>>>>>>> I also opened:
>>>>>>>
https://fedorahosted.org/sssd/ticket/3062
>>>>>>> because the patch is missing documentation and
>>>>>>>
https://fedorahosted.org/sssd/ticket/3063
>>>>>>> to add at least a simple integration test.
>>>>>>
>>>>>> Actually, can we get rid of this warning?
>>>>>>
>>>>>> Error: COMPILER_WARNING:
>>>>>> sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable
'patterns' [-Wunused-variable]
>>>>>> # const char *patterns[] = { "^[^\\.].*\\.conf",
NULL };
>>>>>> # ^
>>>>>> # 214| int ret;
>>>>>> # 215| #ifdef HAVE_LIBINI_CONFIG_V1
>>>>>> # 216|-> const char *patterns[] = {
"^[^\\.].*\\.conf", NULL };
>>>>>> # 217| const char *sections[] = { ".*", NULL };
>>>>>> # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3
>>>>>>
>>>>>> Error: COMPILER_WARNING:
>>>>>> sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function
'sss_ini_get_config'
>>>>>> sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable
'sections' [-Wunused-variable]
>>>>>> # const char *sections[] = { ".*", NULL };
>>>>>> # ^
>>>>>> # 215| #ifdef HAVE_LIBINI_CONFIG_V1
>>>>>> # 216| const char *patterns[] = {
"^[^\\.].*\\.conf", NULL };
>>>>>> # 217|-> const char *sections[] = { ".*", NULL
};
>>>>>> # 218| #ifdef HAVE_LIBINI_CONFIG_V1_3
>>>>>> # 219| uint32_t i = 0;
>>>>>> _______________________________________________
>>>>>
>>>>> New patch attached.
>>>>>
>>>>> Michal
>>>>>
>>>>
>>>> The patch is fine, (so ack also from me), but can someone check if the
>>>> attached rebase on top of Pavel's sssctl is OK?
>>>>
>>>>
>>>
>>> I did not check the differences between the rebased and
>>> non-rebased version, but I just tried branch with sssctl
>>> patches + this rebased patch + startup validation patches
>>> and it worked for me.
>>>
>>> But the CI failed for me with the snippets in place.
>>> Will take a look at what is happening there.
>>>
>> It failed beccause it bas broken with libini_config 1.0 .. 1.2
>> and current CI machines still have libini_config 1.2
>> or 1.1 (el6)
>>
>> Updated patch is attached.
>>
> Ups,
>
> ENOPATCH
>
> fixed in this version :-)
>
Previous version had an issue with SELinux and blocking
access to conf.d.
Hopefully last version is attached.
LS
There are two failures that are not related
to these patches (One is "device is busy" error during
mock and the other was that ldap server failed to start
for one set of tests).