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 2006/08/01 06:36:16 UTC

Re: [merge-tracking] record mergeinfo on wc to wc copy

On Mon, 31 Jul 2006 23:22:51 +0530, Daniel Rall <dl...@collab.net> wrote:

> On Sat, 29 Jul 2006, Madan S. wrote:
>
>> On Sat, 29 Jul 2006 03:12:49 +0530, Daniel Rall <dl...@collab.net> wrote:
>>

[snip]

> The "copyfrom" history is every revision where the merge source
> existed at the path it's pegged at for the 'copy' operation.

I see... my understanding was wrong. Thank you for correcting.

[snip]

> It would matter to the current merge tracking implementation (which
> assumes that "svn:mergeinfo" is accurate and follows it blindly),
> since some other object may've existed at the copy source's path in
> earlier revisions.

yup, yup. Good point.

[snip]

>> yeah, and I think we can use the existing update_wc_merge_info()  
>> function.
>> (Which brings me to another point in a tangential direction - can we
>> abstract the parse_merge_info() call into update_wc_merge_info()?
>> Currently we call parse_merge_info() before calling
>> update_wc_merge_info().)
>
> The call, yes.  The routine, no (since we call it elsewhere).
>
> It's already like this in my WC as I've been working on the
> notification handling code (for skips, etc.).  I'll factor some of
> that out and make an incremental commit today.

Do you mean 'parse_merge_info() must be called inside  
update_wc_merge_info()' - yes
'we can use update_wc_merge_info() for copy operations' - no

?

[snip]

> IIUC, making a WC routine "loggy" consists of writing a set of
> commands for an operation into a file (in XML, I believe), then
> executing the commands in sequence after they've all been written to
> the file.

ah, yes. Karl explained this logging to me once. Will find out more about  
this.

So, we need to do logging for all wc operations? I dont remember seeing  
logging for the svn:mergeinfo property manipulations?

Regards,
Madan.

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

Re: [merge-tracking] record mergeinfo on wc to wc copy

Posted by Daniel Rall <dl...@collab.net>.
On Thu, 03 Aug 2006, Madan S. wrote:

> On Tue, 01 Aug 2006 22:07:57 +0530, Daniel Rall <dl...@collab.net> wrote:
> 
> >On Tue, 01 Aug 2006, Madan S. wrote:
> >...
> >>>>yeah, and I think we can use the existing update_wc_merge_info()
> >>>>function.
...
> IMHO, after reading through update_wc_merge_info(), I think it is the  
> right function. But for one problem, which is:
> 
> I just tried writing some code to call update_wc_merge_info() from  
> svn_wc_copy2(). But for this we need
> 
> - the client ctx in svn_wc_copy2() to pass to update_wc_merge_info(), and  
> hence...
> - remove the cancel_func, cancel_baton, notify_func, notify_baton  
> parameters, introduce the svn_client_ctx_t parameter instead, creating  
> svn_wc_copy3() and deprecating svn_wc_copy2()
> 
> My concern was: Is it acceptable to use a svn_client_ctx_t structure in a  
> wc library?

Good concern -- reference to the svn_client_ctx_t data type in
libsvn_wc is a no-go.

...
> If no, I have a bigger question. The above ctx is required by  
> update_wc_merge_info() to be passed to parse_merge_info(). IIUC,  
> parse_merge_info() will obtain the mergeinfo of the provided wcpath (we  
> dont need to recurse here). For this we should not need a client context -  
> logically.
> 
> But currently, parse_merge_info() calls svn_client__get_prop_from_wc
> (with recurse = TRUE? why?), where the ctx is used.

The merge info of paths under TARGET_WCPATH will have bearing on how
merge info is updated; paths which have had local modifications in the
WC or which were modified by the merge must have their merge info
differences taken into consideration when updating the WC's merge
info.

> Am wondering why parse_merge_info() cannot directly use
> pristine_or_working_propval(). This would not only solve the problem
> of having to pass client ctx from a wc call, but would also be,
> IMHO, the logical thing - not using a client ctx for obtaining a wc
> information, non-recursively.

If we ended up not needing the recursion, a solution which combined
the "pristine or working value" behavior of pristine_or_working_propval()
with the "give me only one property value" behavior of svn_wc_prop_get()
would be ideal.

Assuming we need to call parse_merge_info() for 'copy' (and I think we
do), we should try stripping down the svn_client__get_prop_from_wc()
module-private API -- which I factored out especially for use on the
merge-tracking branch -- to an incarnation closer to the level of the
WC, preferably not polluted by the svn_client_ctx_t data type.
Basically, figure out how you can change those API to suite our needs,
since there are no compatibility concerns.

