Apologize for the delayed comment. Took so long to catch up on mail
backlogs.
On Fri, 04 Aug 2006 01:14:23 +0530, <dlr@tigris.org> wrote:
> Author: dlr
> Date: Thu Aug 3 12:44:22 2006
> New Revision: 20953
>
> Modified:
> branches/merge-tracking/subversion/libsvn_client/diff.c
[snip]
> +/* 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). */
> static svn_error_t *
> update_wc_merge_info(const char *target_wcpath, const svn_wc_entry_t
> *entry,
[snip]
> + /* As some of the merges may've changed the WC's merge info, get
> + a fresh copy before using it to update the WC's merge info. */
> + SVN_ERR(parse_merge_info(&mergeinfo, entry, path, adm_access, ctx,
> + subpool));
> +
> + /* ASSUMPTION: "target_wcpath" is always both a parent and
> + prefix of "path". */
This will not always be the case. IIUC, path will be the source paths of
the merge, while target_wcpath will be, as implied the target wc path. So
very rarely (only in cases of merges into and from the same path) will
this be true. Or am I missing something here?
> + int len = strlen(target_wcpath);
> + if (len < strlen(path))
> + rel_path = apr_pstrcat(subpool, repos_rel_path, "/", path
> + len);
> + else
> + rel_path = repos_rel_path;
sorry, this totally goes over my head - especially given my understanding
stated above, could you please explain a bit on the above chunk of code.
[snip]
> + if (rangelist == NULL)
> + rangelist = apr_array_make(subpool, 0, sizeof(svn_merge_range_t
> *));
> - /* Update the merge info by adjusting the rangelist for REL_PATH. */
> - if (rangelist->nelts == 0)
> - rangelist = NULL;
> - apr_hash_set(mergeinfo, rel_path, APR_HASH_KEY_STRING, rangelist);
> -
> - if (apr_hash_count(mergeinfo) > 0)
> - {
> - /* The WC will contain merge info. */
> - svn_stringbuf_t *mergeinfo_buf;
> - SVN_ERR(svn_mergeinfo_to_string(&mergeinfo_buf, mergeinfo, pool));
> - mergeinfo_str = svn_string_create_from_buf(mergeinfo_buf, pool);
> - }
> + if (is_revert)
> + {
> + ranges = svn_rangelist_dup(ranges, subpool);
> + SVN_ERR(svn_rangelist_reverse(ranges, subpool));
> + SVN_ERR(svn_rangelist_remove(&rangelist, ranges, rangelist,
> + subpool));
> + }
> + else
> + SVN_ERR(svn_rangelist_merge(&rangelist, rangelist, ranges,
> subpool));
> +
> + /* Update the merge info by adjusting the path's rangelist. */
> + if (rangelist->nelts == 0)
> + rangelist = NULL;
This comment is for the above lines. But am quoting from the code itself,
to make the point clearer.
rangelist = apr_hash_get(mergeinfo, rel_path, APR_HASH_KEY_STRING);
if (rangelist == NULL)
rangelist = apr_array_make(subpool, 0, sizeof(svn_merge_range_t
*));
if (is_revert)
{
ranges = svn_rangelist_dup(ranges, subpool);
SVN_ERR(svn_rangelist_reverse(ranges, subpool));
SVN_ERR(svn_rangelist_remove(&rangelist, ranges, rangelist,
subpool));
}
else
SVN_ERR(svn_rangelist_merge(&rangelist, rangelist, ranges,
subpool));
/* Update the merge info by adjusting the path's rangelist. */
if (rangelist->nelts == 0)
rangelist = NULL;
can't the above could be put within a 'if (rangelist == NULL)' and some
minor tweaks, instead of making an apr array and later checking for an
empty array again. (sorry, am not able to write out pseudo/code right away
for you now - gtg)
Regards,
Madan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Aug 9 14:04:54 2006