You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Jon Trowbridge <tr...@ximian.com> on 2002/08/30 05:28:05 UTC

Multiple problems with svn python bindings

In the course of trying to work on gsvn, I've had a number of problems
with the python bindings for the svn_client API.  I don't know the
first thing about swig, so I'm not in a good position to try to
resolve these issues myself.

Here are the problems I've found so far:

* svn_client_commit, svn_client_log and svn_client_diff are %ignored
  in svn_client.i.  A comment in that file says that the problem has
  to do with apr_array_header_t and pools.

* A number of functions take a svn_wc_notify_func_t and a notify baton
  as arguments.  While the comments in svn_client.h say that the
  notify function can be NULL, passing in None for the notify function
  causes a segfault.  As far as I can tell, there is no way to construct
  a non-NULL svn_wc_notify_func_t from inside of python.  Am I missing
  something?

  (This is a problem with svn_client_checkout, svn_client_update,
  svn_client_switch, svn_client_add, svn_client_mkdir, svn_client_delete,
  svn_client_revert, svn_client_resolve, and several others.)

* When calling svn_client_checkout or svn_client_update, a TypeError
  is produced if the xml_src is None.

* svn_client_proplist segfaults when I call it on a file that actually
  has properties set.  (If no properties are set, it correctly returns
  an empty array.)

* I don't see how to access the data returned by svn_client_propget.
  The values of the hash that comes back are not strings, but instead are
  things like: <read-only buffer ptr 0x8179f80, size 111 at 0x81740d8>

I have no idea if these things are easy or hard to fix, or if I've
just misunderstood how the bindings are supposed to work.  Any and all
suggestions are appreciated.

Thanks,
-JT


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Multiple problems with svn python bindings

Posted by Daniel Berlin <db...@dberlin.org>.

On Sat, 31 Aug 2002, Greg Stein wrote:

> On Sat, Aug 31, 2002 at 07:39:44PM -0400, Daniel Berlin wrote:
> > On Sat, 31 Aug 2002, Greg Stein wrote:
> >...
> > > 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).
> > 
> > Um, IIRC, when i tried this, it placed the initialization of obj9 (it was 
> > obj6 in the function i was looking at) after the call we needed it for.
> > IE it didn't work.
> 
> Hmm. Seems like there would have to be some slot to hook it in... oh well.

> >...
> > > 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).
> > 
> > This isn't as hard as one would think.
> 
> Cool. I know a bit about SWIG, but obviously you're the local pro :-)

> 
> > It should only require modifying cwrap.c and typemap.c
> > 
> > You should just be able to change Swig_typemap_apply to merge the param 
> > attribute, then in Swig_cargs (which outputs the argument handling), set 
> > the value.
> > 
> > It actually already does something like this for reference parameters, 
> > setting a default value.
> > 
> > The parameters you pass to a typemap are just set as attributes, so you 
> > don't need to do anything special to pass a new parameter to a typemap.
> > 
> > I'll work up a patch in a moment.
> 
> Cool! I bet if it's clean, we can wrangle Beazley to add it. Or at least use
> it as a demonstration of our particular problem, and have him ponder on a
> "SWIGgish solution" if he doesn't like yours.

I did it an easier way that isn't subversion specific.

All our pool arguments are always last (or at least, we can force them to 
be, no?).

So I added a new SWIG replacement variable, "$last" (like we have $input, 
$1,$2,$3,etc), that will properly expand to the name of the converted last 
argument (IE arg9 in the case we were talking about).

In order to ensure conversion order correctness, I have it defer 
arguments whose conversion contains "$last" till after we've converted 
the last argument.

