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