[MERGE] [BUG 363837] Catch _win32_delete_readonly failure to remove file or directory and try to recover

[Martin Pool]

2009/5/6 Maritza Mendez <martitzam at gmail.com>:
>
>
> On Tue, May 5, 2009 at 3:24 PM, John Arbash Meinel <john at arbash-meinel.com>
> wrote:
>>
>> I would make this very specific to windows + diff handling, rather than
>> do it for all files we want to delete. I suppose 1&2 aren't terrible for
>> generic, but I think we'll often have things open that won't be
>> released, like Visual Studio editing.
>
> This is a very important point.  Although Visual Studio is not part of my
> motivation, I can attest that VS keeps several kinds of files open/locked.
> Standard advice for my teams is "Close VS before attempting any version
> control."  This is true for all version control systems, not just bzr of
> course.  So it is probably not reasonable for users to expect bzr to deal
> gracefully with Visual Studio unless integrated version control is a stated
> goal of the bzr project.  On the other hand, external diff support is built
> into bzr and therefore is expetced to be tolerant of some platform oddness.

I think a requirement to close Visual Studio before doing bzr
operations is a bug in bzr (and probably also in VS but anyhow.)  We
could potentially work around that behaviour by having a mode where we
open and modify the existing files rather than replacing them,
assuming VS will even let you do that.  (But that's a separate bug, so
let's handle it separately.)

> I see two general threads in this discussion.  One thread seems to say that
> bzr could actively diagnose and compensate for the root causes and that this
> should be limited to the win32 layer.  That was my original opinion,
> although my solution was not the best.  The other thread holds that
> platform-specific stale temp files (and directories) are ok, especially if
> the intended operation works and a friendly message is printed instead of an
> uncaught exception.  I am now of this latter opinion and my patch will be
> along those lines, probably this weekend when I get some time to test.

I agree with you too, let's just let them happen.  Thanks.


>> The _win32_delete_readonly function intended to help in the cases where
>> bzr need to remove file marked as read-only. Nothing more. There is special
>> test in test_osutils.py to ensure this behavior.
>
> Point  taken.  I have not seen any design document for bzr, unless email
> archives count.  I'm just saying that the purpose of osutils.rmtree appears
> to be to deal with files which are (for whatever reason) readonly on Windows
> and need to be rendered into a removable state.  If that is the (derived)
> requirement, then the current implemnatation is good, but not foolproof, as
> we see.

There is <http://doc.bazaar-vcs.org/bzr.dev/developers/index.html>
which is derived from doc/ in the source tree, though at the level of
individual functions we'd tend to describe their purpose and
limitations in the docstring.  I'm sure there are many shortcomings in
those documents, but the best way to improve it is for readers like
yourself to point out where things are missing or unclear and we'll
improve it.

-- 
Martin <http://launchpad.net/~mbp/>