Quantcast

[PATCH] Python3 compatible modules - credentials, param, _glue

classic Classic list List threaded Threaded
50 messages Options
123
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] Python3 compatible modules - credentials, param, _glue

Lumir Balhar
Hello.

The changes in this patchset port the samba.credentials, samba.param and
samba._glue modules to Python 3 compatible form. This patchset also
contains a
lot of new tests of mentioned modules. The file compat.py will help us
with porting and to make the future code more readable.

Have a nice day.
Lumír


0010-python-samba.tests-Enable-Python-3-tests-for-ported-.patch (1K) Download Attachment
0009-python-wscript_build-Build-some-modules-for-Python-3.patch (3K) Download Attachment
0008-python-Make-top-level-samba-modules-Python-3.patch (8K) Download Attachment
0007-python-samba.tests.dcerpc-Move-Class-RawDCERPCTest-t.patch (72K) Download Attachment
0006-python-samba.tests.glue-Add-new-tests-for-samba._glu.patch (3K) Download Attachment
0005-python-samba._glue-Port-samba._glue-module-to-Python.patch (3K) Download Attachment
0004-python-samba.tests.param-Add-missing-tests.patch (2K) Download Attachment
0003-python-samba.param-Port-param-module-to-Python-3.patch (13K) Download Attachment
0002-python-samba.tests.credentials-Python-3-compatible-t.patch (1K) Download Attachment
0001-python-samba.credentials-Port-pycredentials.c-to-Pyt.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Andrew Bartlett
On Tue, 2016-12-13 at 12:08 +0100, Lumir Balhar wrote:
> Hello.
>
> The changes in this patchset port the samba.credentials, samba.param
> and
> samba._glue modules to Python 3 compatible form. This patchset also 
> contains a
> lot of new tests of mentioned modules. The file compat.py will help
> us
> with porting and to make the future code more readable.

Thanks for all the hard work, and for addressing my feedback on the
exceptions.  I guess you could imagine that I'm hoping you can port
samba._ldb soon, so we can get that PY3 test out of samba/__init__.py.

I'll give this a closer look as soon as I can. 

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
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Lumir Balhar
On 12/14/2016 08:32 AM, Andrew Bartlett wrote:

> On Tue, 2016-12-13 at 12:08 +0100, Lumir Balhar wrote:
>> Hello.
>>
>> The changes in this patchset port the samba.credentials, samba.param
>> and
>> samba._glue modules to Python 3 compatible form. This patchset also
>> contains a
>> lot of new tests of mentioned modules. The file compat.py will help
>> us
>> with porting and to make the future code more readable.
> Thanks for all the hard work, and for addressing my feedback on the
> exceptions.  I guess you could imagine that I'm hoping you can port
> samba._ldb soon, so we can get that PY3 test out of samba/__init__.py.
>
> I'll give this a closer look as soon as I can.
>
> Thanks,
>
> Andrew Bartlett
>
Hello.

Thank you for starting the review.

I am currently working on the samba._ldb module, but it has a long
dependency line that includes auth, params, a lot of the RPC modules,
and more. Nevertheless, I've managed to port the dependency tree and it
seems to work.

But the next problem I have to solve is tests. The modules mentioned
above have no separate tests for each of them, and to run a more complex
test suites (like the raw test of DCERPC) I need to port more modules
which are included in this complex test suites -- for example NDR.

So, yes, I am working on it and it's progressing well, but it will take
some time to have all the test dependencies prepared and tested as well.

Thank you very much for your cooperation and have a nice day.
Lumír

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Andrew Bartlett
On Thu, 2016-12-22 at 14:34 +0100, Lumir Balhar wrote:

> On 12/14/2016 08:32 AM, Andrew Bartlett wrote:
> >
> > On Tue, 2016-12-13 at 12:08 +0100, Lumir Balhar wrote:
> > >
> > > Hello.
> > >
> > > The changes in this patchset port the samba.credentials,
> > > samba.param
> > > and
> > > samba._glue modules to Python 3 compatible form. This patchset
> > > also
> > > contains a
> > > lot of new tests of mentioned modules. The file compat.py will
> > > help
> > > us
> > > with porting and to make the future code more readable.
> > Thanks for all the hard work, and for addressing my feedback on the
> > exceptions.  I guess you could imagine that I'm hoping you can port
> > samba._ldb soon, so we can get that PY3 test out of
> > samba/__init__.py.
> >
> > I'll give this a closer look as soon as I can.
> >
> > Thanks,
> >
> > Andrew Bartlett
> >
> Hello.
>
> Thank you for starting the review.
>
> I am currently working on the samba._ldb module, but it has a long 
> dependency line that includes auth, params, a lot of the RPC
> modules, 
> and more.

Drat.  I hadn't spotted that. 

> Nevertheless, I've managed to port the dependency tree and it 
> seems to work.

Wow!

> But the next problem I have to solve is tests. The modules mentioned 
> above have no separate tests for each of them, and to run a more
> complex 
> test suites (like the raw test of DCERPC) I need to port more
> modules 
> which are included in this complex test suites -- for example NDR.
>
> So, yes, I am working on it and it's progressing well, but it will
> take 
> some time to have all the test dependencies prepared and tested as
> well.
>
> Thank you very much for your cooperation and have a nice day.
> Lumír

Thank you for all your hard work!

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
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Andrew Bartlett
On Fri, 2016-12-23 at 06:16 +1300, Andrew Bartlett wrote:

> On Thu, 2016-12-22 at 14:34 +0100, Lumir Balhar wrote:
> >
> > On 12/14/2016 08:32 AM, Andrew Bartlett wrote:
> > >
> > >
> > > On Tue, 2016-12-13 at 12:08 +0100, Lumir Balhar wrote:
> > > >
> > > >
> > > > Hello.
> > > >
> > > > The changes in this patchset port the samba.credentials,
> > > > samba.param
> > > > and
> > > > samba._glue modules to Python 3 compatible form. This patchset
> > > > also
> > > > contains a
> > > > lot of new tests of mentioned modules. The file compat.py will
> > > > help
> > > > us
> > > > with porting and to make the future code more readable.
> > > Thanks for all the hard work, and for addressing my feedback on
> > > the
> > > exceptions.  I guess you could imagine that I'm hoping you can
> > > port
> > > samba._ldb soon, so we can get that PY3 test out of
> > > samba/__init__.py.
> > >
> > > I'll give this a closer look as soon as I can.
> > >
> > > Thanks,
> > >
> > > Andrew Bartlett
> > >
> > Hello.
> >
> > Thank you for starting the review.
> >
> > I am currently working on the samba._ldb module, but it has a long 
> > dependency line that includes auth, params, a lot of the RPC
> > modules, 
> > and more. 
>
> Drat.  I hadn't spotted that. 
>
> >
> > Nevertheless, I've managed to port the dependency tree and it 
> > seems to work.
>
> Wow!
>
> >
> > But the next problem I have to solve is tests. The modules
> > mentioned 
> > above have no separate tests for each of them, and to run a more
> > complex 
> > test suites (like the raw test of DCERPC) I need to port more
> > modules 
> > which are included in this complex test suites -- for example NDR.
> >
> > So, yes, I am working on it and it's progressing well, but it will
> > take 
> > some time to have all the test dependencies prepared and tested as
> > well.
> >
> > Thank you very much for your cooperation and have a nice day.
> > Lumír
>
> Thank you for all your hard work!
>
> Andrew Bartlett

