You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2010/08/31 13:13:09 UTC

svn commit: r991158 - in /subversion/trunk/subversion: include/svn_error_codes.h libsvn_wc/adm_ops.c svn/revert-cmd.c tests/cmdline/revert_tests.py

Author: rhuijben
Date: Tue Aug 31 11:13:08 2010
New Revision: 991158

URL: http://svn.apache.org/viewvc?rev=991158&view=rev
Log:
Fix revert_tests.py 21 by making svn_wc_revert4 check if a revert is
possible with only impact at the specified depth.

Before WC-NG libsvn_client didn't take a recursive lock if the specified
depth was less than infinity and some revert operations then failed on
not having a lock. But this didn't fix that you could do a depth empty
revert on a directory which then removes an added (child-of-copy) file.

As far as I can see there is no sane way to keep the original behavior or
reimplement the old behavior inside one of the deprecated wrappers.

svn_wc_revert3 receives an access baton and a depth and failed the revert
if the lock in the access baton wasn't deep enough. (Not on a depth value).

svn_client_revert2 doesn't document it's error behavior, so I think it is
safe to return a different error here, but this might need a later update
to wrap the error with SVN_ERR_WC_NOT_LOCKED to fix clients that handle
this error like we did in svn before this patch.

See also
http://svn.haxx.se/dev/archive-2010-08/0763.shtml

* subversion/include/svn_error_codes.h
  (SVN_ERR_WC_INVALID_OPERATION_DEPTH): New error code.

* subversion/libsvn_wc/adm_ops.c
  (verify_revert_depth): New function.
  (revert_internal): Add depth verification step.

* subversion/svn/revert-cmd.c
  (svn_cl__revert): Update error code check.

* subversion/tests/cmdline/revert_tests.py
  (test_list): Remove XFail marking from revert_added_tree.

Modified:
    subversion/trunk/subversion/include/svn_error_codes.h
    subversion/trunk/subversion/libsvn_wc/adm_ops.c
    subversion/trunk/subversion/svn/revert-cmd.c
    subversion/trunk/subversion/tests/cmdline/revert_tests.py

Modified: subversion/trunk/subversion/include/svn_error_codes.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_error_codes.h?rev=991158&r1=991157&r2=991158&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_error_codes.h (original)
+++ subversion/trunk/subversion/include/svn_error_codes.h Tue Aug 31 11:13:08 2010
@@ -512,6 +512,11 @@ SVN_ERROR_START
              SVN_ERR_WC_CATEGORY_START + 37,
              "Previous operation was interrupted; run 'svn cleanup'")
 
