You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Madan U Sreenivasan <ma...@collab.net> on 2005/09/21 12:00:34 UTC

rfc on structure changes

Hi,

   This is a request for modifying the way certain publicly available
structures are written. The below is the only example am able to give...
but its possible that there are more like this...

   svn_ra_plugin_t structure in subversion/include/svn_ra.h. I need to
change a parameter of the svn_commit_callback_t member.
   To do this, I have to
- deprecate svn_ra_plugin_t with svn_ra_plugin2_t
- replace/deprecate all functions using svn_ra_plugin_t

   This is a non-trivial way to make a change to a data structure...

   An easier way to do such a change would be to deprecate only the
member that needs to change with a new member, added at the bottom.

   I know these are not the best ways to achieve what is required here.
I would like start a thread/issue to discuss and implement the changes
required to make life easier for us in future.


   Pl. comment on the above.

Regards,
Madan.

FWIW, am pasting my monologue on the svn-dev channel...
=================================================================
<madan> hi folks, I need some advice
<madan> In subversion/include/svn_ra.h, 
<madan> I want to replace the get_commit_editor member of the
svn_ra_plugin_t structure
<madan> to take a svn_commit_callback2_t instead of 
<madan> svn_commit_callback_t
<madan> but this would require deprecating the whole of svn_ra_plugin_t
<madan> and replacing all the functions using it
<madan> am wondering if theres an easy way out!
<madan> I was thinking of adding a new member called get_commit_editor2
at the end of the structure...
<madan> and opening an issue to fix this problem....
<madan> the problem - of making changes to structures less intrusive
=================================================================


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: rfc on structure changes

Posted by Branko Čibej <br...@xbc.nu>.
Madan U Sreenivasan wrote:

>Hi,
>
>   This is a request for modifying the way certain publicly available
>structures are written. The below is the only example am able to give...
>but its possible that there are more like this...
>
>   svn_ra_plugin_t structure in subversion/include/svn_ra.h. I need to
>change a parameter of the svn_commit_callback_t member.
>   To do this, I have to
>- deprecate svn_ra_plugin_t with svn_ra_plugin2_t
>- replace/deprecate all functions using svn_ra_plugin_t
>
>   This is a non-trivial way to make a change to a data structure...
>
>   An easier way to do such a change would be to deprecate only the
>member that needs to change with a new member, added at the bottom.
>
>   I know these are not the best ways to achieve what is required here.
>I would like start a thread/issue to discuss and implement the changes
>required to make life easier for us in future.
>  
>
You can only do that with structures that are not allocated by their 
user but by a constructor function. We made this change recently with 
svn_ra_callbacks2_t; the original didn't have a constructor, the new 
structure does.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: rfc on structure changes

Posted by Madan U Sreenivasan <ma...@collab.net>.
On Wed, 2005-09-28 at 15:11, Peter N. Lundblad wrote:
> On Wed, 28 Sep 2005, Madan U Sreenivasan wrote:
> 
> > On Thu, 2005-09-22 at 00:17, Peter N. Lundblad wrote:
> > > On Wed, 21 Sep 2005, Madan U Sreenivasan wrote:
> [snip]
> > > You don't need to change anything in that struct:-) It is deprecated and
> > > not used anymore except for backwards compatibility. The internal
> > > svn_ra__vtable_t can be changed at will.
> >   I'm stuck trying to change svn_ra__vtable_t (at will ;) ). Here, am
> > having to change the signature of the get_commit_editor member to use
> > svn_commit_callback2_t.
> >
> >   Now, svn_ra_get_commit_editor() will have to be deprecated, and will
> > force us to retain usage of svn_commit_callback_t. Now this will force
> > us to retain all callbacks(even private) and functions that use them,
> > till the repos layer and for all three ra libraries.
> 
> Why can't you just create a wrapper callback in the deprecated
> svn_ra_get_commit_editor. You create a function with the callback2
> signature, and a baton where you save the old-style callback and baton.
> Then, in svn_wc_get_commit_editor, you give that callback to
> svn_wc_get_commit_editor2. Your callback will call the old-style callback
> given to the deprecated function, extracting the fields from the struct. I
> haven't lookd in detail, so maybe *I* am missing something.
I was thinking the same too. But where do we map the user given
callback(svn_commit_callback_t) to our new wrapper
callback(svn_commit_callback2_t). In a global variable? I will look at
this option with more focus todya...


> 
> Best,
> //Peter


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: rfc on structure changes

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 28 Sep 2005, Madan U Sreenivasan wrote:

