[merge tracking] WC -> repos 'copy'/'move' operations (r23423)

This commit makes copy test #11 fail over ra_dav on the merge-tracking
branch. I'm greeted by the error:

  subversion/libsvn_ra_dav/util.c:1198: (apr_err=175007)
  svn: REPORT request failed on '/svn-test-work/repositories/copy_tests-11/!svn/bc/1/A/D/H2'
  subversion/libsvn_ra_dav/util.c:462: (apr_err=175007)
  svn: '/svn-test-work/repositories/copy_tests-11/!svn/bc/1/A/D/H2' path not found

The H2 directory exists:

  $ svn ls http://localhost:19742/svn-test-work/repositories/copy_tests-11/A/D
  G/
  H/
  H2/
  gamma

This is apparently a problem with the combination of ra_dav session
and path being passed to svn_ra_get_log(). I'm opening the session to
the URL:

  http://localhost:19742/svn-test-work/repositories/copy_tests-11/A/D/H2

...then trying to use a path like "/A/B/E/beta" in the PATHS argument
to svn_ra_get_log(). I'm assuming that ra_dav is (understandably)
appending the path to the URL used to open its session, rather than to
the repository's root URL.

For some reason, this works fine over ra_svn and ra_local, but not
over ra_dav (and I haven't yet tried ra_serf). I'd like your thoughts
on:

1) Why this works differently over ra_dav?

2) What is a good way to calculate the appropriate path (given a WC
source path)?

Thanks, Dan

On Fri, 16 Feb 2007, dlr@tigris.org wrote:

