On Aug 3, 2009, at 10:43 PM, HuiHuang wrote:
> Hey,
>
> I write a new patch. You can help me to review it. Thank you:)
Over all, the patch looks good; most of the comments below are
stylistic nits.
> If it is ok, I want to remove adm_access batons in
> svn_client__do_commit().
> Do you think it is necessary now?
I haven't looked at svn_client__do_commit(), but if this (and other
changes) make it so the function no longer needs adm_access batons,
please remove them!
>
> Log:
> [[[
> Rip out some adm_access usage in svn_wc_transmit_prop_deltas().
>
> * subversion/include/svn_wc.h
> (svn_wc_transmit_prop_deltas2): New.
> (svn_wc_transmit_prop_deltas): Deprecate.
>
> * subversion/libsvn_wc/adm_crawler.c
> (svn_wc_internal_transmit_prop_deltas): New.
^
Should use double underscore.
> (svn_wc_transmit_prop_deltas2): New.
> (svn_wc_transmit_prop_deltas): Remove.
>
> * subversion/libsvn_wc/deprecated.c
> (svn_wc_transmit_prop_deltas): Reimplement as a wrapper.
>
> * subversion/libsvn_wc/wc.h
> (svn_wc_internal_transmit_prop_deltas): New.
Same.
> ]]]
>
> Modified:
> trunk/subversion/include/svn_wc.h
> trunk/subversion/libsvn_wc/deprecated.c
> trunk/subversion/libsvn_wc/adm_crawler.c
> trunk/subversion/libsvn_wc/wc.h
>
>
>
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 38546)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -5998,10 +5998,9 @@
> apr_pool_t *pool);
>
>
> -/** Given a @a path with its accompanying @a entry, transmit all
> local
> - * property modifications using the appropriate @a editor method (in
> - * conjunction with @a baton). @a adm_access is an access baton set
> - * that contains @a path. Use @a pool for all allocations.
> +/** Given an absolute path @a local_path, transmit all local property
> + * modifications using the appropriate @a editor method (in
> conjunction
> + * with @a baton). Use @a scratch_pool for any temporary allocation.
> *
> * If a temporary file remains after this function is finished, the
> * path to that file is returned in @a *tempfile (so the caller can
> @@ -6009,8 +6008,26 @@
> *
> * @note Starting version 1.5, no tempfile will ever be returned
> * anymore. If @a *tempfile is passed, its value is set to @c
> NULL.
> + *
> + * @since New in 1.7.
> */
> svn_error_t *
> +svn_wc_transmit_prop_deltas2(svn_wc_context_t *wc_ctx,
> + const char *local_path,
By convention, we're calling this parameter local_abspath.
> + const svn_delta_editor_t *editor,
> + void *baton,
> + const char **tempfile,
You can just remove this parameter, since it no longer has a purpose.
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool);
> +
> +
> +/** Similar to svn_wc_transmit_prop_deltas2(), but with a relative
> path
> + * and adm_access baton.
> + *
> + * @deprecated Provided for backwards compatibility with the 1.6 API.
> + */
> +SVN_DEPRECATED
> +svn_error_t *
> svn_wc_transmit_prop_deltas(const char *path,
> svn_wc_adm_access_t *adm_access,
> const svn_wc_entry_t *entry,
> Index: subversion/libsvn_wc/adm_crawler.c
> ===================================================================
> --- subversion/libsvn_wc/adm_crawler.c (revision 38546)
> +++ subversion/libsvn_wc/adm_crawler.c (working copy)
> @@ -1075,37 +1075,51 @@
> }
>
> svn_error_t *
> -svn_wc_transmit_prop_deltas(const char *path,
> - svn_wc_adm_access_t *adm_access,
> - const svn_wc_entry_t *entry,
> - const svn_delta_editor_t *editor,
> - void *baton,
> - const char **tempfile,
> - apr_pool_t *pool)
> +svn_wc_internal_transmit_prop_deltas(svn_wc__db_t *db,
> + const char *local_path,
> + const svn_delta_editor_t
> *editor,
> + void *baton,
> + const char **tempfile,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool)
See above comments about parameter name and removal.
> {
> int i;
> apr_array_header_t *propmods;
> - svn_wc__db_t *db = svn_wc__adm_get_db(adm_access);
> - const char *local_abspath;
> + svn_wc__db_kind_t kind;
>
> - SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
> -
> if (tempfile)
> *tempfile = NULL;
Remove.
>
> + SVN_ERR(svn_wc__db_check_node(&kind, db, local_path,
> scratch_pool));
> /* Get an array of local changes by comparing the hashes. */
> - SVN_ERR(svn_wc__internal_propdiff(&propmods, NULL, db,
> local_abspath,
> - pool, pool));
> + SVN_ERR(svn_wc__internal_propdiff(&propmods, NULL, db, local_path,
> + result_pool, scratch_pool));
>
> /* Apply each local change to the baton */
> for (i = 0; i < propmods->nelts; i++)
> {
> const svn_prop_t *p = &APR_ARRAY_IDX(propmods, i, svn_prop_t);
> - if (entry->kind == svn_node_file)
> - SVN_ERR(editor->change_file_prop(baton, p->name, p->value,
> pool));
> + if (kind == svn_wc__db_kind_file)
> + SVN_ERR(editor->change_file_prop(baton, p->name, p->value,
> + result_pool));
> else
> - SVN_ERR(editor->change_dir_prop(baton, p->name, p->value,
> pool));
> + SVN_ERR(editor->change_dir_prop(baton, p->name, p->value,
> + result_pool));
> }
>
> return SVN_NO_ERROR;
> }
> +
> +svn_error_t *
> +svn_wc_transmit_prop_deltas2(svn_wc_context_t *wc_ctx,
> + const char *local_path,
> + const svn_delta_editor_t *editor,
> + void *baton,
> + const char **tempfile,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool)
Apply the same parameter name and removal changes to the public
version, too.
> +{
> + return svn_wc_internal_transmit_prop_deltas(wc_ctx->db,
> local_path, editor,
> + baton, tempfile,
> result_pool,
> + scratch_pool);
> +}
> Index: subversion/libsvn_wc/deprecated.c
> ===================================================================
> --- subversion/libsvn_wc/deprecated.c (revision 38546)
> +++ subversion/libsvn_wc/deprecated.c (working copy)
> @@ -315,7 +315,29 @@
> fulltext, editor, file_baton,
> pool);
> }
>
> +svn_error_t *
> +svn_wc_transmit_prop_deltas(const char *path,
> + svn_wc_adm_access_t *adm_access,
> + const svn_wc_entry_t *entry,
> + const svn_delta_editor_t *editor,
> + void *baton,
> + const char **tempfile,
> + 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));
> +
When you remove the tempfile param from
svn_wc_transmit_prop_deltas2(), you'll need to do the if (tempfile)
*tempfile = NULL; statement here.
> + SVN_ERR(svn_wc_transmit_prop_deltas2(wc_ctx, local_abspath,
> editor, baton,
> + tempfile, pool, pool));
> +
> + return svn_error_return(svn_wc_context_destroy(wc_ctx));
> +}
> +
> /*** From adm_files.c ***/
> svn_error_t *
> svn_wc_ensure_adm2(const char *path,
> Index: subversion/libsvn_wc/wc.h
> ===================================================================
> --- subversion/libsvn_wc/wc.h (revision 38546)
> +++ subversion/libsvn_wc/wc.h (working copy)
> @@ -458,7 +458,17 @@
> apr_pool_t *result_pool,
> apr_pool_t *scratch_pool);
>
> +/* Internal version of svn_wc_transmit_prop_deltas2(). */
> +svn_error_t *
> +svn_wc_internal_transmit_prop_deltas(svn_wc__db_t *db,
> + const char *local_path,
> + const svn_delta_editor_t
> *editor,
> + void *baton,
> + const char **tempfile,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool);
>
> +
> #ifdef __cplusplus
> }
> #endif /* __cplusplus */
>
> Thank you!
> Huihuang
Thank you! One more round, and I think you've got it (which will help
me in some of the work I've been doing in libsvn_wc/diff.c.)
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2379847
Received on 2009-08-04 06:20:33 CEST