On Wed, Jan 26, 2011 at 2:46 PM, Hyrum K Wright <hyrum_at_hyrumwright.org> wrote:
> On Tue, Jan 25, 2011 at 11:41 PM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
>> Johan Corveleyn wrote on Wed, Jan 26, 2011 at 03:31:11 +0100:
>>> Revving svn_diff_fns_t: what do you mean with parallelizing it? I must
>>> admit that I don't really know (yet) how to go about that. Very early
>>> during the branch work, danielsh pointed out that I modified this
>>> public struct (vtable for reading data from datasources), so it should
>>> be revved. Is it listed/documented somewhere what should be done for
>>> that (community guide perhaps)?
>>
>> Briefly, revving a function includes re-declaring it, updating the old
>> declaration to be a diff against the new one, marking it as deprecated
>> (using doxygen and C preprocessor designators), and re-implementing the
>> old function as a deprecated.c wrapper around the new one.
>>
>> For a struct, you need to re-declare the struct and revv functions that
>> use it. Also, don't forget to add a constructor function
>> (svn_foo_t_create(), but s/_t_/_/) (and possibly a duplicator function),
>> and forbid people from defining variables of the struct type directly.
>>
>> I'm not sure HACKING contains this. On the other hand, the public
>> header files contain plenty of examples of everything I just said...
>
> For testing purposes, I usually find it easiest to work "from the
> bottom up". In other words, rev the lowest API in the call stack,
> write the appropriate wrapper for the existing API, and test and
> commit. This ensures that the existing API (now implemented as a
> wrapper) still behaves the same way, but it doesn't yet exercise
> whatever the new functionality is. I then just work my way up the
> stack for each related function. Structs are a little more involved,
> but can use similar principles.
>
> Johan, if you don't mind, I can put some of my (admittedly limited)
> tuits into looking at what needs to be done to rev the above struct.
I would definitely, absolutely not mind at all :-). Quite the contrary.
As a summary of what I did with svn_diff_fns_t (all in the context of
subversion/libsvn_diff):
- Added new function 'datasources_open' to svn_diff_fns_t (maybe
better to be named 'datasources_open_all' or something to have more
than 1 character difference with the other existing function
'datasource_open' :-), but I said that already).
- This new function is implemented in diff_file.c (the 'real' work of
prefix/suffix scanning) and in diff_memory.c (dummy implementation -
doesn't do anything for prefix/suffix scanning). Those were the only
internal implementors/providers of that vtable.
- I sent a mail once to the dev-list, asking if there were other known
implementors [1], but got no response. Of course, that's not a
definitive survey :-).
- The new vtable->datasources_open function is called from diff.c,
diff3.c and diff4.c.
- The only internal caller of the "old" function 'datasource_open'
(for a single datasource) doesn't call it anymore
(token.c#svn_diff__get_tokens) (there is no need anymore, since the
callers in diff.c, diff3.c and diff4.c already open the datasources
with 'datasources_open' before calling svn_diff__get_tokens).
Cheers,
--
Johan
[1] http://svn.haxx.se/dev/archive-2010-12/0083.shtml
Received on 2011-01-26 15:19:25 CET