You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gs...@apache.org on 2011/05/17 16:59:22 UTC
svn commit: r1104308 - in /subversion/trunk/subversion: libsvn_wc/wc_db.c
tests/cmdline/revert_tests.py
Author: gstein
Date: Tue May 17 14:59:21 2011
New Revision: 1104308
URL: http://svn.apache.org/viewvc?rev=1104308&view=rev
Log:
Fix issue #3859 by rearranging the order of database operations, so that
the triggers fire in a different order. This allows us to ensure that
changes to NODES has precedence over changes to ACTUAL_NODE.
* subversion/libsvn_wc/wc_db.c:
(op_revert_txn, op_revert_recursive_txn): move the ACTUAL_NODE changes
further up in the function and leave a comment about why.
* subversion/tests/cmdline/revert_tests.py:
(revert_empty_actual): remove XFail marker
Modified:
subversion/trunk/subversion/libsvn_wc/wc_db.c
subversion/trunk/subversion/tests/cmdline/revert_tests.py
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1104308&r1=1104307&r2=1104308&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Tue May 17 14:59:21 2011
@@ -5417,6 +5417,21 @@ op_revert_txn(void *baton,
op_depth = svn_sqlite__column_int64(stmt, 0);
SVN_ERR(svn_sqlite__reset(stmt));
+ /* Handle ACTUAL_NODE first. We want any changes to NODES, which fire
+ triggers to update 'revert_list', to take precedence over these
+ changes to ACTUAL_NODE. */
+ SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+ STMT_DELETE_ACTUAL_NODE_LEAVING_CHANGELIST));
+ SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
+ SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
+ if (!affected_rows)
+ {
+ SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+ STMT_CLEAR_ACTUAL_NODE_LEAVING_CHANGELIST));
+ SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
+ SVN_ERR(svn_sqlite__step_done(stmt));
+ }
+
if (op_depth > 0 && op_depth == relpath_depth(local_relpath))
{
/* Can't do non-recursive revert if children exist */
@@ -5456,18 +5471,6 @@ op_revert_txn(void *baton,
SVN_ERR(svn_sqlite__step_done(stmt));
}
- SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
- STMT_DELETE_ACTUAL_NODE_LEAVING_CHANGELIST));
- SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
- SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
- if (!affected_rows)
- {
- SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
- STMT_CLEAR_ACTUAL_NODE_LEAVING_CHANGELIST));
- SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
- SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
- }
-
return SVN_NO_ERROR;
}
@@ -5526,12 +5529,9 @@ op_revert_recursive_txn(void *baton,
if (!op_depth)
op_depth = 1; /* Don't delete BASE nodes */
- SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
- STMT_DELETE_NODES_RECURSIVE));
- SVN_ERR(svn_sqlite__bindf(stmt, "isi", wcroot->wc_id,
- local_relpath, op_depth));
- SVN_ERR(svn_sqlite__step_done(stmt));
-
+ /* Handle ACTUAL_NODE first. We want any changes to NODES, which fire
+ triggers to update 'revert_list', to take precedence over these
+ changes to ACTUAL_NODE. */
SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
STMT_DELETE_ACTUAL_NODE_LEAVING_CHANGELIST_RECURSIVE));
SVN_ERR(svn_sqlite__bindf(stmt, "iss", wcroot->wc_id,
@@ -5544,6 +5544,12 @@ op_revert_recursive_txn(void *baton,
local_relpath, like_arg));
SVN_ERR(svn_sqlite__step_done(stmt));
+ SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+ STMT_DELETE_NODES_RECURSIVE));
+ SVN_ERR(svn_sqlite__bindf(stmt, "isi", wcroot->wc_id,
+ local_relpath, op_depth));
+ SVN_ERR(svn_sqlite__step_done(stmt));
+
/* ### This removes the locks, but what about the access batons? */
SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
STMT_DELETE_WC_LOCK_ORPHAN_RECURSIVE));
Modified: subversion/trunk/subversion/tests/cmdline/revert_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/revert_tests.py?rev=1104308&r1=1104307&r2=1104308&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/revert_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/revert_tests.py Tue May 17 14:59:21 2011
@@ -1261,7 +1261,7 @@ def revert_nested_add_depth_immediates(s
expected_status.remove('A/X', 'A/X/Y')
svntest.actions.run_and_verify_status(wc_dir, expected_status)
-@XFail()
+
@Issue(3859)
def revert_empty_actual(sbox):
"revert with superfluous actual node"
Re: svn commit: r1104308 - in /subversion/trunk/subversion:
libsvn_wc/wc_db.c tests/cmdline/revert_tests.py
Posted by Greg Stein <gs...@gmail.com>.
I think that I'd like to try a SELECT/INSERT directly into
revert_list, rather than using triggers. Thoughts?
On Tue, May 17, 2011 at 11:02, Greg Stein <gs...@gmail.com> wrote:
> Philip: can you review my approach here?
>
>
> On Tue, May 17, 2011 at 10:59, <gs...@apache.org> wrote:
>> Author: gstein
>> Date: Tue May 17 14:59:21 2011
>> New Revision: 1104308
>>
>> URL: http://svn.apache.org/viewvc?rev=1104308&view=rev
>> Log:
>> Fix issue #3859 by rearranging the order of database operations, so that
>> the triggers fire in a different order. This allows us to ensure that
>> changes to NODES has precedence over changes to ACTUAL_NODE.
>>
>> * subversion/libsvn_wc/wc_db.c:
>> (op_revert_txn, op_revert_recursive_txn): move the ACTUAL_NODE changes
>> further up in the function and leave a comment about why.
>>
>> * subversion/tests/cmdline/revert_tests.py:
>> (revert_empty_actual): remove XFail marker
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_wc/wc_db.c
>> subversion/trunk/subversion/tests/cmdline/revert_tests.py
>>
>> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1104308&r1=1104307&r2=1104308&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
>> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Tue May 17 14:59:21 2011
>> @@ -5417,6 +5417,21 @@ op_revert_txn(void *baton,
>> op_depth = svn_sqlite__column_int64(stmt, 0);
>> SVN_ERR(svn_sqlite__reset(stmt));
>>
>> + /* Handle ACTUAL_NODE first. We want any changes to NODES, which fire
>> + triggers to update 'revert_list', to take precedence over these
>> + changes to ACTUAL_NODE. */
>> + SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
>> + STMT_DELETE_ACTUAL_NODE_LEAVING_CHANGELIST));
>> + SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
>> + SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
>> + if (!affected_rows)
>> + {
>> + SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
>> + STMT_CLEAR_ACTUAL_NODE_LEAVING_CHANGELIST));
>> + SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
>> + SVN_ERR(svn_sqlite__step_done(stmt));
>> + }
>> +
>> if (op_depth > 0 && op_depth == relpath_depth(local_relpath))
>> {
>> /* Can't do non-recursive revert if children exist */
>> @@ -5456,18 +5471,6 @@ op_revert_txn(void *baton,
>> SVN_ERR(svn_sqlite__step_done(stmt));
>> }
>>
>> - SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
>> - STMT_DELETE_ACTUAL_NODE_LEAVING_CHANGELIST));
>> - SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
>> - SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
>> - if (!affected_rows)
>> - {
>> - SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
>> - STMT_CLEAR_ACTUAL_NODE_LEAVING_CHANGELIST));
>> - SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
>> - SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
>> - }
>> -
>> return SVN_NO_ERROR;
>> }
>>
>> @@ -5526,12 +5529,9 @@ op_revert_recursive_txn(void *baton,
>> if (!op_depth)
>> op_depth = 1; /* Don't delete BASE nodes */
>>
>> - SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
>> - STMT_DELETE_NODES_RECURSIVE));
>> - SVN_ERR(svn_sqlite__bindf(stmt, "isi", wcroot->wc_id,
>> - local_relpath, op_depth));
>> - SVN_ERR(svn_sqlite__step_done(stmt));
>> -
>> + /* Handle ACTUAL_NODE first. We want any changes to NODES, which fire
>> + triggers to update 'revert_list', to take precedence over these
>> + changes to ACTUAL_NODE. */
>> SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
>> STMT_DELETE_ACTUAL_NODE_LEAVING_CHANGELIST_RECURSIVE));
>> SVN_ERR(svn_sqlite__bindf(stmt, "iss", wcroot->wc_id,
>> @@ -5544,6 +5544,12 @@ op_revert_recursive_txn(void *baton,
>> local_relpath, like_arg));
>> SVN_ERR(svn_sqlite__step_done(stmt));
>>
>> + SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
>> + STMT_DELETE_NODES_RECURSIVE));
>> + SVN_ERR(svn_sqlite__bindf(stmt, "isi", wcroot->wc_id,
>> + local_relpath, op_depth));
>> + SVN_ERR(svn_sqlite__step_done(stmt));
>> +
>> /* ### This removes the locks, but what about the access batons? */
>> SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
>> STMT_DELETE_WC_LOCK_ORPHAN_RECURSIVE));
>>
>> Modified: subversion/trunk/subversion/tests/cmdline/revert_tests.py
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/revert_tests.py?rev=1104308&r1=1104307&r2=1104308&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/tests/cmdline/revert_tests.py (original)
>> +++ subversion/trunk/subversion/tests/cmdline/revert_tests.py Tue May 17 14:59:21 2011
>> @@ -1261,7 +1261,7 @@ def revert_nested_add_depth_immediates(s
>> expected_status.remove('A/X', 'A/X/Y')
>> svntest.actions.run_and_verify_status(wc_dir, expected_status)
>>
>> -@XFail()
>> +
>> @Issue(3859)
>> def revert_empty_actual(sbox):
>> "revert with superfluous actual node"
>>
>>
>>
>
Re: svn commit: r1104308 - in /subversion/trunk/subversion:
libsvn_wc/wc_db.c tests/cmdline/revert_tests.py
Posted by Greg Stein <gs...@gmail.com>.
Philip: can you review my approach here?
On Tue, May 17, 2011 at 10:59, <gs...@apache.org> wrote:
> Author: gstein
> Date: Tue May 17 14:59:21 2011
> New Revision: 1104308
>
> URL: http://svn.apache.org/viewvc?rev=1104308&view=rev
> Log:
> Fix issue #3859 by rearranging the order of database operations, so that
> the triggers fire in a different order. This allows us to ensure that
> changes to NODES has precedence over changes to ACTUAL_NODE.
>
> * subversion/libsvn_wc/wc_db.c:
> (op_revert_txn, op_revert_recursive_txn): move the ACTUAL_NODE changes
> further up in the function and leave a comment about why.
>
> * subversion/tests/cmdline/revert_tests.py:
> (revert_empty_actual): remove XFail marker
>
> Modified:
> subversion/trunk/subversion/libsvn_wc/wc_db.c
> subversion/trunk/subversion/tests/cmdline/revert_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1104308&r1=1104307&r2=1104308&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Tue May 17 14:59:21 2011
> @@ -5417,6 +5417,21 @@ op_revert_txn(void *baton,
> op_depth = svn_sqlite__column_int64(stmt, 0);
> SVN_ERR(svn_sqlite__reset(stmt));
>
> + /* Handle ACTUAL_NODE first. We want any changes to NODES, which fire
> + triggers to update 'revert_list', to take precedence over these
> + changes to ACTUAL_NODE. */
> + SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> + STMT_DELETE_ACTUAL_NODE_LEAVING_CHANGELIST));
> + SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
> + SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
> + if (!affected_rows)
> + {
> + SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> + STMT_CLEAR_ACTUAL_NODE_LEAVING_CHANGELIST));
> + SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
> + SVN_ERR(svn_sqlite__step_done(stmt));
> + }
> +
> if (op_depth > 0 && op_depth == relpath_depth(local_relpath))
> {
> /* Can't do non-recursive revert if children exist */
> @@ -5456,18 +5471,6 @@ op_revert_txn(void *baton,
> SVN_ERR(svn_sqlite__step_done(stmt));
> }
>
> - SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> - STMT_DELETE_ACTUAL_NODE_LEAVING_CHANGELIST));
> - SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
> - SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
> - if (!affected_rows)
> - {
> - SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> - STMT_CLEAR_ACTUAL_NODE_LEAVING_CHANGELIST));
> - SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
> - SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
> - }
> -
> return SVN_NO_ERROR;
> }
>
> @@ -5526,12 +5529,9 @@ op_revert_recursive_txn(void *baton,
> if (!op_depth)
> op_depth = 1; /* Don't delete BASE nodes */
>
> - SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> - STMT_DELETE_NODES_RECURSIVE));
> - SVN_ERR(svn_sqlite__bindf(stmt, "isi", wcroot->wc_id,
> - local_relpath, op_depth));
> - SVN_ERR(svn_sqlite__step_done(stmt));
> -
> + /* Handle ACTUAL_NODE first. We want any changes to NODES, which fire
> + triggers to update 'revert_list', to take precedence over these
> + changes to ACTUAL_NODE. */
> SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> STMT_DELETE_ACTUAL_NODE_LEAVING_CHANGELIST_RECURSIVE));
> SVN_ERR(svn_sqlite__bindf(stmt, "iss", wcroot->wc_id,
> @@ -5544,6 +5544,12 @@ op_revert_recursive_txn(void *baton,
> local_relpath, like_arg));
> SVN_ERR(svn_sqlite__step_done(stmt));
>
> + SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> + STMT_DELETE_NODES_RECURSIVE));
> + SVN_ERR(svn_sqlite__bindf(stmt, "isi", wcroot->wc_id,
> + local_relpath, op_depth));
> + SVN_ERR(svn_sqlite__step_done(stmt));
> +
> /* ### This removes the locks, but what about the access batons? */
> SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> STMT_DELETE_WC_LOCK_ORPHAN_RECURSIVE));
>
> Modified: subversion/trunk/subversion/tests/cmdline/revert_tests.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/revert_tests.py?rev=1104308&r1=1104307&r2=1104308&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/revert_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/revert_tests.py Tue May 17 14:59:21 2011
> @@ -1261,7 +1261,7 @@ def revert_nested_add_depth_immediates(s
> expected_status.remove('A/X', 'A/X/Y')
> svntest.actions.run_and_verify_status(wc_dir, expected_status)
>
> -@XFail()
> +
> @Issue(3859)
> def revert_empty_actual(sbox):
> "revert with superfluous actual node"
>
>
>