Re: svn commit: r37912 - in trunk/subversion: include libsvn_client libsvn_wc

On Tue, Jun 2, 2009 at 19:17, Hyrum K. Wright<hyrum_at_hyrumwright.org> wrote:
>...
> +++ trunk/subversion/libsvn_client/locking_commands.c   Tue Jun  2 10:17:27 2009        (r37912)
> @@ -38,9 +38,10 @@
>  /* For use with store_locks_callback, below. */
>  struct lock_baton
>  {
> -  svn_wc_adm_access_t *adm_access;
> +  const char *base_path;
>   apr_hash_t *urls_to_paths;
>   svn_client_ctx_t *ctx;
> +  svn_wc_context_t *wc_ctx;
>   apr_pool_t *pool;
>  };
>
> @@ -49,7 +50,7 @@ struct lock_baton
>  * BATON is a 'struct lock_baton *', PATH is the path being locked,
>  * and LOCK is the lock itself.
>  *
> - * If BATON->adm_access is not null, then this function either stores
> + * If BATON->base_path is not null, then this function either stores
>  * the LOCK on REL_URL or removes any lock tokens from REL_URL
>  * (depending on whether DO_LOCK is true or false respectively), but
>  * only if RA_ERR is null, or (in the unlock case) is something other
> @@ -63,8 +64,6 @@ store_locks_callback(void *baton,
>                      svn_error_t *ra_err, apr_pool_t *pool)
>  {
>   struct lock_baton *lb = baton;
> -  svn_wc_adm_access_t *adm_access;
> -  const char *abs_path;
>   svn_wc_notify_t *notify;
>
>   /* Create the notify struct first, so we can tweak it below. */
> @@ -80,25 +79,21 @@ store_locks_callback(void *baton,
>   notify->lock = lock;
>   notify->err = ra_err;
>
> -  if (lb->adm_access)
> +  if (lb->base_path)
>     {
> -      const char *base_path = svn_wc_adm_access_path(lb->adm_access);
>       char *path = apr_hash_get(lb->urls_to_paths, rel_url,
>                                 APR_HASH_KEY_STRING);
> -      abs_path = svn_path_join(base_path, path, lb->pool);
> -
> -      SVN_ERR(svn_wc_adm_probe_retrieve(&adm_access, lb->adm_access,
> -                                        abs_path, lb->pool));
> +      const char *abs_path = svn_path_join(lb->base_path, path, lb->pool);
>
>       /* Notify a valid working copy path */
>       notify->path = abs_path;
> -      notify->path_prefix = base_path;
> +      notify->path_prefix = lb->base_path;
>
>       if (do_lock)
>         {
>           if (!ra_err)
>             {
> -              SVN_ERR(svn_wc_add_lock(abs_path, lock, adm_access, lb->pool));
> +              SVN_ERR(svn_wc_add_lock2(lb->wc_ctx, abs_path, lock, lb->pool));

This is incorrect. abs_path is NOT an absolute path. Total misnomer.

>               notify->lock_state = svn_wc_notify_lock_state_locked;
>             }
>           else
> @@ -114,7 +109,7 @@ store_locks_callback(void *baton,
>           if (!ra_err ||
>               (ra_err && (ra_err->apr_err != SVN_ERR_FS_LOCK_OWNER_MISMATCH)))
>             {
> -              SVN_ERR(svn_wc_remove_lock(abs_path, adm_access, lb->pool));
> +              SVN_ERR(svn_wc_remove_lock2(lb->wc_ctx, abs_path, lb->pool));

Same problem.

>               notify->lock_state = svn_wc_notify_lock_state_unlocked;
>             }
>           else
> @@ -425,14 +420,20 @@ svn_client_lock(const apr_array_header_t
>            adm_access, NULL, FALSE, FALSE, ctx, pool));
>
>   cb.pool = pool;
> -  cb.adm_access = adm_access;
> +  if (adm_access)
> +    cb.base_path = svn_wc_adm_access_path(adm_access);

You could use svn_wc__adm_access_abspath() here.

> +  else
> +    cb.base_path = NULL;
>   cb.urls_to_paths = urls_to_paths;
>   cb.ctx = ctx;
> +  SVN_ERR(svn_wc_context_create(&cb.wc_ctx, NULL /* config */, pool, pool));
>
>   /* Lock the paths. */
>   SVN_ERR(svn_ra_lock(ra_session, path_revs, comment,
>                       steal_lock, store_locks_callback, &cb, pool));
>
> +  SVN_ERR(svn_wc_context_destroy(cb.wc_ctx));
> +
>   /* Unlock the wc. */
>   if (adm_access)
>     return svn_wc_adm_close2(adm_access, pool);
> @@ -473,14 +474,20 @@ svn_client_unlock(const apr_array_header
>     SVN_ERR(fetch_tokens(ra_session, path_tokens, pool));
>
>   cb.pool = pool;
> -  cb.adm_access = adm_access;
> +  if (adm_access)
> +    cb.base_path = svn_wc_adm_access_path(adm_access);

And here.

>...
> +++ trunk/subversion/libsvn_wc/adm_ops.c        Tue Jun  2 10:17:27 2009        (r37912)
> @@ -3082,21 +3082,21 @@ svn_wc_resolved_conflict4(const char *pa
>                                      cancel_func, cancel_baton, pool);
>  }
>
> -svn_error_t *svn_wc_add_lock(const char *path, const svn_lock_t *lock,
> -                             svn_wc_adm_access_t *adm_access, apr_pool_t *pool)
> +svn_error_t *
> +svn_wc_add_lock2(svn_wc_context_t *wc_ctx,
> +                 const char *local_abspath,
> +                 const svn_lock_t *lock,
> +                 apr_pool_t *scratch_pool)
>  {
> -  svn_wc__db_t *db = svn_wc__adm_get_db(adm_access);
> -  const char *local_abspath;
>   svn_wc__db_lock_t db_lock;
>   svn_error_t *err;
>   const svn_string_t *needs_lock;
>
> -  SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));

For safety, and to catch bugs (like above, where abs_path is anything
but), you could/should add an assertion here.

>   db_lock.token = lock->token;
>   db_lock.owner = lock->owner;
>   db_lock.comment = lock->comment;
>   db_lock.date = lock->creation_date;
> -  err = svn_wc__db_lock_add(db, local_abspath, &db_lock, pool);
> +  err = svn_wc__db_lock_add(wc_ctx->db, local_abspath, &db_lock, scratch_pool);
>   if (err)
>     {
>       if (err->apr_err != SVN_ERR_WC_PATH_NOT_FOUND)
> @@ -3106,29 +3106,29 @@ svn_error_t *svn_wc_add_lock(const char
>       svn_error_clear(err);
>       return svn_error_createf(SVN_ERR_ENTRY_NOT_FOUND, NULL,
>                                _("'%s' is not under version control"),
> -                               svn_path_local_style(path, pool));
> +                               svn_path_local_style(local_abspath,
> +                                                    scratch_pool));
>     }

You've rev'd the API, so this error remapping should move to deprecated.c.

>...
>  {
> -  svn_wc__db_t *db = svn_wc__adm_get_db(adm_access);
> -  const char *local_abspath;
>   svn_error_t *err;
>   const svn_string_t *needs_lock;
>
> -  SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));

Another assertion.

> -  err = svn_wc__db_lock_remove(db, local_abspath, pool);
> +  err = svn_wc__db_lock_remove(wc_ctx->db, local_abspath, scratch_pool);
>   if (err)
>     {
>       if (err->apr_err != SVN_ERR_WC_PATH_NOT_FOUND)

Another remap to move.

>...
> +++ trunk/subversion/libsvn_wc/deprecated.c     Tue Jun  2 10:17:27 2009        (r37912)
> @@ -528,6 +528,39 @@ svn_wc_resolved_conflict3(const char *pa
>                                    cancel_baton, pool);
>  }
>
> +svn_error_t *
> +svn_wc_add_lock(const char *path,
> +                const svn_lock_t *lock,
> +                svn_wc_adm_access_t *adm_access,
> +                apr_pool_t *pool)
> +{
> +  const char *local_abspath;
> +  svn_wc_context_t *wc_ctx;
> +
> +  SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
> +  SVN_ERR(svn_wc__context_create_with_db(&wc_ctx, NULL /* config */,
> +                                         svn_wc__adm_get_db(adm_access),
> +                                         pool));
> +
> +  return svn_error_return(svn_wc_add_lock2(wc_ctx, local_abspath, lock, pool));

The context is not destroyed, which could mean dangling open-handles on SDBs.

> +}
> +
> +svn_error_t *
> +svn_wc_remove_lock(const char *path,
> +                   svn_wc_adm_access_t *adm_access,
> +                   apr_pool_t *pool)
> +{
> +  const char *local_abspath;
> +  svn_wc_context_t *wc_ctx;
> +
> +  SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
> +  SVN_ERR(svn_wc__context_create_with_db(&wc_ctx, NULL /* config */,
> +                                         svn_wc__adm_get_db(adm_access),
> +                                         pool));
> +
> +  return svn_error_return(svn_wc_remove_lock2(wc_ctx, local_abspath, pool));

Likewise.

>...

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2368886
Received on 2009-07-07 17:36:20 CEST