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!
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).
Working on both,
Paul
Received on 2008-09-09 22:32:21 CEST