[MERGE] [BUG 363837] catch rmtree failure in diff and mutter to logfile

Alexander Belchenko bialix at ukr.net
Tue May 12 08:25:03 BST 2009


BB:resubmit

General rule for bzr (and most python program IMO) is: don't use bare 
except, because it has undesirable effect of catching ALL errors.
We know what error we want to ignore (OSError) so we should explicitly 
use it in except.

--- bzrlib/diff.py	2009-03-28 02:10:54 +0000
+++ bzrlib/diff.py	2009-05-12 05:31:39 +0000
@@ -731,7 +731,10 @@
          return old_disk_path, new_disk_path

      def finish(self):
-        osutils.rmtree(self._root)
+        try:
+            osutils.rmtree(self._root)
+        except:
+            mutter("The temporary directory \"%s\" was not cleanly 
removed." % self._root)

^-- You should use here "except OSerror:"

Also you need to mention the bug you fixing in the NEWS (in the 
corresponding section).

Resubmit your patch, please.


martitzam at gmail.com пишет:
> Hi.
> 
> First, a big thank you to the many people (and especially Alexander) who 
> have helped with this. 
> Any problems which remain are entirely my own, and further feedback is 
> invited.
> 
> After some discussion here on the bazaar listserv, I realize that my 
> original pacth (in osutil.py) was not the best approach.
> 
> This new patch catches the problem at a higher level in diff.py.
> 
> That is the only place I try to catch the problem for now, because it is 
> the only problem I have seen.
> An audit of all calls to osutil.rmtree might be a good idea, but I have 
> not done this.
> 
> The motivation for this patch is as follows:
> 
> 1. The diff operation actually succeeds.  Only the removal of the temp 
> files/dir fails.
> 2. This is not a big deal and therefore should not result in the scary 
> message which is currently printed.
> 2a. The current message makes users wonder if the diff was accurate, 
> when there should be no doubt.
> 2b. The current message may make it harder to write scripts which call bzr.
> 3. The leftover tempfiles are in a well-known win32 temp area and 
> periodic system maintenance should remove them.
> 4. Silent logging (mutter) provides a debug trail in case further 
> investigation is needed.
> 
> All feedback is welcome.
> 
> -M
> 




More information about the bazaar mailing list