[PATCH] Require explicit 'without flags' for acl, ldap and ads

[Andrew Bartlett]

On Thu, 2013-12-19 at 14:04 +1300, Garming Sam wrote:
> On 18/12/13 02:12, Jelmer Vernooij wrote:
> > On Tue, Dec 17, 2013 at 04:12:07PM +1300, Garming Sam wrote:
> >> On 17/12/13 01:54, Jelmer Vernooij wrote:
> >>> On Mon, Dec 16, 2013 at 04:45:19PM +1300, Andrew Bartlett wrote:
> >>>> On Mon, 2013-12-16 at 16:25 +1300, Garming Sam wrote:
> >>>>> These patches are designed to make it more difficult to build Samba
> >>>>> without ACL, LDAP or ADS support. By default, it is already set to
> >>>>> include support for these and by requiring them to included, it makes it
> >>>>> easier to ensure that the build is fully functional (or in any case,
> >>>>> indicates the packages which should be expected by default).
> >>>>>
> >>>>> We would subsequently have to work with the build farm to update it to
> >>>>> have these additional options set. Otherwise, we could attempt to get
> >>>>> the owners to install the missing packages.
> >>>> Thanks Garming!
> >>>>
> >>>> Reviewed-by: Andrew Bartlett <abartlet at samba.org>
> >>>>
> >>>> Can I get some comment and review from others on the team?  Remember,
> >>>> nothing about this prevents a build working that worked before, but it
> >>>> might just require an explicit --without flag or two.  This is based on
> >>>> what I wrote in BUILD_SYSTEMS.txt back before 4.0, but never had time to
> >>>> implement.
> >>>>
> >>>> This is related to the recent enquiry from Gentoo.
> >>> LGTM. The only reservation I have is about showing this error to users who
> >>> can't install the acl or xattr libraries because they're not available for
> >>> their platforms.
> >>>
> >>> Do we consider platforms that don't have these libraries second-class citizens?
> >>> Can we suggest users migrate to a platform that does have those libraries?
> >>>
> >> Modified the messages to be slightly more informative and why the
> >> support should be included.
> >>
> >> I think it would be a good idea to consider platforms that don't
> >> have these libraries to be second class.
> >>
> >> As for suggesting migration, I think it's a little unreasonable and
> >> likely the best thing to do is just to give them a slight nudge by
> >> saying which features they're actually missing out on.
> > Thanks. Some more comments below:
> >
> >> Signed-off-by: Garming Sam <garming at catalyst.net.nz>
> >> ---
> >>   source3/wscript | 8 ++++++--
> >>   1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/source3/wscript b/source3/wscript
> >> index b09c2db..2155c29 100644
> >> --- a/source3/wscript
> >> +++ b/source3/wscript
> >> @@ -428,7 +428,9 @@ utimensat vsyslog _write __write __xstat
> >>                   conf.DEFINE('HAVE_TRU64_ACLS',1)
> >>                   default_static_modules.extend(TO_LIST('vfs_tru64acl'))
> >>           elif (host_os.rfind('darwin') > -1):
> >> -            Logs.warn('ACLs on Dwarwin currently not supported')
> >> +            Logs.warn('ACLs on Darwin currently not supported')
> >> +            conf.fatal("ACL support not available on Darwin/MacOS. Use --without-acl-support for building without ACL support. " \
> >> +                "ACL support is required to change permissions for Windows clients.")
> > Is this technically true? I thought we'd just fall back to a TDB with severely
> > degraded performance, or was that just using the source4/ SMB server?
> >
> >>  From 3be4edb231b1a688dbbbedf0e9e49a290df99d3b Mon Sep 17 00:00:00 2001
> >> From: Garming Sam <garming at catalyst.net.nz>
> >> Date: Mon, 16 Dec 2013 13:31:31 +1300
> >> Subject: [PATCH 2/5] waf: Require ldap support to be specifically disabled
> >>
> >> Signed-off-by: Garming Sam <garming at catalyst.net.nz>
> >> ---
> >>   source3/wscript | 3 +++
> >>   1 file changed, 3 insertions(+)
> >>
> >> diff --git a/source3/wscript b/source3/wscript
> >> index 2155c29..a467065 100644
> >> --- a/source3/wscript
> >> +++ b/source3/wscript
> >> @@ -652,6 +652,9 @@ msg.msg_acctrightslen = sizeof(fd);
> >>               if conf.CONFIG_SET('HAVE_BER_SOCKBUF_ADD_IO') and \
> >>                       conf.CONFIG_SET('HAVE_LDAP_OPT_SOCKBUF'):
> >>                   conf.DEFINE('HAVE_LDAP_SASL_WRAPPING', '1')
> >> +        else:
> >> +            conf.fatal("LDAP support not found. Try installing libldap2-dev or openldap-devel. Otherwise, use --without-ldap to build without LDAP support. " \
> >> +                "LDAP support is required for the LDAP passdb backend.")
> > LDAP support is required for more than just the LDAP passdb backend.
> >
> > Cheers,
> >
> > Jelmer
> 
> I added a couple more things to the LDAP message.

Jelmer,

Can you review this for Garming?  

Garming,

When you get a chance, can you look at making this work for xattrs?
This may need some careful coordination between libreplace and samba,
but I think it should be possible to have libreplace know it's building
Samba, and only do xattr stuff (or the errors for the xattr stuff)
then.  

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba