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

Wed Dec 18 18:04:11 MST 2013

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.

Cheers,

Garming Sam
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-waf-Require-ACL-support-to-be-specifically-disabled.patch
Type: text/x-patch
Size: 1828 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20131219/14a134af/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-waf-Require-ldap-support-to-be-specifically-disabled.patch
Type: text/x-patch
Size: 1210 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20131219/14a134af/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-waf-Require-without-ads-support-to-build-without-ADS.patch
Type: text/x-patch
Size: 1669 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20131219/14a134af/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-waf-fix-a-typo-in-an-ADS-error-message.patch
Type: text/x-patch
Size: 1164 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20131219/14a134af/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-doc-Modify-build-doc-concerning-missing-headers.patch
Type: text/x-patch
Size: 1999 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20131219/14a134af/attachment-0004.bin>