Re: svn commit: r37088 - in trunk/subversion: include libsvn_client libsvn_wc svn tests/cmdline tests/cmdline/upgrade_tests_data

On Tue, Apr 7, 2009 at 23:58, Hyrum K. Wright <hyrum_at_hyrumwright.org> wrote:
>...
> +++ trunk/subversion/libsvn_wc/lock.c   Tue Apr  7 14:58:02 2009        (r37088)
>...
>   /* We can upgrade all formats that are accepted by
>      svn_wc__check_format. */
>   if (adm_access->wc_format < SVN_WC__VERSION)
>     {
> -      svn_boolean_t cleanup_required;
> -      svn_stringbuf_t *log_accum = svn_stringbuf_create("", pool);
> -
> -      /* Don't try to mess with the WC if there are old log files left. */
> -      SVN_ERR(svn_wc__adm_is_cleanup_required(&cleanup_required,
> -                                              adm_access, pool));
> -      if (cleanup_required)
> -        return SVN_NO_ERROR;

You don't want to toss the cleanup test. If there *is* an old log
sitting in there, then the next few statements will *blow it away*.
Not good.

>...
> +++ trunk/subversion/libsvn_wc/log.c    Tue Apr  7 14:58:02 2009        (r37088)
> @@ -2502,13 +2502,37 @@ svn_wc__write_log(svn_wc_adm_access_t *a
>
>
>  /*** Recursively do log things. ***/
> +static svn_error_t *
> +upgrade_working_copy(const char *path,
> +                     svn_cancel_func_t cancel_func,
> +                     void *cancel_baton,
> +                     apr_pool_t *scratch_pool)
> +{
> +  svn_wc_adm_access_t *adm_access;
>
> -svn_error_t *
> -svn_wc_cleanup2(const char *path,
> -                const char *diff3_cmd,
> -                svn_cancel_func_t cancel_func,
> -                void *cancel_baton,
> -                apr_pool_t *pool)
> +  /* Lock this working copy directory, or steal an existing lock */
> +  SVN_ERR(svn_wc__adm_steal_write_lock(&adm_access, path, scratch_pool));

Grab this once in the caller. You'll note that it wants the lock also.
Better to grab once, than to open/close/open again.

And... that means upgrade_working_copy() devolves to a call to
svn_wc__upgrade_format(). (but maybe you have something planned for
it...)

>...

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1586351
Received on 2009-04-08 02:01:53 CEST