On Tue, Jul 31, 2012 at 1:50 PM, Julian Foad <julianfoad_at_btopenworld.com> 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
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
Received on 2012-08-01 20:03:58 CEST