[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-02-28 21:13:01 CET

Hi,

Peter Lundblad wrote:
> Gerco Ballintijn writes:
>> On 2/20/2007 4:19 PM, Peter Lundblad wrote:
>>> Unfortunately, the editor is not documented to be extensible, so
>>> approaches 2 and 3 will require revising that struct, which will be a lot
>>> of work.
>>>
>>
>> I'm not sure what you mean with "not documented to be extensible."
>> Other structures have changed, right? I'm also not sure why it
>
> Other structs have been extended if they either are documented to be
> extensible in the public API (so, the _create/_dup functions are basically
> required to be used) or our APIs doesn't consume such structs. In this
> case, we have a factory function, but nothing requires a library to use
> it. Adding a new field that the library depends on means bad things.
> When we revise this interface, we obviously shouldn't remake this mistake.
>

I guess that in the general case these statements are true. Fortunately, we
are in a more specific case. The editor structure consist only of function
pointers that are initialized and then used. Its fields are not written and
read back, so there is currently no need to translate its contents or have
the ability to copy its contents (We obviously should not try to pass one of
as the other...).

>
> Yes, it is just a simple matter of API revising;)
> It is just that it will require lots of revved functions as well.
> If you want to do that, I'm all for it (in a separate patch, please).
>

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...

>>
>> Any implementation of this functionality will impact functions and/or
>> structures in the public APIs and the changes thus have to stay to
>> "2.0". So, avoiding an unsatisfactory solutions here seems desirable
>> to me.
>
> 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.

>
>> Well, I don't think it is commit-specific. The interface changes could
>> also be used to implement the svnsync and load/dump stuff. I would say
>> its more generic than that. The method should probably be called
>> change_edit_prop() to make it more generic. :-)
>>
> I don't understand why this would be useful for dump. It has direct
> access to the repository (and to be useful, it needs the props before
> any other editing operations anyway).
>

I was approaching this more from the conceptual point of view: dumping and
loading is another place where, beside tree changes, also "revprops" are
processed (unlike for instance diff-ing).

>>>
>>>> + change-rev-prop
>>>> + params: ( name:string [ value:string ] )
>>>> +
>>>
>>> When is it useful to specify no value? This is normally for deleting
>>> properties, but is that applicable here?
>>>
>> I chose this mainly because the lowlevel repos supported it. However,
>> I thought I could also prove useful to be able to set and later clear
>> a property.
>
> Why make this inconsistent with the other property modification
> functions in the editor?
>

Not much reason, but if the method cannot accept NULL values, it
should probably be called set_rev_prop and set-rev-prop since no
values are changed (or even better set_edit_prop and set-edit-prop,
given that "rev" is too commit-centric).

>
> I think we could extend the editor with a mechanism to set
> revision/transaction properties for the target of the editor, but I
> think it would be marginally useful. The only use case for setting
> revision properties after editor->open_root is called is commits with
> signatures, but no one knows how that would work AFAIK. So, I'm
> inclined to say that the approach in the first patch is good for our
> current purposes and that we should let the design of signed commits
> take care of what it needs when that day comes. Gerco, I'm sorry to
> waste your work in this way, so as I said before, let's decide on the
> design before continuing on the implementation. What do others think?
>

Yes, fine people of Subversion land... what say you?

Gerco.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Feb 28 21:13:20 2007

This is an archived mail posted to the Subversion Dev mailing list.