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

Re: Updating swig-py bindings

From: Julian Foad <julianfoad_at_apache.org>
Date: Wed, 14 Mar 2018 21:58:29 +0000

Branko Čibej wrote:
> For anyone who wants to work on updating the bindings: note that we have
> a lot of language-specific stuff in there that's a consequence of Swig 1
> not knowing any better back in the day. Most of those typemaps can be
> made language-independent (thus reducing the size of the Swig files)
> with features from Swig 2 and especially 3, which introduced built-in
> constructs for handling various kinds of data structures that we're
> currently mapping separately for each target language.

Hooray! I was hoping that might be the case.

Another thought. Some comments and code in the current bindings seem
to indicate that a few of our C APIs are written in a way that makes
it harder to bind in a 'natural' way. We could consider changing those
C APIs to use idioms that bind better.

Here is something that at first seemed a very trivial update: adding
'const' to apr_array_header_t input parameters. I added 'const' across
all instances in our C API many years ago (and I see just one that lacks
it now: svn_repos_freeze()), so we can update the typemaps and remove
the obsolete comment seen in the following patch:

Index: subversion/bindings/swig/include/svn_containers.swg
--- subversion/bindings/swig/include/svn_containers.swg (revision 1826615)
+++ subversion/bindings/swig/include/svn_containers.swg (working copy)
@@ -639,26 +639,18 @@
-/* svn_delta_path_driver() mutates its 'paths' argument (by sorting it),
- despite the fact that it is notionally an input parameter - hence, the
- lack of 'const' in that one case.
- svn_wc_get_update_editor3() and svn_wc_get_switch_editor3() aren't changing
- their 'preserved_exts' argument, but it is forwarded to
- svn_cstring_match_glob_list which also doesn't modify it, but does not have
- const in its prototype. */
 %apply const apr_array_header_t *STRINGLIST {
   const apr_array_header_t *args,
   const apr_array_header_t *diff_options,
- apr_array_header_t *paths,
- apr_array_header_t *revprops,
+ const apr_array_header_t *paths,
+ const apr_array_header_t *revprops,
   const apr_array_header_t *targets,
- apr_array_header_t *preserved_exts
+ const apr_array_header_t *preserved_exts
 #if defined(SWIGPERL) || defined(SWIGRUBY)
 %apply const apr_array_header_t *STRINGLIST_MAY_BE_NULL {
- apr_array_header_t *revprops
+ const apr_array_header_t *revprops

But then I went on through that file doing the same thing until I came to
one where both input and output typemaps are applied to the same pattern
(in Ruby):

%typemap(in) apr_array_header_t *PROP_LIST
  VALUE rb_pool;
  apr_pool_t *pool;

  svn_swig_rb_get_pool(argc, argv, self, &rb_pool, &pool);

  $1 = svn_swig_rb_to_apr_array_prop($input, pool);

%typemap(out) apr_array_header_t *PROP_LIST

%apply apr_array_header_t *PROP_LIST {
  apr_array_header_t *wcprop_changes,
  apr_array_header_t *propchanges

Now, a 'wcprop_changes' identifier appears not only as function arguments
(with 'const') but also as a field in 'svn_client_commit_item2_t' (without).

At this point I am for the time being out of my depth -- I don't know enough
to know where to add 'const' and where not.

I'll try to learn more Swig, sooner or later.

- Julian
Received on 2018-03-14 22:58:33 CET

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.