[Svnmerge] [PATCH] Bidirectional merging patch for svnmerge.py

[Blair Zajac]

Giovanni Bajo wrote:
> Raman Gupta <rocketraman at fastmail.fm> wrote:
> 
> 
>>>>I appreciate that it's disabled by default, but it's still very
>>>>slow in my opinion. I think you can achieve a much better speed if
>>>>you add "--verbose" to the single "svn log" invokation done in
>>>>analyze_revs. Does it make sense to you?
>>>
>>>No, unfortunately log --verbose does not print the diffs for the
>>>integrated property, which is necessary in order to determine
>>>whether or
>>>not the revision is reflected. That information can only be obtained
>>>via "diff".
> 
> 
> You can still use "svn log --verbose" to select *only* those revisions where a
> property in the root directory is modified ("M" on the second column), and then
> do a diff only for those revisions. This is an order of magnitude better than
> what the patch does right now.
> 
> 
> 
> 
>>>>>>     # Calculate the base of analysis. If there is a "1-XX"
>>>>>>     interval in the # merged_revs, we do not need to check those.
>>>>>>     base = 1
>>>>>>     r = opts["merged_revs"].normalized()
>>>>>>     if r and r[0][0] == 1:
>>>>>>-        base = r[0][1]
>>>>>>+        base = r[0][1] + 1
>>>>
>>>>I don't think this is correct. Can you explain it?
>>>
>>>Sure, this is unrelated to the bidi patch (my bad) but it is a little
>>>optimization. If the interval is 1-XX, that means XX has already been
>>>merged.
> 
> 
> Ah fine, I'll commit this separately with a testcase.
> 
> 
> 
>>>-    revs = re.compile(r"^r(\d+)", re.M).findall(out)
>>>+
>>>+    revs = re.compile("^r(\d+)", re.M).findall(out)
> 
> 
> Any reason why you're changing the string from raw literal? As a matter of
> style, I prefer all regex literals to be raw strings so not to think about
> escaping issues. Regex are already cryptic without an extra level of
> escaping...
> 
> 
>>>+    # Check to see if the revisions list changed for the specific
>>>+    # target branch involved -- if it has then it is a reflected
>>>+    # revision
> 
> 
> Why do you use findall() on these regular expressions?
> 
> 
> 
>>>+                if(oldrevs != newrevs):
> 
> 
> Extra parentheses.
> 
> Thanks again for working on this!
> 
> Giovanni Bajo

Here's a modified version of Raman's original work.  This also includes the 
patch I sent in earlier today.

It takes into account some of Giovanni's comments above and also does the following:

1) Wrap to 80 characters in many places to keep in line with svn HACKING document.
2) Use svn log -v to find those revisions that have modified the source merge 
directory.
3) Rename to --bi-dir from --bidi.

Here's some timing tests:

		Old method	New method
CPU time	  16.99 sec	  10.14 sec
Clock time	1:09.80 sec	  33.97 sec

This logic reduced the number of revisions to check for reflected merges from 96 
to 30.

Much better.

I noticed that there's duplicate requests to the server for log messages, once 
for the list of revisions that have modified the source merge directory, and 
once for the the revisions that have changed.  This can be done in a separate 
commit.

Regards,
Blair
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: svnmerge-bi-dir-patch-1.txt
Url: /pipermail/svnmerge/attachments/20060221/abe2aa01/attachment.txt