[Svnmerge] [PATCH] Bidirectional patch take 2

[Blair Zajac]

Giovanni Bajo wrote:
> Blair Zajac <blair at orcaware.com> wrote:
> 
> 
>>>Index: svnmerge_test.py
>>>===================================================================
>>>--- svnmerge_test.py (revision 18600)
>>>+++ svnmerge_test.py (working copy)
>>>@@ -336,7 +336,7 @@
>>>
>>>             self.launch("svn co
>>>%(TEMPLATE_REPO_URL)s/branches/test-branch")
>>>
>>>-            atexit.register(lambda: rmtree(template_path))
>>>+        atexit.register(lambda: rmtree(template_path))
> 
> 
> This is a separate fix which I had in my repository and you can commit
> separately.

Committed separately.

>>>+        for rev in potential_reflected_revs:
>>>+            if rev-1 == previous_rev:
>>>+                old_props = previous_props
>>>+            else:
>>>+                old_props = get_revlist_prop(url, opts["prop"], rev-1)
>>>+
>>>+            new_props = get_revlist_prop(url, opts["prop"], rev)
>>>+            previous_props = new_props
>>>+            previous_rev = rev
>>>+
>>>+            old_revisions = old_props.get(target_dir)
>>>+            new_revisions = new_props.get(target_dir)
>>>+
>>>+            if new_revisions != old_revisions:
>>>+                reflected_revs.append("%s" % rev)
> 
> 
> I'd rename potential_reflected_revs into something like prop_changed_revs
> with a comment like "this holds the list of revisions in which the
> svnmerge-integrated property was modified in head".

I renamed it to prop_changed_revs.  The comment is slightly different, as this 
holds property changes where the svnmerge-integrated property could have changed.

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

If Raman or Giovanni want to come up with a test suite for this fix, then we can 
get this fix in.  I didn't commit this one line change.

>>>+    The --bidirectional option will additionally ignore reflected
>>>revisions +    i.e. changes on the source branch that were originally
>>>merged from the +    target, and therefore should generally not be merged
>>>back into the target. +    Reflected revisions will cause svn to produce
>>>spurious conflicts if any +    changes were made to the merged code on
> 
> the
> 
>>>     branch.""", [
> 
> 
> We usually call the "source branch" "head" in the documentation, so I'd
> stick to this convention. I think we should also try and explain a little be
> more. My revised version is this:
> 
> """
> When svnmerge is used to bidirectionally merge changes between the branch
> and its head, it is necessary to not merge the same changes forth and back:
> eg, if
> you committed a merge of a certain revision of the branch into the head, you
> do
> not want that commit to appear as available to merged into the branch
> (as the code originated in the branch itself!). svnmerge can skip these
> so-called
> "reflected" revisions if you specifiy the option --bidirectional.
> """

Thanks.  I put this language into the merge and avail subcommand help.
> 
> 
> Blair, I finished reviewing your patch and it's ok for me to check it in.
> Many thanks to everybody for the patience!

Committed in rev 18614.

Regards,
Blair