On Tue, Apr 28, 2009 at 01:16, Paul T. Burba <pburba_at_collab.net> wrote:
>...
> +++ trunk/subversion/libsvn_client/merge.c Mon Apr 27 16:16:50 2009 (r37491)
>...
> @@ -2847,7 +2902,8 @@ get_full_mergeinfo(svn_mergeinfo_t *reco
> function must have previously been called for PARENT.
> */
> static svn_error_t *
> -filter_merged_revisions(svn_client__merge_path_t *child,
> +filter_merged_revisions(svn_client__merge_path_t *parent,
> + svn_client__merge_path_t *child,
> const svn_wc_entry_t *entry,
> const char *mergeinfo_path,
> svn_mergeinfo_t target_mergeinfo,
> @@ -2942,12 +2998,33 @@ filter_merged_revisions(svn_client__merg
> /* If we haven't already found CHILD->IMPLICIT_MERGEINFO then
> contact the server to get it. */
> if (! (child->implicit_mergeinfo))
> - SVN_ERR(get_full_mergeinfo(NULL, &(child->implicit_mergeinfo),
> - entry, NULL, svn_mergeinfo_inherited,
> - ra_session, child->path,
> - MAX(revision1, revision2),
> - MIN(revision1, revision2),
> - adm_access, ctx, subpool));
> + {
> + /* If CHILD has explicit mergeinfo only because its parent has
> + non-inheritable mergeinfo (see criteria 3 in
> + get_mergeinfo_paths() then CHILD can inherit PARENT's
> + implicit mergeinfo and we can avoid contacting the server.
> +
> + If child->child_of_noninheritable is true, it implies that
> + PARENT must exist per the rules of get_mergeinfo_paths(). */
> + if (child->child_of_noninheritable)
> + SVN_ERR(inherit_implicit_mereginfo_form_parent(parent,
> + child,
> + revision1,
> + revision2,
> + ra_session,
> + adm_access,
> + ctx,
> + pool));
> + else
> + SVN_ERR(get_full_mergeinfo(NULL,
> + &(child->implicit_mergeinfo),
> + entry, NULL,
> + svn_mergeinfo_inherited,
> + ra_session, child->path,
> + MAX(revision1, revision2),
> + MIN(revision1, revision2),
> + adm_access, ctx, subpool));
> + }
>
> target_implicit_rangelist = apr_hash_get(child->implicit_mergeinfo,
> mergeinfo_path,
> @@ -3029,12 +3106,33 @@ filter_merged_revisions(svn_client__merg
> ranges are represented there. If we don't already have
> CHILD->IMPLICIT_MERGEINFO then get it now. */
> if (! (child->implicit_mergeinfo))
> - SVN_ERR(get_full_mergeinfo(NULL, &(child->implicit_mergeinfo),
> - entry, NULL, svn_mergeinfo_inherited,
> - ra_session, child->path,
> - MAX(revision1, revision2),
> - MIN(revision1, revision2),
> - adm_access, ctx, pool));
> + {
> + /* If CHILD has explicit mergeinfo only because its parent has
> + non-inheritable mergeinfo (see criteria 3 in
> + get_mergeinfo_paths() then CHILD can inherit PARENT's
> + implicit mergeinfo and we can avoid contacting the server.
> +
> + If child->child_of_noninheritable is true, it implies that
> + PARENT must exist per the rules of get_mergeinfo_paths(). */
> + if (child->child_of_noninheritable)
> + SVN_ERR(inherit_implicit_mereginfo_form_parent(parent,
> + child,
> + revision1,
> + revision2,
> + ra_session,
> + adm_access,
> + ctx,
> + pool));
> + else
> + SVN_ERR(get_full_mergeinfo(NULL,
> + &(child->implicit_mergeinfo),
> + entry, NULL,
> + svn_mergeinfo_inherited,
> + ra_session, child->path,
> + MAX(revision1, revision2),
> + MIN(revision1, revision2),
> + adm_access, ctx, pool));
> + }
The above couple blocks of code are *exactly* the same.
I see no rationale for condoning this for a backport. I don't know if
this is a single instance of the merge code bloat, or if it is
indicative. But we shouldn't see duplicative code like this.
Stop the bloat.
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2073630
Received on 2009-05-06 01:06:04 CEST