[PATCHES] Port pytalloc to Python 3

Jelmer Vernooij jelmer at samba.org
Fri Nov 28 10:29:58 MST 2014


Hi Petr,

On Thu, Nov 27, 2014 at 03:47:30PM +0100, Petr Viktorin wrote:
> I've seen a discussion on this list from May 2013 [0] with some kind of
> consensus that porting Samba is inevitable, and would be hard, but there was
> no rush. Some posts there cite a wiki page [1] that used to warn against
> moving prematurely, but now it says:
> >Python 2.x is legacy, Python 3.x is the present and future of the language
Thanks for working on this!

I think supporting Python2 and Python3 simultaneously makes sense at least for
our standalone libraries (talloc, ldb, tdb).

Maintaining support for two versions of Python at the same time
is painful and very costly. If we switch to Python3, we should drop
Python2 shortly afterwards if not at the same time.

With my Debian/Ubuntu hat on, dropping Python2 support is fine. What do other
people think? Python3 was released in 2008, so surely it's made its way into
most distributions at this point..

A migration would also need to be coordinated with OpenChange, who
have code that uses our Python bindings (all in Python2).

If we're going to migrate some infrastructure over, we should make sure
that we set up python3 on at least some build farm machines and possibly
sn-devel so that python3 support doesn't regress.

