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

Re: Symmetric merge and deleted subtrees

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Mon, 6 Aug 2012 16:52:38 +0100 (BST)

Hi Paul.

I have only had a chance to skim through your reply so far.  I'm glad that you find it to be an explicable and harmless difference.

I have assigned Issue #4217 to track this, as I want to be able to refer to it before having a chance to study your reply properly.

- Julian

Paul Burba wrote:
> On Tue, Jul 31, 2012 at 1:50 PM, Julian Foad wrote:
>>  I'm investigating a discrepancy that shows up, when 'symmetric
>> merge' is enabled [1], in the notifications printed by some merges. 
>> Merge_tests 78 fails because the expected notification for the last
>> merge is:
>>
>>  --- Merging r6 through r9 into 'H_COPY':
>>  U    H_COPY/psi
>>  D    H_COPY/nu
>>  --- Recording mergeinfo for merge of r6 through r9 into 'H_COPY':
>>   U   H_COPY
>>
>>  but the first line of that changes to:
>>
>>  --- Merging r7 through r9 into 'H_COPY':
>>
>>  while the rest stays the same.  In this instance the difference is
>> probably harmless since r6 is a no-op in the merge source, but we
>> need to check whether this is a symptom of a wider problem.
>>
>>
>>  NEW TEST FOR THIS ISSUE
>>
>>  I have simplified the recipe into a new test, merge_tests-130 in
>>  the attached patch [2].  I removed the '--force' option, and this
>> changes things such that the merge prints a separate notification
>> for the  subtree.  Also
>>  in that patch are some debug prints in the merge code which help to
>>  trace the source of the problem.  A diagram of the test scenario:
>>
>>    #                        merge -c4 A/D/H/nu_at_4 H_COPY/nu
>>    #                        |  merge A/D/H H_COPY
>>    #                        |  |
>>    # A/D/H      A---------------
>>    #     +-psi  +---------M-----
>>    #     +-nu     A---M-D
>>    # H_COPY         C----------G
>>    #     +-psi      +----------+
>>    #     +-nu       +-------G--+
>>    #            1 2 3 4 5 6 wc wc
>>    #
>>    # Key: Add Mod Del Copy merGe
>>
>>  The difference in outputs ('-': existing 1.7 merge, '+':
>> symmetric merge):
>>
>>   I: CMD: svn merge ^/A/D/H H_COPY
>>   I: DBG: merge.c:8568: 1: ...130/H_COPY: 3-6
>>   I: DBG: merge.c:8568: 1: .../H_COPY/nu: 3,5-6
>>   I: DBG: merge.c:8568: 2: ...130/H_COPY: 3-6
>>   I: DBG: merge.c:8568: 2: .../H_COPY/nu: 3,5-6
>>   I: DBG: merge.c:8568: 3: ...130/H_COPY: 3-6
>>  -I: DBG: merge.c:8568: 3: .../H_COPY/nu: 3-6
>>  +I: DBG: merge.c:8568: 3: .../H_COPY/nu: 3,5-6
>>  -I: --- Merging r3 through r6 into 'H_COPY':
>>  +I: --- Merging r4 through r6 into 'H_COPY':
>>   I: U    H_COPY/psi
>>  -I: --- Merging r3 through r6 into 'H_COPY/nu':
>>  +I: --- Merging r5 through r6 into 'H_COPY/nu':
>>   I:    C H_COPY/nu
>>   I: --- Recording mergeinfo for merge of r3 through r6 into
>> 'H_COPY':
>>   I:  U   H_COPY
>>   I: --- Recording mergeinfo for merge of r3 through r6 into
>> 'H_COPY/nu':
>>   I:  G   H_COPY/nu
>>   I: --- Eliding mergeinfo from 'H_COPY/nu':
>>   I:  U   H_COPY/nu
>>   I: Summary of conflicts:
>>   I:   Tree conflicts: 1
>>
>>  The immediate cause of the discrepancy is that the symmetric merge code
>>  passes the YCA revision as the beginning of the range to be merged,
>>  whereas the existing merge code passes revision '1' as the
>>  beginning of
>>  the range to be merged.  At the beginning of the main loop in do_merge():
>>
>>  - source = {loc1 = "/A/D/H_at_1", loc2 = "/A/D/H_at_6",
>> ancestral = TRUE}
>>  + source = {loc1 = "/A/D/H_at_2", loc2 = "/A/D/H_at_6",
>> ancestral = TRUE}
>>
>>  The underlying problem -- the reason why the merge code doesn't resolve
>> these two requests to the same result -- is deeper, involving the
>> function fix_deleted_subtree_ranges().  That function yields
>>
>>    (child 'H_COPY/nu')->remaining_ranges = 3-6
>>
>>  for the 1.7 merge and
>>
>>    (child 'H_COPY/nu')->remaining_ranges = 3,5-6
>>
>>  for the symmetric merge.
>>
>>  Revision 4 has already been merged to 'H_COPY/nu', so it should not
>> be merged again; but that doesn't mean 'remaining_ranges = 3-6' is
>> wrong.  fix_deleted_subtree_ranges() is designed to set a subtree's
>> remaining range to match its parent's remaining range if the subtree
>> doesn't need a separate editor drive.  The function appears to be
>> concluding in the first case that the subtree doesn't need a separate
>> editor drive (since, over the range 3-6, the net result is simply to
>> delete the subtree), but in the second case that it does and should
>> be split into two sub-ranges.
>
> Hi Julian,
>
> I don't think there is a problem here, rather, as you point out above,
> it is a fundamental difference between 1.7 sync merges, which assume a
> starting rev of 1, and symmetric merges, which calculates the YCA and
> passes that as the starting rev to do_merge().  Different inputs to a
> function produce different outputs...nothing terribly shocking about
> that ;-)
>
> Specifically:
>
> When we reach fix_deleted_subtree_ranges we are asking different
> questions during a 1.7 merge vs. a symmetric merge.
>
> First let's look at a couple key parts of the doc string for
> adjust_deleted_subtree_ranges (which does the heavy lifting for
> fix_deleted_subtree_ranges), first:
>
>    Since this function is only invoked for subtrees of the merge target, the
>    guarantees afforded by normalize_merge_sources() don't apply - see the
>    'MERGEINFO MERGE SOURCE NORMALIZATION' comment at the top of this
>    file.  Therefore it is possible that PRIMARY_URL_at_REVISION1 and
>    PRIMARY_URL_at_REVISION2 don't describe the endpoints of an unbroken line of
>    history.  The purpose of this helper is to identify these cases of broken
>    history and adjust CHILD->REMAINING_RANGES in such a way we don't later
>    try to describe nonexistent path/revisions to the merge report editor --
>    see drive_merge_report_editor().
>
> So, we are trying to not break the editor drive.  In the 1.7 case we
> ask adjust_deleted_subtree_ranges about this child:
>
> adjust_deleted_subtree_ranges(svn_client__merge_path_t *child =
> "H_COPY/nu"...,
>                               svn_client__merge_path_t *parent =
> "H_COPY"...,
>                               svn_revnum_t revision1 = 1,
>                               svn_revnum_t revision2 = 6,
>                               const char *primary_url = "^/A/D/H/nu",
>
> *but* neither ^/A/D/H/nu_at_1 or ^/A/D/H/nu_at_6 exist, so CHILD needs no
> special consideration during the editor drive, so we set the CHILD's
> remaining ranges as identical to it's parents.  As the doc string for
> drive_merge_report_editor points out, this means we effectively ignore
> these children during the editor drive:
>
>    ...
>    Note: If the first svn_merge_range_t * element of some subtree
>    child's remaining_ranges array is the same as the first range of
>    that child's nearest path-wise ancestor, then the subtree child
>    *will not* be described to the reporter.
>    ...
>
> The symmetric merge however, is asking about a CHILD that exists at
> the start of the requested range, but not at the end (it is deleted):
>
> adjust_deleted_subtree_ranges(svn_client__merge_path_t *child =
> "H_COPY/nu"...,
>                               svn_client__merge_path_t *parent =
> "H_COPY"...,
>                               svn_revnum_t revision1 = 2,
>                               svn_revnum_t revision2 = 6,
>                               const char *primary_url = "^/A/D/H/nu",
>
> So per the guarantees of adjust_deleted_subtree_ranges we end up with
> 2 discrete remaining ranges for H_COPY/nu:
>
>      If PRIMARY_URL_at_REVISION1 exists but PRIMARY_URL_at_REVISION2 doesn't,
>      then find the revision 'N' in which PRIMARY_URL_at_REVISION1 was
>      deleted.  Leave the subset of CHILD->REMAINING_RANGES that
>      intersects with REVISION1:(N - 1) as-is and set the subset of
>      CHILD->REMAINING_RANGES that intersects with (N - 1):REVISION2 equal
>      to PARENT->REMAINING_RANGES' intersection with (N - 1):REVISION2.
>
>>  I'm not sure whether one of those conclusions is right and the other
>> wrong; maybe both are OK.
>
> I think everything is ok here, at least as far as the resulting merge,
> we should always get the same WC state AFAICT with either the
> symmetric merge or the 1.7 merge.
>
> The only drawback to the symmetric approach is that it performs two
> editor drives:
>
>   Editor Drive #1: ^/merge_tests-130 rev=2 : ^/merge_tests-130 rev=3
>   Editor Drive #2: ^/merge_tests-130 rev=3 : ^/merge_tests-130 rev=6
>
> The first is inoperative obviously, so there is no user visible output.
>
> While 1.7 performs only one:
>
>   Editor Drive #1: ^/merge_tests-130 rev=2 : ^/merge_tests-130 rev=6
>
> Not sure how to avoid this extra editor drive.  Possibly postponing
> the call to remove_noop_subtree_ranges() in
> do_mergeinfo_aware_dir_merge until *after* the call to
> fix_deleted_subtree_ranges() is all that is needed, but I haven't
> checked this out.
>
>>  I'm not sure why there are two different conclusions.
>
> Again, different conclusions based on different inputs.
>
> --
> Paul T. Burba
Received on 2012-08-06 17:53:15 CEST

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