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/02/11 18:49:14 UTC
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.
* 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/workqueue.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));
+
+ if (was_replaced && was_copied)
+ {
+ const char *props_base, *props_revert;
+ svn_error_t *err;
+
+ SVN_ERR(svn_wc__prop_path(&props_base, local_abspath, kind,
+ svn_wc__props_base, scratch_pool));
+ SVN_ERR(svn_wc__prop_path(&props_revert, local_abspath, kind,
+ svn_wc__props_revert, scratch_pool));
+ err = svn_io_file_rename(props_base, props_revert, scratch_pool);
+ if (err && !APR_STATUS_IS_ENOENT(err->apr_err))
+ return svn_error_quick_wrap(err, _("Can't move source to dest"));
+ svn_error_clear(err);
+
+ if (kind != svn_wc__db_kind_dir)
+ {
+ const char *text_base, *text_revert;
+
+ SVN_ERR(svn_wc__text_base_path(&text_base, db, local_abspath,
+ FALSE, scratch_pool));
+
+ SVN_ERR(svn_wc__text_revert_path(&text_revert, db,
+ local_abspath, scratch_pool));
+ err = svn_io_file_rename(text_revert, text_base, scratch_pool);
+ if (err && !APR_STATUS_IS_ENOENT(err->apr_err))
+ return svn_error_quick_wrap(err, _("Can't move source to dest"));
+ svn_error_clear(err);
+ }
+ }
+ if (was_added)
+ {
+ const char *props_base, *props_working;
+ svn_error_t *err;
+
+ SVN_ERR(svn_wc__prop_path(&props_base, local_abspath, kind,
+ svn_wc__props_base, scratch_pool));
+ SVN_ERR(svn_wc__prop_path(&props_working, local_abspath, kind,
+ svn_wc__props_working, scratch_pool));
+
+ err = svn_io_remove_file2(props_base, TRUE, scratch_pool);
+ if (err && !APR_STATUS_IS_ENOENT(err->apr_err))
+ return svn_error_quick_wrap(err, _("Can't move source to dest"));
+ svn_error_clear(err);
+ err = svn_io_remove_file2(props_working, TRUE, scratch_pool);
+ if (err && !APR_STATUS_IS_ENOENT(err->apr_err))
+ return svn_error_quick_wrap(err, _("Can't move source to dest"));
+ svn_error_clear(err);
+ }
+
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_wc__wq_add_delete(svn_wc__db_t *db,
+ const char *parent_abspath,
+ const char *local_abspath,
+ svn_wc__db_kind_t kind,
+ svn_boolean_t was_added,
+ svn_boolean_t was_copied,
+ svn_boolean_t was_replaced,
+ svn_boolean_t base_shadowed,
+ apr_pool_t *scratch_pool)
+{
+ svn_skel_t *work_item = svn_skel__make_empty_list(scratch_pool);
+
+ svn_skel__prepend_int(base_shadowed, work_item, scratch_pool);
+ svn_skel__prepend_int(was_replaced, work_item, scratch_pool);
+ svn_skel__prepend_int(was_copied, work_item, scratch_pool);
+ svn_skel__prepend_int(was_added, work_item, scratch_pool);
+ svn_skel__prepend_int(kind, work_item, scratch_pool);
+ svn_skel__prepend_str(local_abspath, work_item, scratch_pool);
+ svn_skel__prepend_str(OP_DELETE, work_item, scratch_pool);
+
+ SVN_ERR(svn_wc__db_wq_add(db, parent_abspath, work_item, scratch_pool));
+
+ return SVN_NO_ERROR;
+}
+
/* ------------------------------------------------------------------------ */
@@ -1675,6 +1791,7 @@
{ OP_DELETION_POSTCOMMIT, run_deletion_postcommit },
{ OP_POSTCOMMIT, run_postcommit },
{ OP_INSTALL_PROPERTIES, run_install_properties },
+ { OP_DELETE, run_delete },
/* Sentinel. */
{ NULL }
Modified: subversion/trunk/subversion/libsvn_wc/workqueue.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/workqueue.h?rev=909093&r1=909092&r2=909093&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/workqueue.h (original)
+++ subversion/trunk/subversion/libsvn_wc/workqueue.h Thu Feb 11 17:48:57 2010
@@ -108,6 +108,25 @@
svn_boolean_t force_base_install,
apr_pool_t *scratch_pool);
+/* Add a work item to delete a node.
+
+ ### LOCAL_ABSPATH is the node to be deleted and the queue exists in
+ PARENT_ABSPATH (because when LOCAL_ABSPATH is a directory it might
+ not exist on disk). This use of PARENT_ABSPATH is inherited from
+ the log file conversion but perhaps we don't need to use a work
+ queue when deleting a directory that does not exist on disk.
+ */
+svn_error_t *
+svn_wc__wq_add_delete(svn_wc__db_t *db,
+ const char *parent_abspath,
+ const char *local_abspath,
+ svn_wc__db_kind_t kind,
+ svn_boolean_t was_added,
+ svn_boolean_t was_copied,
+ svn_boolean_t was_replaced,
+ svn_boolean_t base_shadowed,
+ apr_pool_t *scratch_pool);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
Re: svn commit: r909093 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c workqueue.c workqueue.h
Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@qqmail.nl> writes:
>> One other thing that worries me is that mark_tree_deleted seems to
>> doing deletes without doing the loggy/workqueue stuff. If I delete a
>> replaced file svn_wc_delete4 sets up a workqueue item to handle the
>> text-bases. However if I delete a directory containing a replaced
>> file it handles the file with a simple call to entry_modify2--that
>> can't be right. It's not a new bug, it pre-dates wc-ng, but it means
>> that the entry_modify2 calls in mark_tree_deleted cannot simply be
>> replaced by svn_wc__db_temp_op_delete, we need to setup wq items in
>> some cases.
>
> In WC-1.0 we left old administrative data in a lot of code paths... (not
> just a few :( ) and in most cases we fix this on adding.
> (Talk about broken behavior...)
In this case it's a real bug that breaks the working copy.
svn rm wc/A/B/f
svn cp wc/g wc/A/B/f
svn rm --force wc/A # doesn't update text-base
svn revert -R wc/A
echo 123 >> wc/A/B/f
svn ci wc # fails with checksum error
> In the final version of WC-NG we don't need most of these operations any
> more.
> * Properties: In DB only. Handled directly. No loggy operation necessary
> * Pristines: Not associated with a single file. No need to remove it in the
> general case. (Maybe schedule a cleanup check as wq item?)
>
> Where the old code 'happens to work' in the intermediate state I wouldn't
> try to solve it cleanly for the period until we have the pristines working.
> (All that work will have to go to /dev/null before 1.7)
Yes, a lot of it gets easier when properties are in the db and
pristines are just a checksum. But mark_tree_deleted will still need
to update the properties and the checksum, and at present it doesn't
do it.
--
Philip
RE: svn commit: r909093 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c workqueue.c workqueue.h
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Philip Martin [mailto:philip.martin@wandisco.com]
> Sent: donderdag 18 februari 2010 13:54
> To: Bert Huijben
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r909093 - in
> /subversion/trunk/subversion/libsvn_wc: adm_ops.c workqueue.c
> workqueue.h
>
> "Bert Huijben" <be...@qqmail.nl> writes:
>
> > My current code is just a copy of the old code, moved to a new location.
No
> > fixes there. (It still uses svn_wc__entry_modify2).
> >
> > In the rest of libsvn_wc we still rewrite BASE_NODE and WORKING_NODE
> records
> > from just the limited state in svn_wc_entry_t... and that would
reintroduce
> > the issue anyway.
> >
> > If we can fix this for the case where we write from the entries that
would
> > be nice.
> >
> > The real solution is stopping to write from entries, which takes much
more
> > effort.
> > (I counted at least a dozen cases where we still change the scheduling
via
> > entries yesterday... And then there are all those other calls for other
> > changes).
> >
> > I would suggest focusing on removing those entry write operations before
> we
> > can focus on the nicest format in the database.
>
> My plan was to write a svn_wc__db_temp_op_delete that modified the
> WORKING_NODE table directly, without calling entry_modify2. I have
> such a patch but it doesn't handle all the cases where delete can be
> called. It was the complexity of this that lead me to create the file
> notes/wc-ng/transitions--that's my attempt at describing what
> svn_wc__db_temp_op_delete should do.
I made the entry based intermediate function as a first step. To find out
what that operation should do to handle the strange cases and to allow
implementing the wc-db version in incremental steps.
This allows handling the easy cases the new way before switching the hard
cases. (Small steps towards the final solution)
> One other thing that worries me is that mark_tree_deleted seems to
> doing deletes without doing the loggy/workqueue stuff. If I delete a
> replaced file svn_wc_delete4 sets up a workqueue item to handle the
> text-bases. However if I delete a directory containing a replaced
> file it handles the file with a simple call to entry_modify2--that
> can't be right. It's not a new bug, it pre-dates wc-ng, but it means
> that the entry_modify2 calls in mark_tree_deleted cannot simply be
> replaced by svn_wc__db_temp_op_delete, we need to setup wq items in
> some cases.
In WC-1.0 we left old administrative data in a lot of code paths... (not
just a few :( ) and in most cases we fix this on adding.
(Talk about broken behavior...)
In the final version of WC-NG we don't need most of these operations any
more.
* Properties: In DB only. Handled directly. No loggy operation necessary
* Pristines: Not associated with a single file. No need to remove it in the
general case. (Maybe schedule a cleanup check as wq item?)
Where the old code 'happens to work' in the intermediate state I wouldn't
try to solve it cleanly for the period until we have the pristines working.
(All that work will have to go to /dev/null before 1.7)
Bert
Re: svn commit: r909093 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c workqueue.c workqueue.h
Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@qqmail.nl> writes:
> My current code is just a copy of the old code, moved to a new location. No
> fixes there. (It still uses svn_wc__entry_modify2).
>
> In the rest of libsvn_wc we still rewrite BASE_NODE and WORKING_NODE records
> from just the limited state in svn_wc_entry_t... and that would reintroduce
> the issue anyway.
>
> If we can fix this for the case where we write from the entries that would
> be nice.
>
> The real solution is stopping to write from entries, which takes much more
> effort.
> (I counted at least a dozen cases where we still change the scheduling via
> entries yesterday... And then there are all those other calls for other
> changes).
>
> I would suggest focusing on removing those entry write operations before we
> can focus on the nicest format in the database.
My plan was to write a svn_wc__db_temp_op_delete that modified the
WORKING_NODE table directly, without calling entry_modify2. I have
such a patch but it doesn't handle all the cases where delete can be
called. It was the complexity of this that lead me to create the file
notes/wc-ng/transitions--that's my attempt at describing what
svn_wc__db_temp_op_delete should do.
One other thing that worries me is that mark_tree_deleted seems to
doing deletes without doing the loggy/workqueue stuff. If I delete a
replaced file svn_wc_delete4 sets up a workqueue item to handle the
text-bases. However if I delete a directory containing a replaced
file it handles the file with a simple call to entry_modify2--that
can't be right. It's not a new bug, it pre-dates wc-ng, but it means
that the entry_modify2 calls in mark_tree_deleted cannot simply be
replaced by svn_wc__db_temp_op_delete, we need to setup wq items in
some cases.
--
Philip
RE: svn commit: r909093 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c workqueue.c workqueue.h
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Philip Martin [mailto:philip.martin@wandisco.com]
> Sent: donderdag 18 februari 2010 11:07
> To: Bert Huijben
> Cc: dev@subversion.apache.org; philip@apache.org
> Subject: Re: svn commit: r909093 - in
> /subversion/trunk/subversion/libsvn_wc: adm_ops.c workqueue.c
> workqueue.h
>
> "Bert Huijben" <be...@qqmail.nl> writes:
>
> > 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.
>
> In my defence I think I just moved that bug from the previous loggy
> code :)
>
> >
> > 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.
>
> OK.
>
> >
> > 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.
>
> Issue 3584, deleting a grandchild of a copied parent creates a base
> node:
>
> svnadmin create repo
> svn import -mm Makefile file://`pwd`/repo/A/B/f
> svn co file://`pwd`/repo wc
> svn cp wc/A wc/A2
> svn rm wc/A2/B/f
>
> Your new code doesn't fix that:
My current code is just a copy of the old code, moved to a new location. No
fixes there. (It still uses svn_wc__entry_modify2).
In the rest of libsvn_wc we still rewrite BASE_NODE and WORKING_NODE records
from just the limited state in svn_wc_entry_t... and that would reintroduce
the issue anyway.
If we can fix this for the case where we write from the entries that would
be nice.
The real solution is stopping to write from entries, which takes much more
effort.
(I counted at least a dozen cases where we still change the scheduling via
entries yesterday... And then there are all those other calls for other
changes).
I would suggest focusing on removing those entry write operations before we
can focus on the nicest format in the database.
It happens to work in most cases now... (And in some cases for the wrong
reasons)
But we really need all that other work before we can switch pristines; move
to a single database; etc. etc.
(I'm hoping that removing those schedule+copied change operations will allow
a hack to switch to pristines sooner; but only time will tell if that is
going to work)
And only after all those steps we can go for 1.7. Or we have to cut WC-NG
completely out of the release.
Bert Huijben
Re: svn commit: r909093 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c workqueue.c workqueue.h
Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@qqmail.nl> writes:
> 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.
In my defence I think I just moved that bug from the previous loggy
code :)
>
> 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.
OK.
>
> 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.
Issue 3584, deleting a grandchild of a copied parent creates a base
node:
svnadmin create repo
svn import -mm Makefile file://`pwd`/repo/A/B/f
svn co file://`pwd`/repo wc
svn cp wc/A wc/A2
svn rm wc/A2/B/f
Your new code doesn't fix that:
sqlite wc/A2/B/.svn/wc.db "select *, kind from base_node"
gives
1|f|1|A2/B/f||normal|file|2|$md5 $2a1af3fc2b77590499e437b2f1cbaa48|34536|2|1266486946928742|pm|infinity||1266486947000000||||
--
Philip
RE: svn commit: r909093 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c workqueue.c workqueue.h
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: philip@apache.org [mailto:philip@apache.org]
> Sent: donderdag 11 februari 2010 18:49
> To: commits@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