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

[Giovanni Bajo]

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