> I've asked about getting personal copyright assignment from Red Hat legal
> (I'm new to that part, too). Hopefully that will get sorted out before
> review is done. Please treat this patchset as a WIP for the time being.
ack.

> With these patches, pytalloc can be built for Python 3.
> To build:
>     cd lib/talloc
>     PYTHON=/usr/bin/python3 ./configure
>     make
> The rest of Samba should not be affected.
> Note that Python 2 is still used to run waf itself.
> 
> The first patch makes it possible to build C extensions for Python 3. I'm
> not sure how much of a complete fork Samba's waf is – should I send this
> patch upstream somewhere?
It's not a fork, just a local copy.

> The second patch is not strictly necessary (Python will load extensions
> without an ABI tag in the filename), but it's good practice.
> I'm curious why pyext_PATTERN contains "%s" but string interpolation was not
> used with it?
I'm not sure why it was done that way either..

> The third patch is mostly your usual C extension porting (a lot of #ifdefs),
> aside from one  change that might be controversial: I removed
> pytalloc_CObject_FromTallocPtr without replacement. I did not see this
> function used much, and since code that needs it would have to be ported
> anyway, I decided to hold off making a PyCapsule version until it's clear
> that it's actually needed.
> 
> On most systems, Python 2 and 3 will coexist. To support this, the fourth
> patch renames pytalloc.o to py3talloc.o under Python 3, and does the same
> with the pkg-config file.

> From 9908f55c38ac7e69c96d094065e3f312c963f293 Mon Sep 17 00:00:00 2001
> From: Petr Viktorin <pviktori at redhat.com>
> Date: Wed, 12 Nov 2014 16:53:33 +0100
> Subject: [PATCH 1/4] buildtools: Honor LDVERSION when looking for Python
>  library
> 
> Since Python 3.2, Python .so files are tagged for ABI compatibility,
> so the library name is something like libpython3.4m.so (note the 'm').
> This information is found in distutils.sysconfig.get_config_var('LDVERSION')
> 
> This fixes waf issue 1405 (https://code.google.com/p/waf/issues/detail?id=1405)
> ---
>  buildtools/wafadmin/Tools/python.py | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/buildtools/wafadmin/Tools/python.py b/buildtools/wafadmin/Tools/python.py
> index 35c61c24664ecac5522b937c33c75e3d14038660..e05f438f5d7c199e8e04febd4079ff1554fe6a9b 100644
> --- a/buildtools/wafadmin/Tools/python.py
> +++ b/buildtools/wafadmin/Tools/python.py
> @@ -170,10 +170,10 @@ def check_python_headers(conf, mandatory=True):
>  
>  	try:
>  		# Get some python configuration variables using distutils
> -		v = 'prefix SO SYSLIBS LDFLAGS SHLIBS LIBDIR LIBPL INCLUDEPY Py_ENABLE_SHARED MACOSX_DEPLOYMENT_TARGET'.split()
> +		v = 'prefix SO SYSLIBS LDFLAGS SHLIBS LIBDIR LIBPL INCLUDEPY Py_ENABLE_SHARED MACOSX_DEPLOYMENT_TARGET LDVERSION'.split()
>  		(python_prefix, python_SO, python_SYSLIBS, python_LDFLAGS, python_SHLIBS,
>  		 python_LIBDIR, python_LIBPL, INCLUDEPY, Py_ENABLE_SHARED,
> -		 python_MACOSX_DEPLOYMENT_TARGET) = \
> +		 python_MACOSX_DEPLOYMENT_TARGET, python_LDVERSION) = \
>  			_get_python_variables(python, ["get_config_var('%s') or ''" % x for x in v],
>  					      ['from distutils.sysconfig import get_config_var'])
>  	except RuntimeError:
> @@ -190,8 +190,10 @@ python_LIBPL = %r
>  INCLUDEPY = %r
>  Py_ENABLE_SHARED = %r
>  MACOSX_DEPLOYMENT_TARGET = %r
> +LDVERSION = %r
>  """ % (python, python_prefix, python_SO, python_SYSLIBS, python_LDFLAGS, python_SHLIBS,
> -	python_LIBDIR, python_LIBPL, INCLUDEPY, Py_ENABLE_SHARED, python_MACOSX_DEPLOYMENT_TARGET))
> +	python_LIBDIR, python_LIBPL, INCLUDEPY, Py_ENABLE_SHARED, python_MACOSX_DEPLOYMENT_TARGET,
> +	python_LDVERSION))
>  
>  	# Allow some python overrides from env vars for cross-compiling
>  	os_env = dict(os.environ)
> @@ -230,7 +232,9 @@ MACOSX_DEPLOYMENT_TARGET = %r
>  		parse_flags(python_LDFLAGS, 'PYEMBED', env)
>  
>  	result = False
> -	name = 'python' + env['PYTHON_VERSION']
> +	if not python_LDVERSION:
> +		python_LDVERSION = env['PYTHON_VERSION']
> +	name = 'python' + python_LDVERSION
>  
>  	if python_LIBDIR is not None:
>  		path = [python_LIBDIR]
> @@ -245,7 +249,7 @@ MACOSX_DEPLOYMENT_TARGET = %r
>  	if not result:
>  		conf.log.write("\n\n# try again with -L$prefix/libs, and pythonXY name rather than pythonX.Y (win32)\n")
>  		path = [os.path.join(python_prefix, "libs")]
> -		name = 'python' + env['PYTHON_VERSION'].replace('.', '')
> +		name = 'python' + python_LDVERSION.replace('.', '')
>  		result = conf.check(lib=name, uselib='PYEMBED', libpath=path)
>  
>  	if result:
> -- 
> 2.1.0
^^ Waf patches should go to waf upstream first. We can then update our copy of waf in Samba.

> From b2cf35b922d4b098136cf680574c148b765d1c5b Mon Sep 17 00:00:00 2001
> From: Petr Viktorin <pviktori at redhat.com>
> Date: Wed, 12 Nov 2014 19:49:45 +0100
> Subject: [PATCH 2/4] buildtools: Use all of pyext_PATTERN in
>  map_shlib_extension
> 
> In Python 3, C extension module filenames have an ABI tag;
> the pyext_PATTERN is e.g. "%s.cpython-34m.so".
> The build system was only using the last dot-separated element
> of that extension (the ".so").
> 
> Use the whole extension when constructing the final filename.
LGTM.

> From 8fdd31ae544fb727479237cd28195caa28354172 Mon Sep 17 00:00:00 2001
> From: Petr Viktorin <pviktori at redhat.com>
> Date: Wed, 12 Nov 2014 17:39:24 +0100
> Subject: [PATCH 3/4] pytalloc: Port to Python 3
> 
> - Use PyUnicode instead of PyString for repr
> 
> - Use rich comparison
>   Removes the deprecated tp_compare in favor of tp_richcompare.
>   Disparate types cannot be compared (except for == and !=),
>   and True or False objects are returned explicitly.
> 
> - Use Py_TYPE instead of ob_type
>   This changed to conform to C aliasing rules,
>   see http://legacy.python.org/dev/peps/pep-3123/
>   The Py_TYPE macro from Python 2.6+ is defined if missing.
> 
> - Don't provide CObject creation function
>   A PyCapsule based replacement would be possible,
>   but might not be necessary considering the function is
>   not used much.
> 
> - Use new-style module initialization
LGTM.

> diff --git a/lib/talloc/pytalloc.c b/lib/talloc/pytalloc.c
> index 80196c6c77b58c395f5dde78e3d19f6d9de4c4a3..15fbcb08cb722357982a0d5c36d1918a43b4c2df 100644
> --- a/lib/talloc/pytalloc.c
> +++ b/lib/talloc/pytalloc.c
> @@ -21,7 +21,10 @@
>  #include <talloc.h>
>  #include <pytalloc.h>
>  
> -void inittalloc(void);
> +/* Py_TYPE is defined in Python 2.6+, and becomes mandatory in Python 3 */
> +#ifndef Py_TYPE
> +#define Py_TYPE(ob) (((PyObject*)(ob))->ob_type)
> +#endif
We can drop this I think, since we don't support Python < 2.6.

> From c9bec3d3345c3d05781d8ec4ce22254dc3118ae8 Mon Sep 17 00:00:00 2001
> From: Petr Viktorin <pviktori at redhat.com>
> Date: Wed, 19 Nov 2014 14:50:07 +0100
> Subject: [PATCH 4/4] pytalloc: Build pytalloc.o as py3talloc.o under Python 3
> 
> This allows two versions of pytalloc to be installed side by side,
> one for Python 2 and another for Python 3.
LGTM

> ---
>  lib/talloc/py3talloc-util.pc.in | 11 +++++++++++
>  lib/talloc/pytalloc-util.pc.in  |  2 +-
>  lib/talloc/pytalloc_guide.txt   |  7 +++++++
>  lib/talloc/wscript              | 10 +++++++---
>  4 files changed, 26 insertions(+), 4 deletions(-)
>  create mode 100644 lib/talloc/py3talloc-util.pc.in
> 
> diff --git a/lib/talloc/py3talloc-util.pc.in b/lib/talloc/py3talloc-util.pc.in
> new file mode 100644
> index 0000000000000000000000000000000000000000..a201b2e840fab5267e5de9c8bc8401c589485132
> --- /dev/null
> +++ b/lib/talloc/py3talloc-util.pc.in
> @@ -0,0 +1,11 @@
> +prefix=@prefix@
> +exec_prefix=@exec_prefix@
> +libdir=@libdir@
> +includedir=@includedir@
> +
> +Name: py3talloc-util
> +Description: Utility functions for using talloc objects with Python 3
> +Version: @TALLOC_VERSION@
> +Libs: @LIB_RPATH@ -L${libdir} -lpy3talloc-util
> +Cflags: -I${includedir}
> +URL: http://talloc.samba.org/
> diff --git a/lib/talloc/pytalloc-util.pc.in b/lib/talloc/pytalloc-util.pc.in
> index b7426bb1124d5c03af4b1351e1e1c55d039d6f82..2a650d91944a5cc356230d58595249a1b7160e8a 100644
> --- a/lib/talloc/pytalloc-util.pc.in
> +++ b/lib/talloc/pytalloc-util.pc.in
> @@ -4,7 +4,7 @@ libdir=@libdir@
>  includedir=@includedir@
>  
>  Name: pytalloc-util
> -Description: Utility functions for using talloc objects with Python
> +Description: Utility functions for using talloc objects with Python 2
>  Version: @TALLOC_VERSION@
>  Libs: @LIB_RPATH@ -L${libdir} -lpytalloc-util
>  Cflags: -I${includedir}
> diff --git a/lib/talloc/pytalloc_guide.txt b/lib/talloc/pytalloc_guide.txt
> index 8abe1d4a1dca9f0e3d20002df7eeead7f6316afe..e79dd4db103ce29d9af6ef6d5211e80cf1665dfe 100644
> --- a/lib/talloc/pytalloc_guide.txt
> +++ b/lib/talloc/pytalloc_guide.txt
> @@ -20,6 +20,13 @@ for objects that wrap talloc-maintained memory in C. It won't write your
>  bindings for you but it will make it easier to write C bindings that involve
>  talloc, and take away some of the boiler plate.
>  
> +Python 3
> +--------
> +
> +pytalloc can be used with Python 3. Usage from Python extension remains
> +the same, but for the C utilities you will need to link against "py3talloc".
> +To make a build for Python 3, configure with PYTHON=/usr/lib/python3.
s,/usr/lib/python3,/usr/bin/python3/

> +.
>  =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
>  pytalloc_Object
>  
> diff --git a/lib/talloc/wscript b/lib/talloc/wscript
> index 986492ccde8c7cdac830dc237893775a2682b2fd..57c0faae778c0427859668e3c5b478f62670af97 100644
> --- a/lib/talloc/wscript
> +++ b/lib/talloc/wscript
> @@ -113,21 +113,25 @@ def build(bld):
>                            manpages='man/talloc.3')
>  
>      if not bld.CONFIG_SET('USING_SYSTEM_PYTALLOC_UTIL') and not bld.env.disable_python:
> -        bld.SAMBA_LIBRARY('pytalloc-util',
> +        if bld.env.PYTHON_VERSION >= '3':
^^ How hard would it be to build with all Python versions found rather than
just the one that we found first? That would make preventing regressions with
python3 a lot easier.

Cheers,

Jelmer


More information about the samba-technical mailing list