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

[Maritza Mendez]

On Mon, May 4, 2009 at 2:37 PM, Aaron Bentley <aaron at aaronbentley.com>wrote:

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

Amen, brother!  Maybe if we both send Bill a Christmas card this year.  :)

> 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.


I agree.  And although I believe that one of these is "almost always" the
truth, you are correct to expose my hidden assumption.  I find I have no
good answer for this in general.



> 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.


Good idea.


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

So maybe the correct approach for diff (and merge) is to tolerate the
leftover temp files and print a friendly message saying that they were left
behind but allow execution to continue as if nothing went wrong.  Since bzr
does not need (and will not re-use) these files, there is no real harm.  The
user has been advised and then it is up to him if to do anything.  Since bzr
uses a well-known system temp folder, routine maintenance should clean it
out anyway.

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

Aha!  I understand now.  I am not in love with bare raise, but I agree it is
much better than bare except!


> 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.


Ok.  I'm with you now.


> 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.
>
> ...
>
> 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.
>

I like that.  I just need to think about how to do that without recursion
and at most a local counter...

> 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.


That's also good.  Might be more than we need, but I will think about it.

Again, I really appreciate this discussion!  Thanks!

-M
-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://lists.ubuntu.com/archives/bazaar/attachments/20090504/21d09ebe/attachment-0001.htm