[PATCH] Python3 compatible tests and credentials module

[Andrew Bartlett]

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/114998.
> > > 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 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