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

[Daniel Rall]

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

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.  ;-)

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

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

Thanks, Dan


...
> Prevent spurious conflicts on the merge property when the
> source branch contains merge property information for other
> branches. Previously, this caused a property conflict (when
> -b was not used) because the initial value on the source branch
> did not match the initial value on the target branch.
> 
> * contrib/client-side/svnmerge.py:
>   (action_merge): Before each merge, set the merge property
>     explicitly to the initial value of the source branch in
>     all cases instead of only when bidirectional merging is
>     enabled.
...

Index: svnmerge_test.py
===================================================================
--- svnmerge_test.py	(revision 22578)
+++ svnmerge_test.py	(working copy)
@@ -1060,20 +1060,14 @@
                     match=r"Committed revision 17")
         os.remove("svnmerge-commit-message.txt")
 
-        ### FIXME: Unfortunately, we're getting a conflict for the
-        ### merge info property in the following svnmerge invocation
-        ### due to Subversion's (reasonable) lack of property value
-        ### merging (which isn't possible without knowing a property's
-        ### MIME type)
-
         # Attempt a merge of changes from test-branch to
-        # testYYY-branch.
-        self.svnmerge("merge -r 16 -S ../test-branch")
-        try:
-            self.launch("svn commit -F svnmerge-commit-message.txt",
-                        match=r"Committed revision 18")
-        except AssertionError:
-            self.assertTrue(os.path.isfile("dir_conflicts.prej"))
+        # testYYY-branch, which causes a property conflict without
+        # machinations by svnmerge.
+        self.svnmerge("merge -r 14,16 -S ../test-branch")
+        self.launch("svn commit -F svnmerge-commit-message.txt",
+                    match=r"Committed revision 18")
+        ### This clobbers the /trunk merge info from r14  :-(
+        self.svnmerge("integrated -S test-branch", match=r"^13-16$")
 
 if __name__ == "__main__":
     # If an existing template repository and working copy for testing
Index: svnmerge.py
===================================================================
--- svnmerge.py	(revision 22577)
+++ svnmerge.py	(working copy)
@@ -1153,14 +1153,13 @@
     old_merge_props = branch_props
     merge_metadata = logs[opts["source-url"]].merge_metadata()
     for start,end in minimal_merge_intervals(revs, phantom_revs):
+        # Set the merge info property now to avoid a property conflict
+        # from Subversion if WC and merge source values differ.
+        new_merge_props = merge_metadata.get(start - 1)
+        if new_merge_props != old_merge_props:
+            set_merge_props(branch_dir, new_merge_props)
+            old_merge_props = new_merge_props
 
-        # Set merge props appropriately if bidirectional support is enabled
-        if opts["bidirectional"]:
-          new_merge_props = merge_metadata.get(start-1)
-          if new_merge_props != old_merge_props:
-              set_merge_props(branch_dir, new_merge_props)
-              old_merge_props = new_merge_props
-
         if not record_only:
             # Do the merge
             svn_command("merge -r %d:%d %s %s" % \
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: not available
URL: </pipermail/svnmerge/attachments/20061205/ed8faca5/attachment-0001.pgp>