Gerco Ballintijn writes:
> Hi,
>
> Peter Lundblad wrote:
> > Gerco Ballintijn writes:
> > 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...).
>
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!
We have binary compatibility guarantees that we can't break in the 1.x series.
You could argue that since svn_delta_default_editor exists, all user ought to
use it. I say that this is not necessarily the case, since there is
nothing requiring it in the documentation. And if I was a user who
was initializing all fields anyway, I might well consider this as a
utility function that I chose not to use. So even if we don't treat
our compatibility rules strictly, this is a dangerous change.
You can read more about our compatibility rules in the hacking
document. Hope this clarifies what I mean.
>
> >
> > 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...
>
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.
>
> >>
> >> 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.
Can you explain this further?
>
>
>
> >>>
> >>>> + 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).
Agreed. These are metadata that's not updated during the edit, but just set.
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 1 09:04:52 2007