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

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

From: Gerco Ballintijn <gerco_at_ballintijn.com>
Date: 2007-02-20 00:34:46 CET

Hi Malcolm,

Malcolm Rowe wrote:
>
> (Could you update libsvn_ra_svn/protocol next time round, though?)
>

Yep. It's on my to do list, but the issues below need to be addressed
first to know what to add/change.

>
> On Fri, Feb 09, 2007 at 05:36:52PM +0100, Gerco Ballintijn wrote:
>> David Glasser wrote:
>>> (For what it's worth, I still think that an API that lets you set
>>> revprops after running through the editor tree would be better, since
>>> you could do signatures and the like, but one could still do that with
>>> a preliminary "dry run" anyway.)
>>>
>> Do you mean pass them as apart of the svn_delta_editor_t::close_edit()
>> call? I guess moving the parameter from the get_commit_editor() calls
>> to the close_edit() is relatively straightforward; however, I have no
>> idea on the impact of that move on the ra_dav implementation. I have
>> not spent much time there (the code was somewhat frightening :-)
>>
>
> I think David might have been suggesting an additional call -
> change_rev_prop(), perhaps (or 'properties', accepting a hash?).
>

Yes, as I realize now there are three ways to go: (1) Provide the revision
properties as part of the get_commit_editor() call, what I did in my patch;
(2) Provide the revision properties through a new change_file_prop() "method"
in the svn_delta_editor_t struct; or (3) Provide the revision properties as
part of the close_edit() method of the svn_delta_editor_t struct. Method (1)
and (3) seem to have the least impact on the existing code, but method (2)
is the cleanest and most generic. (Method (2) and (3) both allow the revision
properties to be provided at the end).

Anyway, since method (2) is the most desirable, I have been looking into the
changes required for method (2). Unfortunately, the svn_delta_editor_t struct
is used throughout the code, which makes it rather laborious to determine the
correct ways to fill-in this structure in all those places. :-( So, any
suggestions on how to deal with adding an extra method to this structure
would be appreciated...

>>
>> A separate issue was (is) whether to ignore, to filter, or to flag the
>> standard revision properties as errors when passed in the table.
>>
>
> I think it would make sense to disallow any changes to the svn:
> properties, since they're already set through other processes (and you
> wouldn't want, for example, to have to work out whether someone could
> override svn:author).
>

But would such a prohibition be for the command-line client *only* or
for all (relevant) public APIs?

>
> I also wondered whether we should be calling the pre- and post-
> revprop-change hooks, or whether we just leave it to the pre-commit
> hook.
>

The notes in the bugtracker seem to suggest that since this new functionality
is not about changes historic data, but about creating new historic data, no
call to the revprop-change hooks is needed. This seems to me a reasonable
stance.

With regards,
Gerco.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Feb 20 00:35:03 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.