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

[Andrew Bartlett]

On Tue, 2013-12-17 at 14:12 +0100, 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?

That's the source4 server only.  Indeed, it's that possible mode of
operation that is why we don't totally block the DC build on this
point. 

> > 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.

Is there anything else we missed that isn't covered by the ADS check?
(That will also be printed, once this warning is silenced).

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