[Svnmerge] [PATCH] Eliminate spurious svnmerge-integrated property conflicts

[Raman Gupta]

Daniel Rall wrote:
> On Fri, 06 Oct 2006, Raman Gupta wrote:
> 
>> Sometimes, svnmerge.py will cause spurious conflicts in the merge
>> memory property. The last step of the action_merge function sets this
>> property explicitly to the new correct value anyway, so any conflicts
>> that occurred during the merge are irrelevant.
> 
> Raman, thanks for the patch.  Sorry it slipped through the cracks for
> so long.  I've written an additional test case for just this problem,
> and below provide both your patch (tweaked) and the activation of my
> test case for review.  (We might want to rename my test case; can you
> think of a better name?)

Thanks for the extra test case. I don't have any better names yet but
perhaps this discussion will help us come up with one.

> While I think your patch makes us quite a bit better off than before,
> I noticed that it does also clobber any transitive merge info from a
> merge source which itself would otherwise attempt to be applied (and
> cause a conflict) as part of the merge.  See the ### comment in my
> test case to make sense of that mouthful.  ;-)

Yes, I should have been clear in my patch description about that. I
think that the ability of svnmerge.py to carry-through this transitive
merge data has been an accidental side-effect of the implementation
rather than a designed-in feature, and so I've never really trusted
it. I tend to want to specifically add merge relationships between
branches rather than have them carry over implicitly.

For example, most of the time, in my workflow at least, the
"transitive" merge information is actually cruft that doesn't make
sense e.g. I end up with merge info about one release branch on
another release branch because they share the trunk as ancestor, and
bidirectionally merge with the trunk also. So the patch I submitted
actually improves this situation for me by eliminating the carry-over
of those values.

If keeping transitive merge data is important to people, perhaps what
is needed is a discussion of the transitive use cases people wish to
support, and then a specific implementation to achieve the
requirements, perhaps enabled by use of a "--transitive" flag.

> Also, I noticed that your patch removes one of the two main use cases
> for the "--bidirectional" option, the other being the gating of
> RevisionLog.merge_metadata().  While I haven't profiled it, I assume
> this is so expensive an operation that we need to continue to gate it
> (e.g. --bidirectional can't be made the default)?

I'd be +1 on making bidirectional the default but we've discussed
other solutions in the past that could obviate the decision -- e.g.
the config pickle for setting options and Michael's suggestion also.

> On an orthogonal note, Michael Haggerty was hoping to make
> --bidirectional "sticky".  Did you have any thoughts on that?

I definitely like the idea :-) Just to throw out a couple of
implementations -- we could prefix the merge property range with a "b"
to indicate bidirectional stickiness. Or we could have a second
property to store this info (though I like this one less as it
destroys the all-in-one-place nature of the merge property).

Cheers,
Raman