[PATCHv2 1/x] Re: Disabling Python Modules
Andrew Bartlett
abartlet at samba.org
Sat Jan 28 01:21:54 UTC 2017
On Fri, 2017-01-27 at 15:24 -0500, Ian Stakenvicius wrote:
> 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?
Waf is strange. When something under bld.env is undefined, it returns
[], not "". We just need to set conf.env['PYTHON_SO_ABI_FLAG'] = '' or
cope with the []. We don't need that when we don't have python, but it
might keep everything more happy.
If all this doesn't work out, then a backup plan could be to make
making gen_python_environments() return an empty list if python is
disabled? The complex yeild stuff can be skipped, and just have it
return [] making:
for env in bld.gen_python_environments(['PKGCONFIGDIR']):
behave just like the
if !bld.python_disabled:
in your original patch
That could make it all fall our naturally, at least once the multi-
python stuff is finished, and the enabled stuff might be enough in the
meantime. I know this is a bit of a contradiction with the previous
requirement - I think we are all trying to feel our way to making this
all as natural as possible.
Sorry for the total confusion here,
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