> -----Original Message-----
> From: philip_at_apache.org [mailto:philip_at_apache.org]
> Sent: donderdag 11 februari 2010 18:49
> To: commits_at_subversion.apache.org
> Subject: svn commit: r909093 - in /subversion/trunk/subversion/libsvn_wc:
> adm_ops.c workqueue.c workqueue.h
>
> Author: philip
> Date: Thu Feb 11 17:48:57 2010
> New Revision: 909093
>
> URL: http://svn.apache.org/viewvc?rev=909093&view=rev
> Log:
> Convert the loggy operations in svn_wc_delete4 into a work queue
> item. This is a precursor to removing the svn_wc__entry_modify2
> call.
Hi Philip,
Thanks for looking into this :)
Sorry for the delay.
>
> * subversion/libsvn_wc/adm_ops.c
> (svn_wc_delete4): Use work queue.
>
> * subversion/libsvn_wc/workqueue.h (svn_wc__wq_add_delete): New.
>
> * subversion/libsvn_wc/workqueue.c
> (svn_wc__wq_add_delete): New.
> (run_delete): New, derived from svn_wc_delete4.
> (dispatch_table): Add OP_DELETE.
>
> Modified:
> subversion/trunk/subversion/libsvn_wc/adm_ops.c
> subversion/trunk/subversion/libsvn_wc/workqueue.c
> subversion/trunk/subversion/libsvn_wc/workqueue.h
>
> Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm
> _ops.c?rev=909093&r1=909092&r2=909093&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Thu Feb 11 17:48:57
> 2010
> @@ -1123,7 +1123,6 @@
> svn_boolean_t was_copied = FALSE;
> svn_boolean_t was_deleted = FALSE; /* Silence a gcc uninitialized warning
> */
> svn_error_t *err;
> - const char *parent_abspath = svn_dirent_dirname(local_abspath, pool);
> svn_wc__db_status_t status;
> svn_wc__db_kind_t kind;
> svn_boolean_t base_shadowed;
> @@ -1229,63 +1228,14 @@
>
> if (kind != svn_wc__db_kind_dir || !was_add || was_deleted)
> {
> - /* We need to mark this entry for deletion in its parent's entries
> - file, so we split off base_name from the parent path, then fold in
> - the addition of a delete flag. */
> - svn_stringbuf_t *log_accum = svn_stringbuf_create("", pool);
> - svn_wc_entry_t tmp_entry;
> -
> - /* Edit the entry to reflect the now deleted state.
> - entries.c:fold_entry() clears the values of copied, copyfrom_rev
> - and copyfrom_url. */
> - tmp_entry.schedule = svn_wc_schedule_delete;
> - SVN_ERR(svn_wc__loggy_entry_modify(&log_accum,
> - parent_abspath,
> - local_abspath, &tmp_entry,
> - SVN_WC__ENTRY_MODIFY_SCHEDULE,
> - pool, pool));
> - SVN_WC__FLUSH_LOG_ACCUM(db, parent_abspath, log_accum, pool);
> + const char *parent_abspath = svn_dirent_dirname(local_abspath, pool);
>
> - /* is it a replacement with history? */
> - if (was_replace && was_copied)
> - {
> - const char *text_base, *text_revert;
> -
> - SVN_ERR(svn_wc__text_base_path(&text_base, wc_ctx->db,
> local_abspath,
> - FALSE, pool));
> -
> - SVN_ERR(svn_wc__text_revert_path(&text_revert, wc_ctx->db,
> - local_abspath, pool));
> -
> - if (kind != svn_wc__db_kind_dir) /* Dirs don't have text-bases */
> - /* Restore the original text-base */
> - SVN_ERR(svn_wc__loggy_move(&log_accum,
> - parent_abspath,
> - text_revert, text_base,
> - pool, pool));
> - SVN_WC__FLUSH_LOG_ACCUM(db, parent_abspath, log_accum,
> pool);
> -
> - SVN_ERR(svn_wc__loggy_revert_props_restore(&log_accum, wc_ctx-
> >db,
> - local_abspath,
> - parent_abspath, pool));
> - SVN_WC__FLUSH_LOG_ACCUM(db, parent_abspath, log_accum,
> pool);
> - }
> - if (was_add)
> - {
> - SVN_ERR(svn_wc__loggy_props_delete(&log_accum, wc_ctx->db,
> - local_abspath, parent_abspath,
> - svn_wc__props_base, pool));
> - SVN_WC__FLUSH_LOG_ACCUM(db, parent_abspath, log_accum,
> pool);
> -
> - SVN_ERR(svn_wc__loggy_props_delete(&log_accum, wc_ctx->db,
> - local_abspath, parent_abspath,
> - svn_wc__props_working, pool));
> - SVN_WC__FLUSH_LOG_ACCUM(db, parent_abspath, log_accum,
> pool);
> - }
> -
> - SVN_ERR(svn_wc__wq_add_loggy(db, parent_abspath, log_accum,
> pool));
> + SVN_ERR(svn_wc__wq_add_delete(wc_ctx->db, parent_abspath,
> local_abspath,
> + kind, was_add, was_copied, was_replace,
> + base_shadowed, pool));
>
> - SVN_ERR(svn_wc__run_log2(db, parent_abspath, pool));
> + SVN_ERR(svn_wc__wq_run(db, parent_abspath, cancel_func,
> cancel_baton,
> + pool));
> }
>
> /* Report the deletion to the caller. */
>
> Modified: subversion/trunk/subversion/libsvn_wc/workqueue.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wor
> kqueue.c?rev=909093&r1=909092&r2=909093&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/workqueue.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/workqueue.c Thu Feb 11
> 17:48:57 2010
> @@ -55,6 +55,7 @@
> #define OP_DELETION_POSTCOMMIT "deletion-postcommit"
> #define OP_POSTCOMMIT "postcommit"
> #define OP_INSTALL_PROPERTIES "install-properties"
> +#define OP_DELETE "delete"
>
>
> struct work_item_dispatch {
> @@ -1664,6 +1665,121 @@
> return SVN_NO_ERROR;
> }
>
> +/* ------------------------------------------------------------------------ */
> +
> +/* OP_DELETE */
> +
> +static svn_error_t *
> +run_delete(svn_wc__db_t *db,
> + const svn_skel_t *work_item,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + apr_pool_t *scratch_pool)
> +{
> + const svn_skel_t *arg = work_item->children->next;
> + const char *local_abspath;
> + svn_wc__db_kind_t kind;
> + svn_boolean_t was_added, was_copied, was_replaced, base_shadowed;
> + svn_wc_entry_t tmp_entry;
> +
> + local_abspath = apr_pstrmemdup(scratch_pool, arg->data, arg->len);
> + arg = arg->next;
> + kind = svn_skel__parse_int(arg, scratch_pool);
> + arg = arg->next;
> + was_added = svn_skel__parse_int(arg, scratch_pool);
> + arg = arg->next;
> + was_copied = svn_skel__parse_int(arg, scratch_pool);
> + arg = arg->next;
> + was_replaced = svn_skel__parse_int(arg, scratch_pool);
> + arg = arg->next;
> + base_shadowed = svn_skel__parse_int(arg, scratch_pool);
> +
> + /* Edit the entry to reflect the now deleted state.
> + entries.c:fold_entry() clears the values of copied, copyfrom_rev
> + and copyfrom_url. */
> + tmp_entry.schedule = svn_wc_schedule_delete;
> + SVN_ERR(svn_wc__entry_modify2(db, local_abspath,
> svn_node_unknown, TRUE,
> + &tmp_entry, SVN_WC__ENTRY_MODIFY_SCHEDULE,
> + scratch_pool));
This type of changes should be handled outside the WQ operation.
All workqueue operations must be able to cope with restarting multiple times.
This operation can delete the entry from the local_abspath. (One of the tiny details in fold_scheduling :( )
In that case you can get yourself a workqueue item that can never complete, because it can't find the entry.
The model for this kind of operations we defined to handle this was:
* Open SqLite Transaction
* Perform database updates
* Insert WQ operation(s)
* Commit Transaction.
* Run WQ operation.
This way you always have a consistent database.
I'm running tests on a change that moves all entry scheduling changes to svn_wc_schedule_delete to a single svn_wc__db_temp_op_delete() operation.
(The reason that it is a temp_op and not the normal op is that it isn't recursive)
It is starting to look like my helper function handles the current corner cases.
Do you have some work in progress on folding the delete states in a wc_db operation?
(I don't like to replicate your work if you already have this part working ;-)
Bert
Received on 2010-02-17 15:53:11 CET