[lttng-dev] [lttng-tools PATCH] lttng-tools python module

[Danny Serres]

Date: Tue, 31 Jul 2012 15:56:47 -0400
Subject: Re: [lttng-dev] [lttng-tools PATCH] lttng-tools python module
From: christian.babeux at efficios.com
To: danny.serres at efficios.com
CC: lttng-dev at lists.lttng.org

Hi Danny,
First of all, thank you for your work on Python bindings!This will be very useful in the near future :).
Here are some comments on your patch:
> SWIG >= 2.0 is used to create the wrapper and its> 'warning md variable unused' bug is patched in Makefile.am.> [...]> diff --git a/src/python/Makefile.am b/src/python/Makefile.am> new file mode 100644> index 0000000..b67d550> --- /dev/null> +++ b/src/python/Makefile.am> [...]> +lttng_wrap.c: lttng.i> +       $(SWIG) -python -I. -I$(top_srcdir)/src/common/sessiond-comm lttng.i> +       sed -i "s/Python.h/python$(PYTHON_VERSION)\/Python.h/g" lttng_wrap.c> +       sed -i "s/PyObject \*m, \*d, \*md;/PyObject \*m, \*d;\n#if defined(SWIGPYTHON_BUILTIN)\nPyObject *md;\n#endif/g" lttng_wrap.c> +       sed -i "s/md = d/d/g" lttng_wrap.c> +       sed -i "s/(void)public_symbol;/(void)public_symbol;\n  md = d;/g" lttng_wrap.c
If I understand correctly your commit message, those sed invocations are necessary to remove an 'unused variable warning'. Is this a known upstreambug in SWIG?











Yes, it is.  So far, it has not been fixed but a patch has been proposed (same as I apply here using sed)
(see http://sourceforge.net/tracker/index.php?func=detail&aid=3530021&group_id=1645&atid=101645 )

> +       sed -i "s/Python.h/python$(PYTHON_VERSION)\/Python.h/g" lttng_wrap.c 
This sed replacement didn't work on my machine. I have a Python3 install and the appropriate Python.h header is located in '/usr/include/python3.2mu'.To reliably detect Python include paths, the 'python-config --includes'command can be used like you did in the main configure.ac.

I would suggest generating the binding like this:
-AM_CFLAGS = -I$(PYTHONINC) -I../lib/lttng-ctl -I../common \     +AM_CFLAGS = -I$(PYTHON_INCLUDE) -I../lib/lttng-ctl -I../common \              $(BUDDY_CFLAGS)                                    [...] lttng_wrap.c: lttng.i        $(SWIG) -python -I. -I$(top_srcdir)/src/common/sessiond-comm lttng.i-       sed -i "s/Python.h/python$(PYTHON_VERSION)\/Python.h/g" lttng_wrap.c[...]
The README should also mention that in order to generate the Python bindings, python headers are also required (python-dev on Debian/Ubuntu).

Fixed.  I will post a second version soon.

Thanks,
Danny


Thank you,
Christian










 		 	   		  
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20120731/a1728183/attachment.html>