[PATCHv2 1-14/14] Re: Disabling Python Modules
Alexander Bokovoy
ab at samba.org
Sat Jan 28 17:25:37 UTC 2017
On la, 28 tammi 2017, Ian Stakenvicius wrote:
> On 28/01/17 05:02 AM, Alexander Bokovoy wrote:
> > On la, 28 tammi 2017, Ian Stakenvicius wrote:
> >> diff --git a/buildtools/wafsamba/samba_python.py b/buildtools/wafsamba/samba_python.py
> >> index 057a017..b6bf079 100644
> >> --- a/buildtools/wafsamba/samba_python.py
> >> +++ b/buildtools/wafsamba/samba_python.py
> >> @@ -40,22 +40,31 @@ def SAMBA_CHECK_PYTHON(conf, mandatory=True, version=(2,4,2)):
> >>
> >> @conf
> >> def SAMBA_CHECK_PYTHON_HEADERS(conf, mandatory=True):
> >> [...]
> >
> > Could you please turn the logic upside down here?
> > Instead of
> >
> > if not conf.env.disable_python:
> > -> re-indent code
> > else:
> > ....
> >
> > please do
> >
> > if conf.env.disable_python:
> > ....
> > return
> >
> > original code
> >
> > This would further reduce the diff size and will not touch the original
> > code at all.
> >
>
> Definitely.
>
>
> >>
> >> # we don't want PYTHONDIR in config.h, as otherwise changing
> >> # --prefix causes a complete rebuild
> >> @@ -77,6 +86,14 @@ def _check_python_headers(conf, mandatory):
> >> conf.env['PYTHON_SO_ABI_FLAG'] = ''
> >>
> >>
> >> +def PYTHON_BUILD_IS_ENABLED(self):
> >> + if self.CONFIG_SET('HAVE_PYTHON_H'):
> >> + return True
> >> + return False
> > CONFIG_SET already returns True or False, so this could be simplified
> > down to
> >
> > def PYTHON_BUILD_IS_ENABLED(self):
> > return self.CONFIG_SET('HAVE_PYTHON_H')
> >
> >
>
> Will do. Note that I found the code pattern I used a lot, so it might
> be worth doing a cleanup commit to change them all to your suggested
> format?
>
Sounds like a good idea, thanks.
> >> +
> >> +Build.BuildContext.PYTHON_BUILD_IS_ENABLED = PYTHON_BUILD_IS_ENABLED
> >> +
> >> +
> >> def SAMBA_PYTHON(bld, name,
> >> source='',
> >> deps='',
> >> @@ -91,6 +108,9 @@ def SAMBA_PYTHON(bld, name,
> >> enabled=True):
> >> '''build a python extension for Samba'''
> >>
> >> + if not bld.PYTHON_BUILD_IS_ENABLED():
> >> + enabled = False
> >> +
> > As PYTHON_BUILD_IS_ENABLED already returns True or False, this could be
> > written as
> >
> > enabled = bld.PYTHON_BUILD_IS_ENABLED()
> >
>
>
> We would lose the passed-in value of 'enabled' if we overwrote it in
> that manner. I could do:
>
> enabled = (enabled and bld.PYTHON_BUILD_IS_ENABLED())
>
> ..if that would be preferable? I'm thinking the earlier form would be
> easier to understand when someone looks at it in a year, though.
Ok, you are right. However, please add a comment there why we are
forcing an override. In a year nobody will remember that.
> -----
>
> So as-is, would this patchset be complete or should I also include the
> code to remove --disable-python from the wscript of standalone modules?
Please add patches for the standalone modules too.
--
/ Alexander Bokovoy
More information about the samba-technical
mailing list