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

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Maritza Mendez wrote:
> On Mon, May 4, 2009 at 7:08 AM, Aaron Bentley <aaron at aaronbentley.com
> <mailto:aaron at aaronbentley.com>> wrote:
> 
>     Unless what you
>     mean is that any arbitrary deletion on Windows can fail because some
>     random program may be holding a file open.

> Yes, that's exactly what I meant.  I don't think any of us would accept
> this behavior on Unix, and support for Windows should be held to the
> same standard.

I wish we could hold Windows to the same standard, which in this case
would be POSIX :-)

> I'm also conditioned to catch these things as soon as possible, before
> they propagate.  This is a low-level problem which should be prevented
> whenever possible.

I think you are making the case that win32's inability to delete an open
file is a bug which we should try to work around.  I think that's fair,
but your solution is not a very general solution.  It's only a solution
if you have reason to believe that the program holding the file open
will release it very soon.

I can think of only two cases where that is likely:
1. The program holding the file open is known to have been closed
   recently
2. The program holding the file open is a scanner of some kind that hold
   the file open briefly.

A very common case will be that the user has the file open in another
program, and in that case, the most productive path may be to request
that the user close the file.

So I think this solution is actually linked to the concept of running an
external program with temp files.

>     Please catch specific exceptions, not using a bare except clause.  Many
>     causes of deletion failure are unrecoverable (file does not exist, file
>     is a directory, etc), so they should propagate immediately.
> 
> 
> That's really my point also.  My objections to what I found in osutils
> are (1) a bare exception comes back to the caller (notice the bare
> "raise" in the released code) and (2) no attempt to do anything useful
> about it is made.

I think we're actually disagreeing.  I see bare except as a bad choice
and a bare raise as a good choice.

A bare raise propagates the original exception, so it gives callers the
opportunity to handle it themselves.  Because it is the original
exception, callers can handle different failure modes in different ways.
 It is also consistent with the behaviour on Unix, so fewer special
cases are required.

A bare except, on the other hand, catches exceptions we shouldn't catch,
including MemoryError, KeyboardInterrupt, and causes of deletion failure
 which we can do nothing about.

> So I am prepared to create and submit a different patch when I get spare
> time.  I still intend to add second-chance code with a small delay, but
> these deltas will then be in the high-level callers (diff and merge). 
> This will result in some duplication and maybe some small obfuscation.

One option would be to define an error handler that does this
second-chance code and provide that as the onerror parameter.  I think
that would duplicate very little code.

> It also means that diff and merge will deal with a low-level problem
> which so far has been seen only on Windows.

Since this solution mostly applies to temp directories used as input for
external programs, perhaps it could be provided as a class:
osutils.TempDir or something like that.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkn/X/gACgkQ0F+nu1YWqI0WBwCeKn/5M4UykQZGfcwqzz0hxpxj
rq4An14XFVCTLFh+U3V4zjB0OkjSNTYO
=8EH5
-----END PGP SIGNATURE-----