Using $last as the pool argument for apr_array_stuff ends up with 
conversion code like this (this is svn_client_log):

 if(!PyArg_ParseTuple(args,(char *)"OOOiiOOOO:svn_client_log",&obj0,,&obj2,&obj3,&arg5,&arg6,&obj6,&obj7,&obj8,&obj1)) goto fail;
    if ((SWIG_ConvertPtr(obj0,(void **) &arg1, SWIGTYPE_p_svn_client_auth_baton_t,SWIG_POINTER_EXCEPTION | 0 )) == -1) SWIG_fail;
    if ((SWIG_ConvertPtr(obj2,(void **) &arg3, SWIGTYPE_p_svn_client_revision_t,SWIG_POINTER_EXCEPTION | 0 )) == -1) SWIG_fail;
    if ((SWIG_ConvertPtr(obj3,(void **) &arg4, SWIGTYPE_p_svn_client_revision_t,SWIG_POINTER_EXCEPTION | 0 )) == -1) SWIG_fail;
    if ((SWIG_ConvertPtr(obj6,(void **) &arg7, SWIGTYPE_svn_log_message_receiver_t,SWIG_POINTER_EXCEPTION | 0 )) == -1) SWIG_fail;
    if ((SWIG_ConvertPtr(obj7,(void **) &arg8, 0, SWIG_POINTER_EXCEPTION | 0 )) == -1) SWIG_fail;
    if ((SWIG_ConvertPtr(obj8,(void **) &arg9, SWIGTYPE_p_apr_pool_t,SWIG_POINTER_EXCEPTION | 0 )) == -1) SWIG_fail;
    {
        arg2 = svn_swig_py_strings_to_array(obj1, arg9);
        if (arg2 == NULL)
        return NULL;
    }
    result = (svn_error_t *)svn_client_log(arg1,(apr_array_header_t const *)arg2,(svn_client_revision_t const *)arg3,(svn_client_revision_t const *)arg4,arg5,arg6,arg7,arg8,arg9);


Which is what we want (arg9 is the apr_pool_t)

This patch to do this to swig requires only changes to the language 
specific modules, rather than any core swig stuff.

That should make it much easier to get into swig.
I think, anyway.

Let me send it in, and if they are cool to the idea of $last, i'll add 
code to the other Language modules.

It's about 20 lines per, max.



> 
> So then the question would become how to tackle the problem? Apply my patch
> for now, then rip it back out when we get a new SWIG?

Without proper arg deferring, you can't use arg9.
If you use arg9 rather than $last, you'll get (i changed it to arg9 in 
the typemap and reran swig, so this is not a guess, but the actual code):

   if ((SWIG_ConvertPtr(obj0,(void **) &arg1, SWIGTYPE_p_svn_client_auth_baton_t,SWIG_POINTER_EXCEPTION | 0 )) == -1) SWIG_fail;
    {
        arg2 = svn_swig_py_strings_to_array(obj1, arg9);
        if (arg2 == NULL)
        return NULL;
    }
    if ((SWIG_ConvertPtr(obj2,(void **) &arg3, SWIGTYPE_p_svn_client_revision_t,SWIG_POINTER_EXCEPTION | 0 )) == -1) SWIG_fail;
    if ((SWIG_ConvertPtr(obj3,(void **) &arg4, SWIGTYPE_p_svn_client_revision_t,SWIG_POINTER_EXCEPTION | 0 )) == -1) SWIG_fail;
    if ((SWIG_ConvertPtr(obj6,(void **) &arg7, SWIGTYPE_svn_log_message_receiver_t,SWIG_POINTER_EXCEPTION | 0 )) == -1) SWIG_fail;
    if ((SWIG_ConvertPtr(obj7,(void **) &arg8, 0, SWIG_POINTER_EXCEPTION | 0 )) == -1) SWIG_fail;
    if ((SWIG_ConvertPtr(obj8,(void **) &arg9, SWIGTYPE_p_apr_pool_t,SWIG_POINTER_EXCEPTION | 0 )) == -1) SWIG_fail;


Which is wrong, since arg9 is used before it's inited.

I'm not sure if any of the other solutions presented will work around this 
yet, my brain is kinda off right now. :)

> > 
Cheers, > -g > > 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Multiple problems with svn python bindings

Posted by Greg Stein <gs...@lyra.org>.
On Sat, Aug 31, 2002 at 07:39:44PM -0400, Daniel Berlin wrote:
> On Sat, 31 Aug 2002, Greg Stein wrote:
>...
> > 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).
> 
> Um, IIRC, when i tried this, it placed the initialization of obj9 (it was 
> obj6 in the function i was looking at) after the call we needed it for.
> IE it didn't work.

Hmm. Seems like there would have to be some slot to hook it in... oh well.

>...
> > 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).
> 
> This isn't as hard as one would think.

Cool. I know a bit about SWIG, but obviously you're the local pro :-)

