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

Re: [PATCH] Small fixes to the Perl bindings

From: Ben Reser <ben_at_reser.org>
Date: Fri, 15 Feb 2013 11:41:40 -0800

On Fri, Feb 15, 2013 at 12:58 AM, Roderich Schupp
<roderich.schupp_at_gmail.com> wrote:
> Err, no. The accessor functions don't go through the typemap, only
> the wrappers for "real" functions do that (add a printf() or warn() at the
> start of svn_swig_pl_set_revision() to see when it's called).

I guess this depends on the SWIG version. I went to copy the code of
_wrap_svn_opt_revision_t_kind_get() out of
subversion/bindings/swig/perl/native/core.c to show you this and
couldn't find the call. I was doing it off my laptop and was very
surprised. My laptop has SWIG 2.0.8 installed. So I looked on my
Ubuntu 12.04 VM that I did this work on and found that it had SWIG
2.0.4 (provided via the Ubuntu package) and indeed the
_wrap_svn_opt_revision_t_kind_get() looks looks like this:

[[[
XS(_wrap_svn_opt_revision_t_kind_get) {
  {
    svn_opt_revision_t *arg1 = (svn_opt_revision_t *) 0 ;
    svn_opt_revision_t rev1 ;
    int argvi = 0;
    enum svn_opt_revision_kind result;
    dXSARGS;

    if ((items < 1) || (items > 1)) {
      SWIG_croak("Usage: svn_opt_revision_t_kind_get(self);");
    }
    {
      arg1 = &rev1;
      svn_swig_pl_set_revision(&rev1, ST(0));
    }
    result = (enum svn_opt_revision_kind) ((arg1)->kind);
    ST(argvi) = SWIG_From_int SWIG_PERL_CALL_ARGS_1((int)(result)); argvi++ ;

    XSRETURN(argvi);
  fail:

    SWIG_croak_null();
  }
}
]]]

This probably explains why you couldn't reproduce the problem I found.

I flipped through the changelog for SWIG and I didn't see anything
that explains this difference in behavior.

> So the ultimate test that the typemap does the right thing for a
> _p_svn_opt_revision_t object is to pass one into a wrapped function
> with a svn_opt_revision_t* parameter (on the C level). Unfortunately
> any such function needs at least a repository to test.
>
> The attached patch adds a test passing an _p_svn_opt_revision_t
> into SVN::Client::log2 (ie. the wrapper for svn_client_log2).

I wouldn't bother to do the testing like this. You can create an
_p_svn_opt_revision_t without needing to get it from something like
svn_wc_parse_externals_description3(). e.g.

my $rev = SVN::_Core::new_svn_opt_revision_t();
$rev->kind($SVN::Core::opt_revision_number);
$rev->value->number(1445267);

In which case we can put the test code in the 3client.t tests, which
already has a repo.

Note that's calling the wrapper function to create a new
_p_svn_opt_revision_t directly. We could and probably should produce
a nice clean Perlish wrapper that uses this.

> Note that I slightly modified the svn:externals string in the existing test
> case
> in order to obtain _p_svn_opt_revision_t objects of kinds "head" and
> "number".

This is probably a good change to have more complete coverage of
testing that externals code. I'll make this change irrespective of
the other changes.
Received on 2013-02-15 20:42:18 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.