[PATCHv2 1-14/14] Re: Disabling Python Modules
Andrew Bartlett
abartlet at samba.org
Thu Feb 9 18:45:55 UTC 2017
On Thu, 2017-02-09 at 16:20 +1300, Andrew Bartlett wrote:
> On Wed, 2017-02-01 at 07:40 +1300, Andrew Bartlett wrote:
> > On Mon, 2017-01-30 at 14:11 -0500, Ian Stakenvicius wrote:
> > > 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
> >
> > I've looked over the diff with the previous patches, and it looks
> > good!
> >
> > Can someone else on the team have a look at this and give review if
> > OK?
> >
> > Reviewed-by: Andrew Bartlett <abartlet at samba.org>
>
> I got a review from Garming and this is waiting for autobuild now.
Sadly this sill doesn't pass autobuild, this time on ctdb. I'll add
it to my stack to look at today, but if I don't get back to you can you
please run:
./script/autobuild.py ctdb --testbase=/tmp
locally. Sadly ctdb is not set up on travis as it exceeded the
resource limits or was flapping (I can't remember which).
Thanks,
Andrew Bartlett
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Catalyst IT http://catalyst.net.nz/services/samba
More information about the samba-technical
mailing list