parameter name of svn_fs_paths_changed2() -- was: Re: SWIG (Perl) prospective patch

John Beranek wrote:
[[[
--- subversion/include/svn_fs.h
+++ subversion/include/svn_fs.h
@@ -1135,11 +1135,11 @@
  * Use @c pool for all allocations, including the hash and its values.
  *
  * @since New in 1.6.
  */
 svn_error_t *
-svn_fs_paths_changed2(apr_hash_t **changed_paths_p,
+svn_fs_paths_changed2(apr_hash_t **changed_paths2_p,
                       svn_fs_root_t *root,
                       apr_pool_t *pool);

 /** Same as svn_fs_paths_changed2(), only with @c svn_fs_path_change_t * values
]]]

I'm not sure that this is ok. I see what you mean -- the data stored in the
hash is now different. Actually, if we were to decide on a rule like this to
be put in place, I'd probably be in favor.

But AFAIK we haven't ever been doing *this* kind of API versioning.
Functionally speaking, it's a bikeshed. And since svn_fs_paths_changed2() is
already in trunk the way that it is, it seems a little silly to do this now.

That said, I see how the bindings sort of *need* it to be a different name.

[[[
 %hash_argout_typemap(changed_paths_p, svn_fs_path_change_t *)
+%hash_argout_typemap(changed_paths2_p, svn_fs_path_change2_t *)
]]]

The bindings code shouldn't be like that. after all, variable names can be
similar and hash tables can contain anything.

Does anyone know how to fix the bindings without changing the actual
parameter name in svn_fs_paths_changed2()?

Alternatively, I see how the variable name change makes sense. Any vetos
from you committers out there?

~Neels

BTW, John, you can create a patch comfortably by making your changes in an
actual working copy and then running "svn diff" in the working copy root
(usually from trunk, but using 1.6.1 like you did:
"
 svn co https://svn.collab.net/repos/svn/tags/1.6.1
 <edit> ./1.6.1/**
 svn diff ./1.6.1 > patch.file
"). That's the standard form normally circulating around here.

> Hello all,
>
> So I've just started using the Subversion API, from Perl. Currently have
> Subversion 1.6.1 rebuilt from a source RPM on my Fedora 10 development
> system.
>
> Was looking into examining transactions in a pre-commit hook, using the
> Perl Subversion API.
>
> Got some way down the road and got to _p_svn_fs_root_t->paths_changed()
> (aka svn_fs_paths_changed() in the C API).
>
> Started reading the C API and discovered svn_fs_paths_changed2() which
> returns the seemingly valuable extra info that svn_fs_path_change2_t was
> created to add in Subversion 1.6.
>
> So, wanted to call this from Perl, only to find the Perl bindings didn't
> implement that call.
>
> After much ado the attached patch is my attempt at adding said call to
> the SWIG bindings for Perl. There are a few provisos/apologies:
>
> * This is a patch against the Subversion 1.6.1 code from the SRPM I
> started from, not trunk or any Subversion branch.
> * I'm not convinced of the change to svn_fs.h and svn_fs.i, whether this
> is the way to achieve what I've done.
> * I've not yet written complete Perldoc for my additions to SVN::Fs.
> * I've tested the additions and ->copyfrom_path() and ->copyfrom_rev()
> and they return what's expected if the change is indeed a copy. If the
> change isn't a copy ->copyfrom_known() still appears to be true, and
> ->copyfrom_path()/->copyfrom_rev() just return empty. Is this as
> expected, the C API docs are far from clear here?
>
> Right, I hope this is helpful to someone...
>
> Cheers,
>
> John.
>
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2362468