Trac #7733: fix severe performance regression when creating ActivityBundle.

src/sugar/bundle/activitybundle.py

@@ -205,13 +205,13 @@ class ActivityBundle(Bundle):
 
     def get_locale_path(self):
         """Get the locale path inside the (installed) activity bundle."""
-        if not self._unpacked:
+        if self._zip_file is not None:
             raise NotInstalledException
         return os.path.join(self._path, 'locale')
 
     def get_icons_path(self):
         """Get the icons path inside the (installed) activity bundle."""
-        if not self._unpacked:
+        if self._zip_file is not None:
             raise NotInstalledException
         return os.path.join(self._path, 'icons')
 
@@ -237,7 +237,7 @@ class ActivityBundle(Bundle):
     def get_icon(self):
         """Get the activity icon name"""
         icon_path = os.path.join('activity', self._icon + '.svg')
-        if self._unpacked:
+        if self._zip_file is None:
             return os.path.join(self._path, icon_path)
         else:
             icon_data = self.get_file(icon_path).read()
@@ -365,7 +365,7 @@ class ActivityBundle(Bundle):
             raise RegistrationException
 
     def uninstall(self, force=False):        
-        if self._unpacked:
+        if self._zip_file is None:
             install_path = self._path
         else:
             if not self.is_installed():

src/sugar/bundle/bundle.py

@@ -19,6 +19,7 @@
 
 import os
 import logging
+import shutil
 import StringIO
 import zipfile
 
@@ -59,9 +60,9 @@ class Bundle:
         self._zip_root_dir = None
 
         if os.path.isdir(self._path):
-            self._unpacked = True
+            self._zip_file = None
         else:
-            self._unpacked = False
+            self._zip_file = zipfile.ZipFile(self._path)
             self._check_zip_bundle()
 
         # manifest = self._get_file(self._infodir + '/contents')
@@ -72,9 +73,12 @@ class Bundle:
         # if signature is None:
         #     raise MalformedBundleException('No signature file')
 
+    def __del__(self):
+        if self._zip_file is not None:
+            self._zip_file.close()
+
     def _check_zip_bundle(self):
-        zip_file = zipfile.ZipFile(self._path)
-        file_names = zip_file.namelist()
+        file_names = self._zip_file.namelist()
         if len(file_names) == 0:
             raise MalformedBundleException('Empty zip file')
 
@@ -99,48 +103,42 @@ class Bundle:
     def get_file(self, filename):
         f = None
 
-        if self._unpacked:
+        if self._zip_file is None:
             path = os.path.join(self._path, filename)
             try:
                 f = open(path,"rb")
             except IOError:
                 return None
         else:
-            zip_file = zipfile.ZipFile(self._path)
             path = os.path.join(self._zip_root_dir, filename)
             try:
-                data = zip_file.read(path)
+                data = self._zip_file.read(path)
                 f = StringIO.StringIO(data)
             except KeyError:
                 logging.debug('%s not found.' % filename)
-            zip_file.close()
 
         return f
     
     def is_file(self, filename):
-        if self._unpacked:
+        if self._zip_file is None:
             path = os.path.join(self._path, filename)
             return os.path.isfile(path)
         else:
-            zip_file = zipfile.ZipFile(self._path)
             path = os.path.join(self._zip_root_dir, filename)
             try:
-                zip_file.getinfo(path)
+                self._zip_file.getinfo(path)
             except KeyError:
                 return False
-            finally:
-                zip_file.close()
 
             return True
 
     def is_dir(self, filename):
-        if self._unpacked:
+        if self._zip_file is None:
             path = os.path.join(self._path, filename)
             return os.path.isdir(path)
         else:
-            zip_file = zipfile.ZipFile(self._path)
             path = os.path.join(self._zip_root_dir, filename, "")
-            for f in zip_file.namelist():
+            for f in self._zip_file.namelist():
                 if f.startswith(path):
                     return True
             return False
@@ -150,7 +148,7 @@ class Bundle:
         return self._path
 
     def _unzip(self, install_dir):
-        if self._unpacked:
+        if self._zip_file is None:
             raise AlreadyInstalledException
 
         if not os.path.isdir(install_dir):
@@ -163,10 +161,13 @@ class Bundle:
         # FIXME: use manifest
         if os.spawnlp(os.P_WAIT, 'unzip', 'unzip', '-o', self._path,
                       '-x', 'mimetype', '-d', install_dir):
+            # clean up install dir after failure
+            shutil.rmtree(install_dir, ignore_errors=True)
+            # indicate failure.
             raise ZipExtractException
 
     def _zip(self, bundle_path):
-        if not self._unpacked:
+        if self._zip_file is not None:
             raise NotInstalledException
 
         raise NotImplementedError

src/sugar/bundle/contentbundle.py

@@ -195,7 +195,7 @@ class ContentBundle(Bundle):
         return "file://" + urllib.pathname2url(self.get_start_path())
 
     def is_installed(self):
-        if self._unpacked:
+        if self._zip_file is None:
             return True
         elif os.path.isdir(self.get_root_dir()):
             return True
@@ -207,7 +207,7 @@ class ContentBundle(Bundle):
         self._run_indexer()
 
     def uninstall(self):
-        if self._unpacked:
+        if self._zip_file is None:
             if not self.is_installed():
                 raise NotInstalledException
             install_dir = self._path