[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