Re: Case-only renames on Windows (issue #3702)

[Johan Corveleyn <jc...@gmail.com>]

On Fri, Mar 25, 2011 at 2:21 PM, Johan Corveleyn <jc...@gmail.com> wrote:
> On Sun, Mar 20, 2011 at 9:32 PM, Johan Corveleyn <jc...@gmail.com> wrote:
>> Some thoughts:
>>
>> - There is only a problem if the dst_path gets case-normalized to one
>> of the source paths. Otherwise, the case-normalization really does
>> need to happen.
>
> Hm, this thought may be incorrect (or at least "unexpected" for
> windows users). In fact, if we look at how the native "move" behaves,
> it never case-normalizes the target path:
>
> [[[
> C:\Temp>dir /B test
> TODO
>
> C:\Temp>echo anothertest > bla
>
> C:\Temp>move bla test\toDO
> Overwrite C:\Temp\test\toDO? (Yes/No/All): y
>        1 file(s) moved.
>
> C:\Temp>dir /B test
> toDO
>
> C:\Temp>type test\todo
> anothertest
> ]]]
>
> So it seems that, if we want "svn mv" to behave more like "move" on
> Windows, the target path should never be normalized to the on-disk
> casing. Just use it as is ...

Finally getting around to this again ... the above is still an open
question for specifying the desired behavior of "svn move" on Win32
... If there is a file A on disk, what should "svn move B a" do?

In theory it could generate a replace of A by a ("copied from" B).
That would be consistent with behavior of Windows' "move" command,
after confirming the overwrite.

But, lacking the "confirm-for-overwrite", I guess the behavior most
consistent with existing svn functionality would be to refuse this
move in the same way as "svn move B A" is refused. Currently, this is
with the (slightly not-to-the-point) error message: "svn: E155007:
Path 'C:\temp\bla\A' is not a directory".

Would that be ok?

Cheers,
-- 
Johan

Re: Case-only renames on Windows (issue #3702)

[Daniel Shahaf <d....@daniel.shahaf.name>]

Johan Corveleyn wrote on Thu, Apr 14, 2011 at 23:49:16 +0200:
> On Tue, Apr 12, 2011 at 11:14 PM, Johan Corveleyn <jc...@gmail.com> wrote:
> > On Fri, Mar 25, 2011 at 2:21 PM, Johan Corveleyn <jc...@gmail.com> wrote:
> >> On Sun, Mar 20, 2011 at 9:32 PM, Johan Corveleyn <jc...@gmail.com> wrote:
> >>> Some thoughts:
> >>>
> >>> - There is only a problem if the dst_path gets case-normalized to one
> >>> of the source paths. Otherwise, the case-normalization really does
> >>> need to happen.
> >>
> >> Hm, this thought may be incorrect (or at least "unexpected" for
> >> windows users). In fact, if we look at how the native "move" behaves,
> >> it never case-normalizes the target path:
> >>
> >> [[[
> >> C:\Temp>dir /B test
> >> TODO
> >>
> >> C:\Temp>echo anothertest > bla
> >>
> >> C:\Temp>move bla test\toDO
> >> Overwrite C:\Temp\test\toDO? (Yes/No/All): y
> >>        1 file(s) moved.
> >>
> >> C:\Temp>dir /B test
> >> toDO
> >>
> >> C:\Temp>type test\todo
> >> anothertest
> >> ]]]
> >>
> >> So it seems that, if we want "svn mv" to behave more like "move" on
> >> Windows, the target path should never be normalized to the on-disk
> >> casing. Just use it as is ...
> >
> > Finally getting around to this again ... the above is still an open
> > question for specifying the desired behavior of "svn move" on Win32
> > ... If there is a file A on disk, what should "svn move B a" do?
> >
> > In theory it could generate a replace of A by a ("copied from" B).
> > That would be consistent with behavior of Windows' "move" command,
> > after confirming the overwrite.
> >
> > But, lacking the "confirm-for-overwrite", I guess the behavior most
> > consistent with existing svn functionality would be to refuse this
> > move in the same way as "svn move B A" is refused. Currently, this is
> > with the (slightly not-to-the-point) error message: "svn: E155007:
> > Path 'C:\temp\bla\A' is not a directory".
> >
> > Would that be ok?
> 
> I think this would be ok: just refuse a move that overwrites another
> local file (need to rm it first).
> 
> Now, I just realized that case-only renames (or renames in general)
> only come into play when there are exactly 2 "targets" of the move
> command: the source and the destination (ok, I'm a bit slow, I know
> :-)).
> 
> So, for fixing issue #3702 (case-only renames on Windows), if we take
> into account the rest that's been discussed in this thread, we only
> need to do something special:
> - If there are exactly 2 targets
> - If those targets are paths, rather than uris.
> - If normalized dst_path == normalized src_path
> 
> In that case I think we should either:
> 
> 1) Normalize the original dst_path again, but this time without the
> APR_FILEPATH_TRUENAME flag to apr_filepath_merge, so it isn't
> converted to on-disk-representation.
> 

