Hi Bert. Nice change again.
> URL: http://svn.apache.org/viewvc?rev=1442598&view=rev
> Log:
> Remove the difference in handling single-file and directory merges in the
> merge notification processing.
>
> * subversion/libsvn_client/merge.c
> (merge_cmd_baton_t): Remove separate variables and add struct specifically
> for this task.
> (notify_merge_begin): Handle single file merges as ordinary merges.
> (do_file_merge): Create a fake children_with_mergeinfo array and keep it
> up to date.
>
> (do_mergeinfo_aware_dir_merge,
> do_directory_merge,
> do_merge): Simplify drive reset code.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/merge.c
>
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> ==============================================================================
[...]
> +
> + /* State for notify_merge_begin() */
> + struct notify_begin_state_t
> + {
> + /* const char * indicating which abspath was last notified for the current
> + operation. */
There's no point in the comment telling us this is a "const char *" :-)
How about "The path that was last notified..."?
> + const char *last_abspath;
> +
> + /* Reference to the on-and-only CHILDREN_WITH_MERGEINFO (see global comment
"one-and-only"
"comment)"
> + or a similar list for single-file-merges */
> + const apr_array_header_t *nodes_with_mergeinfo;
> + } notify_begin;
> +
> } merge_cmd_baton_t;
[...]
> @@ -7143,34 +7135,60 @@ do_file_merge(svn_mergeinfo_catalog_t re
>
> if (!merge_b->record_only)
> {
> - svn_rangelist_t *ranges_to_merge = remaining_ranges;
> + svn_rangelist_t *ranges_to_merge = apr_array_copy(scratch_pool,
> + remaining_ranges);
Instead of copying the array unconditionally here ...
[...]
> + /* If we have ancestrally related sources and more than one
> + range to merge, eliminate no-op ranges before going through
> + the effort of downloading the many copies of the file
> + required to do these merges (two copies per range). */
> + if (remaining_ranges->nelts > 1)
> + {
> + const char *old_sess_url;
> + svn_error_t *err;
> +
> + SVN_ERR(svn_client__ensure_ra_session_url(&old_sess_url,
> + merge_b->ra_session1,
> + primary_src->url,
> + iterpool));
> + err = remove_noop_merge_ranges(&ranges_to_merge,
> + merge_b->ra_session1,
> + remaining_ranges, scratch_pool);
... and then overwriting it with something different here ...
> + SVN_ERR(svn_error_compose_create(
> + err, svn_ra_reparent(merge_b->ra_session1,
> + old_sess_url, iterpool)));
> + }
... add an "else ranges_to_merge = apr_array_copy(...)" here? I think that would make the intent clearer as well as avoiding the copy.
> +
> + /* To support notify_merge_begin() initialize our
> + CHILD_WITH_MERGEINFO. See the comment
> + 'THE CHILDREN_WITH_MERGEINFO ARRAY' at the start of this file. */
> +
> + child_with_mergeinfo = apr_array_make(scratch_pool, 1,
> + sizeof(svn_client__merge_path_t *));
> +
> + /* ### Create a fake copy of merge_target as we don't keep
> + remaining_ranges in sync (yet). */
> + target_info = apr_pcalloc(scratch_pool, sizeof(*target_info));
> +
> + target_info->abspath = merge_target->abspath;
> + target_info->remaining_ranges = ranges_to_merge;
> +
> + APR_ARRAY_PUSH(child_with_mergeinfo, svn_client__merge_path_t *)
> + = target_info;
> +
> + /* And store in baton to allow using it from notify_merge_begin() */
> + merge_b->notify_begin.nodes_with_mergeinfo = child_with_mergeinfo;
> }
>
> - for (i = 0; i < ranges_to_merge->nelts; i++)
> + while (ranges_to_merge->nelts > 0)
> {
[...]
> +
> + /* Poor mans delete first item */
> + SVN_ERR(svn_rangelist_reverse(ranges_to_merge, iterpool));
> + ranges_to_merge->nelts--;
> + SVN_ERR(svn_rangelist_reverse(ranges_to_merge, iterpool));
Eww! We have a better way:
svn_sort__array_delete(ranges_to_merge, 0, 1);
(I suppose the 'svn_sort' prefix makes that function hard to find.)
- Julian
Received on 2013-02-05 17:29:41 CET