Re: svn commit: r23785 - branches/merge-tracking/subversion/libsvn_client

I feel like we should do a better job of differentiating between these
two APIs -- find_wc_merge_info and get_wc_target_merge_info are such
similar names that you really need to read the doc string of each to
get a clear picture of the differences. I don't like the idea of
combining them, since the resulting function would be quite large.
Any suggestions for better function names?

On Mon, 12 Mar 2007, pburba@tigris.org wrote:

> Author: pburba
> Date: Mon Mar 12 13:37:16 2007
> New Revision: 23785
>
> Log:
> On the merge-tracking branch: First stab at fixing Issues #2733 and #2734.
>
> If no merge info exists on a merge target inherit it from the nearest
> ancestor with merge info if one exists.
>
> Patch by: dlr
> me
>
> * subversion/libsvn_client/diff.c
> (find_wc_merge_info): New function.
> (get_wc_target_merge_info): Tweak docstring, add new argument indicating
> if merge info was inherited in the WC, and use find_wc_merge_info()
> rather than svn_client__parse_merge_info() to get target merge info.
> (update_wc_merge_info): Add argument describing merge info existing on
> the WC target and a second argument indicating whether this info was
> explicit or inherited.
> (do_merge, do_single_file_merge): Update callers of
> update_wc_merge_info().
>
> Modified:
> branches/merge-tracking/subversion/libsvn_client/diff.c
>
> Modified: branches/merge-tracking/subversion/libsvn_client/diff.c
> URL: http://svn.collab.net/viewvc/svn/branches/merge-tracking/subversion/libsvn_client/diff.c?pathrev=23785&r1=23784&r2=23785
> ==============================================================================
> --- branches/merge-tracking/subversion/libsvn_client/diff.c (original)
> +++ branches/merge-tracking/subversion/libsvn_client/diff.c Mon Mar 12 13:37:16 2007
> @@ -1784,13 +1784,109 @@
> return SVN_NO_ERROR;
> }
>
> +/* Find explicit or inherited WC merge info for WCPATH, and return it
> + in *MERGEINFO. If the merge info was inherited set *INHERITED to
> + TRUE, set it to FALSE otherwise. */
> +static svn_error_t *
> +find_wc_merge_info(apr_hash_t **mergeinfo,
> + svn_boolean_t *inherited,
> + const svn_wc_entry_t *entry,
> + const char *wcpath,
> + svn_wc_adm_access_t *adm_access,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> +{
> + const char *walk_path = "";
> + apr_hash_t *tmp_mergeinfo = apr_hash_make(pool);
> + apr_hash_index_t *hi;
> + const char *path;
> + void *val, *key;
> + apr_array_header_t *ranges;
> +
> + *inherited = FALSE;
> + *mergeinfo = apr_hash_make(pool);
> + SVN_ERR(svn_path_get_absolute(&wcpath, wcpath, pool));
> +
> + /* ### What about copied paths? */
> + while (TRUE)
> + {
> + /* Look for merge info on WC_FULLPATH. If there isn't any, walk
> + towards the root of the WC until we encounter either (a) an
> + unversioned directory, or (b) merge info. If we encounter (b),
> + use that inherited merge info as our baseline. */
> + SVN_ERR(svn_client__parse_merge_info(&tmp_mergeinfo, entry, wcpath,
> + adm_access, ctx, pool));
> +
> + if (apr_hash_count(tmp_mergeinfo) == 0 &&
> + !svn_path_is_root(wcpath, strlen(wcpath)))
> + {
> + svn_error_t *err;
> +
> + /* No explicit merge info on this path. Look higher up the
> + directory tree while keeping track of what we've walked. */
> + walk_path = svn_path_join(svn_path_basename(wcpath, pool),
> + walk_path, pool);
> + wcpath = svn_path_dirname(wcpath, pool);
> +
> + /* The previous entry didn't lock the parent directory. */
> + err = svn_wc_adm_open3(&adm_access, NULL,
> + wcpath,//svn_path_dirname(wcpath, pool),
> + FALSE, 0, NULL, NULL, pool);
> +
> + if (err)
> + {
> + if (err->apr_err == SVN_ERR_WC_NOT_DIRECTORY)
> + {
> + svn_error_clear(err);
> + return SVN_NO_ERROR;
> + }
> + return err;
> + }
> +
> + SVN_ERR(svn_wc_entry(&entry, wcpath,
> + adm_access, FALSE, pool));
> +
> + if (entry)
> + /* We haven't yet risen above the root of the WC. */
> + continue;
> + }
> + break;
> + }
> +
> + /* Create inherited merge info. */
> + if (svn_path_is_empty(walk_path))
> + {
> + *mergeinfo = tmp_mergeinfo;
> + }
> + else
> + {
> + for (hi = apr_hash_first(pool, tmp_mergeinfo); hi; hi = apr_hash_next(hi))
> + {
> + *inherited = TRUE;
> + apr_hash_this(hi, &key, NULL, &val);
> + ranges = val;
> + path = key;
> + apr_hash_set(*mergeinfo,
> + svn_path_is_empty(walk_path) ? path
> + : svn_path_join(path, walk_path, pool),
> + APR_HASH_KEY_STRING, ranges);
> + }
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
> /* Retrieve any inherited or direct merge info for the target from
> - both the repos and the WC's merge info prop. Combine these two
> + both the repos and the WC's merge info prop (or that of its nearest
> + ancestor if the target has no info of its own). Combine these two
> sets of merge info to determine what's already been merged into the
> - target (reflected by *ENTRY), and store them in *TARGET_MERGEINFO. */
> + target (reflected by *ENTRY), and store them in *TARGET_MERGEINFO.
> + If the target inherited any merge info from a WC ancestor set
> + *INHERITED to TRUE, set it to FALSE otherwise. */
> static svn_error_t *
> get_wc_target_merge_info(apr_hash_t **target_mergeinfo,
> const svn_wc_entry_t **entry,
> + svn_boolean_t *inherited,
> svn_ra_session_t *ra_session,
> const char *target_wcpath,
> svn_wc_adm_access_t *adm_access,
> @@ -1839,8 +1935,10 @@
> SVN_ERR(svn_client__get_merge_info_for_path(ra_session, &repos_mergeinfo,
> repos_rel_path, target_rev,
> pool));
> - SVN_ERR(svn_client__parse_merge_info(target_mergeinfo, *entry, target_wcpath,
> - adm_access, ctx, pool));
> +
> + SVN_ERR(find_wc_merge_info(target_mergeinfo, inherited, *entry,
> + target_wcpath, adm_access, ctx, pool));
> +
> if (repos_mergeinfo != NULL)
> {
> /* If pre-existing local changes reverted some of the merge info
> @@ -2033,10 +2131,13 @@
> /* Calculate the new merge info for the target tree based on the merge
> info for TARGET_WCPATH and MERGES (a mapping of WC paths to range
> lists), and record it in the WC (at, and possibly below,
> - TARGET_WCPATH). */
> + TARGET_WCPATH). IF INHERITED is true then TARGET_WCPATH had no
> + explicit merge info and it's inherited merge info is contained in
> + TARGET_MERGEINFO. */
> static svn_error_t *
> update_wc_merge_info(const char *target_wcpath, const svn_wc_entry_t *entry,
> const char *repos_rel_path, apr_hash_t *merges,
> + apr_hash_t *target_mergeinfo, svn_boolean_t inherited,
> svn_boolean_t is_revert, svn_wc_adm_access_t *adm_access,
> svn_client_ctx_t *ctx, apr_pool_t *pool)
> {
> @@ -2045,6 +2146,11 @@
> apr_hash_t *mergeinfo;
> apr_hash_index_t *hi;
>
> + if (inherited)
> + SVN_ERR(svn_client__record_wc_merge_info(target_wcpath,
> + target_mergeinfo,
> + adm_access, subpool));
> +
> /* Combine the merge info for the revision range just merged into
> the WC with its on-disk merge info. */
> for (hi = apr_hash_first(pool, merges); hi; hi = apr_hash_next(hi))
> @@ -2087,8 +2193,9 @@
> subpool));
> }
> else
> - SVN_ERR(svn_rangelist_merge(&rangelist, ranges, subpool));
> -
> + {
> + SVN_ERR(svn_rangelist_merge(&rangelist, ranges, subpool));
> + }
> /* Update the merge info by adjusting the path's rangelist. */
> if (rangelist->nelts == 0)
> rangelist = NULL;
> @@ -2219,6 +2326,7 @@
> svn_opt_revision_t *revision1, *revision2;
> const svn_wc_entry_t *entry;
> int i;
> + svn_boolean_t inherited;
>
> ENSURE_VALID_REVISION_KINDS(initial_revision1->kind,
> initial_revision2->kind);
> @@ -2284,8 +2392,9 @@
>
> if (notify_b.same_urls)
> {
> - SVN_ERR(get_wc_target_merge_info(&target_mergeinfo, &entry, ra_session,
> - target_wcpath, adm_access, ctx, pool));
> + SVN_ERR(get_wc_target_merge_info(&target_mergeinfo, &entry, &inherited,
> + ra_session, target_wcpath, adm_access,
> + ctx, pool));
>
> is_revert = (merge_type == merge_type_revert);
> SVN_ERR(svn_client__path_relative_to_root(&rel_path, URL1, NULL,
> @@ -2307,8 +2416,8 @@
> SVN_ERR(determine_merges_performed(&merges, target_wcpath,
> &range, &notify_b, pool));
> return update_wc_merge_info(target_wcpath, entry, rel_path,
> - merges, is_revert, adm_access, ctx,
> - pool);
> + merges, target_mergeinfo, inherited,
> + is_revert, adm_access, ctx, pool);
> }
> }
>
> @@ -2431,7 +2540,8 @@
> SVN_ERR(determine_merges_performed(&merges, target_wcpath, r,
> &notify_b, pool));
> SVN_ERR(update_wc_merge_info(target_wcpath, entry, rel_path,
> - merges, is_revert, adm_access,
> + merges, target_mergeinfo,
> + inherited, is_revert, adm_access,
> ctx, pool));
> }
>
> @@ -2513,6 +2623,7 @@
> apr_hash_t *target_mergeinfo;
> const svn_wc_entry_t *entry;
> int i;
> + svn_boolean_t inherited = FALSE;
>
> ENSURE_VALID_REVISION_KINDS(initial_revision1->kind,
> initial_revision2->kind);
> @@ -2571,8 +2682,9 @@
> if (merge_type == merge_type_no_op)
> return SVN_NO_ERROR;
>
> - SVN_ERR(get_wc_target_merge_info(&target_mergeinfo, &entry, ra_session1,
> - target_wcpath, adm_access, ctx, pool));
> + SVN_ERR(get_wc_target_merge_info(&target_mergeinfo, &entry, &inherited,
> + ra_session1, target_wcpath, adm_access,
> + ctx, pool));
>
> is_revert = (merge_type == merge_type_revert);
> SVN_ERR(svn_client__path_relative_to_root(&rel_path, URL1, NULL,
> @@ -2594,8 +2706,9 @@
> SVN_ERR(determine_merges_performed(&merges, target_wcpath,
> &range, &notify_b, pool));
> return update_wc_merge_info(target_wcpath, entry, rel_path,
> - merges, is_revert, adm_access, ctx,
> - pool);
> + merges, target_mergeinfo,
> + inherited, is_revert, adm_access,
> + ctx, pool);
> }
> }
>
> @@ -2688,8 +2801,9 @@
> SVN_ERR(determine_merges_performed(&merges, target_wcpath, r,
> &notify_b, pool));
> SVN_ERR(update_wc_merge_info(target_wcpath, entry, rel_path,
> - merges, is_revert, adm_access, ctx,
> - pool));
> + merges, target_mergeinfo,
> + inherited, is_revert, adm_access,
> + ctx, pool));
> }
>
> /* Clear the notification counter and list of skipped paths
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org