I'm building Samba 4.4.0 on illumos (OpenSolaris). Including just sys/inotify.h is not sufficient on illumos/Solaris (should be the same for the now Oracle Solaris). Adding 2 ifdef's to include sys/filio.h on SunOS makes it compile. Simple patch below, not sure if they are proper as I just do packaging. $NetBSD$ --- source3/smbd/notify_inotify.c.orig 2016-01-26 11:45:46.000000000 +0000 +++ source3/smbd/notify_inotify.c @@ -27,6 +27,9 @@ #include "lib/util/sys_rw_data.h" #include <sys/inotify.h> +#ifdef __sun +#include <sys/filio.h> +#endif /* glibc < 2.5 headers don't have these defines */ #ifndef IN_ONLYDIR $NetBSD$ --- source4/ntvfs/sysdep/inotify.c.orig 2016-01-26 11:45:46.000000000 +0000 +++ source4/ntvfs/sysdep/inotify.c @@ -30,6 +30,9 @@ #include "param/param.h" #include <sys/inotify.h> +#ifdef __sun +#include <sys/filio.h> +#endif /* glibc < 2.5 headers don't have these defines */ #ifndef IN_ONLYDIR
Created attachment 11950 [details] patch for the copy in source3
Created attachment 11951 [details] patch for the copy in source4
Also added both patches as a .patch file. So far Samba 4.4 seems to compile much smoother than 4.3 (I dropped all my patches aside these 2)
(In reply to Jorge Schrauwen from comment #1) > patch for the copy in source3 We don't ususally do the #ifdef <some-platform> anymore. It should be possible to write a configure test that checks whether inotify can be included on its own. As a separate question -- does Illumos really support inotify?
I double checked: https://github.com/joyent/illumos-joyent/blob/master/usr/src/lib/libc/port/sys/inotify.c Exists, but it looks like it did not make it in all distro's yet. (Down side of being fragmented I guess). Probably why configure detects inotify support and tries to enable it. So if inotify.h is found and we are SunOS we need sys/filio.h, else well there is no problem because inotify.h is not found anwyay. Can you point me in the right direction for the configuration test? I it's something relatively trivial I might be able to get a patch for that instread.
(In reply to Jorge Schrauwen from comment #5) > Can you point me in the right direction for the configuration test? I it's > something relatively trivial I might be able to get a patch for that > instread. I'd just do a conf.CHECK_HEADERS('sys/filio.h') and if that exists #include it in the two inotify.c files with a comment that Illumos needs it. Assuming Illumos can't be fixed :-)
(In reply to Volker Lendecke from comment #6) > <jperkin> quite a lot of stuff assumes inotify == Linux, which is why we deliberately break inotify.h in pkgsrc builds to avoid it being picked up Seems the inotify on illumos is not fully the same as on Linux. It may be better to ignore inotify.h on SunOS in general then.
(In reply to Jorge Schrauwen from comment #7) > Seems the inotify on illumos is not fully the same as on Linux. > It may be better to ignore inotify.h on SunOS in general then. Ok, in that case you might avoid the check for inotify in lib/replace/wscript on Illumos. In wscripts it is okay to have platform specific stuff, for example search for "aix" in that file.
More comments from jperkin that may be of interest: > <jperkin> the linux man page _does_ specify sys/ioctl.h which would be enough on illumos to pull in filio.h, so if samba is using ioctl(2) but not including sys/ioctl.h then it's more likely a samba thing (and perhaps they haven't noticed because some other header on linux is pulling in sys/ioctl.h behind the scenes)
(In reply to Jorge Schrauwen from comment #9) <jperkin> sorry, that's not accurate, it's hidden behind -DBSD_COMP <jperkin> I dunno, as I said need a standards wonk ;)
Created attachment 11954 [details] 2nd version of the patch for source3 2nd version updates the configure script to skip detection of inotify if the os is Solaris (Oracle Solaris does not implement it, illumos sometimes does but it is not a drop in replacement for the linux ones) This one is for the script in source3
Created attachment 11955 [details] 2nd version of the patch for source4 2nd version updates the configure script to skip detection of inotify if the os is Solaris (Oracle Solaris does not implement it, illumos sometimes does but it is not a drop in replacement for the linux ones) This one is for the script in source4
Created attachment 11956 [details] lib/nss_wrapper configure script fix for SOLARIS_GETGRENT_R Found another small error but this time in the script for lib/nss
Created attachment 11963 [details] Patch Does this patch also do it I've slightly changed the source4 piece.
Builds fine with the following applied: 11963 11956
Patches apply fine against 4.4.2 also.
Now that badlock has passed, anything holding these back from getting included?
Created attachment 12027 [details] Patches for v4-4-test
Created attachment 12028 [details] Patches for v4-3-test
Created attachment 12029 [details] Patches for v4-2-test
(In reply to Stefan Metzmacher from comment #18) Just tested this version of the patch, looks good to me.
Pushed to autobuild-v4-[4|3|2]-test.
(In reply to Karolin Seeger from comment #22) Pushed to all branches. Closing out bug report. Thanks!
illumos does not deliver inotify.h, SmartOS does. FIONREAD is defined in /usr/include/sys/filio.h. We've worked around the compiling issue and enabled inotify on SmartOS for at least two years, it's a nice feature enabling Samba to do change notification, otherwise you have to use other plugin for change notification. Simply disabling inotify is not really a good solution, IMHO.
(In reply to YOUZHONG YANG from comment #24) Are you sure it is working correctly when inotify support is compiled in? Comment #7, seems to indicate that Joyent themselves do not build with it in pkgsrc tree to avoid issues.
(In reply to Jorge Schrauwen from comment #25) We've used it in production environment for long time, yes, I'm pretty sure it works.
Not so very sure how the build system works but I wipped up a patch. I test build is crunshing but it takes a few hours. $NetBSD$ --- source3/wscript.orig 2016-05-02 07:29:51.000000000 +0000 +++ source3/wscript @@ -142,6 +142,12 @@ long ret = splice(0,0,1,0,400,SPLICE_F_M conf.CHECK_HEADERS('sys/inotify.h') if "HAVE_SYS_INOTIFY_H" in conf.env: conf.DEFINE('HAVE_INOTIFY', 1) + else: + conf.CHECK_HEADERS('sys/inotify.h', add_headers=False) + conf.CHECK_HEADERS('sys/filio.h', add_headers=False) + if (conf.CONFIG_SET('HAVE_SYS_INOTIFY_H') and conf.CONFIG_SET('HAVE_SYS_FILIO_H')): + conf.DEFINE('HAVE_LINUX_INOTIFY', 1) + conf.DEFINE('BSD_COMP', 1) # Check for kernel change notify support conf.CHECK_CODE(''' $NetBSD$ --- source4/ntvfs/sysdep/wscript_configure.orig 2016-05-02 07:29:51.000000000 +0000 +++ source4/ntvfs/sysdep/wscript_configure @@ -9,6 +9,13 @@ if host_os.rfind('sunos') == -1: conf.CHECK_HEADERS('sys/inotify.h', add_headers=False) if (conf.CONFIG_SET('HAVE_SYS_INOTIFY_H')): conf.DEFINE('HAVE_LINUX_INOTIFY', 1) +else: + conf.CHECK_HEADERS('sys/inotify.h', add_headers=False) + conf.CHECK_HEADERS('sys/filio.h', add_headers=False) + if (conf.CONFIG_SET('HAVE_SYS_INOTIFY_H') and conf.CONFIG_SET('HAVE_SYS_FILIO_H')): + conf.DEFINE('HAVE_LINUX_INOTIFY', 1) + conf.DEFINE('BSD_COMP', 1) + conf.CHECK_DECLS('F_SETLEASE', headers='linux/fcntl.h', reverse=True) conf.CHECK_DECLS('SA_SIGINFO', headers='signal.h', reverse=True)
(In reply to Jorge Schrauwen from comment #27) Well that one failed, I'll look more at it tomorrow.
(In reply to YOUZHONG YANG from comment #26) I added a new set of patches here as bug 11939 With these it compiles fine again. I have no real test case for the inotify though.