[Svnmerge] [PATCH] Fix for bidirectional merging corner cases

[David James]

> David James wrote:
> > Nice catch, Raman! Could you convert your 'template.sh' test into a
> > regular test that will run in svnmerge_test.py? If not, don't worry --
> > I'll get to it.
> Sure, but it might take me a few days...

Thanks! CC me when you submit your patch.

> In this new section of code:
>
>             try:
>                 old_value = self.raw_get(log.begin-1)
>             except LaunchError:
>                 # The specified URL might not exist before the
>                 # range of the log. If so, we can safely assume
>                 # that the property was empty at that time.
>                 old_value = { }
>
> Wouldn't it be better to check the situation of the non-existent URL
> explicitly rather than relying on LaunchError? LaunchError might occur
> for other reasons.

Yes, definitely! Can you think of a clean way to implement that?

> In this section of code (the one which my original patch addressed),
> my patch was setting old_revs only for the first loop -- it looks like
> here some extra work is being done because old_revs is initialized at
> the beginning of the loop every time, which shouldn't be required
> because of the last line in the loop:
>
>         for rev in merge_metadata.changed_revs():
>             old_revs = merge_metadata.get(rev-1).get(target_dir)
>             new_revs = merge_metadata.get(rev).get(target_dir)
>             if new_revs != old_revs:
>                 reflected_revs.append("%s" % rev)
>             old_revs = new_revs

merge_metadata maintains its own cache of property values, so there's
no need to maintain a separate cache here (and add complexity). Still,
I intended to remove the 'old_revs = new_revs' line because it is not
needed.

I committed a change which simplified and optimized this bit of code
to avoid the cache lookups in r20074, by simply iterating through the
changed revs and changed values.

Cheers,

David

-- 
David James -- http://www.cs.toronto.edu/~james