BTW, if writing new tests, then a test of pyauth to confirm that
system_session() et al return reasonable structures would be really
cool.  On the flip side, the py_auth_context_new() call is at least a
little tested by the gensec tests, so that may not be as painful.

Now you are getting to the hard stuff - the PIDL generated NDR code.  I
think you would agree that where this where it gets difficult, but it
is also essentially the finish line as I see it.  (There really isn't
much else, is there?). 

Perhaps skip the raw DCE/RPC tests, add some expected value testing to
samba/tests/auth.py and run samba/tests/gensec.py?  

We can cut the dependency chain at pyrpc_util.c by cutting the
NDR/python helpers into pyndr_util_import_export.c (including 
py_return_ndr_struct() / pyrpc_{import,export}_union()).  That way you
don't need to port and convert py_dcerpc_run_function(). 

Would this cover enough to make this work, eliminate IS_PY3 in the
runtime code, but not require you to convert the world at once?

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
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Lumir Balhar
On 12/22/2016 07:00 PM, Andrew Bartlett wrote:

> On Fri, 2016-12-23 at 06:16 +1300, Andrew Bartlett wrote:
>> On Thu, 2016-12-22 at 14:34 +0100, Lumir Balhar wrote:
>>> On 12/14/2016 08:32 AM, Andrew Bartlett wrote:
>>>>
>>>> On Tue, 2016-12-13 at 12:08 +0100, Lumir Balhar wrote:
>>>>>
>>>>> Hello.
>>>>>
>>>>> The changes in this patchset port the samba.credentials,
>>>>> samba.param
>>>>> and
>>>>> samba._glue modules to Python 3 compatible form. This patchset
>>>>> also
>>>>> contains a
>>>>> lot of new tests of mentioned modules. The file compat.py will
>>>>> help
>>>>> us
>>>>> with porting and to make the future code more readable.
>>>> Thanks for all the hard work, and for addressing my feedback on
>>>> the
>>>> exceptions.  I guess you could imagine that I'm hoping you can
>>>> port
>>>> samba._ldb soon, so we can get that PY3 test out of
>>>> samba/__init__.py.
>>>>
>>>> I'll give this a closer look as soon as I can.
>>>>
>>>> Thanks,
>>>>
>>>> Andrew Bartlett
>>>>
>>> Hello.
>>>
>>> Thank you for starting the review.
>>>
>>> I am currently working on the samba._ldb module, but it has a long
>>> dependency line that includes auth, params, a lot of the RPC
>>> modules,
>>> and more.
>> Drat.  I hadn't spotted that.
>>
>>> Nevertheless, I've managed to port the dependency tree and it
>>> seems to work.
>> Wow!
>>
>>> But the next problem I have to solve is tests. The modules
>>> mentioned
>>> above have no separate tests for each of them, and to run a more
>>> complex
>>> test suites (like the raw test of DCERPC) I need to port more
>>> modules
>>> which are included in this complex test suites -- for example NDR.
>>>
>>> So, yes, I am working on it and it's progressing well, but it will
>>> take
>>> some time to have all the test dependencies prepared and tested as
>>> well.
>>>
>>> Thank you very much for your cooperation and have a nice day.
>>> Lumír
>> Thank you for all your hard work!
>>
>> Andrew Bartlett
> BTW, if writing new tests, then a test of pyauth to confirm that
> system_session() et al return reasonable structures would be really
> cool.  On the flip side, the py_auth_context_new() call is at least a
> little tested by the gensec tests, so that may not be as painful.
Ok, perfect. I'll prepare some tests for testing result of
system_session() and underlying arguments, objects and values.
> Now you are getting to the hard stuff - the PIDL generated NDR code.  I
> think you would agree that where this where it gets difficult, but it
> is also essentially the finish line as I see it.  (There really isn't
> much else, is there?).
I already met PIDL generated stuff during porting of DCE/RPC so it won't
be new for me. My next patchset will contain a lot of changes in PIDL
Python parser.
> Perhaps skip the raw DCE/RPC tests, add some expected value testing to
> samba/tests/auth.py and run samba/tests/gensec.py?
Ok, I can skip tests of DCE/RPC and run tests of gensec but first I need
to port samba.gensec module and samba.tests.gensec. It looks like that
it will be simple without another dependency chain.
> We can cut the dependency chain at pyrpc_util.c by cutting the
> NDR/python helpers into pyndr_util_import_export.c (including
> py_return_ndr_struct() / pyrpc_{import,export}_union()).  That way you
> don't need to port and convert py_dcerpc_run_function().
>
> Would this cover enough to make this work, eliminate IS_PY3 in the
> runtime code, but not require you to convert the world at once?
What do you prefer? Do you want to review the current patchset with
credentials, param and _glue modules or should I do rest of work on auth
tests and gensec and send one big patchset which will contain all these
modules at once?
> Thanks,
>
> Andrew Bartlett
>
Thank you and have a nice day.
Lumír

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Andrew Bartlett
On Thu, 2017-01-05 at 14:01 +0100, Lumir Balhar wrote:

