RE: Disabling Python Modules

classic Classic list List threaded Threaded
71 messages Options
1234
Reply | Threaded
Open this post in threaded view
|

RE: Disabling Python Modules

Ian Stakenvicius
On Thu Jan 05 16:59:44 UTC 2017, Jeremy Allison wrote:

> On Thu, Jan 05, 2017 at 02:42:04PM +1300, Andrew Bartlett wrote:
>> On Wed, 2017-01-04 at 17:15 -0800, Jeremy Allison wrote:
>>> It looks like Gentoo Linux have done some work to allow both ldb and samba to build without python:
>>>
>>> https://dev.gentoo.org/~polynomial-c/samba-disable-python-patches-4.5.0_rc1.tar.xz
>>>
>>> Do we want to persue this and add to the build system as an option ? I think there are several embedded vendors who might like this option.
>>
>> My primary concern here is that I think we should be trying to write less C, rather than more, and so if we start to officially support building without python, we create another reason not to write more of our tools in python.
>>
>> I realise that Python hasn't really caught on outside the AD DC, as I had really hoped it would become Samba's standard scripting language for building tools etc.  Even so it would be quite unfortunate to imply a policy that certain parts of samba must be implemented in C.  (I've not used it, but from what I hear about it some rust might be quite helpful).
>>
>> That said, I get that embedded vendors would like one less dependency, particularly where they simply don't use the things we build with/for python.
>
> That's the key. Embedded vendors wanting only smbd or the client
> libsmb or net tools simply don't need the python modules, and it makes buiding
> Samba on these systems really hard (to the point where some give up
> and use alternatives, and we really don't want to see that :-).
>
> I don't think allowing these fixes will have any effect on how
> we use python for our "full" installations (tooling etc.) - after
> all our build system is python so it's not like we're going to
> reduce use of it.
>
> I'll try and find out the ownership of these patches and see if
> we can get them in a shape to merge.

I'm the original author for the vast majority of that patchset; i
definitely give permission for inclusion of them in samba and would be
happy to work with you all to get the portions applied that are
appropriate.

FYI, what the patchset was developed for in particular was so that on
multilib (x86_64 systems that also have 32bit apps and libs) gentoo is
able to build and install as much of samba as possible for 32bit
without needing to have a 32bit python environment installed.  The
patchset as a whole definitely does not provide any sort of working
samba install, nor likely should it.

I'll work on separating things out into appropriate segments, and
getting the patches into git locally so they can be committed as-is
later on.


signature.asc (201 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Disabling Python Modules

Jeremy Allison
On Wed, Jan 18, 2017 at 03:05:58PM -0500, Ian Stakenvicius wrote:

> On Thu Jan 05 16:59:44 UTC 2017, Jeremy Allison wrote:
> > On Thu, Jan 05, 2017 at 02:42:04PM +1300, Andrew Bartlett wrote:
> >> On Wed, 2017-01-04 at 17:15 -0800, Jeremy Allison wrote:
> >>> It looks like Gentoo Linux have done some work to allow both ldb and samba to build without python:
> >>>
> >>> https://dev.gentoo.org/~polynomial-c/samba-disable-python-patches-4.5.0_rc1.tar.xz
> >>>
> >>> Do we want to persue this and add to the build system as an option ? I think there are several embedded vendors who might like this option.
> >>
> >> My primary concern here is that I think we should be trying to write less C, rather than more, and so if we start to officially support building without python, we create another reason not to write more of our tools in python.
> >>
> >> I realise that Python hasn't really caught on outside the AD DC, as I had really hoped it would become Samba's standard scripting language for building tools etc.  Even so it would be quite unfortunate to imply a policy that certain parts of samba must be implemented in C.  (I've not used it, but from what I hear about it some rust might be quite helpful).
> >>
> >> That said, I get that embedded vendors would like one less dependency, particularly where they simply don't use the things we build with/for python.
> >
> > That's the key. Embedded vendors wanting only smbd or the client
> > libsmb or net tools simply don't need the python modules, and it makes buiding
> > Samba on these systems really hard (to the point where some give up
> > and use alternatives, and we really don't want to see that :-).
> >
> > I don't think allowing these fixes will have any effect on how
> > we use python for our "full" installations (tooling etc.) - after
> > all our build system is python so it's not like we're going to
> > reduce use of it.
> >
> > I'll try and find out the ownership of these patches and see if
> > we can get them in a shape to merge.
>
>
> I'm the original author for the vast majority of that patchset; i
> definitely give permission for inclusion of them in samba and would be
> happy to work with you all to get the portions applied that are
> appropriate.
>
> FYI, what the patchset was developed for in particular was so that on
> multilib (x86_64 systems that also have 32bit apps and libs) gentoo is
> able to build and install as much of samba as possible for 32bit
> without needing to have a 32bit python environment installed.  The
> patchset as a whole definitely does not provide any sort of working
> samba install, nor likely should it.
>
> I'll work on separating things out into appropriate segments, and
> getting the patches into git locally so they can be committed as-is
> later on.

Hi Ian, that's fantasic news ! Thanks for helping us with that !

Can you send us in an email as described here:

https://www.samba.org/samba/devel/copyright-policy.html

so we know we have rights to merge.

That would really ease getting these patches into the
upstream tree.

Thanks !

Jeremy.

Reply | Threaded
Open this post in threaded view
|

Re: Disabling Python Modules

Ian Stakenvicius
On 18/01/17 08:03 PM, Jeremy Allison wrote:

> On Wed, Jan 18, 2017 at 03:05:58PM -0500, Ian Stakenvicius wrote:
>> [...]
>> I'll work on separating things out into appropriate segments, and
>> getting the patches into git locally so they can be committed as-is
>> later on.
>
> Hi Ian, that's fantasic news ! Thanks for helping us with that !
>
> Can you send us in an email as described here:
>
> https://www.samba.org/samba/devel/copyright-policy.html
> [..]
Done.

OK, the patchset is prepared for email; there's 27 commits in it, and
I've provided the titles below.  Should some of these be squashed
together before I send them or should they remain separate?


1 waf: disable-python-add configuration parameter to root wscript
2 waf: disable-python-skip python and source4/torture when python is
disabled
3 waf: disable-python-don't check for pytalloc-util when python disabled
4 waf: disable-python-don't check for pyldb-util when python disabled
5 waf: disable-python-don't check for pytevent when python disabled
6 waf: disable-python-don't check for pytdb when python disabled
7 waf: disable-python-don't build pys3param when python disasbled
8 waf: disable-python-don't build pycredentials without python
9 waf: disable-python-don't build python_netbios or pysecurity w/o python
10 waf: disable-python-don't build libcli echo torture test w/o python
11 waf: disable-python-don't build source4/pysmb without python
12 waf: disable-python-don't build samba4/librpc python bits w/o python
13 waf: disable-python-don't build of pysmbd and pysmblib w/o python
14 waf: disable-python-don't build python_samba__ldb w/o python
15 waf: disable-python-don't build pypassdb without python
16 waf: disable-python-don't build pygensec without python
17 waf: disable-python-don't build pyauth without python
18 waf: disable-python-don't build python_dsdb without python
19 waf: disable-python-don't build python_dsdb_dns without python
20 waf: disable-python-don't build pycom without python
21 waf: disable-python-don't build python_messaging w/o python
22 waf: disable-python-don't build samba-policy, py_policy w/o python
23 waf: disable-python-don't build py_registry without python
24 waf: disable-python-don't build pywmi without python
25 waf: disable-python-don't build samba-net et. al. w/o python
26 waf: disable-python-don't build PROVISION, pyparam* w/o python
27 waf: disable-python-don't build python_xattr et. al. w/o python



signature.asc (201 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH] Re: Disabling Python Modules

Ian Stakenvicius
On 20/01/17 01:45 PM, Ian Stakenvicius wrote:

> On 18/01/17 08:03 PM, Jeremy Allison wrote:
>> On Wed, Jan 18, 2017 at 03:05:58PM -0500, Ian Stakenvicius wrote:
>>> [...]
>>> I'll work on separating things out into appropriate segments, and
>>> getting the patches into git locally so they can be committed as-is
>>> later on.
>>
>> Hi Ian, that's fantasic news ! Thanks for helping us with that !
>>
>> Can you send us in an email as described here:
>>
>> https://www.samba.org/samba/devel/copyright-policy.html
>> [..]
>
> Done.
>
> OK, the patchset is prepared for email; there's 27 (edit: 28) commits in it, and
> I've provided the titles below.  Should some of these be squashed
> together before I send them or should they remain separate?
Received advice from #samba-technical.  Please find attached the
patchset as previously linked and described.

Regards,
Ian


disable-python.patches.txt (42K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCH try2] Re: Disabling Python Modules

Ian Stakenvicius
On 23/01/17 11:13 AM, Ian Stakenvicius wrote:

> On 20/01/17 01:45 PM, Ian Stakenvicius wrote:
>> On 18/01/17 08:03 PM, Jeremy Allison wrote:
>>> On Wed, Jan 18, 2017 at 03:05:58PM -0500, Ian Stakenvicius wrote:
>>>> [...]
>>>> I'll work on separating things out into appropriate segments, and
>>>> getting the patches into git locally so they can be committed as-is
>>>> later on.
>>>
>>> Hi Ian, that's fantasic news ! Thanks for helping us with that !
>>>
>>> Can you send us in an email as described here:
>>>
>>> https://www.samba.org/samba/devel/copyright-policy.html
>>> [..]
>>
>> Done.
>>
>> OK, the patchset is prepared for email; there's 27 (edit: 28) commits in it, and
>> I've provided the titles below.  Should some of these be squashed
>> together before I send them or should they remain separate?
>
> Received advice from #samba-technical.  Please find attached the
> patchset as previously linked and described.
>
> Regards,
> Ian
>
New version attached for review, with proper signoffs in place.


disable-python.patches.txt (58K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH try2] Re: Disabling Python Modules

Jeremy Allison
On Mon, Jan 23, 2017 at 11:51:33AM -0500, Ian Stakenvicius wrote:

> On 23/01/17 11:13 AM, Ian Stakenvicius wrote:
> > On 20/01/17 01:45 PM, Ian Stakenvicius wrote:
> >> On 18/01/17 08:03 PM, Jeremy Allison wrote:
> >>> On Wed, Jan 18, 2017 at 03:05:58PM -0500, Ian Stakenvicius wrote:
> >>>> [...]
> >>>> I'll work on separating things out into appropriate segments, and
> >>>> getting the patches into git locally so they can be committed as-is
> >>>> later on.
> >>>
> >>> Hi Ian, that's fantasic news ! Thanks for helping us with that !
> >>>
> >>> Can you send us in an email as described here:
> >>>
> >>> https://www.samba.org/samba/devel/copyright-policy.html
> >>> [..]
> >>
> >> Done.
> >>
> >> OK, the patchset is prepared for email; there's 27 (edit: 28) commits in it, and
> >> I've provided the titles below.  Should some of these be squashed
> >> together before I send them or should they remain separate?
> >
> > Received advice from #samba-technical.  Please find attached the
> > patchset as previously linked and described.
> >
> > Regards,
> > Ian
> >
>
> New version attached for review, with proper signoffs in place.

Thanks a *LOT* for this Ian !

However, in master, when I apply this patchset and configure
using:

buildtools/bin/waf configure --enable-pthreadpool --with-acl-support --enable-developer --picky-developer --disable-python

and then do:

make -j

I get:

WAF_MAKE=1 python ./buildtools/bin/waf build
Waf: Entering directory `/space/jra/src/samba/git/merge-1/bin'
        Selected embedded Heimdal build
symlink: samba-tool -> ./samba-tool
symlink: samba_dnsupdate -> ./samba_dnsupdate
symlink: samba_spnupdate -> ./samba_spnupdate
symlink: samba_kcc -> ./samba_kcc
symlink: samba_upgradeprovision -> ./samba_upgradeprovision
symlink: samba_upgradedns -> ./samba_upgradedns
symlink: smbaddshare -> ./smbaddshare
symlink: smbchangeshare -> ./smbchangeshare
symlink: smbdeleteshare -> ./smbdeleteshare
Checking project rules ...
Unknown dependency 'LIBPYTHON' in 'service_web.objlist'
make: *** [all] Error 1

So I think there's another patch needed for:

source4/web_server/wscript_build

but my python isn't good enough to create it yet,
sorry.

Jeremy.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH try2] Re: Disabling Python Modules

Ian Stakenvicius

> On Jan 24, 2017, at 7:38 PM, Jeremy Allison <[hidden email]> wrote:
>
>> On Mon, Jan 23, 2017 at 11:51:33AM -0500, Ian Stakenvicius wrote:
>>> On 23/01/17 11:13 AM, Ian Stakenvicius wrote:
>>>> On 20/01/17 01:45 PM, Ian Stakenvicius wrote:
>>>>> On 18/01/17 08:03 PM, Jeremy Allison wrote:
>>>>>> On Wed, Jan 18, 2017 at 03:05:58PM -0500, Ian Stakenvicius wrote:
>>>>>> [...]
>>>>>> I'll work on separating things out into appropriate segments, and
>>>>>> getting the patches into git locally so they can be committed as-is
>>>>>> later on.
>>>>>
>>>>> Hi Ian, that's fantasic news ! Thanks for helping us with that !
>>>>>
>>>>> Can you send us in an email as described here:
>>>>>
>>>>> https://www.samba.org/samba/devel/copyright-policy.html
>>>>> [..]
>>>>
>>>> Done.
>>>>
>>>> OK, the patchset is prepared for email; there's 27 (edit: 28) commits in it, and
>>>> I've provided the titles below.  Should some of these be squashed
>>>> together before I send them or should they remain separate?
>>>
>>> Received advice from #samba-technical.  Please find attached the
>>> patchset as previously linked and described.
>>>
>>> Regards,
>>> Ian
>>
>> New version attached for review, with proper signoffs in place.
>
> Thanks a *LOT* for this Ian !
>
> However, in master, when I apply this patchset and configure
> using:
>
> buildtools/bin/waf configure --enable-pthreadpool --with-acl-support --enable-developer --picky-developer --disable-python
>
> and then do:
>
> make -j
>
> I get:
>
> WAF_MAKE=1 python ./buildtools/bin/waf build
> Waf: Entering directory `/space/jra/src/samba/git/merge-1/bin'
>    Selected embedded Heimdal build
> symlink: samba-tool -> ./samba-tool
> symlink: samba_dnsupdate -> ./samba_dnsupdate
> symlink: samba_spnupdate -> ./samba_spnupdate
> symlink: samba_kcc -> ./samba_kcc
> symlink: samba_upgradeprovision -> ./samba_upgradeprovision
> symlink: samba_upgradedns -> ./samba_upgradedns
> symlink: smbaddshare -> ./smbaddshare
> symlink: smbchangeshare -> ./smbchangeshare
> symlink: smbdeleteshare -> ./smbdeleteshare
> Checking project rules ...
> Unknown dependency 'LIBPYTHON' in 'service_web.objlist'
> make: *** [all] Error 1
>
> So I think there's another patch needed for:
>
> source4/web_server/wscript_build
>
> but my python isn't good enough to create it yet,
> sorry.
>
> Jeremy.
>

That part I can do; Gentoo has never built samba in "developer" mode so there may be a number of modules that still expect Python.  I'll give it a shot locally and add a commit or more, as a separate email.

FYI, for anyone that may want to try this, in wscript_build you just have to wrap Python-needing modules by prepending  'if not bld.env.disable_python:'

In wscript it's similar (conf.env.disable_python) but there are a couple more steps.

Lots of examples abound in the patch set, as that's more or less all I did.  The real work is in chasing down module dependencies to know which build lines to disable.

Reply | Threaded
Open this post in threaded view
|

[PATCH 29] Re: Disabling Python Modules

Ian Stakenvicius
On 24/01/17 07:50 PM, Ian Stakenvicius wrote:

>
>> On Jan 24, 2017, at 7:38 PM, Jeremy Allison <[hidden email]> wrote:
>>
>>> On Mon, Jan 23, 2017 at 11:51:33AM -0500, Ian Stakenvicius wrote:
>>>> On 23/01/17 11:13 AM, Ian Stakenvicius wrote:
>>>>> On 20/01/17 01:45 PM, Ian Stakenvicius wrote:
>>>>>> On 18/01/17 08:03 PM, Jeremy Allison wrote:
>>>>>>> On Wed, Jan 18, 2017 at 03:05:58PM -0500, Ian Stakenvicius wrote:
>>>>>>> [...]
>>>>>>> I'll work on separating things out into appropriate segments, and
>>>>>>> getting the patches into git locally so they can be committed as-is
>>>>>>> later on.
>>>>>>
>>>>>> Hi Ian, that's fantasic news ! Thanks for helping us with that !
>>>>>>
>>>>>> Can you send us in an email as described here:
>>>>>>
>>>>>> https://www.samba.org/samba/devel/copyright-policy.html
>>>>>> [..]
>>>>>
>>>>> Done.
>>>>>
>>>>> OK, the patchset is prepared for email; there's 27 (edit: 28) commits in it, and
>>>>> I've provided the titles below.  Should some of these be squashed
>>>>> together before I send them or should they remain separate?
>>>>
>>>> Received advice from #samba-technical.  Please find attached the
>>>> patchset as previously linked and described.
>>>>
>>>> Regards,
>>>> Ian
>>>
>>> New version attached for review, with proper signoffs in place.
>>
>> Thanks a *LOT* for this Ian !
>>
>> However, in master, when I apply this patchset and configure
>> using:
>>
>> buildtools/bin/waf configure --enable-pthreadpool --with-acl-support --enable-developer --picky-developer --disable-python
>>
>> and then do:
>>
>> make -j
>>
>> I get:
>>
>> WAF_MAKE=1 python ./buildtools/bin/waf build
>> Waf: Entering directory `/space/jra/src/samba/git/merge-1/bin'
>>    Selected embedded Heimdal build
>> symlink: samba-tool -> ./samba-tool
>> symlink: samba_dnsupdate -> ./samba_dnsupdate
>> symlink: samba_spnupdate -> ./samba_spnupdate
>> symlink: samba_kcc -> ./samba_kcc
>> symlink: samba_upgradeprovision -> ./samba_upgradeprovision
>> symlink: samba_upgradedns -> ./samba_upgradedns
>> symlink: smbaddshare -> ./smbaddshare
>> symlink: smbchangeshare -> ./smbchangeshare
>> symlink: smbdeleteshare -> ./smbdeleteshare
>> Checking project rules ...
>> Unknown dependency 'LIBPYTHON' in 'service_web.objlist'
>> make: *** [all] Error 1
>>
>> So I think there's another patch needed for:
>>
>> source4/web_server/wscript_build
>>
>> but my python isn't good enough to create it yet,
>> sorry.
>>
>> Jeremy.
>>
>
> That part I can do;
..and, done.  See attached.


disable-python_service_web.patch (831 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 29] Re: Disabling Python Modules

Ian Stakenvicius
On 24/01/17 08:37 PM, Ian Stakenvicius wrote:

> On 24/01/17 07:50 PM, Ian Stakenvicius wrote:
>>
>>> On Jan 24, 2017, at 7:38 PM, Jeremy Allison <[hidden email]> wrote:
>>>
>>>> On Mon, Jan 23, 2017 at 11:51:33AM -0500, Ian Stakenvicius wrote:
>>>>> On 23/01/17 11:13 AM, Ian Stakenvicius wrote:
>>>>>> On 20/01/17 01:45 PM, Ian Stakenvicius wrote:
>>>>>>> On 18/01/17 08:03 PM, Jeremy Allison wrote:
>>>>>>>> On Wed, Jan 18, 2017 at 03:05:58PM -0500, Ian Stakenvicius wrote:
>>>>>>>> [...]
>>>>>>>> I'll work on separating things out into appropriate segments, and
>>>>>>>> getting the patches into git locally so they can be committed as-is
>>>>>>>> later on.
>>>>>>>
>>>>>>> Hi Ian, that's fantasic news ! Thanks for helping us with that !
>>>>>>>
>>>>>>> Can you send us in an email as described here:
>>>>>>>
>>>>>>> https://www.samba.org/samba/devel/copyright-policy.html
>>>>>>> [..]
>>>>>>
>>>>>> Done.
>>>>>>
>>>>>> OK, the patchset is prepared for email; there's 27 (edit: 28) commits in it, and
>>>>>> I've provided the titles below.  Should some of these be squashed
>>>>>> together before I send them or should they remain separate?
>>>>>
>>>>> Received advice from #samba-technical.  Please find attached the
>>>>> patchset as previously linked and described.
>>>>>
>>>>> Regards,
>>>>> Ian
>>>>
>>>> New version attached for review, with proper signoffs in place.
>>>
>>> Thanks a *LOT* for this Ian !
>>>
>>> However, in master, when I apply this patchset and configure
>>> using:
>>>
>>> buildtools/bin/waf configure --enable-pthreadpool --with-acl-support --enable-developer --picky-developer --disable-python
>>>
>>> and then do:
>>>
>>> make -j
>>>
>>> I get:
>>>
>>> WAF_MAKE=1 python ./buildtools/bin/waf build
>>> Waf: Entering directory `/space/jra/src/samba/git/merge-1/bin'
>>>    Selected embedded Heimdal build
>>> symlink: samba-tool -> ./samba-tool
>>> symlink: samba_dnsupdate -> ./samba_dnsupdate
>>> symlink: samba_spnupdate -> ./samba_spnupdate
>>> symlink: samba_kcc -> ./samba_kcc
>>> symlink: samba_upgradeprovision -> ./samba_upgradeprovision
>>> symlink: samba_upgradedns -> ./samba_upgradedns
>>> symlink: smbaddshare -> ./smbaddshare
>>> symlink: smbchangeshare -> ./smbchangeshare
>>> symlink: smbdeleteshare -> ./smbdeleteshare
>>> Checking project rules ...
>>> Unknown dependency 'LIBPYTHON' in 'service_web.objlist'
>>> make: *** [all] Error 1
>>>
>>> So I think there's another patch needed for:
>>>
>>> source4/web_server/wscript_build
>>>
>>> but my python isn't good enough to create it yet,
>>> sorry.
>>>
>>> Jeremy.
>>>
>>
>> That part I can do;
>
> ..and, done.  See attached.
>

I can't be certain, but I think that may indeed be the last of them.
All wscript_build files that contain modules with deps= including
py*util, and LIBPYTHON, are now all covered.




signature.asc (201 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 29] Re: Disabling Python Modules

Stefan Metzmacher-2
Hi Ian,

> I can't be certain, but I think that may indeed be the last of them.
> All wscript_build files that contain modules with deps= including
> py*util, and LIBPYTHON, are now all covered.

I know I'm late in the discussion, but is there a chance
that we could avoid all changes like this:

-bld.SAMBA3_PYTHON('pys3param',
+if not bld.env.disable_python:
+    bld.SAMBA3_PYTHON('pys3param',

And just to that globally within SAMBA_PYTHON()
instead of changing each caller?
The same applies to CHECK_BUNDLED_SYSTEM_PYTHON()
and SAMBA_CHECK_PYTHON_HEADERS()

For all others we should use something like
enabled=bld.PYTHON_BUILD_IS_ENABLED(),
see git grep 'enabled=' for examples.

I think we should also have the --disable-python option
near to the --extra-python option within
buildtools/wafsamba/wscript, so that it is also available
for the standalone builds of talloc, tevent, tdb and ldb.

For the samba build I think --disable-python give an error
unless --without-ad-dc is also given as an ad-dc really requires
python bindings.

And finally we need this checked in script/autobuild.py in order
to prevent future regressions, maybe more or less a copy of
the "samba-libs" task (as "samba-nopython")
just with other configure options, e.g.

samba_nopython_configure_libs = samba_libs_envvars + " ./configure
--abi-check --enable-debug --picky-developer -C ${PREFIX} --disable-python"
samba_nopython_configure_samba = samba_nopython_configure_libs + "
--without-ad-dc"

I hope the changes make some sense to you and don't cause to much pain.

Thanks!
metze


signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 29] Re: Disabling Python Modules

Petr Viktorin
On 01/25/2017 09:29 AM, Stefan Metzmacher wrote:

> Hi Ian,
>
>> I can't be certain, but I think that may indeed be the last of them.
>> All wscript_build files that contain modules with deps= including
>> py*util, and LIBPYTHON, are now all covered.
>
> I know I'm late in the discussion, but is there a chance
> that we could avoid all changes like this:
>
> -bld.SAMBA3_PYTHON('pys3param',
> +if not bld.env.disable_python:
> +    bld.SAMBA3_PYTHON('pys3param',
>
> And just to that globally within SAMBA_PYTHON()
> instead of changing each caller?
> The same applies to CHECK_BUNDLED_SYSTEM_PYTHON()
> and SAMBA_CHECK_PYTHON_HEADERS()

That would also conflicts with the Python3 enablement patches, where all
these calls eventually need to be wrapped for-loop to possibly build
with both Python versions.

--
Petr Viktorin


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 29] Re: Disabling Python Modules

Ian Stakenvicius
In reply to this post by Stefan Metzmacher-2
On 25/01/17 03:29 AM, Stefan Metzmacher wrote:

> Hi Ian,
>
>> I can't be certain, but I think that may indeed be the last of them.
>> All wscript_build files that contain modules with deps= including
>> py*util, and LIBPYTHON, are now all covered.
>
> I know I'm late in the discussion, but is there a chance
> that we could avoid all changes like this:
>
> -bld.SAMBA3_PYTHON('pys3param',
> +if not bld.env.disable_python:
> +    bld.SAMBA3_PYTHON('pys3param',
>
> And just to that globally within SAMBA_PYTHON()
> instead of changing each caller?

I considered doing that, but decided against it due to the fact that
this hides too much from the wscript_build of each module.  When a
bld.env.disable_python conditional exists it makes it clear that all
components the conditional applies to (whether SAMBA_PYTHON() or not,
and there's more than a few that aren't SAMBA_PYTHON()).



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


> I think we should also have the --disable-python option
> near to the --extra-python option within
> buildtools/wafsamba/wscript, so that it is also available
> for the standalone builds of talloc, tevent, tdb and ldb.


--disable-python is already in the standalone builds, I don't recall
patching that in except maybe for one of those; most if not all of the
changes to those modules was to allow the pre-existing
--disable-python functionality to apply when it's _not_ being built
standalone.


>
> For the samba build I think --disable-python give an error
> unless --without-ad-dc is also given as an ad-dc really requires
> python bindings.


I believe I tested this one and it works out fine, however it's worth
noting of course that ADDC ends up being practically disabled anyhow.
The disable-python patchset does NOT disable features of samba at this
point; that wasn't my goal when i built it as Gentoo just cherry-picks
the libs out of the compilation.  Things can definitely be expanded on
to disable full features instead, but that will need to be taken up by
someone that knows more than me.


> And finally we need this checked in script/autobuild.py in order
> to prevent future regressions, maybe more or less a copy of
> the "samba-libs" task (as "samba-nopython")
> just with other configure options, e.g.
>
> samba_nopython_configure_libs = samba_libs_envvars + " ./configure
> --abi-check --enable-debug --picky-developer -C ${PREFIX} --disable-python"
> samba_nopython_configure_samba = samba_nopython_configure_libs + "
> --without-ad-dc"


I'll have to pass this one on to someone else as well, if anyone here
has autobuild.py experience and could take it on?


signature.asc (201 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 29] Re: Disabling Python Modules

Andrew Bartlett
In reply to this post by Petr Viktorin
On Wed, 2017-01-25 at 13:19 +0100, Petr Viktorin wrote:

> On 01/25/2017 09:29 AM, Stefan Metzmacher wrote:
> > Hi Ian,
> >
> > > I can't be certain, but I think that may indeed be the last of
> > > them.
> > > All wscript_build files that contain modules with deps= including
> > > py*util, and LIBPYTHON, are now all covered.
> >
> > I know I'm late in the discussion, but is there a chance
> > that we could avoid all changes like this:
> >
> > -bld.SAMBA3_PYTHON('pys3param',
> > +if not bld.env.disable_python:
> > +    bld.SAMBA3_PYTHON('pys3param',
> >
> > And just to that globally within SAMBA_PYTHON()
> > instead of changing each caller?
> > The same applies to CHECK_BUNDLED_SYSTEM_PYTHON()
> > and SAMBA_CHECK_PYTHON_HEADERS()
>
> That would also conflicts with the Python3 enablement patches, where
> all 
> these calls eventually need to be wrapped for-loop to possibly build 
> with both Python versions.

I would strongly request that any patches we have to disable python
make Petr's work easier, not harder.  I've been putting off merging
Petr's very good patches while I wait for a little more to be
completed, but it would be really unfortunate if he had to re-do a bit
chunk of his work because this landed first.

Please re-base the patches on Petr's patches, or show that they
trivially rebase on yours (say by working inside the SAMBA_PYTHON
layer).

I know this is frustrating, but we need to land this both these ideas
successfully.  

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


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 29] Re: Disabling Python Modules

Andrew Bartlett
In reply to this post by Ian Stakenvicius
On Wed, 2017-01-25 at 23:56 -0500, Ian Stakenvicius wrote:

> On 25/01/17 03:29 AM, Stefan Metzmacher wrote:
> > Hi Ian,
> >
> > > I can't be certain, but I think that may indeed be the last of
> > > them.
> > > All wscript_build files that contain modules with deps= including
> > > py*util, and LIBPYTHON, are now all covered.
> >
> > I know I'm late in the discussion, but is there a chance
> > that we could avoid all changes like this:
> >
> > -bld.SAMBA3_PYTHON('pys3param',
> > +if not bld.env.disable_python:
> > +    bld.SAMBA3_PYTHON('pys3param',
> >
> > And just to that globally within SAMBA_PYTHON()
> > instead of changing each caller?
>
>
> I considered doing that, but decided against it due to the fact that
> this hides too much from the wscript_build of each module.  When a
> bld.env.disable_python conditional exists it makes it clear that all
> components the conditional applies to (whether SAMBA_PYTHON() or not,
> and there's more than a few that aren't SAMBA_PYTHON()).
>
>
>
> > [...]
> >
> > 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.  

> > I think we should also have the --disable-python option
> > near to the --extra-python option within
> > buildtools/wafsamba/wscript, so that it is also available
> > for the standalone builds of talloc, tevent, tdb and ldb.
>
>
> --disable-python is already in the standalone builds, I don't recall
> patching that in except maybe for one of those; most if not all of
> the
> changes to those modules was to allow the pre-existing
> --disable-python functionality to apply when it's _not_ being built
> standalone.

I think having this all in one place is still a good idea however.

> >
> > For the samba build I think --disable-python give an error
> > unless --without-ad-dc is also given as an ad-dc really requires
> > python bindings.
>
>
> I believe I tested this one and it works out fine

I'm sorry, the Samba AD DC strictly requires python.  I don't know what
you tested (presumably just that the binaries built), but building with
the AD DC enabled needs to fail totally and early, to save everybody
time and hair.

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


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 29] Re: Disabling Python Modules

Ian Stakenvicius
In reply to this post by Andrew Bartlett
On 27/01/17 01:02 AM, Andrew Bartlett wrote:

> On Wed, 2017-01-25 at 13:19 +0100, Petr Viktorin wrote:
>> On 01/25/2017 09:29 AM, Stefan Metzmacher wrote:
>>> Hi Ian,
>>>
>>>> I can't be certain, but I think that may indeed be the last of
>>>> them.
>>>> All wscript_build files that contain modules with deps= including
>>>> py*util, and LIBPYTHON, are now all covered.
>>>
>>> I know I'm late in the discussion, but is there a chance
>>> that we could avoid all changes like this:
>>>
>>> -bld.SAMBA3_PYTHON('pys3param',
>>> +if not bld.env.disable_python:
>>> +    bld.SAMBA3_PYTHON('pys3param',
>>>
>>> And just to that globally within SAMBA_PYTHON()
>>> instead of changing each caller?
>>> The same applies to CHECK_BUNDLED_SYSTEM_PYTHON()
>>> and SAMBA_CHECK_PYTHON_HEADERS()
>>
>> That would also conflicts with the Python3 enablement patches, where
>> all
>> these calls eventually need to be wrapped for-loop to possibly build
>> with both Python versions.
>
> I would strongly request that any patches we have to disable python
> make Petr's work easier, not harder.  I've been putting off merging
> Petr's very good patches while I wait for a little more to be
> completed, but it would be really unfortunate if he had to re-do a bit
> chunk of his work because this landed first.
>
> Please re-base the patches on Petr's patches, or show that they
> trivially rebase on yours (say by working inside the SAMBA_PYTHON
> layer).

I already have, and have also committed to rebasing Petr's work myself
if the disable-python patches land before his.

I should note though that reading the above, I am interpreting Petr's
comments to apply to Stefan's, in terms of rolling everything into
SAMBA_PYTHON rather than having them in the wscript_build of
individual modules.  Hopefully Petr can confirm?





signature.asc (201 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 29] Re: Disabling Python Modules

Ian Stakenvicius
In reply to this post by Andrew Bartlett
On 27/01/17 01:10 AM, Andrew Bartlett wrote:

> On Wed, 2017-01-25 at 23:56 -0500, Ian Stakenvicius wrote:
>> On 25/01/17 03:29 AM, Stefan Metzmacher wrote:
>>> Hi Ian,
>>>
>>>> I can't be certain, but I think that may indeed be the last of
>>>> them.
>>>> All wscript_build files that contain modules with deps= including
>>>> py*util, and LIBPYTHON, are now all covered.
>>>
>>> I know I'm late in the discussion, but is there a chance
>>> that we could avoid all changes like this:
>>>
>>> -bld.SAMBA3_PYTHON('pys3param',
>>> +if not bld.env.disable_python:
>>> +    bld.SAMBA3_PYTHON('pys3param',
>>>
>>> And just to that globally within SAMBA_PYTHON()
>>> instead of changing each caller?
>>
>>
>> I considered doing that, but decided against it due to the fact that
>> this hides too much from the wscript_build of each module.  When a
>> bld.env.disable_python conditional exists it makes it clear that all
>> components the conditional applies to (whether SAMBA_PYTHON() or not,
>> and there's more than a few that aren't SAMBA_PYTHON()).
>>
>>
>>
>>> [...]
>>>
>>> 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.  
>
Makes sense.  Is there a particular use of this form already in the
build system that you would recommend I mirror for the implementation
of PYTHON_BUILD_IS_ENABLED()?  Or does waf already provide that simply
because the configure flag exists?

Also, does this mean that the various SAMBA_LIBRARY() modules I'm
turning off due to their dependency on, say, LIBPYTHON, will now also
need to be patched in C so that things won't fail?  Or will the module
still be skipped by the build system since 'enabled=' is false?


>>> I think we should also have the --disable-python option
>>> near to the --extra-python option within
>>> buildtools/wafsamba/wscript, so that it is also available
>>> for the standalone builds of talloc, tevent, tdb and ldb.
>>
>>
>> --disable-python is already in the standalone builds, I don't recall
>> patching that in except maybe for one of those; most if not all of
>> the
>> changes to those modules was to allow the pre-existing
>> --disable-python functionality to apply when it's _not_ being built
>> standalone.
>
> I think having this all in one place is still a good idea however.
>
I'll have to revisit the multi-python patchset to see where the
--extra-python options are going.  Aligning them should not be an
issue at all.


>>>
>>> For the samba build I think --disable-python give an error
>>> unless --without-ad-dc is also given as an ad-dc really requires
>>> python bindings.
>>
>>
>> I believe I tested this one and it works out fine
>
> I'm sorry, the Samba AD DC strictly requires python.  I don't know what
> you tested (presumably just that the binaries built), but building with
> the AD DC enabled needs to fail totally and early, to save everybody
> time and hair.
I tested that if ADDC was set to be "enabled" at configure time, that
the build process overall still successfully completed, yes.  As I
mentioned, likely ADDC was almost of not entirely skipped however and
certainly it (as well as likely other features) would not be usable in
any disable-python case.

SO, to accomplish what's described here:

(A) what other features are python-only or are otherwise useless to
build when there isn't any python, so I can turn them all off at the
same time?

(B) the action taken: to me --disable-python without any addc options
should just disable addc, while --disable-python and --with-addc
should error.  Does that make sense or should --disable-python always
error unless all of the dependent features are disabled?



signature.asc (201 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[PATCHv2 1/x] Re: Disabling Python Modules

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


01-global.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 29] Re: Disabling Python Modules

Andrew Bartlett
In reply to this post by Ian Stakenvicius
On Fri, 2017-01-27 at 11:36 -0500, 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:
> > > On 25/01/17 03:29 AM, Stefan Metzmacher wrote:
> > > > Hi Ian,
> > > >
> > > > > I can't be certain, but I think that may indeed be the last
> > > > > of
> > > > > them.
> > > > > All wscript_build files that contain modules with deps=
> > > > > including
> > > > > py*util, and LIBPYTHON, are now all covered.
> > > >
> > > > I know I'm late in the discussion, but is there a chance
> > > > that we could avoid all changes like this:
> > > >
> > > > -bld.SAMBA3_PYTHON('pys3param',
> > > > +if not bld.env.disable_python:
> > > > +    bld.SAMBA3_PYTHON('pys3param',
> > > >
> > > > And just to that globally within SAMBA_PYTHON()
> > > > instead of changing each caller?
> > >
> > >
> > > I considered doing that, but decided against it due to the fact
> > > that
> > > this hides too much from the wscript_build of each module.  When
> > > a
> > > bld.env.disable_python conditional exists it makes it clear that
> > > all
> > > components the conditional applies to (whether SAMBA_PYTHON() or
> > > not,
> > > and there's more than a few that aren't SAMBA_PYTHON()).
> > >
> > >
> > >
> > > > [...]
> > > >
> > > > 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.  
> >
>
> Makes sense.  Is there a particular use of this form already in the
> build system that you would recommend I mirror for the implementation
> of PYTHON_BUILD_IS_ENABLED()?  Or does waf already provide that
> simply
> because the configure flag exists?

The normal way would be to check:

bld.CONFIG_SET('HAVE_PYTHON')

In your first patch the detection is a little odd.  It seems we still
look for python even if disabled?

     conf.SAMBA_CHECK_PYTHON(mandatory=True, version=(2, 6, 0))
-    conf.SAMBA_CHECK_PYTHON_HEADERS(mandatory=True)
+    if conf.env.disable_python:
+        conf.SAMBA_CHECK_PYTHON_HEADERS(mandatory=False)
+    else:
+        conf.SAMBA_CHECK_PYTHON_HEADERS(mandatory=True)

Shouldn't it be:

if not conf.env.disable_python: 
  conf.SAMBA_CHECK_PYTHON(mandatory=True, version=(2, 6, 0))
  conf.SAMBA_CHECK_PYTHON_HEADERS(mandatory=True)

or such?

> Also, does this mean that the various SAMBA_LIBRARY() modules I'm
> turning off due to their dependency on, say, LIBPYTHON, will now also
> need to be patched in C so that things won't fail?  Or will the
> module
> still be skipped by the build system since 'enabled=' is false?

It may be cleaner to handle some of it there, rather than a cascade of
dependencies.  

See what still needs to be disabled once you do this, and once you no
longer need to care about stuff already disabled by --without-samba-ad-
dc

BTW I'm quite suspicious of

Subject: [PATCH 11/28] waf: disable-python - don't build libcli echo
torture test

What does this have to do with python?

> > > > I think we should also have the --disable-python option
> > > > near to the --extra-python option within
> > > > buildtools/wafsamba/wscript, so that it is also available
> > > > for the standalone builds of talloc, tevent, tdb and ldb.
> > >
> > >
> > > --disable-python is already in the standalone builds, I don't
> > > recall
> > > patching that in except maybe for one of those; most if not all
> > > of
> > > the
> > > changes to those modules was to allow the pre-existing
> > > --disable-python functionality to apply when it's _not_ being
> > > built
> > > standalone.
> >
> > I think having this all in one place is still a good idea however.
> >
>
> I'll have to revisit the multi-python patchset to see where the
> --extra-python options are going.  Aligning them should not be an
> issue at all.

Thanks.

> > > >
> > > > For the samba build I think --disable-python give an error
> > > > unless --without-ad-dc is also given as an ad-dc really
> > > > requires
> > > > python bindings.
> > >
> > >
> > > I believe I tested this one and it works out fine
> >
> > I'm sorry, the Samba AD DC strictly requires python.  I don't know
> > what
> > you tested (presumably just that the binaries built), but building
> > with
> > the AD DC enabled needs to fail totally and early, to save
> > everybody
> > time and hair.
>
> I tested that if ADDC was set to be "enabled" at configure time, that
> the build process overall still successfully completed, yes.  As I
> mentioned, likely ADDC was almost of not entirely skipped however and
> certainly it (as well as likely other features) would not be usable
> in
> any disable-python case.
>
> SO, to accomplish what's described here:
>
> (A) what other features are python-only or are otherwise useless to
> build when there isn't any python, so I can turn them all off at the
> same time?

So far just the AD DC.

> (B) the action taken: to me --disable-python without any addc options
> should just disable addc, while --disable-python and --with-addc
> should error.  Does that make sense or should --disable-python always
> error unless all of the dependent features are disabled?

--disable-python should error unless all of the dependent features are
disabled.  While it looks like this will become a supported feature,
the target is for folks already setting --without-ad-dc and I really
want folks to realise what they are missing out on.

If we don't, I'm quite confident we will get a user saying 'I built
with --disable-python because I don't like python, why can't I use the
AD DC!' ;-)

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


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 29] Re: Disabling Python Modules

Ian Stakenvicius
On 27/01/17 02:46 PM, Andrew Bartlett wrote:

> On Fri, 2017-01-27 at 11:36 -0500, 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:
>>>> On 25/01/17 03:29 AM, Stefan Metzmacher wrote:
>>>>> Hi Ian,
>>>>>
>>>>>> I can't be certain, but I think that may indeed be the last
>>>>>> of
>>>>>> them.
>>>>>> All wscript_build files that contain modules with deps=
>>>>>> including
>>>>>> py*util, and LIBPYTHON, are now all covered.
>>>>>
>>>>> I know I'm late in the discussion, but is there a chance
>>>>> that we could avoid all changes like this:
>>>>>
>>>>> -bld.SAMBA3_PYTHON('pys3param',
>>>>> +if not bld.env.disable_python:
>>>>> +    bld.SAMBA3_PYTHON('pys3param',
>>>>>
>>>>> And just to that globally within SAMBA_PYTHON()
>>>>> instead of changing each caller?
>>>>
>>>>
>>>> I considered doing that, but decided against it due to the fact
>>>> that
>>>> this hides too much from the wscript_build of each module.  When
>>>> a
>>>> bld.env.disable_python conditional exists it makes it clear that
>>>> all
>>>> components the conditional applies to (whether SAMBA_PYTHON() or
>>>> not,
>>>> and there's more than a few that aren't SAMBA_PYTHON()).
>>>>
>>>>
>>>>
>>>>> [...]
>>>>>
>>>>> 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.  
>>>
>>
>> Makes sense.  Is there a particular use of this form already in the
>> build system that you would recommend I mirror for the implementation
>> of PYTHON_BUILD_IS_ENABLED()?  Or does waf already provide that
>> simply
>> because the configure flag exists?
>
> The normal way would be to check:
>
> bld.CONFIG_SET('HAVE_PYTHON')
>
> In your first patch the detection is a little odd.  It seems we still
> look for python even if disabled?
>
>      conf.SAMBA_CHECK_PYTHON(mandatory=True, version=(2, 6, 0))
> -    conf.SAMBA_CHECK_PYTHON_HEADERS(mandatory=True)
> +    if conf.env.disable_python:
> +        conf.SAMBA_CHECK_PYTHON_HEADERS(mandatory=False)
> +    else:
> +        conf.SAMBA_CHECK_PYTHON_HEADERS(mandatory=True)
>
> Shouldn't it be:
>
> if not conf.env.disable_python:
>   conf.SAMBA_CHECK_PYTHON(mandatory=True, version=(2, 6, 0))
>   conf.SAMBA_CHECK_PYTHON_HEADERS(mandatory=True)
>
> or such?
>

When originally building the patchset, I only wanted to strip out
enough of the python dependencies such that it would build without a
python available on the target.  I barely understood waf at the time
(and am still lacking in knowledge now to be honest) and so I wasn't
sure how much the build system itself needed the results of these checks.

In general, with what i've learned to date, I think leaving the checks
for the python runtime is likely still a good idea; this requires
there to be a python available on the build system but that is
necessary for waf anyhow.  It's the header checks, in particular, that
we need to disable in order to bypass a target that doesn't have them
available.




>> Also, does this mean that the various SAMBA_LIBRARY() modules I'm
>> turning off due to their dependency on, say, LIBPYTHON, will now also
>> need to be patched in C so that things won't fail?  Or will the
>> module
>> still be skipped by the build system since 'enabled=' is false?
>
> It may be cleaner to handle some of it there, rather than a cascade of
> dependencies.  
>
> See what still needs to be disabled once you do this, and once you no
> longer need to care about stuff already disabled by --without-samba-ad-
> dc
>
> BTW I'm quite suspicious of
>
> Subject: [PATCH 11/28] waf: disable-python - don't build libcli echo
> torture test
>
> What does this have to do with python?
>
That may be a red herring -- as part of patch #1 in this set, I
skipped recursion into source4/torture due to all of the various
python requirements inside; I believe this made smbtorture
unavailable, which is required for that particular item.

Version 2 of the patchset will need a from-scratch rewrite and so I
will be able to confirm whether or not there is any python depenency
at that point.


>>
>> SO, to accomplish what's described here:
>>
>> (A) what other features are python-only or are otherwise useless to
>> build when there isn't any python, so I can turn them all off at the
>> same time?
>
> So far just the AD DC.
>
>> (B) the action taken: to me --disable-python without any addc options
>> should just disable addc, while --disable-python and --with-addc
>> should error.  Does that make sense or should --disable-python always
>> error unless all of the dependent features are disabled?
>
> --disable-python should error unless all of the dependent features are
> disabled.  While it looks like this will become a supported feature,
> the target is for folks already setting --without-ad-dc and I really
> want folks to realise what they are missing out on.
>
> If we don't, I'm quite confident we will get a user saying 'I built
> with --disable-python because I don't like python, why can't I use the
> AD DC!' ;-)
>
No problem.  I'll add this directly to the root wscript as this looks
to be where the ad-dc configure options are defined.


signature.asc (201 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCHv2 1/x] Re: Disabling Python Modules

Andrew Bartlett
In reply to this post by Ian Stakenvicius
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.

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


1234