> It should only require modifying cwrap.c and typemap.c
> 
> You should just be able to change Swig_typemap_apply to merge the param 
> attribute, then in Swig_cargs (which outputs the argument handling), set 
> the value.
> 
> It actually already does something like this for reference parameters, 
> setting a default value.
> 
> The parameters you pass to a typemap are just set as attributes, so you 
> don't need to do anything special to pass a new parameter to a typemap.
> 
> I'll work up a patch in a moment.

Cool! I bet if it's clean, we can wrangle Beazley to add it. Or at least use
it as a demonstration of our particular problem, and have him ponder on a
"SWIGgish solution" if he doesn't like yours.

So then the question would become how to tackle the problem? Apply my patch
for now, then rip it back out when we get a new SWIG? Is there a middle
ground to avoid the add/rip while waiting on a new SWIG?

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

Re: [PATCH] Re: Multiple problems with svn python bindings

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Sep 02, 2002 at 02:55:13PM -0500, Jon Trowbridge wrote:
> On Sat, 2002-08-31 at 19:27, Greg Stein wrote:
> > I've attached a patch which compiles and visual inspection seems to do the
> > right thing...
> >
> > It introduces a swig_workarounds.h header to prototype the stubs...
> 
> The swig_workarounds.h file is not included in the patch, presumably
> since that file hadn't been added to the repository.

Grr. I noticed that and regenerated the patch. But I included the wrong file
in my email...

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: [PATCH] Re: Multiple problems with svn python bindings

Posted by Jon Trowbridge <tr...@ximian.com>.
On Sat, 2002-08-31 at 19:27, Greg Stein wrote:
> I've attached a patch which compiles and visual inspection seems to do the
> right thing...
>
> It introduces a swig_workarounds.h header to prototype the stubs...

The swig_workarounds.h file is not included in the patch, presumably
since that file hadn't been added to the repository.

-JT



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

[PATCH] Re: Multiple problems with svn python bindings

Posted by Greg Stein <gs...@lyra.org>.
On Sat, Aug 31, 2002 at 04:20:20PM -0700, Greg Stein wrote:
> On Sat, Aug 31, 2002 at 03:46:19PM -0700, Greg Stein wrote:
> >...
> > > 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.
> 
> Working on this now...

I've attached a patch which compiles and visual inspection seems to do the
right thing. I haven't gone thru the whole mix to try out one of these
functions -- figured on at least getting this patch out for conceptual
review.

It introduces a swig_workarounds.h header to prototype the stubs for the
various per-language stubbing. The old STRINGLIST typemap stuff was ripped
from the .i files and use of these stubs put in place. Finally, the new
stubs were inserted into swigutil_py.c.

Since libsvn_swig_py now depends directly on the client and repos libraries,
I also tweaked build.conf to report this. There was also a small change to
add SVN_ERR_SWIG_PY_EXCEPTION_SET so that we could propagate Python errors
thru svn_error_t.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: Multiple problems with svn python bindings

Posted by Greg Stein <gs...@lyra.org>.
On Sat, Aug 31, 2002 at 03:46:19PM -0700, Greg Stein wrote:
>...
> > 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.

Working on this now...

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

Re: Multiple problems with svn python bindings

Posted by Daniel Berlin <db...@dberlin.org>.

On Sat, 31 Aug 2002, Greg Stein wrote:

> 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).

Um, IIRC, when i tried this, it placed the initialization of obj9 (it was 
obj6 in the function i was looking at) after the call we needed it for.
IE it didn't work.

> 
> >...
> > 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).

This isn't as hard as one would think.

It should only require modifying cwrap.c and typemap.c

You should just be able to change Swig_typemap_apply to merge the param 
attribute, then in Swig_cargs (which outputs the argument handling), set 
the value.

It actually already does something like this for reference parameters, 
setting a default value.

The parameters you pass to a typemap are just set as attributes, so you 
don't need to do anything special to pass a new parameter to a typemap.

I'll work up a patch in a moment.

> 
> *shrug*
> 
> We can go with the wrapper workaround unless/until somebody suggests a patch
> to the SWIG folks.
> 
> Cheers,
> -g
> 
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Multiple problems with svn python bindings

Posted by Greg Stein <gs...@lyra.org>.
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

Re: Multiple problems with svn python bindings

Posted by Jon Trowbridge <tr...@ximian.com>.
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.

