Why doesn't this patch work as I expect? This looks like it should be
a functional replacement, as the replaced code is making a new array
that contains all the original elements except the first one, which is
the same thing as deleting the first element. What happens instead is
the merge code goes into an infinite loop during 'svn merge -r9:3
$URL/A $WC/A_COPY --dry-run ...' in merge_tests.py 82. In the
debugger I have traced it around and inserted a bunch of prints to see
that the resulting values are all the same as without the patch, and I
can't see anything wrong. Therefore I suspect a pool lifetime error.
(Continued after the patch...)
[[[
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 1202338)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -5262,28 +5262,13 @@ remove_first_range_from_remaining_ranges
if (child->remaining_ranges->nelts > 0)
{
svn_merge_range_t *first_range =
APR_ARRAY_IDX(child->remaining_ranges, 0, svn_merge_range_t *);
if (first_range->end == revision)
{
- apr_array_header_t *orig_remaining_ranges =
- child->remaining_ranges;
- int j;
-
- child->remaining_ranges =
- apr_array_make(pool, (child->remaining_ranges->nelts - 1),
- sizeof(svn_merge_range_t *));
- for (j = 1; j < orig_remaining_ranges->nelts; j++)
- {
- svn_merge_range_t *range =
- APR_ARRAY_IDX(orig_remaining_ranges,
- j,
- svn_merge_range_t *);
- APR_ARRAY_PUSH(child->remaining_ranges,
- svn_merge_range_t *) = range;
- }
+ svn_sort__array_delete(child->remaining_ranges, 0, 1);
}
}
}
}
/* Get a file's content and properties from the repository.
]]]
This patch would be wrong if the calling code relies on the original
array (that 'child->remaining_ranges' points to) continuing to exist
in memory unchanged. But that doesn't look like the case, and I think
the following patch rules it out:
[[[
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 1202351)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -5231,6 +5231,14 @@ slice_remaining_ranges(apr_array_header_
}
}
+static void
+dbg(const apr_array_header_t *arr, apr_pool_t *pool)
+{
+ svn_string_t *s;
+ svn_rangelist_to_string(&s, arr, pool);
+ SVN_DBG(("[%dx%d/%d p=%p e=%p] %s\n", arr->nelts, arr->elt_size,
arr->nalloc, arr->pool, arr->elts, s->data));
+}
+
/* Helper for do_directory_merge().
For each child in CHILDREN_WITH_MERGEINFO remove the first remaining_ranges
@@ -5281,6 +5289,10 @@ remove_first_range_from_remaining_ranges
APR_ARRAY_PUSH(child->remaining_ranges,
svn_merge_range_t *) = range;
}
+ dbg(child->remaining_ranges, pool);
+ svn_sort__array_delete(orig_remaining_ranges, 0, 1);
+ dbg(orig_remaining_ranges, pool);
+ /* child->remaining_ranges = orig_remaining_ranges;*/
}
}
}
@@ -8458,6 +8470,7 @@ do_directory_merge(svn_mergeinfo_catalog
svn_revnum_t end_rev =
get_most_inclusive_end_rev(notify_b->children_with_mergeinfo,
is_rollback);
+ SVN_DBG(("end_rev=%ld\n", end_rev));
/* While END_REV is valid, do the following:
@@ -8517,6 +8530,8 @@ do_directory_merge(svn_mergeinfo_catalog
if (end_rev > first_target_range->start)
end_rev = first_target_range->start;
}
+ SVN_DBG(("end_rev:=%ld (st=%ld, ftr=%ld:%ld)\n", end_rev,
+ start_rev, first_target_range->start,
first_target_range->end));
}
svn_pool_clear(iterpool);
@@ -8615,6 +8630,7 @@ do_directory_merge(svn_mergeinfo_catalog
get_most_inclusive_start_rev(notify_b->children_with_mergeinfo,
is_rollback);
end_rev = next_end_rev;
+ SVN_DBG(("end_rev:=%ld\n", end_rev));
}
}
svn_pool_destroy(iterpool);
]]]
With this second patch, the test completes successfully (and the debug
prints look fine); but if I uncomment the final line then it goes into
the infinite loop. The debug output is:
DBG: merge.c:8473: end_rev=7
--- Reverse-merging r8 into
'svn-test-work/working_copies/merge_tests-82/A_COPY':
U svn-test-work/working_copies/merge_tests-82/A_COPY/D/gamma
DBG: merge.c:5239: [0x4/1 p=0xb7c3f018 e=0xb7c40690]
DBG: merge.c:5239: [0x4/1 p=0xb7c3f018 e=0xb7c401c0]
DBG: merge.c:8633: end_rev:=3
--- Reverse-merging r4 into
'svn-test-work/working_copies/merge_tests-82/A_COPY/D/H/psi':
U svn-test-work/working_copies/merge_tests-82/A_COPY/D/H/psi
DBG: merge.c:5239: [0x4/1 p=0xb7c3f018 e=0xb7c40840]
DBG: merge.c:5239: [0x4/1 p=0xb7c3f018 e=0xb7c40380]
DBG: merge.c:8633: end_rev:=-1
or with that final line uncommented is:
DBG: merge.c:8473: end_rev=7
--- Reverse-merging r8 into
'svn-test-work/working_copies/merge_tests-82/A_COPY':
U svn-test-work/working_copies/merge_tests-82/A_COPY/D/gamma
DBG: merge.c:5239: [0x4/1 p=0xb7c3f018 e=0xb7c40690]
DBG: merge.c:5239: [0x4/1 p=0xb7c3f018 e=0xb7c401c0]
DBG: merge.c:8633: end_rev:=3
DBG: merge.c:8534: end_rev:=8 (st=4, ftr=8:7)
--- Reverse-merging r4 into
'svn-test-work/working_copies/merge_tests-82/A_COPY/D/H/psi':
U svn-test-work/working_copies/merge_tests-82/A_COPY/D/H/psi
DBG: merge.c:8633: end_rev:=3
DBG: merge.c:8534: end_rev:=8 (st=4, ftr=8:7)
--- Reverse-merging r4 into
'svn-test-work/working_copies/merge_tests-82/A_COPY/D/H/psi':
U svn-test-work/working_copies/merge_tests-82/A_COPY/D/H/psi
DBG: merge.c:8633: end_rev:=3
DBG: merge.c:8534: end_rev:=8 (st=4, ftr=8:7)
--- Reverse-merging r4 into
'svn-test-work/working_copies/merge_tests-82/A_COPY/D/H/psi':
U svn-test-work/working_copies/merge_tests-82/A_COPY/D/H/psi
DBG: merge.c:8633: end_rev:=3
DBG: merge.c:8534: end_rev:=8 (st=4, ftr=8:7)
[... ad infinitum ...]
- Julian