- Dan

Re: [merge-tracking] record mergeinfo on wc to wc copy

Posted by Madan U Sreenivasan <ma...@collab.net>.
On Tue, 01 Aug 2006 22:07:57 +0530, Daniel Rall <dl...@collab.net> wrote:

> On Tue, 01 Aug 2006, Madan S. wrote:
> ...
>> >>yeah, and I think we can use the existing update_wc_merge_info()
>> >>function.

[snip]

> We might be able to use something like update_wc_merge_info() for copy
> ops -- dunno yet.

IMHO, after reading through update_wc_merge_info(), I think it is the  
right function. But for one problem, which is:

I just tried writing some code to call update_wc_merge_info() from  
svn_wc_copy2(). But for this we need

- the client ctx in svn_wc_copy2() to pass to update_wc_merge_info(), and  
hence...
- remove the cancel_func, cancel_baton, notify_func, notify_baton  
parameters, introduce the svn_client_ctx_t parameter instead, creating  
svn_wc_copy3() and deprecating svn_wc_copy2()

My concern was: Is it acceptable to use a svn_client_ctx_t structure in a  
wc library?

(If yes, I will send across the patch that I just wrote. But I doubt it.)

If no, I have a bigger question. The above ctx is required by  
update_wc_merge_info() to be passed to parse_merge_info(). IIUC,  
parse_merge_info() will obtain the mergeinfo of the provided wcpath (we  
dont need to recurse here). For this we should not need a client context -  
logically.

But currently, parse_merge_info() calls svn_client__get_prop_from_wc (with  
recurse = TRUE? why?), where the ctx is used.

Am wondering why parse_merge_info() cannot directly use  
pristine_or_working_propval(). This would not only solve the problem of  
having to pass client ctx from a wc call, but would also be, IMHO, the  
logical thing - not using a client ctx for obtaining a wc information,  
non-recursively.

Regards,
Madan U S
www.symonds.net/~madan

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

Re: [merge-tracking] record mergeinfo on wc to wc copy

Posted by Daniel Rall <dl...@collab.net>.
On Tue, 01 Aug 2006, Madan S. wrote:
...
> >>yeah, and I think we can use the existing update_wc_merge_info()  
> >>function.
> >>(Which brings me to another point in a tangential direction - can we
> >>abstract the parse_merge_info() call into update_wc_merge_info()?
> >>Currently we call parse_merge_info() before calling
> >>update_wc_merge_info().)
> >
> >The call, yes.  The routine, no (since we call it elsewhere).
> >
> >It's already like this in my WC as I've been working on the
> >notification handling code (for skips, etc.).  I'll factor some of
> >that out and make an incremental commit today.
> 
> Do you mean 'parse_merge_info() must be called inside  
> update_wc_merge_info()' - yes
> 'we can use update_wc_merge_info() for copy operations' - no
> 
> ?

As of r20921, parse_merge_info() is called inside
update_wc_merge_info().  I was saying "no" to entirely removing
parse_merge_info().

We might be able to use something like update_wc_merge_info() for copy
ops -- dunno yet.

...
> >IIUC, making a WC routine "loggy" consists of writing a set of
> >commands for an operation into a file (in XML, I believe), then
> >executing the commands in sequence after they've all been written to
> >the file.
> 
> ah, yes. Karl explained this logging to me once. Will find out more about  
> this.
> 
> So, we need to do logging for all wc operations? I dont remember seeing  
> logging for the svn:mergeinfo property manipulations?

We haven't added any special loggy-ness for the "svn:mergeinfo"
property.  It's behavior is currently the same as other properties.

- Dan

Re: [merge-tracking] record mergeinfo on wc to wc copy

Posted by Madan U Sreenivasan <ma...@collab.net>.
On Tue, 01 Aug 2006 12:06:16 +0530, Madan U Sreenivasan <ma...@collab.net>  
wrote:

> On Mon, 31 Jul 2006 23:22:51 +0530, Daniel Rall <dl...@collab.net> wrote:
>>
>> The call, yes.  The routine, no (since we call it elsewhere).
>>
>> It's already like this in my WC as I've been working on the
>> notification handling code (for skips, etc.).  I'll factor some of
>> that out and make an incremental commit today.
>
> Do you mean 'parse_merge_info() must be called inside  
> update_wc_merge_info()' - yes
> 'we can use update_wc_merge_info() for copy operations' - no
>
> ?

Pl. ignore the above question.

I just noticed r20921, where you have committed calling parse_merge_info()  
 from within update_wc_merge_info().

Regards,
Madan.

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