Call me a pedant, but:

Does APR guarantee that it will *not* canonicalize-to-truename if
APR_FILEPATH_TRUENAME is not passed?

That does not follow logically from "APR will canonicalize-to-truename
if APR_FILEPATH_TRUENAME is passed".

> or
> 
> 2) Just use the original dst_path as it was passed to the client, un-normalized.
> 
> 
> I have no idea if it should be 1) (other important normalization-stuff
> going on besides converting it to the on-disk-representation, that
> should still be performed in this case) or 2) ("if the user tells us
> to use this particular representation of the path, let's use it").
> 
> Option 2) would of course be much easier. For option 1) code needs to
> be written/copy-pasted/refactored to do all the normalization stuff,
> except passing the flag to apr_filepath_merge.

Re: Case-only renames on Windows (issue #3702)

[Johan Corveleyn <jc...@gmail.com>]

On Tue, Apr 12, 2011 at 11:14 PM, Johan Corveleyn <jc...@gmail.com> wrote:
> On Fri, Mar 25, 2011 at 2:21 PM, Johan Corveleyn <jc...@gmail.com> wrote:
>> On Sun, Mar 20, 2011 at 9:32 PM, Johan Corveleyn <jc...@gmail.com> wrote:
>>> Some thoughts:
>>>
>>> - There is only a problem if the dst_path gets case-normalized to one
>>> of the source paths. Otherwise, the case-normalization really does
>>> need to happen.
>>
>> Hm, this thought may be incorrect (or at least "unexpected" for
>> windows users). In fact, if we look at how the native "move" behaves,
>> it never case-normalizes the target path:
>>
>> [[[
>> C:\Temp>dir /B test
>> TODO
>>
>> C:\Temp>echo anothertest > bla
>>
>> C:\Temp>move bla test\toDO
>> Overwrite C:\Temp\test\toDO? (Yes/No/All): y
>>        1 file(s) moved.
>>
>> C:\Temp>dir /B test
>> toDO
>>
>> C:\Temp>type test\todo
>> anothertest
>> ]]]
>>
>> So it seems that, if we want "svn mv" to behave more like "move" on
>> Windows, the target path should never be normalized to the on-disk
>> casing. Just use it as is ...
>
> Finally getting around to this again ... the above is still an open
> question for specifying the desired behavior of "svn move" on Win32
> ... If there is a file A on disk, what should "svn move B a" do?
>
> In theory it could generate a replace of A by a ("copied from" B).
> That would be consistent with behavior of Windows' "move" command,
> after confirming the overwrite.
>
> But, lacking the "confirm-for-overwrite", I guess the behavior most
> consistent with existing svn functionality would be to refuse this
> move in the same way as "svn move B A" is refused. Currently, this is
> with the (slightly not-to-the-point) error message: "svn: E155007:
> Path 'C:\temp\bla\A' is not a directory".
>
> Would that be ok?

I think this would be ok: just refuse a move that overwrites another
local file (need to rm it first).

Now, I just realized that case-only renames (or renames in general)
only come into play when there are exactly 2 "targets" of the move
command: the source and the destination (ok, I'm a bit slow, I know
:-)).

So, for fixing issue #3702 (case-only renames on Windows), if we take
into account the rest that's been discussed in this thread, we only
need to do something special:
- If there are exactly 2 targets
- If those targets are paths, rather than uris.
- If normalized dst_path == normalized src_path

In that case I think we should either:

1) Normalize the original dst_path again, but this time without the
APR_FILEPATH_TRUENAME flag to apr_filepath_merge, so it isn't
converted to on-disk-representation.

or

2) Just use the original dst_path as it was passed to the client, un-normalized.


I have no idea if it should be 1) (other important normalization-stuff
going on besides converting it to the on-disk-representation, that
should still be performed in this case) or 2) ("if the user tells us
to use this particular representation of the path, let's use it").

Option 2) would of course be much easier. For option 1) code needs to
be written/copy-pasted/refactored to do all the normalization stuff,
except passing the flag to apr_filepath_merge.

Thoughts?

Cheers,
-- 
Johan