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

[Andrew Bartlett]

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.

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

> I would think
> the latter would be enough to keep the test for checking all other
> headers, but of course I don't know if those headers will also be
> littered with python-related includes...

That would be what this test is trying to find.

> 
> 
> ----
> 
> 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.  Perhaps metze has
some ideas?

Andrew Bartlett

> > [...]
> > ==> ldb.stderr <==
> > 
> > 'clean' finished successfully (0.074s)
> > 
> > ldb: Completed OK
> > 
> > All OK
> > 
> > waiting for tail to flush
> > 
> > Running: 'stat /tmp/b10417/tmp' in '.'
> > 
> > stat: cannot stat ‘/tmp/b10417/tmp’: No such file or directory
> > 
> > Traceback (most recent call last):
> > 
> >   File "./script/autobuild.py", line 819, in <module>
> > 
> >     cleanup()
> > 
> >   File "./script/autobuild.py", line 474, in cleanup
> > 
> >     run_cmd("stat %s" % test_tmpdir, show=True)
> > 
> >   File "./script/autobuild.py", line 278, in run_cmd
> > 
> >     return check_call(cmd, shell=True, cwd=dir)
> > 
> >   File "/opt/python/2.7.12/lib/python2.7/subprocess.py", line 541,
> > in check_call
> > 
> >     raise CalledProcessError(retcode, cmd)
> > 
> > subprocess.CalledProcessError: Command 'stat /tmp/b10417/tmp'
> > returned non-zero exit status 1
> > 
> > The command "./script/autobuild.py --tail --testbase=/tmp $TASK"
> > exited with 1.
> > cache.2
> > 
> > store build cache
> > 
> > 0.00s
> > 
> > 2.08schanges detected, packing new archive
> > 
> > uploading archive
> > 
> > Done. Your build exited with 1.
> 
-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba