[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Fix issue #4316 - Merge errors out after resolving conflicts

From: Paul Burba <ptburba_at_gmail.com>
Date: Thu, 21 Mar 2013 11:23:19 -0400

On Wed, Mar 20, 2013 at 4:15 PM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
>
> --
> Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
> ----- Original Message -----
>> From: Julian Foad <julianfoad_at_btopenworld.com>
>> To: C. Michael Pilato <cmpilato_at_collab.net>
>> Cc: Subversion Development <dev_at_subversion.apache.org>
>> Sent: Wednesday, 20 March 2013, 15:38
>> Subject: Re: [PATCH] Fix issue #4316 - Merge errors out after resolving conflicts
>>
>> C. Michael Pilato wrote:
>>> On 03/15/2013 06:03 PM, Julian Foad wrote:
>>>> NOTIFICATIONS CHANGED
>>>>
>>>> As I mentioned in my "Conflict resolver callback design"
>> email,
>>>> doing this does mean that the notification receiver will get a
>> 'C'
>>>> (conflict) notification for every conflict even if that conflict is
>> going to be
>>>> resolved to a pre-determined choice. In terms of the 'svn'
>> client and
>>>> the 'svn merge' command, this means that 'svn merge
>>>> --accept=[mine-full, etc.]' will, if we don't take further
>> action, print
>>>> something like in this example:
>>>>
>>>> [[[
>>>> --- Merging r3 through r4 into 'merge_tests-135/A2/mu':
>>>> C merge_tests-135/A2/mu
>>>> --- Recording mergeinfo for merge of r3 through r4 into
>>>> 'merge_tests-135/A2/mu':
>>>> G merge_tests-135/A2/mu
>>>> Resolved conflicted state of 'merge_tests-135/A2/mu'
>>>> --- Merging r6 through r8 into 'merge_tests-135/A2/mu':
>>>> U merge_tests-135/A2/mu
>>>> --- Merging r10 through r11 into 'merge_tests-135/A2/mu':
>>>> U merge_tests-135/A2/mu
>>>> --- Recording mergeinfo for merge of r5 through r11 into
>>>> 'merge_tests-135/A2/mu':
>>>> G merge_tests-135/A2/mu
>>>> Summary of conflicts:
>>>> Property conflicts: 1
>>>> ]]]
>>>>
>>>> I think if this was changed to print a slightly different summary,
>>>> something like...
>>>>
>>>> [[[
>>>> Summary of conflicts:
>>>> Property conflicts: 0 (and 1 already resolved)
>>>> ]]]
>>>>
>>>> ... then it would be fine. I don't see that the interleaved
>>>> 'Resolved ...' line is a problem.
>>>>
>>>> Do others agree?
>>>
>>> The point of the summary section is to draw attention to details that might
>>> have whizzed by the screen. Given that, I agree it's a bit misleading
>> to
>>> alert the user to a problem which may not really be a problem any longer.
>>> So yeah, a change such as what you've suggested makes sense to me.
>>>
>>> (Sorry, no feedback on your actual patch.)
>>
>> I have committed a complete fix, with the Summary of Conflicts as discussed
>> here, in <http://svn.apache.org/r1459012>.
>
> Remaining issues:
>
> * There is an inconsistency with what rev ranges get recorded as merged, in a file merge vs. a dir merge -- see the comment and work-around in merge_tests.py 138.

Hi Julian,

I see the inconsistency, but I don't believe there is a bug (assuming
we are talking about the same thing -- if not please point me in the
right direction). Looking at the spot in the test where your comment
is, the test currently does this merge:

Uniform revision:

  1.8.0-dev>svn up
  Updating '.':
  At revision 11.

Local prop mod that will conflict with the coming merge:

  1.8.0-dev>svn st
   M A2\mu

  1.8.0-dev>svn diff
  Index: A2/mu
  ===================================================================
  --- A2/mu (revision 11)
  +++ A2/mu (working copy)

  Property changes on: A2/mu
  ___________________________________________________________________
  Modified: prop-4
  ## -1 +1 ##
  -Old pval 4
  \ No newline at end of property
  +Conflict pval 4
  \ No newline at end of property

Fragmented mergeinfo inherited by the target...

  1.8.0-dev>svn pg svn:mergeinfo -vR A2
  Properties on 'A2':
    svn:mergeinfo
      /A:5,9

...breaks the merge into multiple editor drives, starting with r3:

  1.8.0-dev>svn.exe merge ^^/A/mu_at_11 A2\mu --accept mine-full
  --- Merging r3 through r4 into 'A2\mu':
   C A2\mu
  --- Recording mergeinfo for merge of r3 through r4 into 'A2\mu':
   G A2\mu
  Resolved conflicted state of 'A2\mu'
  --- Merging r6 through r8 into 'A2\mu':
   U A2\mu
  --- Merging r10 through r11 into 'A2\mu':
   U A2\mu
  --- Recording mergeinfo for merge of r5 through r11 into 'A2\mu':
   G A2\mu
  Summary of conflicts:
    Property conflicts: 0 remaining (and 1 already resolved)