> On 12/22/2016 07:00 PM, Andrew Bartlett wrote:
> >
> > On Fri, 2016-12-23 at 06:16 +1300, Andrew Bartlett wrote:
> > >
> > > On Thu, 2016-12-22 at 14:34 +0100, Lumir Balhar wrote:
> > > >
> > > > On 12/14/2016 08:32 AM, Andrew Bartlett wrote:
> > > > >
> > > > >
> > > > > On Tue, 2016-12-13 at 12:08 +0100, Lumir Balhar wrote:
> > > > > >
> > > > > >
> > > > > > Hello.
> > > > > >
> > > > > > The changes in this patchset port the samba.credentials,
> > > > > > samba.param
> > > > > > and
> > > > > > samba._glue modules to Python 3 compatible form. This
> > > > > > patchset
> > > > > > also
> > > > > > contains a
> > > > > > lot of new tests of mentioned modules. The file compat.py
> > > > > > will
> > > > > > help
> > > > > > us
> > > > > > with porting and to make the future code more readable.
> > > > > Thanks for all the hard work, and for addressing my feedback
> > > > > on
> > > > > the
> > > > > exceptions.  I guess you could imagine that I'm hoping you
> > > > > can
> > > > > port
> > > > > samba._ldb soon, so we can get that PY3 test out of
> > > > > samba/__init__.py.
> > > > >
> > > > > I'll give this a closer look as soon as I can.
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Andrew Bartlett
> > > > >
> > > > Hello.
> > > >
> > > > Thank you for starting the review.
> > > >
> > > > I am currently working on the samba._ldb module, but it has a
> > > > long
> > > > dependency line that includes auth, params, a lot of the RPC
> > > > modules,
> > > > and more.
> > > Drat.  I hadn't spotted that.
> > >
> > > >
> > > > Nevertheless, I've managed to port the dependency tree and it
> > > > seems to work.
> > > Wow!
> > >
> > > >
> > > > But the next problem I have to solve is tests. The modules
> > > > mentioned
> > > > above have no separate tests for each of them, and to run a
> > > > more
> > > > complex
> > > > test suites (like the raw test of DCERPC) I need to port more
> > > > modules
> > > > which are included in this complex test suites -- for example
> > > > NDR.
> > > >
> > > > So, yes, I am working on it and it's progressing well, but it
> > > > will
> > > > take
> > > > some time to have all the test dependencies prepared and tested
> > > > as
> > > > well.
> > > >
> > > > Thank you very much for your cooperation and have a nice day.
> > > > Lumír
> > > Thank you for all your hard work!
> > >
> > > Andrew Bartlett
> > BTW, if writing new tests, then a test of pyauth to confirm that
> > system_session() et al return reasonable structures would be really
> > cool.  On the flip side, the py_auth_context_new() call is at least
> > a
> > little tested by the gensec tests, so that may not be as painful.
> Ok, perfect. I'll prepare some tests for testing result of 
> system_session() and underlying arguments, objects and values.
> >
> > Now you are getting to the hard stuff - the PIDL generated NDR
> > code.  I
> > think you would agree that where this where it gets difficult, but
> > it
> > is also essentially the finish line as I see it.  (There really
> > isn't
> > much else, is there?).
> I already met PIDL generated stuff during porting of DCE/RPC so it
> won't 
> be new for me. My next patchset will contain a lot of changes in
> PIDL 
> Python parser.

That will be interesting :-)

> >
> > Perhaps skip the raw DCE/RPC tests, add some expected value testing
> > to
> > samba/tests/auth.py and run samba/tests/gensec.py?
> Ok, I can skip tests of DCE/RPC and run tests of gensec but first I
> need 
> to port samba.gensec module and samba.tests.gensec. It looks like
> that 
> it will be simple without another dependency chain.

Good!

> >
> > We can cut the dependency chain at pyrpc_util.c by cutting the
> > NDR/python helpers into pyndr_util_import_export.c (including
> > py_return_ndr_struct() / pyrpc_{import,export}_union()).  That way
> > you
> > don't need to port and convert py_dcerpc_run_function().
> >
> > Would this cover enough to make this work, eliminate IS_PY3 in the
> > runtime code, but not require you to convert the world at once?
> What do you prefer? Do you want to review the current patchset with 
> credentials, param and _glue modules or should I do rest of work on
> auth 
> tests and gensec and send one big patchset which will contain all
> these 
> modules at once?

I'm pretty happy with those changes, so don't expect me to come back
with big objections later.  If you could build on those with the larger
patch set, that will be perfect. 

I know it is much more satisfying to get the code merged, but I really
don't want to make a release with IS_PY3 outside the test code. 

> >
> > Thanks,
> >
> > Andrew Bartlett
> >
> Thank you and have a nice day.
> Lumír

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
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Lumir Balhar
> On 01/05/2017 07:10 PM, Andrew Bartlett wrote:
>> On Thu, 2017-01-05 at 14:01 +0100, Lumir Balhar wrote:
>>> On 12/22/2016 07:00 PM, Andrew Bartlett wrote:
[...]
>>> Perhaps skip the raw DCE/RPC tests, add some expected value testing
>>> to
>>> samba/tests/auth.py and run samba/tests/gensec.py?
>> Ok, I can skip tests of DCE/RPC and run tests of gensec but first I
>> need
>> to port samba.gensec module and samba.tests.gensec. It looks like
>> that
>> it will be simple without another dependency chain.
> Good!

It seems that my expectations were wrong. I found out that tests of the
gensec module are planned old way with planoldpythontestsuite() function
which is not prepared for Python 3 yet.
Of course, I can port this functionality as well but I've discovered
another long dependency chain which goes through modules getopt,
hostconfig, dsdb, dsdb_dns and maybe more - so it will take some time to
do it and it will add other changes to patchset.

This is just a heads-up that the patchset will be even larger than
expected, unless you see a different way forward.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Andrew Bartlett
On Fri, 2017-01-13 at 12:01 +0100, Lumir Balhar wrote:

> > On 01/05/2017 07:10 PM, Andrew Bartlett wrote: 
> > > On Thu, 2017-01-05 at 14:01 +0100, Lumir Balhar wrote: 
> > > > On 12/22/2016 07:00 PM, Andrew Bartlett wrote: 
>  [...] 
> > > > Perhaps skip the raw DCE/RPC tests, add some expected value
> > > > testing 
> > > > to 
> > > > samba/tests/auth.py and run samba/tests/gensec.py? 
> > >  Ok, I can skip tests of DCE/RPC and run tests of gensec but
> > > first I 
> > > need 
> > > to port samba.gensec module and samba.tests.gensec. It looks
> > > like 
> > > that 
> > > it will be simple without another dependency chain. 
> >  Good! 
>  
> It seems that my expectations were wrong. I found out that tests of
> the 
> gensec module are planned old way with planoldpythontestsuite()
> function 
> which is not prepared for Python 3 yet. 
> Of course, I can port this functionality as well but I've discovered 
> another long dependency chain which goes through modules getopt, 
> hostconfig, dsdb, dsdb_dns and maybe more - so it will take some time
> to 
> do it and it will add other changes to patchset. 
>
> This is just a heads-up that the patchset will be even larger than
> expected, unless you see a different way forward. 

I do.  

You should be able to prove to yourself that HostConfig is unused,
except for tests, but the easiest course of action is to remove
get_hostconfig() from the getopt.py script, as that is both unused and
untested.

The HostConfig abstraction never really took off, despite looking like
a reasonable idea, so you can also remove the get_samdb() call from it
as well if that is helpful somehow.

That should cut down the dep chain for now.  When we get back to it, it
looks like there is some other code here we should remove or finally
use.

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
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Andrew Bartlett
On Sat, 2017-01-14 at 07:05 +1300, Andrew Bartlett wrote:

