[Svnmerge] [PATCH] Bidirectional patch take 2

[Giovanni Bajo]

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.


>> +        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".


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

This is a minor optimization and should be done in a separate commit
(possibly with a testcase, shouldn't be too hard).


>> +    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.
"""


Blair, I finished reviewing your patch and it's ok for me to check it in.
Many thanks to everybody for the patience!
-- 
Giovanni Bajo