On Wed, Jan 25, 2012 at 10:29 AM, Bert Huijben <b...@qqmail.nl> wrote: > > >> -----Original Message----- >> From: Johan Corveleyn [mailto:jcor...@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 <jcor...@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'.
IMO, the caller still does all the work, except that the caller must be able to do things to wc-items which are case-shadowed. If it can't, well, let's just revert case-only renames on Windows then. > 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? I'm sorry, but this is clearly a bug, not a feature. If a script makes use of this, it's already broken. > > 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. Agreed that someone needs to make a design. Just the amount of discussion in this thread warrants some decent design documentation. > 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. Ok, but we don't need to case-normalize everywhere. I'm perhaps naïve, but it's definitely limited to places where we call I/O API's without caring if the file really exists or not (in this case: calling 'delete' on a file, it doesn't matter if it exists or not, so I'm not going to check, so I might as well just call 'delete' immediately). > 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? Ok, it's definitely an edge case. Only been reported by 1 user on the users-list. > 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. Ok, we'll leave #4023 as an open issue / known problem then. No problem for me. -- Johan
