š Hi Alexander, Thanks.š You are right.š All three exceptions are a WindowsError.š The first two are access denied (Windows system error 0x05) and the third one is the sharing violation (Windows system error 0x20 = the 32 (decimal) reported when bzr passes it through).š Although I have not gone so far as to see what process is holding the files -- and the directory -- but its pretty obvious that it must be BeyondCompare.š Here is the actual output I generated from _win32_delete_readonly: Path: C:/docume~1/martitza/locals~1/temp/bzr-diff-yribs_\new\IssueTrackingNeeds.txt Exception Type: <type 'exceptions.WindowsError'> ['__class__', '__delattr__', '__dict__', '__doc__', '__format__', '__getattribute__', '__getitem__', '__getslice__', '__hash__', '__init__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__setstate__', '__sizeof__', '__str__', '__subclasshook__', '__unicode__', 'args', 'errno', 'filename', 'message', 'strerror', 'winerror'] Exception Attributes:5 Path: C:/docume~1/martitza/locals~1/temp/bzr-diff-yribs_\old\IssueTrackingNeeds.txt Exception Type: <type 'exceptions.WindowsError'> ['__class__', '__delattr__', '__dict__', '__doc__', '__format__', '__getattribute__', '__getitem__', '__getslice__', '__hash__', '__init__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__setstate__', '__sizeof__', '__str__', '__subclasshook__', '__unicode__', 'args', 'errno', 'filename', 'message', ' strerror', 'winerror'] Exception Attributes:5 Path: C:/docume~1/martitza/locals~1/temp/bzr-diff-yribs_ Exception Type: <type 'exceptions.WindowsError'> ['__class__', '__delattr__', '__dict__', '__doc__', '__format__', '__getattribute__', '__getitem__', '__getslice__', '__hash__', '__init__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__setstate__', '__sizeof__', '__str__', '__subclasshook__', '__unicode__', 'args', 'errno', 'filename', 'message', ' strerror', 'winerror'] Exception Attributes:32 So the options to make _win32_delete_readonly more fine grained remain either an ugly delay (like my first patch) which may break later or some awkward code to try to figure out what process (BC3 in this case) is at fault. Seems like a lot of work for the reward. I think it's still best to just wrap the osutils.rmtree in diff.py (line 734) in a try/except: ššš def finish(self): ššššššš try: ššššššššššš osutils.rmtree(self._root) ššššššš except: ššššššššššš note("The temporary directory [%s] was not cleanly removed." % self._root) The result is output which looks like this: The temporary directory [C:/docume~1/kferrio/locals~1/temp/bzr-diff-yribs_] was not cleanly removed. instead of this: bzr: ERROR: [Error 32] The process cannot access the file because it is being used by another process: 'C:/docume~1/martitza/locals~1/temp/bzr-diff-yribs_' I would rather make this a debug-level error than an info-level error, so that it does not show up on stderr by default.š As I read trace.py it is not obvious to me what is the preferred way to create debug-level logging.š So rather than violate that convention, I opted for note/info.š I may look through the sources for examples of text which get printed only to bzr.log to figure out the debug level. Thanks again.š I will test a bit more before submitting a merge request. -M <div class="gmail_quote">2009/5/7 Alexander Belchenko <<a href="mailto:bialix@ukr.net">bialix@ukr.net</a>> <blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">Hi Maritza, Because I'm the one who was involved in writing _win32_delete_readonly, I'll try to provide some hints for you. š šdef _win32_delete_readonly(function, path, excinfo): š š š š"""Error handler for shutil.rmtree function [for win32] š š š šHelps to remove files and dirs marked as read-only. š š š š""" š š š šexception = excinfo[1] š š š šif function in (os.remove, os.rmdir) \ š š š š š šand isinstance(exception, OSError) \ š š š š š šand exception.errno == errno.EACCES: š š š š š šmake_writable(path) š š š š š šfunction(path) š š š šelse: š š š š š šraise The problem in the code above is that it used too-broad testing for error, e.g. š š š š š šand exception.errno == errno.EACCES: errno.EACCES it's a POSIX-style error constant, IIUC it's about Access Denied. Fortunately it seems like actually we have there WindowsError exception, and therefore can get real Windows Error Code (as of GetLastError() returns). So, if you're adding following debugging prints to the function: š šdef _win32_delete_readonly(function, path, excinfo): š š š š"""Error handler for shutil.rmtree function [for win32] š š š šHelps to remove files and dirs marked as read-only. š š š š""" š š š šexception = excinfo[1] + š š š print type(exception), dir(exception) + š š š print getattr(exception, 'winerror', None) š š š šif function in (os.remove, os.rmdir) \ š š š š š šand isinstance(exception, OSError) \ š š š š š šand exception.errno == errno.EACCES: š š š š š šmake_writable(path) š š š š š šfunction(path) š š š šelse: š š š š š šraise You will obtain additional information about your error with BeyondCompare3. Especially the second print should print to you actual Windows error code (<a href="http://msdn.microsoft.com/en-us/library/ms681381%28VS.85%29.aspx" target="_blank">http://msdn.microsoft.com/en-us/library/ms681381(VS.85).aspx</a>) Knowing actual error code there is possible to imprtove _win32_delete_readonly function to make more fine-grained analysis why deletion of file fails and provide appropriate solution for different cases: deleting read-only files, files currently in use etc. HTH Maritza Mendez ĐÉŰĹÔ: <blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div class="im"> On Wed, May 6, 2009 at 2:01 AM, Alexander Belchenko <<a href="mailto:bialix@ukr.net" target="_blank">bialix@ukr.net</a> <mailto:<a href="mailto:bialix@ukr.net" target="_blank">bialix@ukr.net</a>>> wrote: š šMaritza Mendez ĐÉŰĹÔ: š š š šOn Tue, May 5, 2009 at 8:54 AM, Aaron Bentley š š š š<<a href="mailto:aaron@aaronbentley.com" target="_blank">aaron@aaronbentley.com</a> <mailto:<a href="mailto:aaron@aaronbentley.com" target="_blank">aaron@aaronbentley.com</a>> </div> š š š š<mailto:<a href="mailto:aaron@aaronbentley.com" target="_blank">aaron@aaronbentley.com</a> <mailto:<a href="mailto:aaron@aaronbentley.com" target="_blank">aaron@aaronbentley.com</a>>>><div><div></div> <div class="h5"> š š š šwrote: š š š š š -----BEGIN PGP SIGNED MESSAGE----- š š š š š Hash: SHA1 š š š š š Maritza Mendez wrote: š š š š š š> The more I think about this, the more I think that a cure š š š šmay be š š š š š worse š š š š š š> than the problem. šSo I am leaning toward... š š š š š š> bzr would catch the exception, print a polite message š š š š š š> about the external misbehavior, and just keep going. š š š š š I think that this would be fine, so long as it's an opt-in š š š šbehaviour. š š š š š If you do it as an onerror callback, note that problems may š š š šcascade, š š š š š because failure to delete a file can cause failure to delete its š š š š š containing directory. š š š š š Aaron š š š šI think it is "opt-in" enough to limit the response to catching š š š š OSError and ignoring all other exceptions. šWhat do you think? š š š š At worst, bzr would continue to miss some things it misses now. š š š šI have to remind myself that this whole discussion comes from š š š šthe fact that _win32_delete_readonly does not robustly handle š š š šfailures in osutils.rmtree call to shutil.rmtree. šSince that š š š šseems to be the whole purpose of _win32_delete_readonly, I might š š š šsuggest that osutils.rmtree and _win32_delete_readonly do not š š š šmeet design requirements. š šWhich one design requirements? š š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. -M </div></div></blockquote> </blockquote></div>