[SSSD] [sssd PR#81][comment] Please see the commit message, the fix is hopefully simple

[lslebodn]

  URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

lslebodn commented:
"""
master:
* 2927dc45b9bc810f4f55bce165bb96405129e693

sssd-1-14:
* 495289cfa922b00278aa91d433489403e792304e

sssd-1-13:
* 331a7c3d60a4dadd43565f339095daef5de5e7bf

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/81#issuecomment-262269392
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#81][comment] Please see the commit message, the fix is hopefully simple

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

fidencio commented:
"""
Acked-by: Fabiano Fidêncio 
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/81#issuecomment-262235759
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#81][comment] Please see the commit message, the fix is hopefully simple

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

jhrozek commented:
"""
Thanks for the review, I pushed a new version. I did test the patch (otherwise 
the conditional build of the files provider where I added AM_CONDITIONAL didn't 
work) but I have no idea how it could work :)

Anyway, let's see if this version is OK..
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/81#issuecomment-262228673
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#81][comment] Please see the commit message, the fix is hopefully simple

[lslebodn]

  URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

lslebodn commented:
"""
On (21/11/16 13:53), fidencio wrote:
>For future interactions would be way simpler if the reviewer could just push 
>the patch with the simple fix and point it in the review instead of having it 
>blocked here for a few days.
A) It is not a hight priority or critical fix; so nothing was blocked.

B) In this case, the fixup change was simple. But there is still question
which changes are approrpiate to be done before push which are not.
The author of the patch need't agree with some changes.

It would be OK if author wrote please change that before pushing the patch.
But it would take the same amount of time as changing code and push it.

LS

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/81#issuecomment-261933794
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#81][comment] Please see the commit message, the fix is hopefully simple

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

fidencio commented:
"""
So, AFAIU what's is missing is this small patch:

```
[ffidenci@cat x86_64]$ git diff
diff --git a/src/external/inotify.m4 b/src/external/inotify.m4
index bcf9408..25259a8 100644
--- a/src/external/inotify.m4
+++ b/src/external/inotify.m4
@@ -7,7 +7,7 @@ AC_DEFUN([AM_CHECK_INOTIFY],
 AC_LINK_IFELSE(
 [AC_LANG_SOURCE([
 #ifdef HAVE_SYS_INOTIFY_H
-#include ,
+#include 
 #endif
 int main () {
 return (-1 == inotify_init());
```

Feel free to use the gist above as a different patch (I don't care about the 
ownership).

For future interactions would be way simpler if the reviewer could just push 
the patch with the simple fix and point it in the review instead of having it 
blocked here for a few days.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/81#issuecomment-261929087
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#81][comment] Please see the commit message, the fix is hopefully simple

[lslebodn]

  URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

lslebodn commented:
"""
This is only a problem with strict warning flags.
which is usually a develer build.

There is not a problem in downstream centos/fedora
->see 
https://kojipkgs.fedoraproject.org//packages/sssd/1.14.2/1.fc24/data/logs/x86_64/build.log

```
checking sys/inotify.h usability... yes
checking sys/inotify.h presence... yes
checking for sys/inotify.h... yes
checking whether sys/inotify.h actually works... yes
```

And more verbose version:
```
configure:25860: checking sys/inotify.h usability
configure:25860: gcc -c -g -O2 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-D_LARGEFILE64_SOURCE  conftest.c >&5
configure:25860: $? = 0
configure:25860: result: yes
configure:25860: checking sys/inotify.h presence
configure:25860: gcc -E  conftest.c
configure:25860: $? = 0
configure:25860: result: yes
configure:25860: checking for sys/inotify.h
configure:25860: result: yes
configure:25871: checking whether sys/inotify.h actually works
configure:25883: gcc -o conftest -g -O2 -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE   conftest.c  >&5
conftest.c: In function 'main':
conftest.c:186:19: warning: implicit declaration of function 'inotify_init'
[-Wimplicit-function-declaration]
 return (-1 == inotify_init());
^~~~
configure:25883: $? = 0
configure:25884: result: yes
```

Anyway,
have you tested your patch?
I can still see issue there.
```
configure:25883: gcc -o conftest -g -O2 -ftrack-macro-expansion=0 -Wextra 
-Wno-unused-parameter -Wno-sign-compare -Werror -D_FILE_OFFSET_BITS=64 
-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE  -Wl,--as-needed conftest.c  >&5
conftest.c:183:25: error: extra tokens at end of #include directive [-Werror]
 #include ,
 ^
cc1: all warnings being treated as errors
configure:25883: $? = 1
```

LS

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/81#issuecomment-261513452
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org