Re: [PATCH] Make merge-tests 53 (detect_copy_src_for_target_with_multiple_ancestors) pass.

On Tue, 05 Jun 2007, Kamesh Jayachandran wrote:
...
> [[[
> Make merge_tests.py's 'detect_copy_src_for_target_with_multiple_ancestors'
> pass.
>
> * subversion/libsvn_client/log.c
> include "svn_sorts.h".
      ^
Capitalize.

> (copyfrom_info_receiver): Sort changed_paths based on path. Iterate
> from last to first and check for the copy source.
>
> * subversion/tests/cmdline/merge_tests.py
> (detect_copy_src_for_target_with_multiple_ancestors):
> Fix for merge range notifications.

I'm unable follow this description without reading the patch. Perhaps
a comparsion including what was wrong before would help.

...
> Patch by: kameshj

You can drop this "Patch by" contribulyzer stuff when committing
yourself, of course. :-)

> Index: subversion/libsvn_client/log.c
> ===================================================================
> --- subversion/libsvn_client/log.c (revision 25291)
> +++ subversion/libsvn_client/log.c (working copy)
> @@ -33,6 +33,7 @@
> #include "svn_client.h"
> #include "svn_error.h"
> #include "svn_path.h"
> +#include "svn_sorts.h"
>
> #include "svn_private_config.h"
> #include "private/svn_wc_private.h"
> @@ -112,17 +113,23 @@
>
> if (changed_paths)
> {
> - apr_hash_index_t *hi;
> - char *path;
> + int i;
> + const char *path;
> svn_log_changed_path_t *changed_path;
> + apr_array_header_t *changed_paths_in_DF_order;
> + changed_paths_in_DF_order = svn_sort__hash(

I'd rather use a shorter name for this, and if we think the type of
sorting is important, note that as a comment. New patch attached.

> + changed_paths,
> + svn_sort_compare_items_as_paths,
> + pool);
>
> - for (hi = apr_hash_first(NULL, changed_paths);
> - hi;
> - hi = apr_hash_next(hi))
> +
> + for (i = (changed_paths_in_DF_order->nelts -1) ; i >= 0 ; i--)
> {
> - void *val;
> - apr_hash_this(hi, (void *) &path, NULL, &val);
> - changed_path = val;
> + svn_sort__item_t *item = &APR_ARRAY_IDX(changed_paths_in_DF_order,
> + i,
> + svn_sort__item_t);
> + path = item->key;
> + changed_path = item->value;
>
> /* Consider only the path we're interested in. */
> if (changed_path->copyfrom_path &&
> Index: subversion/tests/cmdline/merge_tests.py
> ===================================================================
> --- subversion/tests/cmdline/merge_tests.py (revision 25291)
> +++ subversion/tests/cmdline/merge_tests.py (working copy)
> @@ -6220,7 +6220,9 @@
> saved_cwd = os.getcwd()
> try:
> os.chdir(A_copy_of_B_C_path)
> - svntest.actions.run_and_verify_svn(None, [], [], 'merge', '-g')
> + svntest.actions.run_and_verify_svn(None,
> + svntest.main.merge_notify_line(2),
> + [], 'merge', '-g')
> finally:
> os.chdir(saved_cwd)
>
> @@ -6432,7 +6434,7 @@
> XFail(merge_to_path_with_switched_children),
> merge_with_implicit_target_file,
> XFail(empty_rev_range_mergeinfo),
> - XFail(detect_copy_src_for_target_with_multiple_ancestors),
> + detect_copy_src_for_target_with_multiple_ancestors,
> prop_add_to_child_with_mergeinfo,
> diff_repos_does_not_update_mergeinfo,

+1 to commit with suggested tweaks.