You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ph...@apache.org on 2010/01/27 17:54:16 UTC
svn commit: r903735 - in /subversion/trunk/subversion: libsvn_client/copy.c
libsvn_wc/wc_db.c
Author: philip
Date: Wed Jan 27 16:54:16 2010
New Revision: 903735
URL: http://svn.apache.org/viewvc?rev=903735&view=rev
Log:
Remove access batons from wc-to-wc copies.
* subversion/libsvn_wc/wc_db.c
(kind_map): Add "none".
* subversion/libsvn_client/copy.c
(do_wc_to_wc_copies): Split and call svn_wc__call_with_write_lock.
(do_wc_to_wc_copies_with_write_lock): New.
Modified:
subversion/trunk/subversion/libsvn_client/copy.c
subversion/trunk/subversion/libsvn_wc/wc_db.c
Modified: subversion/trunk/subversion/libsvn_client/copy.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/copy.c?rev=903735&r1=903734&r2=903735&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/copy.c (original)
+++ subversion/trunk/subversion/libsvn_client/copy.c Wed Jan 27 16:54:16 2010
@@ -258,61 +258,78 @@
}
-/* Copy each COPY_PAIR->SRC into COPY_PAIR->DST. Use POOL for temporary
- allocations. */
+struct do_wc_to_wc_copies_with_write_lock_baton {
+ const apr_array_header_t *copy_pairs;
+ svn_client_ctx_t *ctx;
+ const char *dst_parent;
+};
+
+/* The guts of do_wc_to_wc_copies */
static svn_error_t *
-do_wc_to_wc_copies(const apr_array_header_t *copy_pairs,
- svn_client_ctx_t *ctx,
- apr_pool_t *pool)
+do_wc_to_wc_copies_with_write_lock(void *baton,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
{
+ struct do_wc_to_wc_copies_with_write_lock_baton *b = baton;
int i;
- apr_pool_t *iterpool = svn_pool_create(pool);
- const char *dst_parent;
- svn_wc_adm_access_t *dst_access;
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
svn_error_t *err = SVN_NO_ERROR;
- get_copy_pair_ancestors(copy_pairs, NULL, &dst_parent, NULL, pool);
- if (copy_pairs->nelts == 1)
- dst_parent = svn_dirent_dirname(dst_parent, pool);
-
- /* Because all copies are to the same destination directory, we can open
- the directory once, and use it for each copy. */
- /* ### If we didn't potentially use DST_ACCESS as the SRC_ACCESS, we
- ### could use a read lock here. */
- SVN_ERR(svn_wc__adm_open_in_context(&dst_access, ctx->wc_ctx, dst_parent,
- TRUE, -1, ctx->cancel_func, ctx->cancel_baton,
- pool));
-
- for (i = 0; i < copy_pairs->nelts; i++)
+ for (i = 0; i < b->copy_pairs->nelts; i++)
{
const char *dst_parent_abspath;
const char *dst_abspath;
- svn_client__copy_pair_t *pair = APR_ARRAY_IDX(copy_pairs, i,
+ svn_client__copy_pair_t *pair = APR_ARRAY_IDX(b->copy_pairs, i,
svn_client__copy_pair_t *);
svn_pool_clear(iterpool);
/* Check for cancellation */
- if (ctx->cancel_func)
- SVN_ERR(ctx->cancel_func(ctx->cancel_baton));
+ if (b->ctx->cancel_func)
+ SVN_ERR(b->ctx->cancel_func(b->ctx->cancel_baton));
/* Perform the copy */
- SVN_ERR(svn_dirent_get_absolute(&pair->src_abs, pair->src, pool));
+ SVN_ERR(svn_dirent_get_absolute(&pair->src_abs, pair->src, scratch_pool));
SVN_ERR(svn_dirent_get_absolute(&dst_parent_abspath, pair->dst_parent,
- pool));
+ scratch_pool));
dst_abspath = svn_dirent_join(dst_parent_abspath, pair->base_name,
iterpool);
- err = svn_wc_copy3(ctx->wc_ctx, pair->src_abs, dst_abspath,
- ctx->cancel_func, ctx->cancel_baton,
- ctx->notify_func2, ctx->notify_baton2, iterpool);
+ err = svn_wc_copy3(b->ctx->wc_ctx, pair->src_abs, dst_abspath,
+ b->ctx->cancel_func, b->ctx->cancel_baton,
+ b->ctx->notify_func2, b->ctx->notify_baton2, iterpool);
if (err)
break;
}
svn_pool_destroy(iterpool);
- svn_io_sleep_for_timestamps(dst_parent, pool);
+ svn_io_sleep_for_timestamps(b->dst_parent, scratch_pool);
SVN_ERR(err);
+ return SVN_NO_ERROR;
+}
+
+/* Copy each COPY_PAIR->SRC into COPY_PAIR->DST. Use POOL for temporary
+ allocations. */
+static svn_error_t *
+do_wc_to_wc_copies(const apr_array_header_t *copy_pairs,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ const char *dst_parent, *dst_parent_abspath;
+ struct do_wc_to_wc_copies_with_write_lock_baton baton;
+
+ get_copy_pair_ancestors(copy_pairs, NULL, &dst_parent, NULL, pool);
+ if (copy_pairs->nelts == 1)
+ dst_parent = svn_dirent_dirname(dst_parent, pool);
+
+ SVN_ERR(svn_dirent_get_absolute(&dst_parent_abspath, dst_parent, pool));
- return svn_error_return(svn_wc_adm_close2(dst_access, pool));
+ baton.copy_pairs = copy_pairs;
+ baton.ctx = ctx;
+ baton.dst_parent = dst_parent;
+ SVN_ERR(svn_wc__call_with_write_lock(do_wc_to_wc_copies_with_write_lock,
+ &baton, ctx->wc_ctx, dst_parent_abspath,
+ pool, pool));
+
+ return SVN_NO_ERROR;
}
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=903735&r1=903734&r2=903735&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed Jan 27 16:54:16 2010
@@ -261,6 +261,7 @@
{ "symlink", svn_wc__db_kind_symlink },
{ "subdir", svn_wc__db_kind_subdir },
{ "unknown", svn_wc__db_kind_unknown },
+ { "none", svn_wc__db_kind_unknown },
{ NULL }
};
Re: svn commit: r903735 - in /subversion/trunk/subversion: libsvn_client/copy.c libsvn_wc/wc_db.c
Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@qqmail.nl> writes:
>> * subversion/libsvn_wc/wc_db.c
>> (kind_map): Add "none".
>
> Why did you add a 'none' value?
>
> svn_wc__db_kind_t doesn't have a none value, and the database schema
> doesn't allow a none value?
The code allows the value "none" to get into the database. The path is
entries.c:insert_working_node calling svn_node_kind_to_word which can
return "none".
> (Most likely the bug is where you saw this value)
Copy a file:
$ svnadmin create repo
$ svn import -mm Makefile file://`pwd`/repo/foo
$ subversion/svn/svn co file://`pwd`/repo wc
$ gdb -args subversion/svn/.libs/lt-svn cp wc/foo wc/bar
(gdb) b svn_wc__entry_modify2
(gdb) r
Breakpoint 1, svn_wc__entry_modify2 (db=0xf7aec0,
(gdb) fin
Run till exit from #0 svn_wc__entry_modify2 (db=0xf7aec0,
517 err = svn_wc__entry_modify2(loggy->db,
Value returned is $1 = (svn_error_t *) 0x0
(gdb) shell echo "select * from working_node;" | sqlite3 wc/.svn/wc.db
1|bar||normal|none||||||infinity||1|foo|1|||0||0
(gdb)
The next call to svn_wc__entry_modify2 would abort if
svn_wc__db_read_info read the value "none" from the database. This
didn't happen if access batons were being used and the old entry could
be retrieved from the access baton cache, which is why the old code
didn't fall over.
> This type doesn't map to svn_node_kind_t as Greg determined can't
> add new values to this existing enum without breaking most of our
> other code. svn_wc__db_kind_t is an internal type that is directly
> mapped to the database value and this would always write the
> "unknown" string in the database. (In most cases this value would be
> invalid, except for a few cases where the repository decides not to
> tell the workingcopy the node kind)
I'll see if I can work out how to get it write "unknown" instead.
We currently enforce this constraint when reading but not writing,
that's fragile. Why does entries.c:insert_working_node allow invalid
values into the database? Is this a bug? Should it be changed to
enforce constraints like this?
--
Philip
RE: svn commit: r903735 - in /subversion/trunk/subversion: libsvn_client/copy.c libsvn_wc/wc_db.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: philip@apache.org [mailto:philip@apache.org]
> Sent: woensdag 27 januari 2010 17:54
> To: commits@subversion.apache.org
> Subject: svn commit: r903735 - in /subversion/trunk/subversion:
> libsvn_client/copy.c libsvn_wc/wc_db.c
>
> Author: philip
> Date: Wed Jan 27 16:54:16 2010
> New Revision: 903735
>
> URL: http://svn.apache.org/viewvc?rev=903735&view=rev
> Log:
> Remove access batons from wc-to-wc copies.
>
> * subversion/libsvn_wc/wc_db.c
> (kind_map): Add "none".
Why did you add a 'none' value?
svn_wc__db_kind_t doesn't have a none value, and the database schema doesn't allow a none value? (Most likely the bug is where you saw this value)
This type doesn't map to svn_node_kind_t as Greg determined can't add new values to this existing enum without breaking most of our other code. svn_wc__db_kind_t is an internal type that is directly mapped to the database value and this would always write the "unknown" string in the database. (In most cases this value would be invalid, except for a few cases where the repository decides not to tell the workingcopy the node kind)
See wc_metadata.sql around line 20:
/*
* the KIND column in these tables has one of the following values:
* "file"
* "dir"
* "symlink"
* "unknown"
* "subdir"
*
Bert
>
> * subversion/libsvn_client/copy.c
> (do_wc_to_wc_copies): Split and call svn_wc__call_with_write_lock.
> (do_wc_to_wc_copies_with_write_lock): New.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/copy.c
> subversion/trunk/subversion/libsvn_wc/wc_db.c
>
> Modified: subversion/trunk/subversion/libsvn_client/copy.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/
> copy.c?rev=903735&r1=903734&r2=903735&view=diff
> =======================================================================
> =======
> --- subversion/trunk/subversion/libsvn_client/copy.c (original)
> +++ subversion/trunk/subversion/libsvn_client/copy.c Wed Jan 27
> 16:54:16 2010
> @@ -258,61 +258,78 @@
> }
>
>
> -/* Copy each COPY_PAIR->SRC into COPY_PAIR->DST. Use POOL for
> temporary
> - allocations. */
> +struct do_wc_to_wc_copies_with_write_lock_baton {
> + const apr_array_header_t *copy_pairs;
> + svn_client_ctx_t *ctx;
> + const char *dst_parent;
> +};
> +
> +/* The guts of do_wc_to_wc_copies */
> static svn_error_t *
> -do_wc_to_wc_copies(const apr_array_header_t *copy_pairs,
> - svn_client_ctx_t *ctx,
> - apr_pool_t *pool)
> +do_wc_to_wc_copies_with_write_lock(void *baton,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool)
> {
> + struct do_wc_to_wc_copies_with_write_lock_baton *b = baton;
> int i;
> - apr_pool_t *iterpool = svn_pool_create(pool);
> - const char *dst_parent;
> - svn_wc_adm_access_t *dst_access;
> + apr_pool_t *iterpool = svn_pool_create(scratch_pool);
> svn_error_t *err = SVN_NO_ERROR;
>
> - get_copy_pair_ancestors(copy_pairs, NULL, &dst_parent, NULL, pool);
> - if (copy_pairs->nelts == 1)
> - dst_parent = svn_dirent_dirname(dst_parent, pool);
> -
> - /* Because all copies are to the same destination directory, we can
> open
> - the directory once, and use it for each copy. */
> - /* ### If we didn't potentially use DST_ACCESS as the SRC_ACCESS, we
> - ### could use a read lock here. */
> - SVN_ERR(svn_wc__adm_open_in_context(&dst_access, ctx->wc_ctx,
> dst_parent,
> - TRUE, -1, ctx->cancel_func, ctx-
> >cancel_baton,
> - pool));
> -
> - for (i = 0; i < copy_pairs->nelts; i++)
> + for (i = 0; i < b->copy_pairs->nelts; i++)
> {
> const char *dst_parent_abspath;
> const char *dst_abspath;
> - svn_client__copy_pair_t *pair = APR_ARRAY_IDX(copy_pairs, i,
> + svn_client__copy_pair_t *pair = APR_ARRAY_IDX(b->copy_pairs, i,
>
> svn_client__copy_pair_t *);
> svn_pool_clear(iterpool);
>
> /* Check for cancellation */
> - if (ctx->cancel_func)
> - SVN_ERR(ctx->cancel_func(ctx->cancel_baton));
> + if (b->ctx->cancel_func)
> + SVN_ERR(b->ctx->cancel_func(b->ctx->cancel_baton));
>
> /* Perform the copy */
> - SVN_ERR(svn_dirent_get_absolute(&pair->src_abs, pair->src,
> pool));
> + SVN_ERR(svn_dirent_get_absolute(&pair->src_abs, pair->src,
> scratch_pool));
> SVN_ERR(svn_dirent_get_absolute(&dst_parent_abspath, pair-
> >dst_parent,
> - pool));
> + scratch_pool));
> dst_abspath = svn_dirent_join(dst_parent_abspath, pair-
> >base_name,
> iterpool);
> - err = svn_wc_copy3(ctx->wc_ctx, pair->src_abs, dst_abspath,
> - ctx->cancel_func, ctx->cancel_baton,
> - ctx->notify_func2, ctx->notify_baton2,
> iterpool);
> + err = svn_wc_copy3(b->ctx->wc_ctx, pair->src_abs, dst_abspath,
> + b->ctx->cancel_func, b->ctx->cancel_baton,
> + b->ctx->notify_func2, b->ctx->notify_baton2,
> iterpool);
> if (err)
> break;
> }
> svn_pool_destroy(iterpool);
>
> - svn_io_sleep_for_timestamps(dst_parent, pool);
> + svn_io_sleep_for_timestamps(b->dst_parent, scratch_pool);
> SVN_ERR(err);
> + return SVN_NO_ERROR;
> +}
> +
> +/* Copy each COPY_PAIR->SRC into COPY_PAIR->DST. Use POOL for
> temporary
> + allocations. */
> +static svn_error_t *
> +do_wc_to_wc_copies(const apr_array_header_t *copy_pairs,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> +{
> + const char *dst_parent, *dst_parent_abspath;
> + struct do_wc_to_wc_copies_with_write_lock_baton baton;
> +
> + get_copy_pair_ancestors(copy_pairs, NULL, &dst_parent, NULL, pool);
> + if (copy_pairs->nelts == 1)
> + dst_parent = svn_dirent_dirname(dst_parent, pool);
> +
> + SVN_ERR(svn_dirent_get_absolute(&dst_parent_abspath, dst_parent,
> pool));
>
> - return svn_error_return(svn_wc_adm_close2(dst_access, pool));
> + baton.copy_pairs = copy_pairs;
> + baton.ctx = ctx;
> + baton.dst_parent = dst_parent;
> +
> SVN_ERR(svn_wc__call_with_write_lock(do_wc_to_wc_copies_with_write_lock
> ,
> + &baton, ctx->wc_ctx,
> dst_parent_abspath,
> + pool, pool));
> +
> + return SVN_NO_ERROR;
> }
>
>
>
> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_d
> b.c?rev=903735&r1=903734&r2=903735&view=diff
> =======================================================================
> =======
> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed Jan 27 16:54:16
> 2010
> @@ -261,6 +261,7 @@
> { "symlink", svn_wc__db_kind_symlink },
> { "subdir", svn_wc__db_kind_subdir },
> { "unknown", svn_wc__db_kind_unknown },
> + { "none", svn_wc__db_kind_unknown },
> { NULL }
> };
>
>