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

[Giovanni Bajo]

Raman Gupta <rocketraman at fastmail.fm> wrote:

>> Attached is a bidirectional merging patch for the latest version of
>> svnmerge.py (rev 238).  It removes reflected revisions (i.e. changes
>> on the branch that were originally merged from the target, and therefore
>> should generally not be merged back into the target) from the
>> available/merge list. Reflected revisions can cause spurious
>> conflicts if any changes were made to the merged code on the branch.

Hi Raman, thanks for the patch. I don't feel very fond of this feature because
I don't think it really solves anything, but I'm not going to stay in its way.
I admit I have no widespread experience with bidi merges, so I'm going to trust
your and others' word on this. So I'll comment only on its technical merit and
happily commit it when it's technically sound.

>> The upside to this is that no dot-files are required, the downside
>> is that it is slow because it requires a remote svn diff call for
>> each possible revision to be merged.

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?

>> +    def __len__(self):
>> +        return len(self._revs.keys())

len(self._revs) is sufficient.

>>      # 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?

>> tracking (merging from multiple heads)"),
>> +    Option("-b", "--bidi", value=True, default=False,
>> +              help="check for reflected revisions that can cause conflicts
"
>> +                  "when merging bidirectionally between branches)"), ]

closing parens ')' without matching opening parens. I'd appreciate if you also
add some blurb to the "svnmerge merge" description which explains what
reflected revisions are. In fact, I was going to add something about phantom
revisions in the "svnmerge avail" description.

Thanks again for the patch!

Giovanni Bajo