On (27/06/16 14:00), Jakub Hrozek wrote: >On Fri, Jun 24, 2016 at 03:55:14PM +0200, Michal Židek wrote: >> On 06/24/2016 02:52 PM, Lukas Slebodnik wrote: >> > 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?= >> > > > > > > > > > > > > <[email protected]> >> > > > > > > > > > > > > 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 <[email protected]> >> > > > > > > > > > > > > --- >> > > > > > > > > > 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 >> > >> >> Thanks, this version works for all CI machines. >> >> Here is result of the CI: >> http://sssd-ci.duckdns.org/logs/job/46/08/summary.html >> >> 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). > >I would like to push the sssctl patches first, can you rebase? (You can >see what I changed in the previous mail to rebase the patch atop >sssctl). The patch was created on top of Pavel's patches.
It depends on which patches will be pushed to master. but I was able to apply patch from this thread with 3way merge on Pavel's sssctl branch from fedorapeople; there weren't any conflicts LS _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
