Kamesh, this works great!  I noticed that it doesn't use the RA
session reparenting suggested by Peter Lundblad -- any particular
reason for that?  Also, there's some error handling in
get_implied_merge_info() to account for locally-added but uncommitted
versioned resources; is it still correct as written (e.g. do we still
need the test for SVN_ERR_RA_DAV_REQUEST_FAILED)?  Seems a little odd
that there's no special-casing for ra_svn's protocol.
- Dan
On Tue, 20 Mar 2007, Kamesh Jayachandran wrote:
> [[[
> On the merge-tracking branch: Fix 11th copy_test failure.
> 
> * subversion/libsvn_client/copy.c
>   (get_implied_merge_info, calculate_target_merge_info):
>    Remove FIXME marker.
>   (wc_to_repos_copy):
>    Remove FIXME marker.
>    Call 'calculate_target_merge_info' with ra_session of repos_root.
> 
> Patch by: kameshj
> ]]]
> Index: subversion/libsvn_client/copy.c
> ===================================================================
> --- subversion/libsvn_client/copy.c	(revision 23922)
> +++ subversion/libsvn_client/copy.c	(working copy)
> @@ -390,9 +390,6 @@
>       was created (copied or added). */
>    err = svn_ra_get_log(ra_session, rel_paths, 1, rev, 1, FALSE, TRUE,
>                         revnum_receiver, &oldest_rev, pool);
> -  /* ### FIXME: ra_dav may fail with SVN_ERR_RA_DAV_PATH_NOT_FOUND.
> -     ### This is possibly related to path construction mentioned in
> -     ### calculate_target_merge_info()... */
>    if (err)
>      {
>        if (err->apr_err == SVN_ERR_FS_NOT_FOUND ||
> @@ -441,10 +438,6 @@
>                                              pool));
>  
>    /* Obtain any implied and/or existing (explicit) merge info. */
> -  /* ### FIXME: May fail with SVN_ERR_RA_DAV_PATH_NOT_FOUND over
> -     ### ra_dav, because we're providing a path relative to the
> -     ### repository root instead of the ra_session (which may've been
> -     ### opened to a path somewhere under the root). */
>    SVN_ERR(get_implied_merge_info(ra_session, target_mergeinfo,
>                                   src_rel_path, src_path, src_revnum, pool));
>    SVN_ERR(svn_client__get_merge_info_for_path(ra_session, &src_mergeinfo,
> @@ -956,7 +949,7 @@
>                   apr_pool_t *pool)
>  {
>    const char *message;
> -  const char *top_src_path, *top_dst_url;
> +  const char *top_src_path, *top_dst_url, *repos_root;
>    svn_ra_session_t *ra_session;
>    const svn_delta_editor_t *editor;
>    void *edit_baton;
> @@ -1013,8 +1006,6 @@
>        svn_client__copy_pair_t *pair = APR_ARRAY_IDX(copy_pairs, i,
>                                                      svn_client__copy_pair_t *);
>  
> -      /* ### FIXME: I want the path relative to the ra_session
> -         ### instead. */
>        SVN_ERR(svn_client__path_relative_to_root(&pair->src_rel, pair->src,
>                                                  NULL, ra_session, adm_access,
>                                                  pool));
> @@ -1086,6 +1077,11 @@
>                                       APR_HASH_KEY_STRING)))
>      goto cleanup;
>  
> +  /* Change the ra_session to repos_root. So that 'svn_ra_get_log'
> +     on paths relative to repos_root would work fine. */
> +  SVN_ERR(svn_ra_get_repos_root(ra_session, &repos_root, pool));
> +  SVN_ERR(svn_client_open_ra_session(&ra_session, repos_root, ctx, pool));
> +
>    /* ### TODO: This extra loop would be unnecessary if this code lived
>       ### in svn_client__get_copy_committables(), which is incidentally
>       ### only used above (so should really be in this source file). */
- application/pgp-signature attachment: stored
 
 
Received on Tue Mar 27 03:33:59 2007