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 2011/05/04 14:55:04 UTC
svn commit: r1099436 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c
update_editor.c wc_db.c wc_db.h
Author: philip
Date: Wed May 4 12:55:03 2011
New Revision: 1099436
URL: http://svn.apache.org/viewvc?rev=1099436&view=rev
Log:
Drop the delete list after calling svn_wc__db_op_delete.
* subversion/libsvn_wc/adm_ops.c
(svn_wc_delete4): Always invoke notification to drop the delete list.
* subversion/libsvn_wc/update_editor.c
(add_directory, close_file): Do NULL notification to drop delete list.
* subversion/libsvn_wc/wc_db.c
(svn_wc__db_delete_list_notify): Allow NULL notify callback, destroy
iteration pool.
* subversion/libsvn_wc/wc_db.h
(svn_wc__db_delete_list_notify): Tweak docstring.
Modified:
subversion/trunk/subversion/libsvn_wc/adm_ops.c
subversion/trunk/subversion/libsvn_wc/update_editor.c
subversion/trunk/subversion/libsvn_wc/wc_db.c
subversion/trunk/subversion/libsvn_wc/wc_db.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=1099436&r1=1099435&r2=1099436&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Wed May 4 12:55:03 2011
@@ -740,10 +740,8 @@ svn_wc_delete4(svn_wc_context_t *wc_ctx,
SVN_ERR(svn_wc__db_op_delete(db, local_abspath, pool));
- if (notify_func)
- SVN_ERR(svn_wc__db_delete_list_notify(notify_func, notify_baton,
- db, local_abspath, pool));
- /* ### else: Delete the list */
+ SVN_ERR(svn_wc__db_delete_list_notify(notify_func, notify_baton,
+ db, local_abspath, pool));
/* By the time we get here, anything that was scheduled to be added has
become unversioned */
Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=1099436&r1=1099435&r2=1099436&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Wed May 4 12:55:03 2011
@@ -2047,7 +2047,12 @@ add_directory(const char *path,
### we record a delete instead, which will allow resolving the conflict
### to theirs with 'svn revert'. */
if (db->shadowed && db->obstruction_found)
- SVN_ERR(svn_wc__db_op_delete(eb->db, db->local_abspath, pool));
+ {
+ SVN_ERR(svn_wc__db_op_delete(eb->db, db->local_abspath, pool));
+ SVN_ERR(svn_wc__db_delete_list_notify(NULL, NULL,
+ eb->db, db->local_abspath,
+ pool));
+ }
/* If this add was obstructed by dir scheduled for addition without
history let close_file() handle the notification because there
@@ -3954,7 +3959,12 @@ close_file(void *file_baton,
### we record a delete instead, which will allow resolving the conflict
### to theirs with 'svn revert'. */
if (fb->shadowed && fb->obstruction_found)
- SVN_ERR(svn_wc__db_op_delete(eb->db, fb->local_abspath, pool));
+ {
+ SVN_ERR(svn_wc__db_op_delete(eb->db, fb->local_abspath, pool));
+ SVN_ERR(svn_wc__db_delete_list_notify(NULL, NULL,
+ eb->db, fb->local_abspath,
+ scratch_pool));
+ }
/* ### ugh. deal with preserving the file external value in the database.
### there is no official API, so we do it this way. maybe we should
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1099436&r1=1099435&r2=1099436&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed May 4 12:55:03 2011
@@ -4817,35 +4817,41 @@ svn_wc__db_delete_list_notify(svn_wc_not
{
svn_wc__db_wcroot_t *wcroot;
const char *local_relpath;
- svn_sqlite__stmt_t *stmt;
- svn_boolean_t have_row;
- apr_pool_t *iterpool = svn_pool_create(scratch_pool);
SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
SVN_ERR(svn_wc__db_wcroot_parse_local_abspath(&wcroot, &local_relpath,
- db, local_abspath, scratch_pool, iterpool));
+ db, local_abspath, scratch_pool, scratch_pool));
VERIFY_USABLE_WCROOT(wcroot);
- SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
- STMT_SELECT_DELETE_LIST));
- SVN_ERR(svn_sqlite__step(&have_row, stmt));
- while (have_row)
+ if (notify_func)
{
- const char *notify_relpath = svn_sqlite__column_text(stmt, 0, NULL);
- const char *notify_abspath = svn_dirent_join(wcroot->abspath,
- notify_relpath,
- iterpool);
- notify_func(notify_baton,
- svn_wc_create_notify(notify_abspath,
- svn_wc_notify_delete,
- iterpool),
- iterpool);
+ svn_sqlite__stmt_t *stmt;
+ svn_boolean_t have_row;
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+ SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+ STMT_SELECT_DELETE_LIST));
SVN_ERR(svn_sqlite__step(&have_row, stmt));
- }
+ while (have_row)
+ {
+ const char *notify_relpath = svn_sqlite__column_text(stmt, 0, NULL);
+ const char *notify_abspath = svn_dirent_join(wcroot->abspath,
+ notify_relpath,
+ iterpool);
+ notify_func(notify_baton,
+ svn_wc_create_notify(notify_abspath,
+ svn_wc_notify_delete,
+ iterpool),
+ iterpool);
- SVN_ERR(svn_sqlite__reset(stmt));
+ SVN_ERR(svn_sqlite__step(&have_row, stmt));
+ }
+
+ SVN_ERR(svn_sqlite__reset(stmt));
+
+ svn_pool_destroy(iterpool);
+ }
SVN_ERR(svn_sqlite__exec_statements(wcroot->sdb, STMT_DROP_DELETE_LIST));
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=1099436&r1=1099435&r2=1099436&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Wed May 4 12:55:03 2011
@@ -1215,7 +1215,10 @@ svn_wc__db_op_delete(svn_wc__db_t *db,
/* Make delete notifications for all paths in the delete list created
* by deleting LOCAL_ABSPATH.
*
- * This function drops the delete list.
+ * This function drops the delete list. NOTIFY_FUNC may be NULL in
+ * which case the table is dropped without any notification.
+ *
+ * ### Perhaps this should be part of svn_wc__db_op_delete?
*/
svn_error_t *
svn_wc__db_delete_list_notify(svn_wc_notify_func2_t notify_func,
Re: svn commit: r1099436 - in /subversion/trunk/subversion/libsvn_wc:
adm_ops.c update_editor.c wc_db.c wc_db.h
Posted by Greg Stein <gs...@gmail.com>.
On Wed, May 4, 2011 at 12:34, Hyrum K Wright <hy...@hyrumwright.org> wrote:
> On Wed, May 4, 2011 at 11:31 AM, Greg Stein <gs...@gmail.com> wrote:
>> On Wed, May 4, 2011 at 08:55, <ph...@apache.org> wrote:
>>>...
>>> +++ subversion/trunk/subversion/libsvn_wc/wc_db.h Wed May 4 12:55:03 2011
>>> @@ -1215,7 +1215,10 @@ svn_wc__db_op_delete(svn_wc__db_t *db,
>>> /* Make delete notifications for all paths in the delete list created
>>> * by deleting LOCAL_ABSPATH.
>>> *
>>> - * This function drops the delete list.
>>> + * This function drops the delete list. NOTIFY_FUNC may be NULL in
>>> + * which case the table is dropped without any notification.
>>> + *
>>> + * ### Perhaps this should be part of svn_wc__db_op_delete?
>>
>> I say "yes". If the actual deletion produces an error, then we still
>> want the table dropped before returning that error. Tho... it may be
>> that the sqlite transaction rollback takes the table with it(?). ...
>> in any case, these two functions need to be called as a pair, and that
>> is a bad pattern to enforce on the caller.
>
> I'll also note that we have a few other places where we do this, too.
> My spidey sense has been telling me we need to make the action and
> notification bits a single wc_db function call, but I haven't yet
> acted on it.
Likewise. These separate notification functions have been bugging me,
too. It introduces a coupling between wc_db and its callers -- those
callers need to do "something more" when they call into wc_db. If they
don't... then things start to go wrong.
I'll look into fixing these.
Cheers,
-g
Re: svn commit: r1099436 - in /subversion/trunk/subversion/libsvn_wc:
adm_ops.c update_editor.c wc_db.c wc_db.h
Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Wed, May 4, 2011 at 11:31 AM, Greg Stein <gs...@gmail.com> wrote:
> On Wed, May 4, 2011 at 08:55, <ph...@apache.org> wrote:
>>...
>> +++ subversion/trunk/subversion/libsvn_wc/wc_db.h Wed May 4 12:55:03 2011
>> @@ -1215,7 +1215,10 @@ svn_wc__db_op_delete(svn_wc__db_t *db,
>> /* Make delete notifications for all paths in the delete list created
>> * by deleting LOCAL_ABSPATH.
>> *
>> - * This function drops the delete list.
>> + * This function drops the delete list. NOTIFY_FUNC may be NULL in
>> + * which case the table is dropped without any notification.
>> + *
>> + * ### Perhaps this should be part of svn_wc__db_op_delete?
>
> I say "yes". If the actual deletion produces an error, then we still
> want the table dropped before returning that error. Tho... it may be
> that the sqlite transaction rollback takes the table with it(?). ...
> in any case, these two functions need to be called as a pair, and that
> is a bad pattern to enforce on the caller.
I'll also note that we have a few other places where we do this, too.
My spidey sense has been telling me we need to make the action and
notification bits a single wc_db function call, but I haven't yet
acted on it.
-Hyrum
Re: svn commit: r1099436 - in /subversion/trunk/subversion/libsvn_wc:
adm_ops.c update_editor.c wc_db.c wc_db.h
Posted by Greg Stein <gs...@gmail.com>.
On Wed, May 4, 2011 at 08:55, <ph...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.h Wed May 4 12:55:03 2011
> @@ -1215,7 +1215,10 @@ svn_wc__db_op_delete(svn_wc__db_t *db,
> /* Make delete notifications for all paths in the delete list created
> * by deleting LOCAL_ABSPATH.
> *
> - * This function drops the delete list.
> + * This function drops the delete list. NOTIFY_FUNC may be NULL in
> + * which case the table is dropped without any notification.
> + *
> + * ### Perhaps this should be part of svn_wc__db_op_delete?
I say "yes". If the actual deletion produces an error, then we still
want the table dropped before returning that error. Tho... it may be
that the sqlite transaction rollback takes the table with it(?). ...
in any case, these two functions need to be called as a pair, and that
is a bad pattern to enforce on the caller.
Cheers,
-g