Luke review : add try/catch and be safer, documentation, follow conventions

slapos/recipe/build.py

@@ -164,23 +164,30 @@ class Script:
     self.cleanup_dir_list.append(extract_dir)
     return extract_dir
 
-  def copytree(self, origin, destination, overwrite=False, ignore_dir_list=[]):
-    """Recursively Copy directory. if "overwrite" is set to False, will stop if
-      destination already exists. ignore_dir_list is an array of directories
-      you want to exclude.
-      Example : copytree("/from", "/to", overwrite=True, ignore_dir_list=["a_private_dir"])
+  def copyTree(self, origin, destination, ignore_dir_list=None):
+    """Recursively Copy directory.
+    
+    ignore_dir_list is a list of relative directories you want to exclude.
+    Example :
+    copytree("/from", "/to", ignore_dir_list=["a_private_dir"])
     """
-    if os.path.exists(destination) and not overwrite:
-      self.logger.info('Destination already exists, aborting.')
-      return False
+    # Check existence before beginning. We don't want to cleanup something
+    # that does not belong to us.
+    if os.path.exists(destination):
+      raise shutil.Error('Destination already exists: %s' % destination)
     self.logger.info("Copying %s to %s" % (origin, destination))
+    if ignore_dir_list is None:
+      ignore_dir_list = []
     try:
       shutil.copytree(origin, destination,
                       ignore=lambda src,names:ignore_dir_list)
-    except shutil.Error:
-      self.logger.error("Error occurred : %s")
-      shutil.rmtree(destination)
-      return False
+    except (shutil.Error, OSError) as error:
+      # Cleanup in case of failure
+      try:
+        shutil.rmtree(destination, ignore_errors=True)
+      except (shutil.Error, OSError), strerror:
+        self.logger.error("Error occurred when cleaning after error: %s", strerror)
+      raise error
     return True
 
   script = 'raise NotImplementedError'