Philip: can you review my approach here?
On Tue, May 17, 2011 at 10:59, <gst...@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" > > >
