<div>Hi Danny,</div><div><br></div><div>First of all, thank you for your work on Python bindings!</div><div>This will be very useful in the near future :).</div><div><br></div><div>Here are some comments on your patch:</div>

<div><br></div><div>> SWIG >= 2.0 is used to create the wrapper and its</div><div>> 'warning md variable unused' bug is patched in Makefile.am.</div><div>> [...]</div><div>> diff --git a/src/python/Makefile.am b/src/python/Makefile.am</div>

<div>> new file mode 100644</div><div>> index 0000000..b67d550</div><div>> --- /dev/null</div><div>> +++ b/src/python/Makefile.am</div><div>> [...]</div><div>> +lttng_wrap.c: lttng.i</div><div>> +       $(SWIG) -python -I. -I$(top_srcdir)/src/common/sessiond-comm lttng.i</div>

<div>> +       sed -i "s/Python.h/python$(PYTHON_VERSION)\/Python.h/g" lttng_wrap.c</div><div>> +       sed -i "s/PyObject \*m, \*d, \*md;/PyObject \*m, \*d;\n#if defined(SWIGPYTHON_BUILTIN)\nPyObject *md;\n#endif/g" lttng_wrap.c</div>

<div>> +       sed -i "s/md = d/d/g" lttng_wrap.c</div><div>> +       sed -i "s/(void)public_symbol;/(void)public_symbol;\n  md = d;/g" lttng_wrap.c</div><div><br></div><div>If I understand correctly your commit message, those sed invocations are </div>

<div>necessary to remove an 'unused variable warning'. Is this a known upstream</div><div>bug in SWIG?</div><div><br></div><div>> +       sed -i "s/Python.h/python$(PYTHON_VERSION)\/Python.h/g" lttng_wrap.c </div>

<div><br></div><div>This sed replacement didn't work on my machine. I have a Python3 install and </div><div>the appropriate Python.h header is located in '/usr/include/python3.2mu'.</div><div>To reliably detect Python include paths, the 'python-config --includes'</div>

<div>command can be used like you did in the main <a href="http://configure.ac" target="_blank">configure.ac</a>.</div><div><br></div><div>I would suggest generating the binding like this:</div><div><br></div><div>-AM_CFLAGS = -I$(PYTHONINC) -I../lib/lttng-ctl -I../common \     </div>

<div>+AM_CFLAGS = -I$(PYTHON_INCLUDE) -I../lib/lttng-ctl -I../common \</div><div>              $(BUDDY_CFLAGS)                                    </div><div>[...]</div><div> lttng_wrap.c: lttng.i</div><div>        $(SWIG) -python -I. -I$(top_srcdir)/src/common/sessiond-comm lttng.i</div>

<div>-       sed -i "s/Python.h/python$(PYTHON_VERSION)\/Python.h/g" lttng_wrap.c</div><div>[...]</div><div><br></div><div>The README should also mention that in order to generate </div><div>the Python bindings, python headers are also required </div>

<div>(python-dev on Debian/Ubuntu).</div><div><br></div><div>Thank you,</div><div><br></div><div>Christian</div><div><br></div>