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"


Reply via email to