[Svnmerge] [PATCH] Handle transitive merging property conflicts for both merged/blocked revs

[Raman Gupta]

Dustin J. Mitchell wrote:
> On Thu, Jul 05, 2007 at 09:02:37PM -0400, Raman Gupta wrote:
>> The patch simply avoids conflicts be removing the integrated/blocked
>> properties altogether before calling merge, and then resets them to
>> their known values after the merge is complete.
> 
> If I understand correctly, this basically means that there will be a
> property conflict on almost every merge, but those conflicts are ignored
> and the property simply set to the correct value at the end.  I have two
> concerns:

No, the point of removing the properties before the merge is to
completely avoid a property conflict -- since they are removed the
source branch can have absolutely anything. This works because merge
--force is being called.

> 1. Why do anything with the properties if they're going to cause
> conflicts and be overwritten anyway?

See above. The other way is to not do anything with the properties,
and then do "svn revert ." to deal with conflicts. However, without a
bunch of extra logic that will also revert other non-svnmerge property
changes too, so this solution is cleaner.

> 2. Are we losing information in this process?  That is, if we're merging
> A -> B -> C, and revision r on B was a merge from A to B, then
> svnmerge-integrated would change from, let's say, A:1-36,38-39 to
> A:1-39 in that revision.  In merging revision r to C, doesn't it seem
> reasonable to also indicate that r37 from A is now present in C?

If you are consistently merging transitively from A -> B -> C, then it
doesn't matter that r37 from A is present in C. In only matters that
revision r in B is present in C, since it is clear that r37 in A is
included in r.

So no information is lost UNLESS you eventually want to merge from A
directly into C after merging via B. In that case you're in the realm
of graph merging and I can pretty much guarantee you'll run into other
issues with this since svnmerge.py currently supports that only by
accident and is likely to fail on various corner cases, and in various
ways, including property conflicts.

You will also notice the TODO in the patch that hints at the solution
for this too.

This has all been argued before in previous threads :-)

> All that said, this is a small change, and if it makes the test case
> pass, then it will satisfy those folks who aren't concerned with merges
> from A to C, so I'm on board with it.

Yes it does, and ok.

> Dustin

Thanks for the review.

Cheers,
Raman