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 2011/05/17 14:28:53 UTC

svn commit: r1104191 - in /subversion/trunk/subversion: libsvn_client/copy.c tests/libsvn_wc/op-depth-test.c

Author: rhuijben
Date: Tue May 17 12:28:53 2011
New Revision: 1104191

URL: http://svn.apache.org/viewvc?rev=1104191&view=rev
Log:
Fix the libsvn_client portion of issues #3702 and #3865, by handling case
only renames of files and directories properly on case insensitive filesystems.

Suggested by: jcorvel

* subversion/libsvn_client/copy.c
  (do_wc_to_wc_moves_with_locks2): Don't try to delete what was there, as we
    just removed it. On case insensitive filesystems this might remove
    something else. (Like: Everything you just moved).
  (verify_wc_srcs_and_dsts): Add is_moved argument and detect and allow case
    sensitive renames of a single item within a single parent directory.
  (try_copy): Update caller.

* subversion/tests/libsvn_wc/op-depth-test.c
  (test_case_rename): Also do a file rename. Return success, as it should pass
    on all platforms now.
  (test_funcs): Remove XFail marking from test_case_rename.

Modified:
    subversion/trunk/subversion/libsvn_client/copy.c
    subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c

Modified: subversion/trunk/subversion/libsvn_client/copy.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/copy.c?rev=1104191&r1=1104190&r2=1104191&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/copy.c (original)
+++ subversion/trunk/subversion/libsvn_client/copy.c Tue May 17 12:28:53 2011
@@ -348,7 +348,7 @@ do_wc_to_wc_moves_with_locks2(void *bato
                              scratch_pool));
 
   SVN_ERR(svn_wc_delete4(b->ctx->wc_ctx, b->pair->src_abspath_or_url,
-                         FALSE, FALSE,
+                         TRUE, FALSE,
                          b->ctx->cancel_func, b->ctx->cancel_baton,
                          b->ctx->notify_func2, b->ctx->notify_baton2,
                          scratch_pool));
@@ -452,6 +452,7 @@ do_wc_to_wc_moves(const apr_array_header
 static svn_error_t *
 verify_wc_srcs_and_dsts(const apr_array_header_t *copy_pairs,
                         svn_boolean_t make_parents,
+                        svn_boolean_t is_move,
                         svn_client_ctx_t *ctx,
                         apr_pool_t *pool)
 {
@@ -482,10 +483,51 @@ verify_wc_srcs_and_dsts(const apr_array_
       SVN_ERR(svn_io_check_path(pair->dst_abspath_or_url, &dst_kind,
                                 iterpool));
       if (dst_kind != svn_node_none)
-        return svn_error_createf(
-          SVN_ERR_ENTRY_EXISTS, NULL,
-          _("Path '%s' already exists"),
-          svn_dirent_local_style(pair->dst_abspath_or_url, pool));
+        {
+          if (is_move
+              && copy_pairs->nelts == 1
+              && strcmp(svn_dirent_dirname(pair->src_abspath_or_url, iterpool),
+                        svn_dirent_dirname(pair->dst_abspath_or_url,
+                                           iterpool)) == 0)
+            {
+              const char *dst;
+              char *dst_apr;
+              apr_status_t apr_err;
+              /* We have a rename inside a directory, which might collide
+                 just because the case insensivity of the filesystem makes
+                 the source match the destination. */
+
+              SVN_ERR(svn_path_cstring_from_utf8(&dst,
+                                                 pair->dst_abspath_or_url,
+                                                 pool));
+
+              apr_err = apr_filepath_merge(&dst_apr, NULL, dst,
+                                           APR_FILEPATH_TRUENAME, iterpool);
+
+              if (!apr_err)
+                {
+                  /* And now bring it back to our canonical format */
+                  SVN_ERR(svn_path_cstring_to_utf8(&dst, dst_apr, iterpool));
+                  dst = svn_dirent_canonicalize(dst, iterpool);
+                }
+              /* else: Don't report this error; just report the normal error */
+
+              if (!apr_err && strcmp(dst, pair->src_abspath_or_url) == 0)
+                {
+                  /* Ok, we have a single case only rename. Get out of here */
+                  svn_dirent_split(&pair->dst_parent_abspath, &pair->base_name,
+                                   pair->dst_abspath_or_url, pool);
+
+                  svn_pool_destroy(iterpool);
+                  return SVN_NO_ERROR;
+                }
+            }
+
+          return svn_error_createf(
+                      SVN_ERR_ENTRY_EXISTS, NULL,
+                      _("Path '%s' already exists"),
+                      svn_dirent_local_style(pair->dst_abspath_or_url, pool));
+        }
 
       svn_dirent_split(&pair->dst_parent_abspath, &pair->base_name,
                        pair->dst_abspath_or_url, pool);
@@ -2222,7 +2264,8 @@ try_copy(const apr_array_header_t *sourc
   /* Now, call the right handler for the operation. */
   if ((! srcs_are_urls) && (! dst_is_url))
     {
-      SVN_ERR(verify_wc_srcs_and_dsts(copy_pairs, make_parents, ctx, pool));
+      SVN_ERR(verify_wc_srcs_and_dsts(copy_pairs, make_parents, is_move,
+                                      ctx, pool));
 
       /* Copy or move all targets. */
       if (is_move)

Modified: subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c?rev=1104191&r1=1104190&r2=1104191&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c Tue May 17 12:28:53 2011
@@ -3418,17 +3418,29 @@ test_copy_of_deleted(const svn_test_opts
   return SVN_NO_ERROR;
 }
 
+/* Part of issue #3702, #3865 */
 static svn_error_t *
 test_case_rename(const svn_test_opts_t *opts, apr_pool_t *pool)
 {
   svn_test__sandbox_t b;
+  apr_hash_t *dirents;
 
   SVN_ERR(svn_test__sandbox_create(&b, "case_rename", opts, pool));
   SVN_ERR(add_and_commit_greek_tree(&b));
 
   SVN_ERR(wc_move(&b, "A", "a"));
+  SVN_ERR(wc_move(&b, "iota", "iotA"));
 
-  return svn_error_create(APR_EOF, NULL, NULL);
+  SVN_ERR(svn_io_get_dirents3(&dirents, wc_path(&b, ""), TRUE, pool, pool));
+
+  /* A shouldn't be there, but a should */
+  SVN_TEST_ASSERT(apr_hash_get(dirents, "a", APR_HASH_KEY_STRING));
+  SVN_TEST_ASSERT(apr_hash_get(dirents, "A", APR_HASH_KEY_STRING) == NULL);
+  /* iota shouldn't be there, but iotA should */
+  SVN_TEST_ASSERT(apr_hash_get(dirents, "iotA", APR_HASH_KEY_STRING));
+  SVN_TEST_ASSERT(apr_hash_get(dirents, "iota", APR_HASH_KEY_STRING) == NULL);
+
+  return SVN_NO_ERROR;
 }
 /* ---------------------------------------------------------------------- */
 /* The list of test functions */
@@ -3480,7 +3492,7 @@ struct svn_test_descriptor_t test_funcs[
                        "test_shadowed_update"),
     SVN_TEST_OPTS_PASS(test_copy_of_deleted,
                        "test_copy_of_deleted (issue #3873)"),
-    SVN_TEST_OPTS_XFAIL(test_case_rename,
+    SVN_TEST_OPTS_PASS(test_case_rename,
                        "test_case_rename on case (in)sensitive system"),
     SVN_TEST_NULL
   };