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?= <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
> >
>
> 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