David James wrote:
> On 5/25/06, Jelmer Vernooij <firstname.lastname@example.org> wrote:
>> The attached patch allows the callbacks in svn_ra_callbacks2_t() to be
>> implemented in Python. This is one of the things required in order to
>> get svn_ra_commit() working from Python.
> Could you update our test suite with a few new tests to make sure your
> code works? For example, you could register a few callback functions
> in Python and make sure they get called with appropriate arguments.
ra_local doesn't actually call these callbacks, which makes it very hard
to write tests using the file:// RA backend. Apparently ra_dav uses them
most often, but there are no Python tests that use it. I have used them
successfully here using other protocols though.
> You're converting SVN strings into SWIG pointers, here. Instead, we
> should convert svn strings into real Python strings using
> PyString_FromStringAndSize((void *)s->data, s->len). See
> convert_svn_string_t for an example.
> You're converting from SWIG "apr_file_t *" pointers into C "apr_file_t
> *" pointers, here. It's good that we can handle apr_file_t * pointers,
> but we should also be able to handle regular Python file objects. See
> svn_swig_py_make_file for some example code which works with Python
> file objects.
> This code converts Python "svn_string_t *" pointers into C
> "svn_string_t *" pointers. It'd be much better if we let users supply
> Python strings instead of pointers. You can convert from Python
> strings into "svn_string_t *" objects using the
> make_svn_string_from_ob function.
> If result == Py_None, there's a memory leak here. If you're going to
> throw away a reference, you always need to run Py_DECREF, unless
> result == NULL. Py_XDECREF will do the trick here. I noticed this
> memory leak in several places, but I only commented on it here.
> You're converting a SWIG auth baton pointer directly into a C auth
> baton here. I see you also implicitly increased the reference count of
> auth_baton when you called PyObject_GetAttrString. Oops. You should
> Py_DECREF(py_auth_baton) unconditionally so as to prevent memory
That should be ok, as the functions on svn.client that return auth_batons return opaque pointers. I've fixed the
> Also, do we need to check for "-1" specifically? Other code in
> swigutil_py.c checks for nonzero values instead.
Here's the summary again:
Allow callbacks in svn_ra_callbacks2_t() to be implemented in Python.
(svn_swig_py_setup_ra_callbacks): Fill in callback function pointers
ra_callbacks_progress_funcs): Add wrappers for Python callbacks
(make_ob_string): Add function
Received on Mon Jun 12 02:49:47 2006