[SSSD] [patch] Python 3 support for sssd

Bohuslav Kabrda bkabrda at redhat.com
Wed Jul 9 12:43:27 UTC 2014


----- Original Message -----
> On Tue, Jul 08, 2014 at 10:38:00AM -0400, Bohuslav Kabrda wrote:
> > Some comments:
> > - The patch tries to maintain backward compatibility for Python 2.6 and 2.7
> > - I admit I only tested 2.7 so far, but it seemed to work ok.
> 
> The oldest OS release we support is RHEL-6. I haven't checked what
> Ubuntu uses, but I would expect at least 2.7

Yeah, RHEL 6 has 2.6, so we should be fine there. I'll do the testing as soon as I find some time for it :)

> > - I didn't yet solve the build part - for now, I just handedit
> > configure.ac to set "PYTHON=python3" and src/external/python.m4 to set
> > "AC_PATH_PROG(PYTHON, python3)" and compile with that.
> 
> This is fine for testing, but do you have some experience from other
> projects on the best approach? I think we can help with coding up the
> build system part, I'm just not sure what the right approach is..

So, there are basically two different approaches to this and I don't think any of them is "the best". I do admit I prefer the second one though:
1) configure can be modified to accept an argument for --with-python, e.g. user would be able to use "--with-python=python2" or "--with-python=python3"; you have to do two builds if you want to build for two Python releases
2) configure can be modified to accept a "--with-python3" option in addition to "--with-python"
(libreport is doing something that is closer to 2), although they strictly require both Python major versions and don't build if one is not present [3].

> > - I managed to run Python tests in src/tests except python-test.py itself
> > (it says "OSError: [Errno 5] Could not initialize connection to the
> > confdb" and I didn't yet investigate why that happens).
> 
> Yeah, don't worry about that one, it requires root currently because the
> files are owned by root. I wonder if we should remove that test and the
> python sysdb API altogether, I don't think anyone ever uses it.

Ok, thanks for the info.

> > - To run tests, one must modify the hashbangs to /usr/bin/python3 (I now
> > see that I left one of these in the actual patch, so please disregard
> > that).
> 
> Maybe I miss something here, but if the changes are backwards
> compatible, why does it matter to use python3 explicitly?

So, the way I think this should be done is that you run tests for all versions for Python version(s) that you're currently building for. The tests have explicit hashbang that refers to /usr/bin/python2, so you have to modify that when running tests for Python 3. The way I think this should be done systematically is, that you should invoke tests using the Python interpreter executable found by configure - for example, invoke "/usr/bin/python2 src/test/foo-test.py" rather than just "foo-test.py".

> > 
> > I'd like to ask you, sssd devels, to give me some comments on the patch (or
> > ask questions) and if you have some time to spare, doing more extensive
> > testing would also be very welcome.
> > 
> > --
> > Regards,
> > Bohuslav "Slavek" Kabrda.
> > 
> > [1]
> > https://fedorahosted.org/sssd/attachment/ticket/2017/sssd-python3-compat.patch
> > [2] https://fedorahosted.org/sssd/ticket/2017

[3] https://github.com/abrt/libreport/commit/6ee995deaf7195c024d77a9ba44509b2507a6768#diff-67e997bcfdac55191033d57a16d1408a

Thanks for the feedback,
Slavek



More information about the sssd-devel mailing list