[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH]svn_wc_transmit_prop_deltas2-wc-ng

From: Hyrum K. Wright <hyrum_at_hyrumwright.org>
Date: Mon, 3 Aug 2009 11:13:22 -0500

On Jul 31, 2009, at 3:37 AM, HuiHuang wrote:

> Hey Stefan,
>
> I think you are very busy these days because subversion heads up:
> 1.6.4 on Aug. 4.
> When I try to design commit based on wc-ng I find it is a little
> hard for me. I do not
> konw where to start when dig deep into the code. So I think it is
> better to move
> forward little by little.
>
> I make the following patch to try to use wc_db and svn_wc_context_t.
> I know the
> docstring may be not good. I want to know if I do the right thing?
> If it is right, I will
> write a new patch instead.
>
> log message:
>
> [[[
> Add a new function svn_wc_transmit_prop_deltas2. It uses
> svn_wc_context_t
> instead of svn_wc_adm_access_t. And replace
> svn_wc_transmit_prop_deltas by
> it in some reference.
>
> * subversion/include/svn_wc.h
> (svn_wc_transmit_prop_deltas2): New API.
> * subversion/libsvn_client/commit_util.c
> (adm_access): Remove.
> (tmp_entry): Remove.
> (svn_wc_transmit_prop_deltas): Use
> svn_wc_transmit_prop_deltas2 instead.
> (cb_baton.adm_access): Remove.
> * subversion/libsvn_wc/adm_crawler.c
> (svn_wc_transmit_prop_deltas2): New function.
> ]]]
>
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 38507)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -5952,7 +5952,18 @@
> const char **tempfile,
> apr_pool_t *pool);
>
> +/** This function is the same as svn_wc_transmit_prop_deltas except
> that it use
> + * svn_wc_context_t instead of svn_wc_adm_access_t.
> + */

New functions should have the complete description, while older ones
are described in terms of the newer functions. (See other examples in
svn_wc.h.)

> +svn_error_t *
> +svn_wc_transmit_prop_deltas2(const char *path,
> + svn_wc_context_t *wc_ctx,
> + const svn_delta_editor_t *editor,
> + void *baton,
> + const char **tempfile,
> + 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

You need to note that the old API is deprecated, using @deprecated and
SVN_DEPRECATED.

> Index: subversion/libsvn_client/commit_util.c
> ===================================================================
> --- subversion/libsvn_client/commit_util.c (revision 38507)
> +++ subversion/libsvn_client/commit_util.c (working copy)
> @@ -1274,7 +1274,6 @@
> void *file_baton = NULL;
> const char *copyfrom_url = NULL;
> apr_pool_t *file_pool = NULL;
> - svn_wc_adm_access_t *adm_access = cb_baton->adm_access;
> const svn_delta_editor_t *editor = cb_baton->editor;
> apr_hash_t *file_mods = cb_baton->file_mods;
> svn_client_ctx_t *ctx = cb_baton->ctx;
> @@ -1435,8 +1434,6 @@
> /* Now handle property mods. */
> if (item->state_flags & SVN_CLIENT_COMMIT_ITEM_PROP_MODS)
> {
> - const svn_wc_entry_t *tmp_entry;
> -
> if (kind == svn_node_file)
> {
> if (! file_baton)
> @@ -1469,25 +1466,19 @@
> }
> }
>
> - /* Ensured by harvest_committables(), item->path will never
> be an
> - excluded path. However, will it be deleted/absent items?
> I think
> - committing an modification on a deleted/absent item does
> not make
> - sense. So it's probably safe to turn off the show_hidden
> flag here.*/
> - SVN_ERR(svn_wc_entry(&tmp_entry, item->path, adm_access,
> FALSE, pool));
> -
> /* When committing a directory that no longer exists in the
> repository, a "not found" error does not occur immediately
> upon opening the directory. It appears here during the delta
> transmisssion. */
> - err = svn_wc_transmit_prop_deltas
> - (item->path, adm_access, tmp_entry, editor,
> + err = svn_wc_transmit_prop_deltas2
> + (item->path, ctx->wc_ctx, editor,
> (kind == svn_node_dir) ? *dir_baton : file_baton, NULL,
> pool);

I wouldn't change any callers initially, just have them exercise the
old API which should be reimplemented as a wrapper around the new API.

Also, no-space-before-paren :)

>
> if (err)
> return fixup_out_of_date_error(path, kind, err);
>
> - SVN_ERR(svn_wc_transmit_prop_deltas
> - (item->path, adm_access, tmp_entry, editor,
> + SVN_ERR(svn_wc_transmit_prop_deltas2
> + (item->path, ctx->wc_ctx, editor,
> (kind == svn_node_dir) ? *dir_baton : file_baton,
> NULL, pool));

Same.

> /* Make any additional client -> repository prop changes. */
> @@ -1610,7 +1601,6 @@
> }
>
> /* Setup the callback baton. */
> - cb_baton.adm_access = adm_access;
> cb_baton.editor = editor;
> cb_baton.edit_baton = edit_baton;
> cb_baton.file_mods = file_mods;
> Index: subversion/libsvn_wc/adm_crawler.c
> ===================================================================
> --- subversion/libsvn_wc/adm_crawler.c (revision 38507)
> +++ subversion/libsvn_wc/adm_crawler.c (working copy)
> @@ -1060,7 +1060,43 @@
> fulltext, editor, file_baton,
> pool);
> }
>
> +svn_error_t *
> +svn_wc_transmit_prop_deltas2(const char *path,

This should be an absolute path, and documented as such in the header.

> + svn_wc_context_t *wc_ctx,

We've been standardizing the parameter order as wc_ctx, path not path,
wc_ctx.

> + const svn_delta_editor_t *editor,
> + void *baton,
> + const char **tempfile,
> + apr_pool_t *pool)
> +{
> + int i;
> + apr_array_header_t *propmods;
> + svn_wc__db_t *db = wc_ctx->db;

Unneeded temp variable.

> + const char *local_abspath;

Unneeded, since the caller should provide an abspath.

> + svn_wc__db_kind_t *kind = NULL;

This shouldn't be a pointer; the check_node() API below will segfault
if it is.

> + SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));

Don't need this, since the caller should provide an abspath.

> + SVN_ERR(svn_wc__db_check_node(kind, db, local_abspath, pool));
> +
> + if (tempfile)
> + *tempfile = NULL;
> +
> + /* Get an array of local changes by comparing the hashes. */
> + SVN_ERR(svn_wc__internal_propdiff(&propmods, NULL, db,
> local_abspath,
> + pool, 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 (*kind == svn_wc__db_kind_file)
> + SVN_ERR(editor->change_file_prop(baton, p->name, p->value,
> pool));
> + else
> + SVN_ERR(editor->change_dir_prop(baton, p->name, p->value,
> pool));
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
> svn_error_t *
> svn_wc_transmit_prop_deltas(const char *path,
> svn_wc_adm_access_t *adm_access,

This function should be reimplemented as wrapper around the new one,
and moved to deprecated.c

>
> Thank you.
> Huihuang

That's all the stuff I caught on the initial pass, feel free to
followup with questions.

-Hyrum

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2379585
Received on 2009-08-03 18:13:45 CEST

This is an archived mail posted to the Subversion Dev mailing list.