> On Thu, 2005-09-22 at 00:17, Peter N. Lundblad wrote:
> > On Wed, 21 Sep 2005, Madan U Sreenivasan wrote:
> >
> > >    This is a request for modifying the way certain publicly available
> > > structures are written. The below is the only example am able to give...
> > > but its possible that there are more like this...
> > >
> > >    svn_ra_plugin_t structure in subversion/include/svn_ra.h. I need to
> > > change a parameter of the svn_commit_callback_t member.
> >
> > You don't need to change anything in that struct:-) It is deprecated and
> > not used anymore except for backwards compatibility. The internal
> > svn_ra__vtable_t can be changed at will.
>   I'm stuck trying to change svn_ra__vtable_t (at will ;) ). Here, am
> having to change the signature of the get_commit_editor member to use
> svn_commit_callback2_t.
>
>   Now, svn_ra_get_commit_editor() will have to be deprecated, and will
> force us to retain usage of svn_commit_callback_t. Now this will force
> us to retain all callbacks(even private) and functions that use them,
> till the repos layer and for all three ra libraries.

Why can't you just create a wrapper callback in the deprecated
svn_ra_get_commit_editor. You create a function with the callback2
signature, and a baton where you save the old-style callback and baton.
Then, in svn_wc_get_commit_editor, you give that callback to
svn_wc_get_commit_editor2. Your callback will call the old-style callback
given to the deprecated function, extracting the fields from the struct. I
haven't lookd in detail, so maybe *I* am missing something.

Best,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: rfc on structure changes

Posted by Madan U Sreenivasan <ma...@collab.net>.
On Thu, 2005-09-22 at 00:17, Peter N. Lundblad wrote:
> On Wed, 21 Sep 2005, Madan U Sreenivasan wrote:
> 
> >    This is a request for modifying the way certain publicly available
> > structures are written. The below is the only example am able to give...
> > but its possible that there are more like this...
> >
> >    svn_ra_plugin_t structure in subversion/include/svn_ra.h. I need to
> > change a parameter of the svn_commit_callback_t member.
> 
> You don't need to change anything in that struct:-) It is deprecated and
> not used anymore except for backwards compatibility. The internal
> svn_ra__vtable_t can be changed at will.
  I'm stuck trying to change svn_ra__vtable_t (at will ;) ). Here, am
having to change the signature of the get_commit_editor member to use
svn_commit_callback2_t.

  Now, svn_ra_get_commit_editor() will have to be deprecated, and will
force us to retain usage of svn_commit_callback_t. Now this will force
us to retain all callbacks(even private) and functions that use them,
till the repos layer and for all three ra libraries.
  What more, svn_commit_callback_t uses individual commit info values
instead of the comprehensive svn_commit_info_t structure. This means, we
need to retain all private functions with individual commit information
parameters, instead of changing to the more efficient svn_commit_info_t
structure. :/

  Am I missing something here? Is there a easy way to do this, other
than retaining all private functions that use svn_commit_callback_t and
individual commit information vars instead of svn_commit_info_t?

Regards,
Madan.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: rfc on structure changes

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 21 Sep 2005, Madan U Sreenivasan wrote:

>    This is a request for modifying the way certain publicly available
> structures are written. The below is the only example am able to give...
> but its possible that there are more like this...
>
>    svn_ra_plugin_t structure in subversion/include/svn_ra.h. I need to
> change a parameter of the svn_commit_callback_t member.

You don't need to change anything in that struct:-) It is deprecated and
not used anymore except for backwards compatibility. The internal
svn_ra__vtable_t can be changed at will.


 >    To do this, I have to
> - deprecate svn_ra_plugin_t with svn_ra_plugin2_t
> - replace/deprecate all functions using svn_ra_plugin_t
>
Yes, it got cumbersome. That's the reason for revving the whole RA API.

>    An easier way to do such a change would be to deprecate only the
> member that needs to change with a new member, added at the bottom.

That has happened before. See r11155. It introduced a get_log2 function.
As someone said, you can do this in certain situations.
>
>    I know these are not the best ways to achieve what is required here.
> I would like start a thread/issue to discuss and implement the changes
> required to make life easier for us in future.

Trying to make it easier to extend APIs in the future is a good idea IMO,
if it can be done without cluttering the current API (like adding dummy
arguments "just in case" or such things).  One way to do this is by
providing a constructor as Branko said. But, we are forgetting about it
all the time, so keeping an eye on this is good...

Best,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org