I've revisited the wc context creation/destruction issue in r38345.
Hopefully it addresses these concerns (as well as the other stuff
discussed in IRC).
-Hyrum
On Jul 5, 2009, at 12:00 PM, Greg Stein wrote:
> After reviewing r38328, I'm really not liking this pattern. It seems
> to be very error-prone. You're destroying the context in cases where
> you probably shouldn't. Via empirical evidence, mistakes seem to be
> pretty easy.
>
> In general, why aren't we just creating a wc_ctx when the client
> context is created? They are *cheap* to create (the state pool is the
> most expensive). It only gets expensive when the context and its
> contained DB are used for an operation and an SDB is opened.
>
> Cheers,
> -g
>
> On Sun, Jul 5, 2009 at 15:26, Greg Stein<gstein_at_gmail.com> wrote:
>> This seems like a strange pattern. Why don't we stash that wc_context
>> into the client context? Seems better to lazy-create the wc_context,
>> than to have pairs of create/destroy. ??
>>
>> On Sun, Jul 5, 2009 at 02:28, Hyrum K.
>> Wright<hyrum_at_hyrumwright.org> wrote:
>>> Author: hwright
>>> Date: Sat Jul 4 17:28:19 2009
>>> New Revision: 38337
>>>
>>> Log:
>>> Attempt to fix the windows tests by ensuring we destroy the
>>> wc_context
>>> prior to exiting svn_client_commit4().
>>>
>>> * subversion/libsvn_client/commit.c
>>> (svn_client_commit4): Destroy the wc_context. Also, create the
>>> wc_context
>>> a bit later (after all the early outs).
>>>
>>> Modified:
>>> trunk/subversion/libsvn_client/commit.c
>>>
>>> Modified: trunk/subversion/libsvn_client/commit.c
>>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/commit.c?pathrev=38337&r1=38336&r2=38337
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> ====================================================================
>>> --- trunk/subversion/libsvn_client/commit.c Sat Jul 4
>>> 17:26:21 2009 (r38336)
>>> +++ trunk/subversion/libsvn_client/commit.c Sat Jul 4
>>> 17:28:19 2009 (r38337)
>>> @@ -1338,12 +1338,6 @@ svn_client_commit4(svn_commit_info_t **c
>>> depth == svn_depth_infinity,
>>> pool, pool));
>>>
>>> - /* Get a wc context. */
>>> - if (!ctx->wc_ctx)
>>> - SVN_ERR(svn_wc_context_create(&wc_ctx, NULL /* config */,
>>> pool, pool));
>>> - else
>>> - wc_ctx = ctx->wc_ctx;
>>> -
>>> /* No targets means nothing to commit, so just return. */
>>> if (! base_dir)
>>> goto cleanup;
>>> @@ -1522,6 +1516,12 @@ svn_client_commit4(svn_commit_info_t **c
>>> svn_pool_destroy(subpool);
>>> }
>>>
>>> + /* Get a wc context. */
>>> + if (!ctx->wc_ctx)
>>> + SVN_ERR(svn_wc_context_create(&wc_ctx, NULL /* config */,
>>> pool, pool));
>>> + else
>>> + wc_ctx = ctx->wc_ctx;
>>> +
>>> SVN_ERR(svn_wc_adm_open3(&base_dir_access, NULL, base_dir,
>>> TRUE, /* Write lock */
>>> lock_base_dir_recursive ? -1 : 0, /*
>>> lock levels */
>>> @@ -1702,6 +1702,9 @@ svn_client_commit4(svn_commit_info_t **c
>>> pool);
>>> }
>>>
>>> + if (!ctx->wc_ctx)
>>> + SVN_ERR(svn_wc_context_destroy(wc_ctx));
>>> +
>>> /* Sleep to ensure timestamp integrity. */
>>> svn_io_sleep_for_timestamps(base_dir, pool);
>>>
>>> ------------------------------------------------------
>>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=2368048
>>>
>>
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2368159
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2368405
Received on 2009-07-06 17:10:45 CEST