>>> +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() ??
>
>>> +{
>>> + /* Dereference to obtain only the merge info of the src_path
>>> provided */
>>> + if (merge_info)
>>>
>> No need to check for nullity of merge_info. Unless rel_paths->nelts
>> == 0 merge_info will never be null.
>
> We need to ensure that src_mergeinfo is set to NULL, thats why the
> null-check.
No need to ensure that. Across our code, atleast the places where I have
seen, the callee always sets the apr_hash_t*. Checking for NULL there is
redundancy.
In this particular case refer 'svn_repos_fs_get_merge_info' in
subversion/libsvn_repos/fs-wrap.c you will realize that 'src_mergeinfo'
will never be NULL.
Anyway I could see this kind of too cautious(????) null checking atleast
in 2 instances(for which my patch found its place in issue tracker!!!,
and hence did not give much motivation to dig further.)
>
>>> +
>>> + /* Obtain copyfrom information of the source */
>>> SVN_ERR(get_copyfrom_merge_info(ra_session, ©from_mergeinfo,
>>> src_rel_path, copyfrom_path,
>>> src_revnum, pool));
>>>
>> What about using only 'get_src_merge_info'?
>
> get_src_merge_info() is to get the existing mergeinfo of the source.
> get_copyfrom_merge_info() is to get the copyfrom information of the
> source. How can we use the same function?
>
Oh I misunderstood!.
I thought 'get_implied_merge_info' would get the 'svn:mergeinfo' as
available on 'copy_from_source_path'. I am wrong it just gives the
'copy_from_source_path:created_rev-cur_rev'. So we need both this
mergeinfo and mergeinfo crawl on 'copy_from_mergeinfo'.
With regards
Kamesh Jayachandran
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Oct 26 09:01:58 2006