[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r38328 - trunk/subversion/libsvn_client

From: Hyrum K. Wright <hyrum_at_hyrumwright.org>
Date: Mon, 6 Jul 2009 10:28:21 -0500

On Jul 5, 2009, at 11:55 AM, Greg Stein wrote:

> On Fri, Jul 3, 2009 at 21:25, Hyrum K. Wright<hyrum_at_hyrumwright.org>
> wrote:
>> ...
>> +++ trunk/subversion/libsvn_client/copy.c Fri Jul 3 12:25:44
>> 2009 (r38328)
>> ...
>> @@ -1992,7 +1996,7 @@ svn_client_copy5(svn_commit_info_t **com
>> svn_wc_context_t *wc_ctx;
>>
>> if (!ctx->wc_ctx)
>> - SVN_ERR(svn_wc_context_create(&wc_ctx, NULL /* config */,
>> subpool,
>> + SVN_ERR(svn_wc_context_create(&wc_ctx, NULL /* config */, pool,
>> subpool));
>
> This doesn't seem like a proper change. You need to destroy the
> context at the end of the function, so the subpool seems the proper
> choice. If you were going to stash it into the client context, then
> *maybe* pool would be proper.

This (and the other wc_ctx issues) should be addressed by r38345.

>> ...
>> +++ trunk/subversion/libsvn_client/mergeinfo.c Fri Jul 3 12:25:44
>> 2009 (r38328)
>> ...
>> @@ -113,17 +118,17 @@ svn_client__record_wc_mergeinfo(const ch
>> /* Record the new mergeinfo in the WC. */
>> /* ### Later, we'll want behavior more analogous to
>> ### svn_client__get_prop_from_wc(). */
>> - SVN_ERR(svn_wc_prop_set3(SVN_PROP_MERGEINFO, mergeinfo_str,
>> wcpath,
>> - adm_access, TRUE /* skip checks */,
>> NULL, NULL,
>> - pool));
>> + SVN_ERR(svn_wc_prop_set4(wc_ctx, local_abspath,
>> SVN_PROP_MERGEINFO,
>> + mergeinfo_str, TRUE /* skip checks */,
>> NULL, NULL,
>> + scratch_pool));
>>
>> if (ctx->notify_func2)
>> {
>> ctx->notify_func2(ctx->notify_baton2,
>> - svn_wc_create_notify(wcpath,
>> + svn_wc_create_notify(local_abspath,
>
> Is this a valid change? To change from relative to absolute paths?
> ISTR that the notification callbacks do a lot of path munging to get
> "good looking paths", and this could end up changing the "contract"
> about the types of paths we feed into the notification system. Maybe a
> different notification structure that explicitly contains abspaths?

IIRC, we don't include the type of path in the docs for our
notification structure, so either absolute or relative paths are
acceptable. Our current notification system does some munging of the
absolute path to create a relative path, if it can, so most
notification output won't change. I think the only case where this
doesn't happen is when we give parent relative paths on the command
line.

For example 'svn up ../..' will produce absolute paths in the output,
but 'svn up foo/bar' will produce paths relative to the cwd.

-Hyrum

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2368409
Received on 2009-07-06 17:29:58 CEST

This is an archived mail posted to the Subversion Dev mailing list.