> On Fri, 2017-01-13 at 12:01 +0100, Lumir Balhar wrote:
> > > On 01/05/2017 07:10 PM, Andrew Bartlett wrote: 
> > > > On Thu, 2017-01-05 at 14:01 +0100, Lumir Balhar wrote: 
> > > > > On 12/22/2016 07:00 PM, Andrew Bartlett wrote: 
> >
> >  [...] 
> > > > > Perhaps skip the raw DCE/RPC tests, add some expected value
> > > > > testing 
> > > > > to 
> > > > > samba/tests/auth.py and run samba/tests/gensec.py? 
> > > >
> > > >  Ok, I can skip tests of DCE/RPC and run tests of gensec but
> > > > first I 
> > > > need 
> > > > to port samba.gensec module and samba.tests.gensec. It looks
> > > > like 
> > > > that 
> > > > it will be simple without another dependency chain. 
> > >
> > >  Good! 
> >
> >  
> > It seems that my expectations were wrong. I found out that tests of
> > the 
> > gensec module are planned old way with planoldpythontestsuite()
> > function 
> > which is not prepared for Python 3 yet. 
> > Of course, I can port this functionality as well but I've
> > discovered 
> > another long dependency chain which goes through modules getopt, 
> > hostconfig, dsdb, dsdb_dns and maybe more - so it will take some
> > time
> > to 
> > do it and it will add other changes to patchset. 
> >
> > This is just a heads-up that the patchset will be even larger than
> > expected, unless you see a different way forward. 
>
> I do.  
>
> You should be able to prove to yourself that HostConfig is unused,
> except for tests, but the easiest course of action is to remove
> get_hostconfig() from the getopt.py script, as that is both unused
> and
> untested.
>
> The HostConfig abstraction never really took off, despite looking
> like
> a reasonable idea, so you can also remove the get_samdb() call from
> it
> as well if that is helpful somehow.
>
> That should cut down the dep chain for now.  When we get back to it,
> it
> looks like there is some other code here we should remove or finally
> use. 

Do you have an updated patch set?  I'm hoping we can land this without
a disastrous conflict with the --disable-python patch set, and it would
be helpful to see the current state of play.

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
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Lumir Balhar
On 01/27/2017 07:17 AM, Andrew Bartlett wrote:

> On Sat, 2017-01-14 at 07:05 +1300, Andrew Bartlett wrote:
>> On Fri, 2017-01-13 at 12:01 +0100, Lumir Balhar wrote:
>>>> On 01/05/2017 07:10 PM, Andrew Bartlett wrote:
>>>>> On Thu, 2017-01-05 at 14:01 +0100, Lumir Balhar wrote:
>>>>>> On 12/22/2016 07:00 PM, Andrew Bartlett wrote:
>>>   [...]
>>>>>> Perhaps skip the raw DCE/RPC tests, add some expected value
>>>>>> testing
>>>>>> to
>>>>>> samba/tests/auth.py and run samba/tests/gensec.py?
>>>>>   Ok, I can skip tests of DCE/RPC and run tests of gensec but
>>>>> first I
>>>>> need
>>>>> to port samba.gensec module and samba.tests.gensec. It looks
>>>>> like
>>>>> that
>>>>> it will be simple without another dependency chain.
>>>>   Good!
>>>  
>>> It seems that my expectations were wrong. I found out that tests of
>>> the
>>> gensec module are planned old way with planoldpythontestsuite()
>>> function
>>> which is not prepared for Python 3 yet.
>>> Of course, I can port this functionality as well but I've
>>> discovered
>>> another long dependency chain which goes through modules getopt,
>>> hostconfig, dsdb, dsdb_dns and maybe more - so it will take some
>>> time
>>> to
>>> do it and it will add other changes to patchset.
>>>
>>> This is just a heads-up that the patchset will be even larger than
>>> expected, unless you see a different way forward.
>> I do.
>>
>> You should be able to prove to yourself that HostConfig is unused,
>> except for tests, but the easiest course of action is to remove
>> get_hostconfig() from the getopt.py script, as that is both unused
>> and
>> untested.
>>
>> The HostConfig abstraction never really took off, despite looking
>> like
>> a reasonable idea, so you can also remove the get_samdb() call from
>> it
>> as well if that is helpful somehow.
>>
>> That should cut down the dep chain for now.  When we get back to it,
>> it
>> looks like there is some other code here we should remove or finally
>> use.
> Do you have an updated patch set?  I'm hoping we can land this without
> a disastrous conflict with the --disable-python patch set, and it would
> be helpful to see the current state of play.
>
> Thanks,
>
> Andrew Bartlett
>
Hello.

Sorry for later reply but I was on DevConf in Brno and I was a bit sick
before it.

Here is updated patchset. It contains a lot of changes discussed before,
fixes, new tests and more.

Commit fc31cd9 (patch 12) adds one static function to the header file
py3compat.h, which is not the best but I cannot find a better solution
here because I need to redefine two function calls and I can't do it
just with macro. Another possible solution for this problem is to use
#if/#else/#endif everywhere is PyCObject_FromVoidPtrAndDesc used
(including in the PIDL generator).

I hope that everything will be good for you.

Have a nice day.
Lumír

0027-python-selftest-Add-possibility-to-run-old-Python-te.patch (3K) Download Attachment
0026-python-samba.gensec-Port-module-to-Python-3-compatib.patch (10K) Download Attachment
0025-python-samba.gensec-Fix-error-handling-in-set_creden.patch (996 bytes) Download Attachment
0024-python-selftests-Enable-samba.getopt-tests-execution.patch (1K) Download Attachment
0023-python-samba.getopt-Port-module-to-Python-3-compatib.patch (1K) Download Attachment
0022-python-samba.tests.core-Port-and-enable-core-tests-i.patch (2K) Download Attachment
0021-python-samba.tests-Move-import-of-ported-modules-out.patch (1012 bytes) Download Attachment
0020-python-samba._ldb-Port-of-samba._ldb-to-Python-3-com.patch (3K) Download Attachment
0019-python-samba.tests.auth-Add-tests-for-samba.auth-mod.patch (2K) Download Attachment
0018-python-samba.auth-Port-samba.auth-to-Python-3-compat.patch (5K) Download Attachment
0017-python-wscript_build-Build-some-DCE-RPC-modules-with.patch (6K) Download Attachment
0016-python-samba.dcerpc-Port-security-module-to-Python-3.patch (3K) Download Attachment
0015-python-samba.tests.dcerpc.misc-Port-and-enable-tests.patch (2K) Download Attachment
0014-python-samba.dcerpc-Port-RPC-related-stuff-to-Python.patch (15K) Download Attachment
0013-python-pidl-Port-Python-interface-generator.patch (15K) Download Attachment
0012-python-Add-PyCapsule-related-macros-and-function-to-.patch (2K) Download Attachment
0011-pytalloc-Add-new-function-pytalloc_PyCapsule_FromTal.patch (2K) Download Attachment
0010-python-samba.tests-Enable-Python-3-tests-for-ported-.patch (2K) Download Attachment
0009-python-wscript_build-Build-some-modules-for-Python-3.patch (3K) Download Attachment
0008-python-Make-top-level-samba-modules-Python-3.patch (8K) Download Attachment
0007-python-samba.tests.dcerpc-Move-Class-RawDCERPCTest-t.patch (72K) Download Attachment
0006-python-samba.tests.glue-Add-new-tests-for-samba._glu.patch (3K) Download Attachment
0005-python-samba._glue-Port-samba._glue-module-to-Python.patch (3K) Download Attachment
0004-python-samba.tests.param-Add-missing-tests.patch (2K) Download Attachment
0003-python-samba.param-Port-param-module-to-Python-3.patch (13K) Download Attachment
0002-python-samba.tests.credentials-Python-3-compatible-t.patch (5K) Download Attachment
0001-python-samba.credentials-Port-pycredentials.c-to-Pyt.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Andrew Bartlett
On Mon, 2017-01-30 at 16:59 +0100, Lumir Balhar wrote:

