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

RE: How to fix issue #4023 (on Windows, 'svn rm missingItem' deletes the unversioned 'MissingItem' from disk)?

From: Bert Huijben <bert_at_qqmail.nl>
Date: Wed, 25 Jan 2012 10:29:12 +0100

> -----Original Message-----
> From: Johan Corveleyn [mailto:jcorvel_at_gmail.com]
> Sent: woensdag 25 januari 2012 1:48
> To: Julian Foad
> Cc: Subversion Development; Bert Huijben; Philip Martin
> Subject: Re: How to fix issue #4023 (on Windows, 'svn rm missingItem'
> deletes the unversioned 'MissingItem' from disk)?
>
> On Tue, Jan 24, 2012 at 1:18 AM, Johan Corveleyn <jcorvel_at_gmail.com>
> wrote:
>
> [ ... ]
>
> > I'd like to note though that #4023 is also potentially an issue for
> > svn < 1.7 (pre-wcng). At least if you're thinking about
> > non-commandline usage. As of #3865, one can now also run into this
> > situation with the commandline client, because you're now able to
> > successfully invoke 'svn rm foo' while foo is missing and unversioned
> > FOO is in the way. But if you're talking directly to the client layer,
> > it's easy to pass 'foo', and the current code will then happily remove
> > foo from wc.db, and FOO from disk. There is no need for any of my
> > "fixes" to do that.
>
> Just to confirm this point: I've been able to reproduce #4023 (or a
> variation thereof) with TortoiseSVN 1.6:
>
> - Add 'foo' as a versioned file. Commit.
> - Delete 'foo' from filesystem (non-svn delete).
> - Create new file 'FOO'.
> - Right-click and TSVN -> Add
> - You get a warning that another item which only differs in case
> already exists. Click 'Add the item anyway'.
> - Right-click in the directory, and choose 'Check for modifications'
> (i.e. 'svn status')
> - Now you can see foo as missing, and FOO as added.
> - Right-click foo, and choose Delete. Now FOO is deleted from disk.
>
> I'll try to update the issue later tonight, to remove the 'since 1.7'
> denomination, and rephrase it independently of the use of 'svn' (the
> commandline executable). It's a libsvn_client or libsvn_wc issue.
>
> So my conclusion is: libsvn_client / libsvn_wc shouldn't assume that
> they always receive a truepath-canoncalized local path. They can
> always receive a path that's a valid wc-item, but is not present on
> disk and is case-shadowed by another item on disk [1]. This means that
> they should be very careful when calling filesystem API's where they
> don't care whether the exact same file exists or not (in casu:
> 'delete'): better to check whether the file (with the same exact case)
> exists, before calling delete, to avoid accidentally deleting the
> wrong file.

libsvn_wc never used true path assumptions, except that it assumed that the
caller already did all the work, which is usually 'svn'.

The 'svn rm' case that opened this thread was the first case where libsvn_wc
would be involved and this opens the door to many other changes.
If we change svn rm, then it no longer matches 'svn revert' for these cases.

Where do we stop?

Do we break all shell scripts and library userw that assume the current
behavior on the way?

Julian made a good point in asking for a design on how we should handle
this.
We don't want to go the same way as with the file externals implementation:
exceptions everywhere.

The fix for allowing multiple path forms from 'svn' was a very local change.
Changing 'svn rm' scemantics in libsvn_wc is not. This has impact on the
entire client library (E.g. svn merge).

Just case normalizing everywhere will make Subversion at least 5* slower on
Windows than on other platforms in the normal cases where the user didn't
care before 1.7.

I don't think this 'svn rm wROngCased' is such a common case that we should
use it to go that way. At least not without a proper design.

I was never bitten by this problem. How many users were?

If some users are surprised by the current behavior that is not necessary a
reason to change the implementation if there are good reasons for the
current design.

        Bert
>
> [1] The fact that the svn <1.7 commandline client cannot throw such
> case-shadowed targets at the client layer is just a very thin layer of
> protection, and IMHO a shortcoming which was rectified with #3865.
>
> P.S.: It would be interesting to know if #4023 is also an issue on OSX
> (with case-insensitive filesystem). I guess it is, but I don't have
> such a system, so if anyone could try this ...
>
> --
> Johan
Received on 2012-01-25 10:30:46 CET

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