[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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

From: Johan Corveleyn <jcorvel_at_gmail.com>
Date: Mon, 18 Apr 2011 20:01:20 +0200

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

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.