Re: [PATCH][MERGE-TRACKING] Step 3 of recording copyfrom mergeinfo during repos to repos copy

On Fri, 27 Oct 2006, Kamesh Jayachandran wrote:

> Madan U Sreenivasan wrote:
> >On Thu, 26 Oct 2006 12:31:38 +0530, Kamesh Jayachandran
> ><kamesh@collab.net> wrote:
> >
> >>
> >>>>>+static svn_error_t *
> >>>>>+get_src_merge_info(svn_ra_session_t *ra_session,
> >>>>>+ apr_hash_t **src_mergeinfo,
> >>>>>+ const char *src_path,
> >>>>>+ svn_revnum_t src_rev,
> >>>>>+ apr_pool_t *pool)
> >>>>>
> >>>>May be we should export this function so that
> >>>>subversion/libsvn_client/diff.c:get_wc_target_merge_info can make
> >>>>use of the same. I could see the scope for its usage when we might
> >>>>have 'mergeaudit' subcommands.
> >>>
> >>>Could you explain a bit more in detail. All this function does is
> >>>call svn_ra_get_merge_info(), and wraps it up in a mergeinfo hash.
> >>Yes we have a similar code lying out in the function
> >>'get_wc_target_merge_info' inside subversion/libsvn_client/diff.c. If
> >>you make get_src_merge_info public(that is what I meant when I said
> >>export(static functions are not exported) it ) we can reuse it there
> >>and possible future consumers like subcommands like 'svn mergeaudit'.
> >>May be we can call it svn_client_get_mergeinfo_for_single_path() ??
> >
> >No, these are not the same...
>
> <snip from libsvn_client/diff.c>
> APR_ARRAY_PUSH(mergeinfo_paths, const char *) = repos_rel_path;
>
> /* ### TODO: To handle sub-tree merge info, the list will need to
> ### include the those child paths which have merge info which
> ### differs from that of TARGET_WCPATH, and if those paths are
> ### directories, their children as well. */
>
> SVN_ERR(svn_ra_get_merge_info(ra_session, &repos_mergeinfo,
> mergeinfo_paths,
> target_rev, TRUE, pool));
> //This line is independent of the earlier one and the following one so
> can be moved above or below.
> SVN_ERR(parse_merge_info(target_mergeinfo, *entry, target_wcpath,
> adm_access, ctx, pool));
>
> if (repos_mergeinfo != NULL)
> {
> apr_hash_t *target_repos_mergeinfo =
> apr_hash_get(repos_mergeinfo, repos_rel_path, APR_HASH_KEY_STRING);
>
>
> </snip>
> <snip from your patch>
>
> apr_hash_t *merge_info;
> apr_array_header_t *rel_paths = apr_array_make(pool, 1,
> sizeof(src_path));
>
> APR_ARRAY_PUSH(rel_paths, const char *) = src_path;
>
> SVN_ERR(svn_ra_get_merge_info(ra_session, &merge_info, rel_paths,
> src_rev, TRUE, pool));
>
> /* Dereference to obtain only the merge info of the src_path provided */
> if (merge_info)
> *src_mergeinfo = apr_hash_get(merge_info, src_path, APR_HASH_KEY_STRING);
> else
> *src_mergeinfo = NULL;
>
> return SVN_NO_ERROR;
>
> </snip>
> I meant to reuse this implementation in
> libsvn_client/diff.c:get_wc_target_merge_info.
>
> Hope I made it clear.

Good suggestion, Kamesh. I've added an
svn_client__get_merge_info_for_path() API in r22130, which is now used
from get_wc_target_merge_info(). As WC -> repos 'copy' is implemented
in terms of Merge Tracking, we'll likely refactor more of
get_wc_target_merge_info() into the shared subroutine.

I will also use this new API when committing Madan's patch.