On Sat, Apr 16, 2011 at 10:17 AM, Johan Corveleyn <jcorvel_at_gmail.com> wrote:
> 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, ...
In attachment is a second attempted patch that implements the above
suggestion: in copy.c, when verifying srcs_and_dsts, if a dst clashes
with an existing file, check if its truepath is the same as the src
(if so, let it go through). This uses some copy-pasted code from
opt.c, so maybe some refactoring is needed here ... Or another way to
pass both the truepath and the originally-requested-path.
After that the "svn move" went through, but the moved file was deleted
from the file system. I fixed that by setting keep_local to TRUE in
the call to svn_wc_delete4 in do_wc_to_wc_moves_with_locks2. At first
sight this seems ok to me, but I'm very much in unfamiliar territory,
so I'm not sure at all :-). See the comment in the patch for some
additional explanation.
Comments, review, ... very much appreciated.
With this patch, one can perform case-only renames on Windows.
BUT it raises some additional questions/issues:
- How to commit such a move? Committing the parent directory
recursively works fine, but if you try to specify only the move
targets (src and dst paths), commit runs into the same problem as what
I was trying to address here: both paths are converted to their
"truepath", which means only the added side is seen by commit (the
only part that's still on the filesystem):
C:\Temp\test4>svn mv foo2 Foo2
A Foo2
D foo2
C:\Temp\test4>svn commit -mtest foo2 Foo2
Adding Foo2
Committed revision 96322.
This is very undesirable, because after this commit this cannot be
checked out or updated anymore on a Windows client (case-clashing
files).
- In fact: the same problem holds true for other commands as well: how
to revert both sides of this move? Ok, one can revert in two steps ...
- Maybe a more general solution is needed, so all commands can
adequately see which path the user actually means? The "truepath"
corresponding to a given path (modulo case), or really the path in the
case given by the user? A shot in the dark: (1) first look in the
wc-db - if the path matches a path in the wc-db, accept it as is, else
(2) convert it to its truepath (path on the filesystem that matches
modulo case). Except for "svn move", as implemented in this patch ...
- Note that the above problem is already present now on trunk (without
my patch): since we can now represent case-clashing paths in the WC
even on a case-insensitive filesystem. (See Bert's example in [2],
"svn ren TODO todoQ; svn ren todoQ todo").
Cheers,
--
Johan
> [1] http://svn.haxx.se/dev/archive-2011-04/0232.shtml
> [2] http://subversion.tigris.org/issues/show_bug.cgi?id=3702#desc6
Received on 2011-04-18 20:02:11 CEST