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

[Jelmer Vernooij]

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