pburba_at_tigris.org writes:
> Log:
> Partial fix for issue #3199, Subtree merges broken when required ranges
> don't intersect with merge target.
>
> * subversion/libsvn_client/merge.c
> (remove_first_range_from_remaining_ranges): Add end_rev argument. Don't
> always remove the first remaining range for each child. This is the wrong
> behavior if the first range in the target and a child doesn't intersect.
> In that case, two separate editor drives are required and if we remove the
> non-intersecting first range then one of those ranges is never merged.
> (do_directory_merge): Update call to
> remove_first_range_from_remaining_ranges().
Note that I tweaked that log message slightly.
> --- trunk/subversion/libsvn_client/merge.c (r31300)
> +++ trunk/subversion/libsvn_client/merge.c (r31301)
> @@ -2584,17 +2592,24 @@ remove_first_range_from_remaining_ranges
> continue;
> if (child->remaining_ranges->nelts > 0)
> {
> - apr_array_header_t *orig_remaining_ranges = child->remaining_ranges;
> - 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_merge_range_t *first_range =
> + APR_ARRAY_IDX(child->remaining_ranges, 0, svn_merge_range_t *);
> + if (first_range->end == end_rev)
> + {
> + apr_array_header_t *orig_remaining_ranges =
> + child->remaining_ranges;
> + 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;
> + }
> }
> }
> }
Durn, there ought to be an APR_ARRAY_SHIFT(array, n) in APR; then you
could do this very efficiently, without creating a new array. Actually,
we could implement a version of APR_ARRAY_SHIFT() ourselves -- not as
well as APR could do it, since APR can play tricks with the internals,
but still better than the above:
/* Left-shift the elements of ARRAY by SLOTS_TO_SHIFT, losing
* elements shifted off the left end and shortening ARRAY.
* If SLOTS_TO_SHIFT is greater than ARRAY->nelts, post to dev@
* asking Stefan Küng if it's okay to abort().
*/
static void
array_shift(apr_array_header_t *array, int slots_to_shift)
{
int i;
if (slots_to_shift > array->nelts)
post_to_dev_list();
/* else */
for (i = 0; i < array->nelts - slots_to_shift; i++)
{
APR_ARRAY_IDX(array, i,
uh_oh_can't_pass_types_as_first_class_objects_in_C)
= APR_ARRAY_IDX(array, i + slots_to_shift, but_see_above);
}
}
Hmmm, well as you can see from the problem above, it would probably have
to be written inline. But the basic idea of shifting -- in this case,
by 1 slot -- would work, and be somewhat more efficient than the current
code. I don't know if it's worth it, partly because I don't know how
often this case comes up.
Aside from that (which may apply to more places in the code... not sure
how often we do this kind of looping shift), the change looks good to
me.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-06-23 04:46:13 CEST