The gaps in the mergeinfo are all filled:

  1.8.0-dev>svn pg svn:mergeinfo -vR A2
  Properties on 'A2':
    svn:mergeinfo
      /A:5,9
  Properties on 'A2\mu':
    svn:mergeinfo
      /A/mu:3-11

If we instead try the above merge as a directory merge targeting
A2/mu's parent, the merge starts with r2:

  1.8.0-dev>svn.exe merge ^^/A_at_11 A2 --accept mine-full
  --- Merging r2 through r4 into 'A2':
   C A2\B
   C A2\mu
  --- Recording mergeinfo for merge of r2 through r4 into 'A2':
   U A2
  Resolved conflicted state of 'A2\B'
  Resolved conflicted state of 'A2\mu'
  --- Merging r6 through r8 into 'A2':
   U A2\B
   U A2\mu
  --- Merging r10 through r11 into 'A2':
   U A2\B
   U A2\mu
  --- Recording mergeinfo for merge of r6 through r11 into 'A2':
   G A2
  Summary of conflicts:
    Property conflicts: 0 remaining (and 2 already resolved)

  1.8.0-dev>svn st
   M A2
   M A2\B
   M A2\mu

And the recorded mergeinfo also starts with r2:

  1.8.0-dev>svn pg svn:mergeinfo -vR A2
  Properties on 'A2':
    svn:mergeinfo
      /A:2-11

If that is the inconsistency you mean? I don't think that's a
problem. Let's look at the history of ^/A2 vs ^/A2/mu:

  1.8.0-dev>svn log -v -r1:HEAD ^^/A2
  ------------------------------------------------------------------------
  r1 | jrandom | 2013-03-21 10:16:18 -0400 (Thu, 21 Mar 2013) | 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.
  ------------------------------------------------------------------------
  r3 | jrandom | 2013-03-21 10:16:26 -0400 (Thu, 21 Mar 2013) | 1 line
  Changed paths:
     A /A2 (from /A:1)
     R /A2/B (from /A/B:2)
     R /A2/mu (from /A/mu:2)

  File 'C:\SVN\src-trunk-3\\subversion\tests\cmdline\merge_tests.py',
  line 18796, in conflicted_split_merge_with_resolve
  ------------------------------------------------------------------------
  r11 | jrandom | 2013-03-21 10:16:27 -0400 (Thu, 21 Mar 2013) | 1 line
  Changed paths:
     M /A2
     M /A2/B
     M /A2/mu

  File 'C:\SVN\src-trunk-3\\subversion\tests\cmdline\merge_tests.py',
  line 18814, in conflicted_split_merge_with_resolve
  ------------------------------------------------------------------------

Since ^/A2/mu was replaced with a copy of ^/A/mu_at_2 wouldn't we expect
a merge targeting the former (since ^/A2/mu_at_2 is the yca of the two
branches) to merge -r2:HEAD? Likewise, if we target /A2, the yca is
older, ^/A_at_1, so the merge is -r1:HEAD. This explains the mergeinfo
differences between the file and dir merge.

~~~~~

r3 is explained by the fact that your test makes a copy of a
mixed-revision WC to create the branch in r3:

  1.8.0-dev>svn st -v
                   1 1 jrandom .
                   1 1 jrandom A
                   2 2 jrandom A\B
                   1 1 jrandom A\B\E
                   1 1 jrandom A\B\E\alpha
                   1 1 jrandom A\B\E\beta
                   1 1 jrandom A\B\F
                   1 1 jrandom A\B\lambda
                   1 1 jrandom A\C
                   1 1 jrandom A\D
                   1 1 jrandom A\D\G
                   1 1 jrandom A\D\G\pi
                   1 1 jrandom A\D\G\rho
                   1 1 jrandom A\D\G\tau
                   1 1 jrandom A\D\H
                   1 1 jrandom A\D\H\chi
                   1 1 jrandom A\D\H\omega
                   1 1 jrandom A\D\H\psi
                   1 1 jrandom A\D\gamma
                   2 2 jrandom A\mu
                   1 1 jrandom iota

  1.8.0-dev>svn copy A A2
  A A2

  1.8.0-dev>svn ci -m ""
  Adding A2
  Adding A2\B
  Adding A2\B\E
  Adding A2\B\F
  Adding A2\B\lambda
  Adding A2\mu

  Committed revision 3.

  1.8.0-dev>svn log -v -r3
  ------------------------------------------------------------------------
  r3 | pburba | 2013-03-21 11:19:39 -0400 (Thu, 21 Mar 2013) | 1 line
  Changed paths:
     A /A2 (from /A:1)
     R /A2/B (from /A/B:2)
     R /A2/mu (from /A/mu:2)

  ------------------------------------------------------------------------

Not sure if that was an oversight or purposeful.

-- 
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
Received on 2013-03-21 16:23:52 CET

This is an archived mail posted to the Subversion Dev mailing list.