+  /** @since New in 1.7. */
+  SVN_ERRDEF(SVN_ERR_WC_INVALID_OPERATION_DEPTH,
+             SVN_ERR_WC_CATEGORY_START + 38,
+             "This operation can not be performed with just this depth.")
+
   /* fs errors */
 
   SVN_ERRDEF(SVN_ERR_FS_GENERAL,

Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=991158&r1=991157&r2=991158&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue Aug 31 11:13:08 2010
@@ -1615,6 +1615,69 @@ revert_entry(svn_depth_t *depth,
   return SVN_NO_ERROR;
 }
 
+/* Verifies if an add (or copy) to LOCAL_ABSPATH can be reverted with depth
+ * DEPTH, without touching nodes that are filtered by DEPTH.
+ *
+ * Use ROOT_ABSPATH for generating error messages.
+ */
+static svn_error_t *
+verify_revert_depth(svn_wc__db_t *db,
+                    const char *local_abspath,
+                    svn_depth_t depth,
+                    const char *root_abspath,
+                    apr_pool_t *scratch_pool)
+{
+  apr_array_header_t *children;
+  apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+  int i;
+
+  SVN_ERR_ASSERT(depth >= svn_depth_empty && depth < svn_depth_infinity);
+
+  SVN_ERR(svn_wc__db_read_children(&children, db, local_abspath,
+                                   scratch_pool, iterpool));
+
+  for (i = 0; i < children->nelts; i++)
+    {
+      const char *name = APR_ARRAY_IDX(children, i, const char *);
+      const char *child_abspath;
+      svn_wc__db_status_t status;
+      svn_wc__db_kind_t kind;
+
+      svn_pool_clear(iterpool);
+
+      child_abspath = svn_dirent_join(local_abspath, name, iterpool);
+
+      SVN_ERR(svn_wc__db_read_info(&status, &kind, NULL, NULL, NULL, NULL,
+                                   NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+                                   NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+                                   NULL, NULL, NULL, NULL,
+                                   db, child_abspath, iterpool, iterpool));
+
+      if (status == svn_wc__db_status_not_present
+          || status == svn_wc__db_status_absent
+          || status == svn_wc__db_status_excluded)
+        continue;
+
+      if (depth == svn_depth_empty
+          || (depth == svn_depth_files && kind == svn_wc__db_kind_dir))
+        {
+          return svn_error_createf(
+                        SVN_ERR_WC_INVALID_OPERATION_DEPTH, NULL,
+                        _("Can't revert '%s' with this depth, as that requires"
+                          " reverting '%s'."),
+                        svn_dirent_local_style(root_abspath, iterpool),
+                        svn_dirent_local_style(child_abspath, iterpool));
+        }
+
+      if (kind == svn_wc__db_kind_dir)
+        SVN_ERR(verify_revert_depth(db, child_abspath, svn_depth_empty,
+                                    root_abspath, iterpool));
+    }
+
+  svn_pool_destroy(iterpool);
+
+  return SVN_NO_ERROR;
+}
 
 /* This is just the guts of svn_wc_revert4() save that it accepts a
    hash CHANGELIST_HASH whose keys are changelist names instead of an
@@ -1727,6 +1790,24 @@ revert_internal(svn_wc__db_t *db,
        _("Cannot revert '%s': unsupported node kind in working copy"),
        svn_dirent_local_style(local_abspath, pool));
 
+  /* Safeguard 4:  Make sure we don't revert deeper then asked */
+  if (status == svn_wc__db_status_added
+      && db_kind == svn_wc__db_kind_dir
+      && depth >= svn_depth_empty
+      && depth < svn_depth_infinity)
+    {
+      const char *op_root_abspath;
+      SVN_ERR(svn_wc__db_scan_addition(NULL, &op_root_abspath, NULL, NULL,
+                                       NULL, NULL, NULL, NULL, NULL,
+                                       db, local_abspath, pool, pool));
+
+      /* If this node is an operation root for a copy/add, then reverting
+         it will change its descendants, if it has any. */
+      if (strcmp(local_abspath, op_root_abspath) == 0)
+        SVN_ERR(verify_revert_depth(db, local_abspath, depth,
+                                    local_abspath, pool));
+    }
+
   /* If the entry passes changelist filtering, revert it!  */
   if (svn_wc__internal_changelist_match(db, local_abspath, changelist_hash,
                                         pool))

Modified: subversion/trunk/subversion/svn/revert-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/revert-cmd.c?rev=991158&r1=991157&r2=991158&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/revert-cmd.c (original)
+++ subversion/trunk/subversion/svn/revert-cmd.c Tue Aug 31 11:13:08 2010
@@ -67,7 +67,7 @@ svn_cl__revert(apr_getopt_t *os,
                            opt_state->changelists, ctx, scratch_pool);
 
   if (err
-      && (err->apr_err == SVN_ERR_WC_NOT_LOCKED)
+      && (err->apr_err == SVN_ERR_WC_INVALID_OPERATION_DEPTH)
       && (! SVN_DEPTH_IS_RECURSIVE(opt_state->depth)))
     {
       err = svn_error_quick_wrap

Modified: subversion/trunk/subversion/tests/cmdline/revert_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/revert_tests.py?rev=991158&r1=991157&r2=991158&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/revert_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/revert_tests.py Tue Aug 31 11:13:08 2010
@@ -1059,7 +1059,7 @@ test_list = [ None,
               revert_replaced_with_history_file_2,
               revert_tree_conflicts_in_updated_files,
               revert_add_over_not_present_dir,
-              XFail(revert_added_tree),
+              revert_added_tree,
               XFail(revert_child_of_copy),
              ]