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
