[PATCHv2 1-14/14] Re: Disabling Python Modules

[Ian Stakenvicius]

On 30/01/17 01:44 PM, Andrew Bartlett wrote:
> We seem to have moved off-list.  Can we move this back on-list?

Done.


> On Mon, 2017-01-30 at 10:06 -0500, Ian Stakenvicius wrote:
>> On 29/01/17 10:56 PM, Andrew Bartlett wrote:
>>>
>>> With:
>>>
>>> ./configure.developer  --picky-developer --
>>> prefix=/tmp/b9407/prefix/samba-nopython  --with-profiling-data  
>>> --without-ad-dc --disable-python && make -j
>>>
>>> I now get:
>>>
>>> In file included from ../lib/replace/replace.h:32:0,
>>>                  from ../dynconfig/dynconfig.c:41:
>>> default/include/config.h:431:0: error: "PYTHONDIR" redefined [-
>>> Werror]
>>>  #define PYTHONDIR "/tmp/b9407/prefix/samba-
>>> nopython/lib/python2.7/site-packages"
>>>  
>>> <command-line>:0:0: note: this is the location of the previous
>>> definition
>>> In file included from ../lib/replace/replace.h:32:0,
>>>                  from ../dynconfig/dynconfig.c:41:
>>> default/include/config.h:432:0: error: "PYTHONARCHDIR" redefined [-
>>> Werror]
>>>  #define PYTHONARCHDIR "/tmp/b9407/prefix/samba-
>>> nopython/lib64/python2.7/site-packages"
>>>  
>>
>>
>> RIGHT, now I recall why I had coded the SAMBA_CHECK_PYTHON_HEADERS()
>> the way I did originally, it was so that deleting the defines would
>> still happen regardless of whether the check was skipped.  The
>> following should work, alternatively I can put it back to the way it
>> was with most of the function wrapped by the conditional..
>>
>> --- a/buildtools/wafsamba/samba_python.py
>> +++ b/buildtools/wafsamba/samba_python.py
>> @@ -47,6 +47,10 @@ def SAMBA_CHECK_PYTHON_HEADERS(conf,
>> mandatory=True):
>>                                   "--disable-python specified")
>>
>>          conf.msg("python headers", "Check disabled due to
>> --disable-python")
>> +        # we don't want PYTHONDIR in config.h, as otherwise changing
>> +        # --prefix causes a complete rebuild
>> +        del(conf.env.defines['PYTHONDIR'])
>> +        del(conf.env.defines['PYTHONARCHDIR'])
>>          return
>>
>>      if conf.env["python_headers_checked"] == []:
>>
> 
> My preference would be that the call is:
> 
> if not conf.env.disable_python:
>         conf.SAMBA_CHECK_PYTHON_HEADERS(mandatory=True)
> 
> I didn't make a bother about it before as I really didn't want to
> bikeshed you to death, but I don't think we should call
> SAMBA_CHECK_PYTHON_HEADERS if we disable python.  
> 
> I'm OK with checking for the python interpreter: it might be handy, or
> it might just bite us later.  The main hope is that we bail early
> before hitting future python 2.7 syntax in the remainder of the build
> system.
> 

I don't think that would fix this particular issue, in fact I think
avoiding the call to SAMBA_CHECK_PYTHON_HEADERS would cause the
dynconfig problem to occur more rather than less.  The original
version of my patch ensured that those calls to
del(conf.env.defines['PYTHON*]) always occurred regardless of whether
the header check(s) were being skipped, the issue you ran into was
when they don't occur, which would happen if
SAMBA_CHECK_PYTHON_HEADERS isn't called at all.


>> Travis is still running on a PR with the above change, with luck
>> it'll
>> pass on everything except for samba-nopython; that one's already
>> failed due to:
>>
>>> [2770/3250] Compiling testsuite/headers/test_headers.c
>>>
>>> ../testsuite/headers/test_headers.c:26:20: fatal error: Python.h:
>>> No such file or directory
>>>
>>>  #include <Python.h>
>>>
>>>                     ^
>>>
>>> compilation terminated.
>>>
>>> Waf: Leaving directory `/tmp/b10412/samba-nopython/bin'
>>>
>>> Build failed:  -> task failed (err #1): 
>>>
>>> 	{task: cc test_headers.c -> test_headers_2.o}
>>>
>>> make: *** [all] Error 1
>>>
>>
>> ...which makes sense given the code.  However, I'm not sure if this
>> should be skipped in the build system or if the include line above
>> should just be wrapped with #ifdef HAVE_PYTHON_H  ..?  
> 
> I think an #ifdef is appropriate here.
> 

*nod*, tested already and it works, travis reports samba-nopython
succeeds:

https://github.com/samba-team/samba/pull/74


>> ----
>>
>> On a separate note, i'm also seeing the following, which looks to be
>> an unrelated failure related to the cleanup of the test??
> 
> I never got it to run that far.  I agree it is odd.  Punt that one back
> to the list and ask metze.

It's not just my PR that does this, all open PR's from December 2016
onwards seems to have this issue right now.  Will open a new thread.




-------------- 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/20170130/bb73f6bc/signature.sig>