On Wed, Nov 26, 2008 at 5:36 PM, Bert Huijben <bert_at_vmoo.com> wrote:
>
>
>> -----Original Message-----
>> From: Paul Burba [mailto:ptburba_at_gmail.com]
>> Sent: Wednesday, November 26, 2008 6:05 PM
>> To: Michael Susser
>> Cc: dev_at_subversion.tigris.org
>> Subject: Re: Bug when merging with dry-run switch?
>>
>> On Wed, Nov 26, 2008 at 2:17 AM, Michael Susser
>> <hdmlist_at_googlemail.com> wrote:
>> > Hello community,
>> >
>> > i think i have found a small bug regarding dry-run-merge in svn
>> versions >
>> > 1.5.3. The error message i get is the following:
>> >
>> > svn merge --accept postpone --dry-run -c43053
>> > http://vmprjsrv02/svn/APPL/SCU/SPC/branches/b09A D:/wc/p09A/Tp/Spc
>> > -- Zusammenführen von r43053 in
>> > »D:\wc\p09A\Tp\Spc\Tp\TestCenter\Src\CoatingUnitTest«:
>> > U D:\wc\p09A\Tp\Spc\Tp\TestCenter\Src\CoatingUnitTest\Id_18519.tsd
>> > U D:\wc\p09A\Tp\Spc\Tp\TestCenter\Src\CoatingUnitTest\Id_18520.tsd
>> > U D:\wc\p09A\Tp\Spc\Tp\TestCenter\Src\CoatingUnitTest\Id_18521.tsd
>> > U D:\wc\p09A\Tp\Spc\Tp\TestCenter\Src\CoatingUnitTest\Id_18525.tsd
>> > G D:\wc\p09A\Tp\Spc\Tp\TestCenter\Src\CoatingUnitTest
>> > d:\svnserver\src-tags-1.5.4\subversion\libsvn_wc\lock.c:1433:
>> > (apr_err=155005)
>> > svn: Keine Schreibsperre in
>> > »D:\wc\p09A\Tp\Spc\Tp\TestCenter\Src\CoatingUnitTest«
>> >
>> > ("Keine Schreibsperre" means "No write-lock")
>> >
>> > I tried to create a recipe to reproduce this message but did not
>> succeed.
>> > Maybe the history of my working copy is too complex. The bug does not
>> occur
>> > in svn versions 1.5.0-1.5.3.
>> >
>> > Then i debugged a little bit and found the reason for the error in
>> > libsvn_client/merge.c in the new function
>> > "process_children_with_new_mergeinfo" (which was introduced in
>> 1.5.4). Here
>> > is a patch that worked for me, but i do not know what side effects it
>> has:
>> >
>> > Index: merge.c
>> > ===================================================================
>> > --- merge.c (revision 34373)
>> > +++ merge.c (working copy)
>> > @@ -5144,9 +5144,12 @@
>> >
>> SVN_ERR(svn_mergeinfo_merge(path_explicit_mergeinfo,
>> >
>> path_inherited_mergeinfo,
>> > iterpool));
>> > -
>> > SVN_ERR(svn_client__record_wc_mergeinfo(path_with_new_mergeinfo,
>> > -
>> > path_explicit_mergeinfo,
>> > -
>> adm_access,
>> > iterpool));
>> > + if (!merge_b->dry_run)
>> > + {
>> > +
>> > SVN_ERR(svn_client__record_wc_mergeinfo(path_with_new_mergeinfo,
>> > +
>> > path_explicit_mergeinfo,
>> > +
>> adm_access,
>> > iterpool));
>> > + }
>> > }
>> >
>> > /* If the path is not in NOTIFY_B-
>> >CHILDREN_WITH_MERGEINFO
>> >
>> > Can anyone confirm that this is a bug or not, please?
>>
>> Hi Michael,
>>
>> It is indeed a bug and your patch is basically the right approach.
>> But we can go a bit further and not bother creating/populating the
>> paths_with_new_mergeinfo hash during a dry-run and also make the
>> entirety of process_children_with_new_mergeinfo a no-op for dry-runs.
>> That should solve your bug and eliminate some unnecessary work during
>> dry-runs.
>>
>> Committed this change in r34432 and will nominate it for backport to
>> 1.5.x.
>
> Hi Paul,
>
> Looking at this change I wonder if r34432 changes the notifications
> send on the dry merge action. I would expect it to miss notifications
> as the rest of the code depends on batton->paths_with_new_mergeinfo
> for looping over these items.
Hi Bert,
Dry-run merge notifications shouldn't change as a result of r34432,
the same notifications will result before and after r34432. The only
difference is that post-r34432 there won't be an error once the merge
is complete and merge.c:process_children_with_new_mergeinfo() tries to
account for inherited mergeinfo (by which point all the notifications
have been printed).
> Should these notifications match the real merge or is it ok to just
> skip the changes that are only for mergeinfo recording?
I think it is ok to skip these. We have never produced notifications
for mergeinfo changes that describe the merge itself, rather
notifications are only shown if there was some actual difference in
the merge source. The new merge_cmd_baton_t member
paths_with_new_mergeinfo introduced in r32975 falls into a middle
ground between these two, but seems much more akin to the former.
While it takes a change in the merge source for
process_children_with_new_mergeinfo() to actually do anything, it is
only fixing up the mergeinfo already added and for which a
notification has already been produced.
Of course things do get a bit tricky when we consider that, with 1.5,
it is now possible to have a merge where the real and --dry-run output
differ slightly (regardless of the changes I made in r32975 and
r34432). This is because a single merge command might now actually
result in multiple editor drives, the simplest example is where we
specify two discrete ranges. For example, say we have this very
simple repository:
>svn log -v -r1:HEAD merge_tests-109
------------------------------------------------------------------------
r1 | jrandom | 2008-12-01 10:40:49 -0500 (Mon, 01 Dec 2008) | 1 line
Changed paths:
A /A
A /A/B
A /A/B/E
A /A/B/E/alpha
A /A/B/E/beta
A /A/B/F
A /A/B/lambda
A /A/C
A /A/D
A /A/D/G
A /A/D/G/pi
A /A/D/G/rho
A /A/D/G/tau
A /A/D/H
A /A/D/H/chi
A /A/D/H/omega
A /A/D/H/psi
A /A/D/gamma
A /A/mu
A /iota
Log message for revision 1.
------------------------------------------------------------------------
r2 | jrandom | 2008-12-01 10:40:50 -0500 (Mon, 01 Dec 2008) | 1 line
Changed paths:
A /A_COPY (from /A:1)
log msg
------------------------------------------------------------------------
r3 | pburba | 2008-12-01 10:41:11 -0500 (Mon, 01 Dec 2008) | 1 line
Changed paths:
M /A/mu
------------------------------------------------------------------------
r4 | pburba | 2008-12-01 10:41:17 -0500 (Mon, 01 Dec 2008) | 1 line
Changed paths:
M /A/B/lambda
------------------------------------------------------------------------
r5 | pburba | 2008-12-01 10:41:26 -0500 (Mon, 01 Dec 2008) | 1 line
Changed paths:
M /A/mu
------------------------------------------------------------------------
If we merge two revisions which change the same file with one merge
command, we see two notifications:
>svn merge %url109%/A merge_tests-109\A_COPY -c3,5
--- Merging r3 into 'merge_tests-109\A_COPY':
U merge_tests-109\A_COPY\mu
--- Merging r5 into 'merge_tests-109\A_COPY':
G merge_tests-109\A_COPY\mu
>svn revert -R merge_tests-109
Reverted 'merge_tests-109\A_COPY'
Reverted 'merge_tests-109\A_COPY\mu'
But if we do the same merge as a --dry-run we see a conflict because
the first range was never really applied:
>svn merge %url109%/A merge_tests-109\A_COPY -c3,5 --dry-run
--- Merging r3 into 'merge_tests-109\A_COPY':
U merge_tests-109\A_COPY\mu
--- Merging r5 into 'merge_tests-109\A_COPY':
C merge_tests-109\A_COPY\mu
Summary of conflicts:
Text conflicts: 1
Since merging -c3,5 is equivalent to merging -c3 and then merging -c5
I don't think this is a problem:
>svn merge %url109%/A merge_tests-109\A_COPY -c3 --dry-run
--- Merging r3 into 'merge_tests-109\A_COPY':
U merge_tests-109\A_COPY\mu
>svn merge %url109%/A merge_tests-109\A_COPY -c5 --dry-run
--- Merging r5 into 'merge_tests-109\A_COPY':
C merge_tests-109\A_COPY\mu
Summary of conflicts:
Text conflicts: 1
Similar differences can occur when a path get's explicit mergeinfo in
one range and then a change to that mergeinfo in another range. Say
that r3 and r5 where not text changes but rather mergeinfo changes to
mu:
>svn merge %url109%/A merge_tests-109\A_COPY -c3,5 --dry-run
--- Merging r3 into 'merge_tests-109\A_COPY':
U merge_tests-109\A_COPY\mu
--- Merging r5 into 'merge_tests-109\A_COPY':
U merge_tests-109\A_COPY\mu
^
The second merge thinks it is simply updating unchanged mergeinfo.
>svn merge %url109%/A merge_tests-109\A_COPY -c3,5
--- Merging r3 into 'merge_tests-109\A_COPY':
U merge_tests-109\A_COPY\mu
--- Merging r5 into 'merge_tests-109\A_COPY':
G merge_tests-109\A_COPY\mu
^
The second merge merges new mergeinfo into the local mod created by
the first merge.
But again, even *if* it this a problem, it is not unique to r32975 and r34432.
Does that answer your questions?
Paul
Received on 2008-12-01 17:12:13 CET