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

Re: Build error in swig-pl

From: David James <james82_at_gmail.com>
Date: Tue, 30 Jun 2009 11:10:41 -0700

On Tue, Jun 30, 2009 at 5:55 AM, Hyrum K. Wright<hyrum_at_hyrumwright.org> wrote:
> John, Nik,
> I know it's been a while since either of you did much with the perl
> swig bindings, but I think you're still the domain experts around
> here, so this plea goes to you.  (If anybody else can fix this,
> though, feel free to do so.)
>
> I rev'd an API in libsvn_wc the other day, and now the perl swig
> bindings won't build.  Specifically, I added svn_wc_get_ignores2(),
> and inside the generated wrapper, I see the following error:
>
> svn_wc.c: In function ‘_wrap_svn_wc_get_ignores2’:
> svn_wc.c:28992: error: ‘_global_pool’ undeclared (first use in this
> function)
> svn_wc.c:28992: error: (Each undeclared identifier is reported only once
> svn_wc.c:28992: error: for each function it appears in.)

Hi Hyrum,

I haven't looked at the Perl bindings in a long time, but I have a
suggestion. It looks like your new function gets rid of the simple
"pool" argument and replaces it with two new arguments, result_pool,
and scratch_pool. I don't think any of the bindings have been taught
how to handle functions that accept multiple pool arguments, or to
understand the meaning of the words "result_pool" and "scratch_pool".

Teaching the Perl bindings to just treat the result_pool and
scratch_pool as a regular pool is relatively easy -- there's an %apply
statement in svn_types.swg that looks like this:

#ifdef SWIGPERL
%apply apr_pool_t *pool {
    apr_pool_t *dir_pool,
    apr_pool_t *file_pool,
    apr_pool_t *node_pool
};
#endif

It's not really the best idea, though, to treat result_pool and
scratch_pool as regular pools, since they aren't regular pools. Our
bindings would be much happier if you just stuck with a single pool
argument, and used that pool as our result pool. If you need to create
a subpool from your result pool to work as a scratch pool, that is no
problem.

The Python / Ruby bindings may compile, but I doubt that they will
work properly with your function. The bindings assume that each
function only accepts a single pool argument, so if you have multiple
pool arguments they will likely get very confused.

You could also teach all three sets of bindings to understand the
difference between result pools and scratch pools, but that would be a
much bigger project.

In general, it is quite confusing and hard to maintain that our SWIG
bindings are trying to make general decisions about how to handle
function wrapping based on the names of variables in function
declarations. Unfortunately, the current design of our SWIG bindings
requires this type of trickery. In the ctypes python bindings, we
simply wrap functions "as is", so there is no need for any fancy rules
based on variable names. This type of design, I think, is much more
maintainable for the long term, because it allows Subversion API
designers to write their functions any way they want without needing
to think about how our fancy SWIG rules will interpret your changes.

Cheers,

David

>
> Looking at the generated code it appears that sure enough, that
> function is trying to use _global_pool in line 28992:
>
>     {
>       arg4 = svn_swig_pl_objs_to_hash_by_name (ST(2), "svn_config_t *",
>         _global_pool);
>     }
>
> But _global_pool is never declared or initialized anywhere, like it is
> in the wrapper for the now-deprecated API, svn_wc_get_ignores().  I
> think part of the problem is that we've started using a dual-pool
> structure to allow callers to better manage memory, and instead of
> having a single pool argument, our new APIs now have two pool
> arguments: a result_pool and a scratch_pool.
>
> The build problem doesn't happen with the swig python or ruby
> bindings, and I feel that it might be pretty easy to solve, once one
> acquires the knowledge needed to do so.  Do either of you have any
> pointers?
>
> -Hyrum
>
> PS - The above error information can also be seen on the x64-ubuntu
> buildbot: http://crest.ics.uci.edu/buildbot/builders/x64-ubuntu%20gcc/builds/803/steps/Build/logs/stdio
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2366690
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2366793
Received on 2009-06-30 20:11:25 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.