> On 01/27/2017 07:17 AM, Andrew Bartlett wrote:
> > On Sat, 2017-01-14 at 07:05 +1300, Andrew Bartlett wrote:
> > > On Fri, 2017-01-13 at 12:01 +0100, Lumir Balhar wrote:
> > > > > On 01/05/2017 07:10 PM, Andrew Bartlett wrote:
> > > > > > On Thu, 2017-01-05 at 14:01 +0100, Lumir Balhar wrote:
> > > > > > > On 12/22/2016 07:00 PM, Andrew Bartlett wrote:
> > > >
> > > >   [...]
> > > > > > > Perhaps skip the raw DCE/RPC tests, add some expected
> > > > > > > value
> > > > > > > testing
> > > > > > > to
> > > > > > > samba/tests/auth.py and run samba/tests/gensec.py?
> > > > > >
> > > > > >   Ok, I can skip tests of DCE/RPC and run tests of gensec
> > > > > > but
> > > > > > first I
> > > > > > need
> > > > > > to port samba.gensec module and samba.tests.gensec. It
> > > > > > looks
> > > > > > like
> > > > > > that
> > > > > > it will be simple without another dependency chain.
> > > > >
> > > > >   Good!
> > > >
> > > >   
> > > > It seems that my expectations were wrong. I found out that
> > > > tests of
> > > > the
> > > > gensec module are planned old way with planoldpythontestsuite()
> > > > function
> > > > which is not prepared for Python 3 yet.
> > > > Of course, I can port this functionality as well but I've
> > > > discovered
> > > > another long dependency chain which goes through modules
> > > > getopt,
> > > > hostconfig, dsdb, dsdb_dns and maybe more - so it will take
> > > > some
> > > > time
> > > > to
> > > > do it and it will add other changes to patchset.
> > > >
> > > > This is just a heads-up that the patchset will be even larger
> > > > than
> > > > expected, unless you see a different way forward.
> > >
> > > I do.
> > >
> > > You should be able to prove to yourself that HostConfig is
> > > unused,
> > > except for tests, but the easiest course of action is to remove
> > > get_hostconfig() from the getopt.py script, as that is both
> > > unused
> > > and
> > > untested.
> > >
> > > The HostConfig abstraction never really took off, despite looking
> > > like
> > > a reasonable idea, so you can also remove the get_samdb() call
> > > from
> > > it
> > > as well if that is helpful somehow.
> > >
> > > That should cut down the dep chain for now.  When we get back to
> > > it,
> > > it
> > > looks like there is some other code here we should remove or
> > > finally
> > > use.
> >
> > Do you have an updated patch set?  I'm hoping we can land this
> > without
> > a disastrous conflict with the --disable-python patch set, and it
> > would
> > be helpful to see the current state of play.
> >
> > Thanks,
> >
> > Andrew Bartlett
> >
>
> Hello.
>
> Sorry for later reply but I was on DevConf in Brno and I was a bit
> sick 
> before it.
>
> Here is updated patchset. It contains a lot of changes discussed
> before, 
> fixes, new tests and more.
>
> Commit fc31cd9 (patch 12) adds one static function to the header
> file 
> py3compat.h, which is not the best but I cannot find a better
> solution 
> here because I need to redefine two function calls and I can't do it 
> just with macro. Another possible solution for this problem is to
> use 
> #if/#else/#endif everywhere is PyCObject_FromVoidPtrAndDesc used 
> (including in the PIDL generator).
>
> I hope that everything will be good for you.

This is really good.  There is a stray PY3 import into
samba/__init__.py

The thing that scared me more was the regular expression for expanding
the code to call PyCObject_FromVoidPtrAndDesc in pidl :-)

I can see why it was done however, and I don't really mine.

I would really appreciate a second or more set of eyes over this, and
I'll get you a formal review shortly after more examination, but I'm
reasonably comfortable with how this is landing!

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
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Andrew Bartlett
On Tue, 2017-01-31 at 08:24 +1300, Andrew Bartlett wrote:

