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

Keeping these function names of a reasonable length but descriptive is
harder than finding a beer that tastes great and is less filling.

Maybe something like this?
                               
find_wc_merge_info() -->
get_wc_inherited_or_explicit_merge_info()*
                           --> get_inherited_or_explicit_merge_info()*

get_wc_target_merge_info() --> get_wc_and_repos_merge_info()

Long, but not much worse than "grok_range_info_from_opt_revisions()"

Paul B.

> -----Original Message-----
> From: Daniel Rall [mailto:dlr@collab.net]
> Sent: Tuesday, March 13, 2007 1:50 PM
> To: dev@subversion.tigris.org
> Cc: pburba@tigris.org
> Subject: 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/li
> > bsvn_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
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Mar 13 19:21:16 2007