[PATCHv2 1/x] Re: Disabling Python Modules

[Ian Stakenvicius]

On 27/01/17 03:02 PM, Andrew Bartlett wrote:
> On Fri, 2017-01-27 at 14:21 -0500, Ian Stakenvicius wrote:
>> On 27/01/17 11:36 AM, Ian Stakenvicius wrote:
>>> On 27/01/17 01:10 AM, Andrew Bartlett wrote:
>>>> On Wed, 2017-01-25 at 23:56 -0500, Ian Stakenvicius wrote:
>>>>>>
>>>>>> For all others we should use something like
>>>>>> enabled=bld.PYTHON_BUILD_IS_ENABLED(),
>>>>>> see git grep 'enabled=' for examples.
>>>>>
>>>>>
>>>>> Is this just for style? or..
>>>>
>>>> No, it is more than style.  It allows the build system to know
>>>> that the
>>>>  name of the target subsystem exists, but it not in use.  This
>>>> allows
>>>> dependencies to be resolved as nothing, rather than having 'if
>>>> python'
>>>> in dep strings.  While this may just move the dep problem to the
>>>> C
>>>> layer, it allow an #ifdef at that layer.  
>>>>
>>>> It is also cleaner, in general, and while not totally consistent,
>>>> it is
>>>> how we have tried to handle other features.  
>>>>
>>
>> OK, let me know if this is a bit more on track.
>>
>> The following patch adds the --disable-python option globally.
>>
>> It also adds some code to skip meat in SAMBA_PYTHON_CHECK_HEADERS ,
>> so
>> that even if python is available the HAVE_PYTHON_H will not be
>> defined
>> (an appropriate exception is raised if this check is mandatory).
>>
>> It also adds bld.PYTHON_BUILD_IS_ENABLED() which uses HAVE_PYTHON_H
>> to
>> determine its value.
>>
>> My thought here is that, since the codebase has the HAVE_PYTHON_H
>> define, there's no need to make a PYTHON_BUILD_IS_ENABLED define to
>> essentially do the same thing.  Also, the check sets 'disable_python'
>> in the env so that the subprojects do not need to update their checks
>> and logic to some new variable.
>>
>> If this looks good, I'll adjust the rest of the patchset using this
>> as
>> the starting point.
> 
> This looks pretty good.
> 
> Does it still work if you change:
> 
> @@ -89,8 +106,9 @@ def SAMBA_PYTHON(bld, name,
>                   vars=None,
>                   install=True,
>                   enabled=True):
> -    '''build a python extension for Samba'''
> +  '''build a python extension for Samba'''
>  
> +  if bld.PYTHON_BUILD_IS_ENABLED():
>      if bld.env['IS_EXTRA_PYTHON']:
>          name = 'extra-' + name
> 
> To instead set enabled=False?  That would still declare the (disabled)
> build object to waf.  Either way, this should not be hard to tweak
> later. 
> 

I will check and adjust the patch if it works, thanks!

---

So, one thing that I just discovered with this method, is that the
existing wscript for lib/talloc happens to work the same way more or
less that my whole original patchset did, in that if python is
disabled then the entirety of pytalloc-util is skipped.

I attempted to change its structure back so that the SAMBA_LIBRARY and
SAMBA_PYTHONs are skipped via enabled=, but this line is still causing
problems:

> name = bld.pyembed_libname('pytalloc-util')


...resulting in:


>   File "./lib/talloc/wscript", line 128, in build
>     name = bld.pyembed_libname('pytalloc-util')
>   File "./buildtools/wafsamba/samba_python.py", line 159, in pyembed_libname
>     return name + bld.env['PYTHON_SO_ABI_FLAG']
> TypeError: cannot concatenate 'str' and 'list' objects
> 
> Makefile:8: recipe for target 'all' failed
> make: *** [all] Error 1


Any thoughts on how to get around this one?


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170127/57a1ae42/signature.sig>