> > 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
svn_client_workarounds.[ch] containing functions like:

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.

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
looked at the swig docs for the first time late last night, and I don't
really know what the hell is going on.  And forgive me if this is
painfully, blindingly obvious to anyone who has thought about swig for
longer than a day.

(Of course, we'd add comments explaining what was going on, pick a
better name than svn_client_diff_foo, etc.)

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.

-JT

Re: Multiple problems with svn python bindings

Posted by Greg Stein <gs...@lyra.org>.
On Sat, Aug 31, 2002 at 01:36:56AM -0500, Jon Trowbridge wrote:
> On Fri, 2002-08-30 at 16:42, Greg Stein wrote:
> > The issue here is that we need to accept a (Python) list and construct an
> > apr_array_header_t for passing into those functins. To do *that*, we need a
> > pool for that construction. So what needs to happen with those functions is
> > to somehow tell SWIG that the pool to use is "arg6" or whatever.
> 
> Is there a way to get SWIG to do this that isn't totally abusive?  I
> figured out one way, but it is so overwhelmingly hacky that I'm too
> embarrassed to mention it in public. :)

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 :-)

> > I'm not yet sure what the cleanest approach would be for this. Hardcode it
> > for each function that takes an array? Is there some other approach?
> 
> 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)

> And speaking of svn_client_diff, is there a way to construct something
> in python that maps to an apr_file_t?  Or do we also need to put that on
> the to-do list?

On the to-do list. Essentially, we need to get the "fileno" from the Python
file and then call apr_os_file_put() to wrap an apr_file_t around it. (the
function is awfully named and in the awfully named apr_portable.h header,
but hey... that is the guy). Then the apr_file_t is wrapped by SWIG and
returned.

Another alternative is to provide bindings for functions in apr_file_io.h. I
didn't really want to do that, however, since all languages will have their
own file manipulation systems -- we don't really need APR's. So I think just
the ability to turn a Python (true) file into apr_file_t should be fine.

I'm not sure if we need the reverse (given an apr_file_t, get a Python
file). That would involve using apr_os_file_get() and os.fdopen().

> > But for each and every callback, we need some "thunk" types of layers...
> 
> Yeah, it looks like thunks should be pretty doable using a
> multiple-argument typemap.

Right.

> I can at least help out with the trivial bits.  Attached are two (very
> small) patches: the first one fixes the crash in the svn_client_proplist
> wrapper, and the second allows the xml_src and xml_dst arguments to be
> None.

These look good. Could you go ahead and check them in with an appropriate
log message?

(technically, you do have privs, and you didn't send me a log msg :-)

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

Re: Multiple problems with svn python bindings

Posted by Daniel Berlin <db...@dberlin.org>.
y

On 31 Aug 2002, Jon Trowbridge wrote:

> On Fri, 2002-08-30 at 16:42, Greg Stein wrote:
> > The issue here is that we need to accept a (Python) list and construct an
> > apr_array_header_t for passing into those functins. To do *that*, we need a
> > pool for that construction. So what needs to happen with those functions is
> > to somehow tell SWIG that the pool to use is "arg6" or whatever.
> 
> Is there a way to get SWIG to do this that isn't totally abusive?  I
> figured out one way, but it is so overwhelmingly hacky that I'm too
> embarrassed to mention it in public. :) 

It's probably easiest to modify swig, actually.
I would imagine this is not a feature that would be unused or is specific 
to subversion.
You just need to be able to have a parameter that is matched somewhere 
else.

Like so:
%typemap(python, in, param=pool) svn_stringbuf_t * {
	$1 = svn_stringbuf_create(...,pool);
}

Then, any function where this typemap is used, either has to specifically 
have a parameter named "pool" (which it would use by default), or specify 
the value somehow (a "param_is").

We do this in GCC's garbage collector walker generator because of variable 
arrays (which are unions of about 30 different types) and whatnot, where 
we know at the point we use them what type of thing we are storing in 
them, but not at the point of the structure definition. Which is like what 
we have here.

We know what pool to use at the point of the use of the conversion, but 
not at the definition of how to do that conversion.


> -JT
> 
> 
> 

Re: Multiple problems with svn python bindings

