[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Multiple problems with svn python bindings

From: Greg Stein <gstein_at_lyra.org>
Date: 2002-09-01 00:46:19 CEST

On Sat, Aug 31, 2002 at 03:43:33PM -0500, Jon Trowbridge wrote:
> On Sat, 2002-08-31 at 14:53, Greg Stein wrote:
> > I haven't applied enough thought or investigation to the problem. I'd be
> > interested in seeing your hacky version. We might be able to reduce the Hack
> > from it :-)
>
> OK, you asked for it. :) I'm attaching a patch that implements the hack
> for svn_client_diff. Since it depends on calling internal swig
> functions, this approach is very fragile. Not to mention amazingly
> wrong-headed.

Using obj9 directly was the idea that I had in mind, actually :-) That's
why I said it was fragile. If SWIG changed how it generated code, then it
would break (and I've already seen changes in the generated output, in the
past, which would have broken this kind of construct).

> > > Hardcoding it seems like the simplest approach. It is only three
> > > functions, after all.
> >
> > Yup. It will end up being a little more fragile than an automated approach,
> > but I'm not sure that we'll ever be able to truly automate this situation
> > (without pushing apr_array_header_t construction up to Python, which is just
> > wrong; any sequence ought to be fine)
>
> I'm not sure if would be more fragile. We could just create

Well, not what you're proposing below :-) (I was prolly just confused about
your suggestion). I like the one below quite a bit.

> svn_client_workarounds.[ch] containing functions like:

These would just go into the swigutil libraries, rather than svn_client
proper.

> svn_error_t *svn_client_diff_foo (PyObject *diff_options_py_object,
> ...blah blah blah...,
> apr_pool_t *pool)
> {
> const apr_array_header_t *diff_options;
>
> diff_options = svn_swig_py_strings_to_array (diff_options_py_object,
> pool);
>
> return svn_client_diff (diff_options, ...blah blah blah..., pool)
> }
>
> Then in svn_client.i we would %include svn_client_workarounds.h and add
> stuff like:
>
> %rename(svn_client_diff) svn_client_diff_foo
> %typemap(python,in) PyObject *diff_options_py_object {
> $1 = $input
> }
>
> In other words, we just pass the PyObject through to the workaround _foo
> function unchanged.

Yup, looks great. And because of the PyObject*, this would be Python
specific, so it goes into the swigutil_py.* files.

> I haven't tried this, but it seems like it should work, and it doesn't
> even seem like it would be all that fragile. Massive disclaimer: I

Not fragile at all, w.r.t SWIG changes. However, if the svn_client prototype
changed, then our wrapper would also need to change. So it is fragile in
that respect (as opposed to all the other swig-wrapped functions which
automatically adjust to changing prototypes).

>...
> All of that said, Dan Berlin's proposal for modifying swig is obviously
> much cleaner than this... but that would require getting the patch into
> swig, having to wait for a new swig release containing the patch, etc.

Right. I'm not entirely sure what that change would be, nor did I really
understand what Dan wrote. I imagine something along the lines of enabling
parameters to the typemaps. You could then define one typemap to invoke the
other pameterized-typemap (e.g. pass the name of the pool arg).

*shrug*

We can go with the wrapper workaround unless/until somebody suggests a patch
to the SWIG folks.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Sep 1 00:41:25 2002

This is an archived mail posted to the Subversion Dev mailing list.