[PATCH] Python3 compatible tests and credentials module

[Andrew Bartlett]

On Sat, 2016-11-26 at 09:34 +1300, Andrew Bartlett wrote:
> On Fri, 2016-11-25 at 17:20 +0100, Petr Viktorin wrote:
> > 
> > On 11/25/2016 03:40 AM, Andrew Bartlett wrote:
> > > 
> > > 
> > > On Thu, 2016-11-24 at 14:59 +0100, Lumir Balhar wrote:
> > > > 
> > > > 
> > > > Hello.
> > > > 
> > > > Because I have no response for two weeks on GitHub [0], I
> > > > decided
> > > > to
> > > > send my patches here. Which way is preferred, please?
> > > > 
> > > > The first part of this PR contains changes necessary to run
> > > > tests
> > > > with
> > > > Python 3. These changes should not have any impact on
> > > > functionality
> > > > with
> > > > Python 2.
> > > > The second part ports the samba.credentials module to be Python
> > > > 3
> > > > compatible, and also ports the credentials tests. The files
> > > > py3compat.h
> > > > and compat.py will help us with porting and to make the future
> > > > code
> > > > more
> > > > readable. The py3compat.h file is the same as proposed by Petr
> > > > Viktorin
> > > > and discussed in this thread:
> > > > https://lists.samba.org/archive/samba-technical/2016-July/11499
> > > > 8.
> > > > html
> > > > 
> > > > I am submitting the two parts as one PR because, without at
> > > > least
> > > > one
> > > > module prepared for Python 3, it is impossible to try running
> > > > tests
> > > > with
> > > > Python 3.
> > > > 
> > > > [0] https://github.com/samba-team/samba/pull/68
> > > 
> > > Thanks for the patch.  The main thing I don't like is the if PY3
> > > stuff,
> > > and the .args usage.  I would rather we ported param and glue to
> > > python3 at the same time, they shouldn't be too hard.
> > 
> > 
> > The problem the PY3 guards are solving is that the main test
> > module, 
> > samba.tests, depends on most of Samba (dcerpc, param, ndr), and the
> > main 
> > samba module depends on param and _ldb
> > If porting needs to come with tests, either we need to port most
> > of 
> > Samba in one patchset, or the circular dependency needs to be cut
> > somewhere.
> > I think the `if PY3` guards are the best solution here, but if you 
> > disagree, sending a bigger patchset is of course possible.
> 
> I'm more comfortable with if PY3 in tests than I am in mainline code.
> 
> For the mainline code, I'm concerned that this effort may not
> continue,
> and don't want if PY3 littered all over our code for an extended
> period
> of time.
> 
> If at all possible, I would like to continue to have this effort move
> from the outside in, that is from modules that can be converted
> without
> such guards first. 
> 
> > 
> > As for param specifically, Python3 file objects are not directly
> > backed 
> > by FILE*, so the dump() methods in param are one of the tougher
> > nuts
> > to 
> > crack during the porting. 
> 
> Look at how ndr_print and the ldb ldif printer takes different types
> of
> printing callbacks.  Just make the param code do the same.  It is a
> bit
> more work, but don't feel afraid to change how the underlying code
> works, I don't think we are particularly attached to the
> implementation. 

I'm also entirely comfortable if the stream argument is lost from
python, and dump() simply pushes to stdout always by passing stdout
inside the C layer.  That would seem to be the least intrusive fix.

> > 
> > I think param would benefit from a separate 
> > review, after preparations to run tests on py3 are done.
> 
> OK.  I think we are at that point, I don't want to merge it until we
> get rid of those PY3 hooks in the mainline code, but I think we have
> successfully discussed the only issues with the preparations, so we
> can
> get on to that step as soon as you are ready. 
> 
> 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