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
Received on 2009-07-05 19:00:28 CEST