[PATCH] First step for issue #3702 (case-only renames on Windows) - now blocked by libsvn_client

Hi,

Following discussion in [1], I tried to write a patch for issue #3702
("svn ren TODO todo" not work on windows). As mentioned by Bert in
comment to the issue [2], we need to avoid letting 'svn move' convert
the destination path to on-disk casing in this case, so that's what
the below patch does.

However, it seems that's not enough. I'm getting further, but now the
move is blocked libsvn_client/copy.c. The error I'm getting now is:

    svn: E155007: Path 'C:\Temp\test\todo' is not a directory

The problem is that, in copy.c#svn_client_move6, it first tries the
move, but if the dst_path already exists (which comes down to an
apr_stat inside copy.c#verify_wc_srcs_and_dsts), it tries the move
again with moving the src_path to a child of (the presumed directory)
dst_path:

libsvn_client/copy.c: 2313
[[[
  err = try_copy(sources, dst_path,
                 TRUE /* is_move */,
                 make_parents,
                 FALSE,
                 revprop_table,
                 commit_callback, commit_baton,
                 ctx,
                 subpool);

  /* If the destination exists, try to move the sources as children of the
     destination. */
  if (move_as_child && err && (src_paths->nelts == 1)
        && (err->apr_err == SVN_ERR_ENTRY_EXISTS
            || err->apr_err == SVN_ERR_FS_ALREADY_EXISTS))
    {
      ...
      err = try_copy(sources,
                     dst_is_url
                         ? svn_path_url_add_component2(dst_path,
                                                       src_basename, pool)
                         : svn_dirent_join(dst_path, src_basename, pool),
                      ...
]]]

So, a fix in the client layer is needed as well. Any suggestions on
how to best approach this?

Right now, I'm thinking the fix should be in verify_wc_srcs_and_dsts
(similar to the "special case" of my below patch for move-cmd.c):
while checking each copy_pair, if the dst_path already exists, check
the "true path" of dst_path (APR_FILEPATH_TRUENAME), and see if it's
the same as the src_path. If so, don't return an already-exists-error.
But I'm not sure if that's correct, may have side-effects, ...

Thoughts?

Below is the patch-in-progress for svn/move-cmd.c. Any feedback on
this patch is also welcome of course. I'm pretty sure it's not perfect
:-), but it allowed me to get further.

Note: it's definitely not finished yet, because for getting the
"original dst_path", I only look at the apr_getopt_t *os structure,
and not at the opt_state->targets. I should probably consider both,
just like cmdline.c#svn_client_args_to_target_array does. But I don't
know yet how to do that nicely/concisely (maybe introduce another
helper function somewhere? In cmdline.c?). Any suggestions welcome
:-).

Log message:
Allow case-only renames on case-insensitive filesystems (issue #3702).

* subversion/svn/move-cmd.c
  (svn_cl__move): If the targets are paths, and there are exactly two, and
   the normalized DST_PATH and SRC_PATH are equal, canonicalize the original
   DST_PATH again, but without converting it to on-disk casing.
-------
Patch:
[[[
Index: subversion/svn/move-cmd.c
===================================================================
--- subversion/svn/move-cmd.c (revision 1092816)
+++ subversion/svn/move-cmd.c (working copy)
@@ -30,6 +30,7 @@
 #include "svn_client.h"
 #include "svn_error.h"
 #include "svn_path.h"
+#include "svn_utf.h"
 #include "cl.h"

 #include "svn_private_config.h"
@@ -76,6 +77,27 @@ svn_cl__move(apr_getopt_t *os,
           (SVN_ERR_CL_UNNECESSARY_LOG_MESSAGE, NULL,
            _("Local, non-commit operations do not take a log message "
              "or revision properties"));
+
+ /* For allowing case-only renames on Windows (issue #3702):
+ If DST_PATH is a path, and there is exactly one source path, check if
+ the normalized DST_PATH and SRC_PATH are equal. If so, canonicalize
+ the original DST_PATH again, but without converting it to on-disk
+ casing, so we can pass the intended destination to
+ svn_client_move6. */
+ if (targets->nelts == 1)
+ {
+ const char *src_path = APR_ARRAY_IDX(targets, targets->nelts - 1,
+ const char *);
+
+ if (strcmp(src_path, dst_path) == 0)
+ {
+ const char *utf8_target;
+
+ SVN_ERR(svn_utf_cstring_to_utf8(&utf8_target,
+ os->argv[os->argc - 1], pool));
+ dst_path = svn_dirent_canonicalize(utf8_target, pool);
+ }
+ }
     }

   if (ctx->log_msg_func3)
]]]

[1] http://svn.haxx.se/dev/archive-2011-04/0232.shtml
[2] http://subversion.tigris.org/issues/show_bug.cgi?id=3702#desc6

Cheers,

-- 
Johan