> Author: dlr
> Date: Fri Feb 16 18:25:53 2007
> New Revision: 23423
>
> Log:
> On the merge-tracking branch: Checkpoint commit for handling of merge
> info for WC -> repos 'copy'/'move' operations.
>
> While this is mostly working, it introduces test failures over ra_dav
> due to some path miscalculations (we're using a repository-relative
> path, rather than the expected RA session root-relative path).
>
> * subversion/libsvn_client/copy.c
> (get_implied_merge_info): Handle the case where a locally-added but
> uncommitted versioned resource which doesn't exist in the
> repository causes an error (see copy test #48).
> (calculate_target_merge_info): Add FIXME note about the cause of the
> ra_dav test failure.
> (wc_to_repos_copy): Acquire a relative path and revnum for each copy
> source. Use it to calculate the merge info for each destination
> path, and stuff that into the outgoing_prop_changes list for the
> corresponding svn_client_commit_item3_t.
>
> * subversion/libsvn_client/commit_util.c
> (do_item_commit): Take into account the item's outgoing_prop_changes
> value when the item's state flag has SVN_CLIENT_COMMIT_ITEM_PROP_MODS
> set.
>
> * subversion/tests/cmdline/copy_tests.py
> (copy_added_paths_to_URL): Assure that the merge info for a copied
> path which was copied from a versioned but uncommitted source is
> not set.
> (test_list): Unmark wc_to_repos (though it still fails over ra_dav).
>
> Patch by: me
> madanus
> cmpilato
>
>
> Modified:
> branches/merge-tracking/subversion/libsvn_client/commit_util.c
> branches/merge-tracking/subversion/libsvn_client/copy.c
> branches/merge-tracking/subversion/tests/cmdline/copy_tests.py
>
> Modified: branches/merge-tracking/subversion/libsvn_client/commit_util.c
> URL: http://svn.collab.net/viewvc/svn/branches/merge-tracking/subversion/libsvn_client/commit_util.c?pathrev=23423&r1=23422&r2=23423
> ==============================================================================
> --- branches/merge-tracking/subversion/libsvn_client/commit_util.c (original)
> +++ branches/merge-tracking/subversion/libsvn_client/commit_util.c Fri Feb 16 18:25:53 2007
> @@ -1150,6 +1150,28 @@
> copyfrom_url ? item->copyfrom_rev : SVN_INVALID_REVNUM,
> pool, dir_baton));
> }
> +
> + /* Set other prop-changes, if available in the baton */
> + if (item->outgoing_prop_changes)
> + {
> + svn_prop_t *prop;
> + apr_array_header_t *prop_changes = item->outgoing_prop_changes;
> + int ctr;
> + for (ctr = 0; ctr < prop_changes->nelts; ctr++)
> + {
> + prop = APR_ARRAY_IDX(prop_changes, ctr, svn_prop_t *);
> + if (kind == svn_node_file)
> + {
> + editor->change_file_prop(file_baton, prop->name,
> + prop->value, pool);
> + }
> + else
> + {
> + editor->change_dir_prop(*dir_baton, prop->name,
> + prop->value, pool);
> + }
> + }
> + }
> }
>
> /* Now handle property mods. */
>
> Modified: branches/merge-tracking/subversion/libsvn_client/copy.c
> URL: http://svn.collab.net/viewvc/svn/branches/merge-tracking/subversion/libsvn_client/copy.c?pathrev=23423&r1=23422&r2=23423
> ==============================================================================
> --- branches/merge-tracking/subversion/libsvn_client/copy.c (original)
> +++ branches/merge-tracking/subversion/libsvn_client/copy.c Fri Feb 16 18:25:53 2007
> @@ -27,6 +27,7 @@
> #include "svn_wc.h"
> #include "svn_client.h"
> #include "svn_error.h"
> +#include "svn_error_codes.h"
> #include "svn_path.h"
> #include "svn_opt.h"
> #include "svn_time.h"
> @@ -376,27 +377,39 @@
> svn_revnum_t rev,
> apr_pool_t *pool)
> {
> + svn_error_t *err;
> svn_revnum_t oldest_rev = SVN_INVALID_REVNUM;
> svn_merge_range_t *range;
> apr_array_header_t *rangelist;
> apr_array_header_t *rel_paths = apr_array_make(pool, 1, sizeof(rel_path));
>
> + *implied_mergeinfo = apr_hash_make(pool);
> APR_ARRAY_PUSH(rel_paths, const char *) = rel_path;
>
> /* Trace back in history to find the revision at which this node
> was created (copied or added). */
> - SVN_ERR(svn_ra_get_log(ra_session, rel_paths, 1, rev, 1, FALSE, TRUE,
> - revnum_receiver, &oldest_rev, pool));
> + 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 && (err->apr_err == SVN_ERR_FS_NOT_FOUND ||
> + err->apr_err == SVN_ERR_RA_DAV_REQUEST_FAILED))
> + {
> + /* A locally-added but uncommitted versioned resource won't
> + exist in the repository. */
> + svn_error_clear(err);
> + return SVN_NO_ERROR;
> + }
>
> range = apr_palloc(pool, sizeof(*range));
> range->start = oldest_rev;
> range->end = rev;
> rangelist = apr_array_make(pool, 1, sizeof(range));
> APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = range;
> - *implied_mergeinfo = apr_hash_make(pool);
> apr_hash_set(*implied_mergeinfo, path, APR_HASH_KEY_STRING, rangelist);
>
> - return SVN_NO_ERROR;
> + return err;
> }
>
> /* Obtain the implied merge info and the existing merge info of the
> @@ -421,6 +434,10 @@
> 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, &implied_mergeinfo,
> src_rel_path, src_path, src_revnum, pool));
> SVN_ERR(svn_client__get_merge_info_for_path(ra_session, &src_mergeinfo,
> @@ -944,6 +961,7 @@
> svn_error_t *cmt_err = SVN_NO_ERROR;
> svn_error_t *unlock_err = SVN_NO_ERROR;
> svn_error_t *cleanup_err = SVN_NO_ERROR;
> + const svn_wc_entry_t *entry;
> int i;
>
> /* The commit process uses absolute paths, so we need to open the access
> @@ -988,6 +1006,14 @@
> 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));
> + SVN_ERR(svn_wc_entry(&entry, pair->src, adm_access, FALSE, pool));
> + pair->src_revnum = entry->revision;
> +
> pair->dst_rel = svn_path_is_child(top_dst_url, pair->dst, pool);
> SVN_ERR(svn_ra_check_path(ra_session,
> svn_path_uri_decode(pair->dst_rel, pool),
> @@ -1052,6 +1078,42 @@
> APR_HASH_KEY_STRING))))
> goto cleanup;
>
> + /* ### 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). */
> + for (i = 0; i < copy_pairs->nelts; i++)
> + {
> + svn_prop_t *mergeinfo_prop;
> + apr_hash_t *mergeinfo, *wc_mergeinfo;
> + svn_client__copy_pair_t *pair = APR_ARRAY_IDX(copy_pairs, i,
> + svn_client__copy_pair_t *);
> + svn_client_commit_item3_t *item =
> + APR_ARRAY_IDX(commit_items, i, svn_client_commit_item3_t *);
> +
> + /* Set the merge info for the destination to the combined merge
> + info known to the WC and the repository. */
> + item->outgoing_prop_changes = apr_array_make(pool, 1,
> + sizeof(svn_prop_t *));
> + mergeinfo_prop = apr_palloc(item->outgoing_prop_changes->pool,
> + sizeof(svn_prop_t));
> + mergeinfo_prop->name = SVN_PROP_MERGE_INFO;
> + SVN_ERR(calculate_target_merge_info(ra_session, &mergeinfo,
> + adm_access, pair->src,
> + pair->src_rel, pair->src_revnum,
> + pool));
> + SVN_ERR(svn_wc_entry(&entry, pair->src, adm_access, FALSE, pool));
> + SVN_ERR(svn_client__parse_merge_info(&wc_mergeinfo, entry,
> + pair->src, adm_access, ctx,
> + pool));
> + SVN_ERR(svn_mergeinfo_merge(&mergeinfo, mergeinfo,
> + wc_mergeinfo, pool));
> + SVN_ERR(svn_mergeinfo__to_string((svn_string_t **)
> + &mergeinfo_prop->value,
> + mergeinfo, pool));
> + APR_ARRAY_PUSH(item->outgoing_prop_changes, svn_prop_t *) =
> + mergeinfo_prop;
> + }
> +
> /* Sort and condense our COMMIT_ITEMS. */
> if ((cmt_err = svn_client__condense_commit_items(&top_dst_url,
> commit_items,
>
> Modified: branches/merge-tracking/subversion/tests/cmdline/copy_tests.py
> URL: http://svn.collab.net/viewvc/svn/branches/merge-tracking/subversion/tests/cmdline/copy_tests.py?pathrev=23423&r1=23422&r2=23423
> ==============================================================================
> --- branches/merge-tracking/subversion/tests/cmdline/copy_tests.py (original)
> +++ branches/merge-tracking/subversion/tests/cmdline/copy_tests.py Fri Feb 16 18:25:53 2007
> @@ -2661,6 +2661,11 @@
> svntest.actions.run_and_verify_svn(None, None, [], 'cp', '-m', '',
> upsilon_path, upsilon_copy_URL)
>
> + # Validate that the merge info of the copy destination matches the
> + # implied merge info from the copy source.
> + svntest.actions.run_and_verify_svn(None, ['\n'], [], 'propget',
> + 'svn:mergeinfo', upsilon_copy_URL)
> +
> # Copy added dir A/D/I to URL://A/D/G/I
> I_copy_URL = sbox.repo_url + '/A/D/G/I'
> svntest.actions.run_and_verify_svn(None, None, [], 'cp', '-m', '',
> @@ -3386,7 +3391,7 @@
> copy_delete_commit,
> mv_and_revert_directory,
> Skip(copy_preserve_executable_bit, (os.name != 'posix')),
> - XFail(wc_to_repos),
> + wc_to_repos,
> repos_to_wc,
> copy_to_root,
> url_copy_parent_into_child,
>