Posted by Jon Trowbridge <tr...@ximian.com>.
On Fri, 2002-08-30 at 16:42, Greg Stein wrote:
> The issue here is that we need to accept a (Python) list and construct an
> apr_array_header_t for passing into those functins. To do *that*, we need a
> pool for that construction. So what needs to happen with those functions is
> to somehow tell SWIG that the pool to use is "arg6" or whatever.

Is there a way to get SWIG to do this that isn't totally abusive?  I
figured out one way, but it is so overwhelmingly hacky that I'm too
embarrassed to mention it in public. :) 

> I'm not yet sure what the cleanest approach would be for this. Hardcode it
> for each function that takes an array? Is there some other approach?

Hardcoding it seems like the simplest approach.  It is only three
functions, after all.

And speaking of svn_client_diff, is there a way to construct something
in python that maps to an apr_file_t?  Or do we also need to put that on
the to-do list?

> But for each and every callback, we need some "thunk" types of layers...

Yeah, it looks like thunks should be pretty doable using a
multiple-argument typemap. 

I can at least help out with the trivial bits.  Attached are two (very
small) patches: the first one fixes the crash in the svn_client_proplist
wrapper, and the second allows the xml_src and xml_dst arguments to be
None.

Thanks,
-JT



Re: Multiple problems with svn python bindings

Posted by Greg Stein <gs...@lyra.org>.
On Fri, Aug 30, 2002 at 12:28:05AM -0500, Jon Trowbridge wrote:
>...
> Here are the problems I've found so far:
> 
> * svn_client_commit, svn_client_log and svn_client_diff are %ignored
>   in svn_client.i.  A comment in that file says that the problem has
>   to do with apr_array_header_t and pools.

Ah. Now I understand your offlist email. I was hoping to get back to that,
and ask what you meant about "commit", "log", and "diff" :-)

The issue here is that we need to accept a (Python) list and construct an
apr_array_header_t for passing into those functins. To do *that*, we need a
pool for that construction. So what needs to happen with those functions is
to somehow tell SWIG that the pool to use is "arg6" or whatever.

I'm not yet sure what the cleanest approach would be for this. Hardcode it
for each function that takes an array? Is there some other approach?

> * A number of functions take a svn_wc_notify_func_t and a notify baton
>   as arguments.  While the comments in svn_client.h say that the
>   notify function can be NULL, passing in None for the notify function
>   causes a segfault.  As far as I can tell, there is no way to construct
>   a non-NULL svn_wc_notify_func_t from inside of python.  Am I missing
>   something?

Note that I just kind of "turned on" the bindings for libsvn_client/wc a few
weeks ago. "hey, they compile, if I disable <this>". And that was it. Across
the board, we need to flesh out mechanisms for the callbacks. I did some of
that with the editor stuff. But for each and every callback, we need some
"thunk" types of layers.

Thankfully, every callback in SVN also has a piece of context associated
with that. So what we do for these cases is accept a single callable at the
Python scripting level, and underneath, we pass that callable as the
context, and the function is something that we write explicitly in
swig_util_py.c.

>...
> * When calling svn_client_checkout or svn_client_update, a TypeError
>   is produced if the xml_src is None.

Take a look at svn_repos.i. I changed the argument parsing character for
"src_entry" to "z". That allows None to be passed, so NULL will be passed to
the underlying function.

A similar situation could be defined in svn_client.i to allow None for any
parameter named "xml_src".

(you can also attach them to specific functions and whatnot, but I suspect
 that *any* parameter named xml_src will have that behavior.)

> * svn_client_proplist segfaults when I call it on a file that actually
>   has properties set.  (If no properties are set, it correctly returns
>   an empty array.)

The proplist conversion code has never been exercised. See svn_client.i for
that code. There is (obviously) a bug in there somewhere.

> * I don't see how to access the data returned by svn_client_propget.
>   The values of the hash that comes back are not strings, but instead are
>   things like: <read-only buffer ptr 0x8179f80, size 111 at 0x81740d8>

Buffer objects are string-like. You can use them in most places as a string.
But if something doesn't like that, then just do str(ob).

Regardless, I have a comment in the code about whether it is wise to use
buffer objects in this scenario. It is possible that the underlying memory
might drop out. Thus, using a true string object would be safer.

> I have no idea if these things are easy or hard to fix, or if I've
> just misunderstood how the bindings are supposed to work.  Any and all
> suggestions are appreciated.

Mostly, it is simply that you're treading into uncharted waters :-)

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