> On Mon, 2017-01-30 at 16:59 +0100, Lumir Balhar wrote:
> > On 01/27/2017 07:17 AM, Andrew Bartlett wrote:
> > > On Sat, 2017-01-14 at 07:05 +1300, Andrew Bartlett wrote:
> > > > On Fri, 2017-01-13 at 12:01 +0100, Lumir Balhar wrote:
> > > > > > On 01/05/2017 07:10 PM, Andrew Bartlett wrote:
> > > > > > > On Thu, 2017-01-05 at 14:01 +0100, Lumir Balhar wrote:
> > > > > > > > On 12/22/2016 07:00 PM, Andrew Bartlett wrote:
> > > > >
> > > > >   [...]
> > > > > > > > Perhaps skip the raw DCE/RPC tests, add some expected
> > > > > > > > value
> > > > > > > > testing
> > > > > > > > to
> > > > > > > > samba/tests/auth.py and run samba/tests/gensec.py?
> > > > > > >
> > > > > > >   Ok, I can skip tests of DCE/RPC and run tests of gensec
> > > > > > > but
> > > > > > > first I
> > > > > > > need
> > > > > > > to port samba.gensec module and samba.tests.gensec. It
> > > > > > > looks
> > > > > > > like
> > > > > > > that
> > > > > > > it will be simple without another dependency chain.
> > > > > >
> > > > > >   Good!
> > > > >
> > > > >   
> > > > > It seems that my expectations were wrong. I found out that
> > > > > tests of
> > > > > the
> > > > > gensec module are planned old way with
> > > > > planoldpythontestsuite()
> > > > > function
> > > > > which is not prepared for Python 3 yet.
> > > > > Of course, I can port this functionality as well but I've
> > > > > discovered
> > > > > another long dependency chain which goes through modules
> > > > > getopt,
> > > > > hostconfig, dsdb, dsdb_dns and maybe more - so it will take
> > > > > some
> > > > > time
> > > > > to
> > > > > do it and it will add other changes to patchset.
> > > > >
> > > > > This is just a heads-up that the patchset will be even larger
> > > > > than
> > > > > expected, unless you see a different way forward.
> > > >
> > > > I do.
> > > >
> > > > You should be able to prove to yourself that HostConfig is
> > > > unused,
> > > > except for tests, but the easiest course of action is to remove
> > > > get_hostconfig() from the getopt.py script, as that is both
> > > > unused
> > > > and
> > > > untested.
> > > >
> > > > The HostConfig abstraction never really took off, despite
> > > > looking
> > > > like
> > > > a reasonable idea, so you can also remove the get_samdb() call
> > > > from
> > > > it
> > > > as well if that is helpful somehow.
> > > >
> > > > That should cut down the dep chain for now.  When we get back
> > > > to
> > > > it,
> > > > it
> > > > looks like there is some other code here we should remove or
> > > > finally
> > > > use.
> > >
> > > Do you have an updated patch set?  I'm hoping we can land this
> > > without
> > > a disastrous conflict with the --disable-python patch set, and it
> > > would
> > > be helpful to see the current state of play.
> > >
> > > Thanks,
> > >
> > > Andrew Bartlett
> > >
> >
> > Hello.
> >
> > Sorry for later reply but I was on DevConf in Brno and I was a bit
> > sick 
> > before it.
> >
> > Here is updated patchset. It contains a lot of changes discussed
> > before, 
> > fixes, new tests and more.
> >
> > Commit fc31cd9 (patch 12) adds one static function to the header
> > file 
> > py3compat.h, which is not the best but I cannot find a better
> > solution 
> > here because I need to redefine two function calls and I can't do
> > it 
> > just with macro. Another possible solution for this problem is to
> > use 
> > #if/#else/#endif everywhere is PyCObject_FromVoidPtrAndDesc used 
> > (including in the PIDL generator).
> >
> > I hope that everything will be good for you.
>
> This is really good.  There is a stray PY3 import into
> samba/__init__.py
>
> The thing that scared me more was the regular expression for
> expanding
> the code to call PyCObject_FromVoidPtrAndDesc in pidl :-)
>
> I can see why it was done however, and I don't really mine.
>
> I would really appreciate a second or more set of eyes over this, and
> I'll get you a formal review shortly after more examination, but I'm
> reasonably comfortable with how this is landing!

I'm hitting (due to our strict build, please build with --enable-
developer):

../lib/talloc/pytalloc_util.c: In function
‘pytalloc_PyCapsule_FromTallocPtr’:
../lib/talloc/pytalloc_util.c:201:34: error: passing argument 3 of
‘PyCapsule_New’ from incompatible pointer type [-Werror=incompatible-
pointer-types]
  return PyCapsule_New(ptr, NULL, py_cobject_talloc_free);
                                  ^~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/python3.5m/Python.h:96:0,
                 from ../lib/talloc/pytalloc_util.c:20:
