On Wed, Sep 10, 2008 at 11:40 AM, Paul Burba <ptburba_at_gmail.com> wrote:
> On Tue, Sep 9, 2008 at 4:32 PM, Paul Burba <ptburba_at_gmail.com> wrote:
>> On Mon, Sep 1, 2008 at 11:46 AM, Arfrever Frehtes Taifersar Arahesis
>> <arfrever.fta_at_gmail.com> wrote:
>>> 2008-09-01 17:39:20 Arfrever Frehtes Taifersar Arahesis napisał(a):
>>>> I discovered 2 bugs in handling of svn:mergeinfo when I was performing some
>>>> merges on the svnpatch-diff branch.
>>>> 1. Addition of incorrect svn:mergeinfo on some files or directories:
>>>> Steps to reproduce:
>>>> svn co -r32624 https://svn.collab.net/repos/svn/branches/svnpatch-diff subversion-svnpatch-diff-r32624
>>>> cd subversion-svnpatch-diff-r32624
>>>> svn merge -r32500:32620 ^/trunk
>>>> This merge adds incorrect svn:mergeinfo on:
>>>> These files / directories don't have svn:mergeinfo before merging.
>>>> After merging, svn:mergeinfo on these files / directories doesn't contain:
>>>> (Next `svn merge ^/trunk` would try to merge r25664:30782 from trunk which causes
>>>> many conflicts.)
>>>> 2. Incomplete reversion of changes during reverse merging:
>>>> Steps to reproduce:
>>>> svn co -r32625 https://svn.collab.net/repos/svn/branches/svnpatch-diff subversion-svnpatch-diff-r32625
>>>> cd subversion-svnpatch-diff-r32625
>>>> svn merge -c-32625 ^/branches/svnpatch-diff
>>>> r32625 added (incorrect) svn:mergeinfo on:
>>>> Reverse merging should delete svn:mergeinfo property, but it leaves empty svn:mergeinfo.
>>> 3. Reverse merging mentioned above removes '/trunk:r1-31375' from svn:mergeinfo on branch root:
>>> $ svn pg svn:mergeinfo .
>> Yes, that is most definitely wrong, we should see mergeinfo of
>> '/trunk:1-31375,31378-32500', i.e. the mergeinfo that existed @32624.
>> It is worth noting that a portion of this mergeinfo, '/trunk:1-28961',
>> is self-referential, that is to say it explicitly describes
>> paths/revisions that are part of the svn/branches/svnpatch-diff
>> branch's natural history (the branch was copied from trunk_at_28961).
>> This came about because back in January when this branch was created
>> we were still explicitly recording natural history in explicit
>> mergeinfo when doing a copy.
>> Now for the specifics: What's going wrong here is that when we perform
>> the reverse merge of -c-32625,
>> libsvn_subr/merge.c:filter_self_referential_mergeinfo() is filtering
>> out /trunk:1-31375' portion of svnpatch-diff's mergeinfo. As
>> mentioned above it should only filter out '/trunk:1-28961', so that is
>> one bug. But I can't see any reason to ever do filtering of this type
>> during a *reverse* merge. Currently the svn_wc_diff_callbacks3_t
>> callback merge.c:merge_props_changed() calls
>> filter_self_referential_mergeinfo() without consideration of the
>> merge's direction.
>> So we need to do the following:
>> 1) Stop filtering self-referential mergeinfo during reverse merges
>> (easier said than done as we don't have enough context in the callback
>> to determine merge direction right now). If you or anything else can
>> see a reason this filtering needs to be done in reverse merges please
>> let me know!
> The attached patch takes care of this. I'll commit later today if
> there are no objections.
>> 2) Determine if the filtering bug can occur during forward merges and
>> fix it (I'm pretty certain it can, just trying to come up with a
> Not too hard to do it turns out, test and fix in progress.
Added a new merge test in r33013 where valid mergeinfo (i.e. not part
of the target's natural history) is actually filtered out.
Received on 2008-09-11 15:44:41 CEST