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

Re: [PATCH] (2nd approach) Fix issue #1976: Set arbitrary revision properties (revprops) during commit.

From: Gerco Ballintijn <gerco_at_ballintijn.com>
Date: 2007-03-01 23:40:02 CET

Hi,

Peter Lundblad wrote:
>
> Say I am a user of libsvn_ra.h. I do:
> ...
> svn_delta_editor_t editor; /* Could allocate on heap as well. */
> /* Initialize the editor fields that are available in svn 1.4. */
> SomeType edit_baton;
> svn_ra_do_update(..., &editor, &edit_baton, ...);
> ...
>
> The RA layer calls your new function in my editor that was allocated with the
> old size and no function pointer for your new function. CRASH!
>

Hhmm, I think I see now. I was (mostly) think of "old code" calling
"new code," but you're refering to "new code" calling "old code."
One has to be careful when thinking about these callbacks. :-/

>
> You can read more about our compatibility rules in the hacking
> document.
>

I did read it (several times actually). The crux is the correct and
*complete* application of the ideas. :-)

I am correct in thinking that *it is solvable* by copying the user
provided editor structure, and extending it with a dummy set_edit_prop
function, as in the following. Right?

---- code ---- code ---- code ---- code ---- code ---- code ----

struct svn_delta_editor2_t;

static svn_error_t *
set_edit_prop(...)
{
   return SVN_ERR_NOT_IMPLEMENTED; /* or SVN_NO_ERR */
}

svn_delta_editor2_t *
svn_delta_editor2_from_editor(const svn_delta_editor_t *editor,
                               apr_pool_t *pool)
{
   svn_delta_editor2_t *editor2 = apr_palloc(pool, sizeof(*editor2))

   editor2->set_target_revision = editor->set_target_revision;
   editor2->set_edit_prop = set_edit_prop; /* <--- */
   editor2->open_root = editor2->open_root;
   ...
   editor2->close_edit = editor2->close_edit;

   return editor2;
}

svn_error_t *
svn_ra_do_update2(... , const svn_delta_editor2_t *update_editor,
                   void *update_baton, ...)
{
   ...
}

svn_error_t *
svn_ra_do_update(..., const svn_delta_editor_t *editor, void *baton, ...)
{
   svn_delta_editor2_t *editor2;

   editor2 = svn_delta_editor2_from_editor(editor, pool);

   return svn_ra_do_update2(..., editor2, baton, ...);
}

---- code ---- code ---- code ---- code ---- code ---- code ----

A quick scan of the subversion include directory reveals the
following affected functions, function-pointers, and structure.

svn_delta.h
-----------
svn_delta_editor_t
svn_delta_default_editor
svn_delta_get_cancellation_editor
svn_delta_path_driver

svn_ra.h
--------
svn_ra_get_commit_editor2
svn_ra_do_update
svn_ra_do_switch
svn_ra_do_status
svn_ra_do_diff2
svn_ra_replay
svn_ra_plugin_t::get_commit_editor
svn_ra_plugin_t::do_update
svn_ra_plugin_t::do_switch
svn_ra_plugin_t::do_status
svn_ra_plugin_t::do_diff
svn_ra_plugin_t (because of previous five)
svn_ra_get_ra_library (because of previous)

svn_ra_svn.h
------------
svn_ra_svn_get_editor
svn_ra_svn_drive_editor2

svn_repos.h
-----------
svn_repos_begin_report
svn_repos_dir_delta
svn_repos_replay2
svn_repos_get_commit_editor4
svn_repos_node_editor

svn_wc.h
--------
svn_wc_get_status_editor2
svn_wc_get_update_editor3
svn_wc_get_switch_editor3
svn_wc_get_diff_editor3
svn_wc_transmit_text_deltas2
svn_wc_transmit_prop_deltas

I'm not complete sure what to do with the svn_ra_plugin_t
structure, though. I think we can simply have two independent
svn_ra_get_ra_library() svn_ra_get_ra_library2() functions here
that return the svn_ra_plugin_t and svn_ra_plugin2_t structures.

>>
>> My intention was always to discuss and, if needed, create a v2 editor
>> structure. I don't think its that much work. I'm not sure what you mean
>> with a separate patch though...
>>
> I mean that this patch is getting big if you revise the editor as well and
> therefore gets hard to review. A nice way of splitting it would be to revise
> the editor in one patch and do the rest in another. The former patch is
> useful even if the an incarnation of set_edit_or_txn_or_rev_..._prop is
> not added.
>

I'm not sure what the purpose would be of the part without the method
set_edit_prop. What (generic) aspect of the editor would be revised?
(and in what way?)

>>>
>>> Yes. OTOH, no one has come up with a design for something that actually
>>> needs the ability to set revprops at abitrary times, so we don't know if
>>> this solution is satisfactory either. Designing an API for some needs
>>> we don't know yet might just be wasted work.
>>>
>>
>> Fair enough. I had this idea about adding client-specific summarizing
>> information in the revision, but nothing concrete.
>
> Can you explain this further?
>

I was thinking along the lines of properties specifing the number of lines
changed in plain text files, the number of changed files in general, or
some other commit or change statistic; a language-specific client that
stores the names of the changed functions, classes, etc.; or a performance
tracking client that stores commit durations in the server for later
aggragation. Something like that, nothing particularly worked out.

Gerco.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 1 23:40:30 2007

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.