/usr/include/python3.5m/pycapsule.h:28:24: note: expected
‘PyCapsule_Destructor {aka void (*)(struct _object *)}’ but argument is
of type ‘void (*)(void *)’
 PyAPI_FUNC(PyObject *) PyCapsule_New(

This is because we are so strict we care about the difference between
fn(void *) and fn(PyObject *).

The other issue I hit is:
                        ^~~~~~~~~~~~~
In file included from ../source4/param/pyparam_util.c:21:0:
../python/py3compat.h:179:0: error: "PyCapsule_CheckExact" redefined [-
Werror]
 #define PyCapsule_CheckExact(capsule) (PyCObject_Check(capsule))
 
I've started a thread to have us move to just using PyCapsule by
requiring 2.7.  I think this is likely to get up, but we will know
soon.  This will kill a number of birds with one stone, so can you re-
work the patches to rely on PyCapsule?

Thanks,

Andrew Bartlett

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Andrew Bartlett
On Thu, 2017-02-09 at 14:20 +1300, Andrew Bartlett wrote:

>
> > This is really good.  There is a stray PY3 import into
> > samba/__init__.py
> >
> > The thing that scared me more was the regular expression for
> > expanding
> > the code to call PyCObject_FromVoidPtrAndDesc in pidl :-)
> >
> > I can see why it was done however, and I don't really mine.
> >
> > I would really appreciate a second or more set of eyes over this,
> > and
> > I'll get you a formal review shortly after more examination, but
> > I'm
> > reasonably comfortable with how this is landing!
>
> I'm hitting (due to our strict build, please build with --enable-
> developer):
>
> ../lib/talloc/pytalloc_util.c: In function
> ‘pytalloc_PyCapsule_FromTallocPtr’:
> ../lib/talloc/pytalloc_util.c:201:34: error: passing argument 3 of
> ‘PyCapsule_New’ from incompatible pointer type [-Werror=incompatible-
> pointer-types]
>   return PyCapsule_New(ptr, NULL, py_cobject_talloc_free);
>                                   ^~~~~~~~~~~~~~~~~~~~~~
> In file included from /usr/include/python3.5m/Python.h:96:0,
>                  from ../lib/talloc/pytalloc_util.c:20:
> /usr/include/python3.5m/pycapsule.h:28:24: note: expected
> ‘PyCapsule_Destructor {aka void (*)(struct _object *)}’ but argument
> is
> of type ‘void (*)(void *)’
>  PyAPI_FUNC(PyObject *) PyCapsule_New(
>
> This is because we are so strict we care about the difference between
> fn(void *) and fn(PyObject *).
>
> The other issue I hit is: 
>                         ^~~~~~~~~~~~~
> In file included from ../source4/param/pyparam_util.c:21:0:
> ../python/py3compat.h:179:0: error: "PyCapsule_CheckExact" redefined
> [-
> Werror]
>  #define PyCapsule_CheckExact(capsule) (PyCObject_Check(capsule))
>  
> I've started a thread to have us move to just using PyCapsule by
> requiring 2.7.  I think this is likely to get up, but we will know
> soon.  This will kill a number of birds with one stone, so can you
> re-
> work the patches to rely on PyCapsule?
I've decided to have a stab at this.  Please look at this rough re-
working of your patch set, and if you are happy I'll tidy it up and get
some review into master.

Thanks!

Andrew Bartlett

py3.patch (275K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Petr Viktorin
In reply to this post by Andrew Bartlett
On 02/09/2017 02:20 AM, Andrew Bartlett wrote:
> On Tue, 2017-01-31 at 08:24 +1300, Andrew Bartlett wrote:
[...]
>
> I've started a thread to have us move to just using PyCapsule by
> requiring 2.7.  I think this is likely to get up, but we will know
> soon.  This will kill a number of birds with one stone, so can you re-
> work the patches to rely on PyCapsule?

Requiring 2.7 is two questions in one:
- the minimum version for Samba
- the minimum version for independent libraries like pytalloc

But thinking about this, I realized pytalloc can
- export pytalloc_CObject_FromTallocPtr for Python 2
- export pytalloc_PyCapsule_FromTallocPtr for Python 3 *and* 2.7

Then Samba could use only PyCapsule (if it doesn't need 2.6
compatibility), while talloc maintains backwards compatibility.


--
Petr Viktorin


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Petr Viktorin
In reply to this post by Andrew Bartlett
On 02/09/2017 02:20 AM, Andrew Bartlett wrote:
...

>
> I'm hitting (due to our strict build, please build with --enable-
> developer):
>
> ../lib/talloc/pytalloc_util.c: In function
> ‘pytalloc_PyCapsule_FromTallocPtr’:
> ../lib/talloc/pytalloc_util.c:201:34: error: passing argument 3 of
> ‘PyCapsule_New’ from incompatible pointer type [-Werror=incompatible-
> pointer-types]
>   return PyCapsule_New(ptr, NULL, py_cobject_talloc_free);
>                                   ^~~~~~~~~~~~~~~~~~~~~~
> In file included from /usr/include/python3.5m/Python.h:96:0,
>                  from ../lib/talloc/pytalloc_util.c:20:
> /usr/include/python3.5m/pycapsule.h:28:24: note: expected
> ‘PyCapsule_Destructor {aka void (*)(struct _object *)}’ but argument is
> of type ‘void (*)(void *)’
>  PyAPI_FUNC(PyObject *) PyCapsule_New(
>
> This is because we are so strict we care about the difference between
> fn(void *) and fn(PyObject *).

This is a real bug: PyCObject gets the "context", an opaque pointer,
while PyCapsule gets the PyCapsule object itself.
Sorry for missing that in pre-review.


--
Petr Viktorin


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Stefan Metzmacher-2
In reply to this post by Petr Viktorin
Hi Petr,

>> I've started a thread to have us move to just using PyCapsule by
>> requiring 2.7.  I think this is likely to get up, but we will know
>> soon.  This will kill a number of birds with one stone, so can you re-
>> work the patches to rely on PyCapsule?
>
> Requiring 2.7 is two questions in one:
> - the minimum version for Samba
> - the minimum version for independent libraries like pytalloc
>
> But thinking about this, I realized pytalloc can
> - export pytalloc_CObject_FromTallocPtr for Python 2
> - export pytalloc_PyCapsule_FromTallocPtr for Python 3 *and* 2.7
>
> Then Samba could use only PyCapsule (if it doesn't need 2.6
> compatibility), while talloc maintains backwards compatibility.
Maybe to naive, but can't we somehow simulate PyCapsule on top of
CObject for 2.6?
And use PyCapsule everywhere in the code?

At least I seem to have thought about something like this (maybe the
reverse) here
https://git.samba.org/?p=samba.git;a=commitdiff;h=e0324c0cf7e7c363a579

+               ##
+               ## PyCapsule (starting with 2.7) vs. PyCObject (up to 3.2)
+               ##
+               ## As we need to support python 2.6, we can't use
PyCapsule yet.
+               ##
+               ## When we'll get support fpr Python3 we'll have to emulate
+               ## PyCObject using PyCapsule and convert these functions to
+               ## use PyCapsule.
+               ##

metze


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

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Petr Viktorin
On 02/09/2017 03:55 PM, Stefan Metzmacher wrote:

> Hi Petr,
>
>>> I've started a thread to have us move to just using PyCapsule by
>>> requiring 2.7.  I think this is likely to get up, but we will know
>>> soon.  This will kill a number of birds with one stone, so can you re-
>>> work the patches to rely on PyCapsule?
>>
>> Requiring 2.7 is two questions in one:
>> - the minimum version for Samba
>> - the minimum version for independent libraries like pytalloc
>>
>> But thinking about this, I realized pytalloc can
>> - export pytalloc_CObject_FromTallocPtr for Python 2
>> - export pytalloc_PyCapsule_FromTallocPtr for Python 3 *and* 2.7
>>
>> Then Samba could use only PyCapsule (if it doesn't need 2.6
>> compatibility), while talloc maintains backwards compatibility.
>
> Maybe to naive, but can't we somehow simulate PyCapsule on top of
> CObject for 2.6?
> And use PyCapsule everywhere in the code?

That's roughly what the proposed patchset does -- it even references the
comment below. It's possible, but not particularly pretty.

>
> At least I seem to have thought about something like this (maybe the
> reverse) here
> https://git.samba.org/?p=samba.git;a=commitdiff;h=e0324c0cf7e7c363a579
>
> +               ##
> +               ## PyCapsule (starting with 2.7) vs. PyCObject (up to 3.2)
> +               ##
> +               ## As we need to support python 2.6, we can't use
> PyCapsule yet.
> +               ##
> +               ## When we'll get support fpr Python3 we'll have to emulate
> +               ## PyCObject using PyCapsule and convert these functions to
> +               ## use PyCapsule.
> +               ##
>
> metze
>


--
Petr Viktorin

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Stefan Metzmacher-2
Hi Petr,

>>>> I've started a thread to have us move to just using PyCapsule by
>>>> requiring 2.7.  I think this is likely to get up, but we will know
>>>> soon.  This will kill a number of birds with one stone, so can you re-
>>>> work the patches to rely on PyCapsule?
>>>
>>> Requiring 2.7 is two questions in one:
>>> - the minimum version for Samba
>>> - the minimum version for independent libraries like pytalloc
>>>
>>> But thinking about this, I realized pytalloc can
>>> - export pytalloc_CObject_FromTallocPtr for Python 2
>>> - export pytalloc_PyCapsule_FromTallocPtr for Python 3 *and* 2.7
>>>
>>> Then Samba could use only PyCapsule (if it doesn't need 2.6
>>> compatibility), while talloc maintains backwards compatibility.
>>
>> Maybe to naive, but can't we somehow simulate PyCapsule on top of
>> CObject for 2.6?
>> And use PyCapsule everywhere in the code?
>
> That's roughly what the proposed patchset does -- it even references the
> comment below. It's possible, but not particularly pretty.
Why I had a brief look and everything but PyCapsule_Import() should be
pretty easy
to similate on top of PyCObject.

In the last patchset from Andrew I'm wondering why
PyCapsulate_GetContext() is used,
I'd guess PyCObject_GetDesc should be replaced by PyCapsule_GetName().

I don't think we actually need PyCapsulate_{Get,Set}Context() for our usage.
PyCapsule_New() and PyCapsule_GetPointer() should be enough.
I only used PyCObject_GetDesc() because PyCObject_AsVoidPtr() doesn't have
a way to check the description.

Something like this should work and provide PyCapsulate
for all samba callers even with python 2.6

#ifdef PYTHON2.6
#define PyCapsule_Destructor \
        _compat_PyCapsule_Destructor
#define PyCapsule_New(pointer, name, destructor_fn) \
        _compat_PyCapsule_New(pointer, name, destructor_fn)
#define PyCapsule_CheckExact(p) \
        _compat_PyCapsule_CheckExact(p)
#define PyCapsule_IsValid(capsule, name) \
        _compat_PyCapsule_IsValid(capsule, name)
#define PyCapsule_GetPointer(capsule, name) \
        _compat_PyCapsule_GetPointer(capsule, name)

typedef void (*PyCapsule_Destructor)(PyObject *);
PyObject* PyCapsule_New(void *pointer, const char *name,
PyCapsule_Destructor destructor_fn);
int PyCapsule_CheckExact(PyObject *p);
int PyCapsule_IsValid(PyObject *capsule, const char *name);
void* PyCapsule_GetPointer(PyObject *capsule, const char *name);

#endif /* PYTHON2.7 */

struct _compat_PyCapsule {
        PyObject *self;
        void *pointer;
        const char *name;
        PyCapsule_Destructor destructor_fn;
}

static const char *_compat_PyCapsule_desc = "_compat_PyCapsule_desc";

static void _compat_PyCapsule_destructor_wrapper(void *_capsule, void
*_desc)
{
       
        struct _compat_PyCapsule *capsule =
                (struct _compat_PyCapsule *)_capsule;

        assert(_desc == _compat_PyCapsule_desc);

        if (capsule->destructor_fn != NULL) {
                capsule->destructor_fn(capsule->self);
        }

        free(capsule);
}

PyObject *_compat_PyCapsule_New(void *pointer, const char *name,
PyCapsule_Destructor destructor_fn)
{
        struct _compat_PyCapsule *capsule = NULL;

        assert(pointer != NULL);
        assert(name != NULL);

        capsule = calloc(1, sizeof(*capsule));
        if (capsule == NULL) {
                return NULL;
        }
        capsule->pointer = pointer;
        capsule->name = name;
        capsule->destructor_fn = destructor_fn;
        capsule->self = PyCObject_FromVoidPtrAndDesc(capsule,
                                _compat_PyCapsule_desc,
                                _compat_PyCapsule_destructor_wrapper);
        if (capsule->self == NULL) {
                free(capsule);
                return NULL;
        }

        return capsule->self;
}

int PyCapsule_CheckExact(PyObject *p)
{
        int tmp;
        void *desc = NULL;
        void *_capsule = NULL;
        struct _compat_PyCapsule *capsule = NULL;

        tmp = PyCObject_Check(p);
        if (tmp == 0) {
                return 0;
        }

        desc = PyCObject_GetDesc(p);
        if (desc == NULL) {
                return 0;
        }

        if (desc == _compat_PyCapsule_desc) {
                return 1;
        }

        return 0;
}

void *PyCapsule_GetPointer(PyObject *self, const char *name)
{
        int tmp;
        void *_capsule = NULL;
        struct _compat_PyCapsule *capsule = NULL;

        tmp = PyCapsule_CheckExact(self);
        if (tmp == 0) {
                return 0;
        }

        _capsule = PyCObject_AsVoidPtr(self);
        capsule = (struct _compat_PyCapsule *)_capsule;

        tmp = strcmp(capsule->name, name);
        if (tmp != 0) {
                return 0;
        }

        return capsule->pointer;
}

int PyCapsule_IsValid(PyObject *self, const char *name)
{
        void *pointer;

        pointer = PyCapsule_GetPointer(self);
        if (pointer != NULL) {
                return 1;
        }

        return 0;
}


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

Re: [PATCH] Python3 compatible modules - credentials, param, _glue

Andrew Bartlett
On Thu, 2017-02-09 at 17:18 +0100, Stefan Metzmacher wrote:

> Hi Petr,
>
> > > > > I've started a thread to have us move to just using PyCapsule
> > > > > by
> > > > > requiring 2.7.  I think this is likely to get up, but we will
> > > > > know
> > > > > soon.  This will kill a number of birds with one stone, so
> > > > > can you re-
> > > > > work the patches to rely on PyCapsule?
> > > >
> > > > Requiring 2.7 is two questions in one:
> > > > - the minimum version for Samba
> > > > - the minimum version for independent libraries like pytalloc
> > > >
> > > > But thinking about this, I realized pytalloc can
> > > > - export pytalloc_CObject_FromTallocPtr for Python 2
> > > > - export pytalloc_PyCapsule_FromTallocPtr for Python 3 *and*
> > > > 2.7
> > > >
> > > > Then Samba could use only PyCapsule (if it doesn't need 2.6
> > > > compatibility), while talloc maintains backwards compatibility.
> > >
> > > Maybe to naive, but can't we somehow simulate PyCapsule on top of
> > > CObject for 2.6?
> > > And use PyCapsule everywhere in the code?
> >
> > That's roughly what the proposed patchset does -- it even
> > references the
> > comment below. It's possible, but not particularly pretty.
>
> Why I had a brief look and everything but PyCapsule_Import() should
> be
> pretty easy
> to similate on top of PyCObject.
>
> In the last patchset from Andrew I'm wondering why
> PyCapsulate_GetContext() is used,
> I'd guess PyCObject_GetDesc should be replaced by
> PyCapsule_GetName().
>
> I don't think we actually need PyCapsulate_{Get,Set}Context() for our
> usage.
> PyCapsule_New() and PyCapsule_GetPointer() should be enough.
> I only used PyCObject_GetDesc() because PyCObject_AsVoidPtr() doesn't
> have
> a way to check the description.
I'm afraid this is all a little above my head right now.  I was just
trying to un-wrap it, but clearly I need to read up on this area.

> Something like this should work and provide PyCapsulate
> for all samba callers even with python 2.6

I'm happy with whatever outcome you and Petr think is best, because my
primary goal is to get this in to master, but my preference would be to
actually expect Python from 2010 rather than still support Python from
2008.  

In particular, I would really like to avoid unnecessary wrappers: if we
have reason to expect we can write a wrapper, perhaps only add it if we
get substantial negative feedback on the 2.7 requirement when Samba 4.7
is released in September?

Thanks,

Andrew Bartlett

signature.asc (879 bytes) Download Attachment
123
Loading...