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:
>>> build.conf
>>> subversion/include
>>> subversion/libsvn_subr
>>> subversion/tests/libsvn_subr
>>>
>>> These files / directories don't have svn:mergeinfo before merging.
>>> After merging, svn:mergeinfo on these files / directories doesn't contain:
>>> /trunk:1-31375,31378-32620
>>>
>>> (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:
>>> build.conf
>>> subversion/include
>>> subversion/libsvn_subr
>>> subversion/tests/libsvn_subr
>>>
>>> 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 .
>> /branches/1.5.x-r30215:30238
>> /branches/bdb-reverse-deltas:31976-32455
>> /branches/diff-callbacks3:29985-30687
>> /branches/dont-save-plaintext-passwords-by-default:30654-31044
>> /branches/gnome-keyring:30484-31336
>> /branches/in-memory-cache:29755-31378
>> /branches/issue-3000:31639,31642-31645,31647-31652,31654,31660
>> /branches/issue-3220-dev:32136-32152
>> /branches/kwallet:30711-31240
>> /branches/log-g-performance:30867-30958
>> /branches/svn-mergeinfo-enhancements:30045-30214
>> /branches/svnserve-logging:29754-30819
>> /trunk:31378-32500
>
> Arfrever,
>
> 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
> test).
Not too hard to do it turns out, test and fix in progress.
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-10 17:41:20 CEST