Patch by Jameson Chema Quinn.

Readability cleanups and little fixes by me.

use MANIFEST. Deprecate bundle_name. fix_manifest().
bundlebuilder.config() cleanup.

src/sugar/activity/bundlebuilder.py

@@ -23,6 +23,8 @@ import subprocess
 import re
 import gettext
 from optparse import OptionParser
+import logging
+from fnmatch import fnmatch
 
 from sugar import env
 from sugar.bundle.activitybundle import ActivityBundle
@@ -31,10 +33,14 @@ def list_files(base_dir, ignore_dirs=None, ignore_files=None):
     result = []
 
     for root, dirs, files in os.walk(base_dir):
-        for f in files:
+        if ignore_files:
-            if ignore_files and f not in ignore_files:
+            for pattern in ignore_files:
+                files = [f for f in files if not fnmatch(f, pattern)]
+                
         rel_path = root[len(base_dir) + 1:]
+        for f in files:
             result.append(os.path.join(rel_path, f))
+            
         if ignore_dirs and root == base_dir:
             for ignore in ignore_dirs:
                 if ignore in dirs:
@@ -43,25 +49,27 @@ def list_files(base_dir, ignore_dirs=None, ignore_files=None):
     return result
 
 class Config(object):
-    def __init__(self, bundle_name):
+    def __init__(self, source_dir=None, dist_dir = None, dist_name = None):
-        self.source_dir = os.getcwd()
+        self.source_dir = source_dir or os.getcwd()
             
-        bundle = ActivityBundle(self.source_dir)
+        self.bundle = bundle = ActivityBundle(self.source_dir)
-        version = bundle.get_activity_version()
+        self.version = bundle.get_activity_version()
-
+        self.activity_name = bundle.get_name()
-        self.bundle_name = bundle_name
-        self.xo_name = '%s-%d.xo' % (self.bundle_name, version)
-        self.tarball_name = '%s-%d.tar.bz2' % (self.bundle_name, version)
         self.bundle_id = bundle.get_bundle_id()
+        self.bundle_name = reduce(lambda x, y:x+y, self.activity_name.split())
         self.bundle_root_dir = self.bundle_name + '.activity'
-        self.tarball_root_dir = '%s-%d' % (self.bundle_name, version)
+        self.tar_root_dir = '%s-%d' % (self.bundle_name, self.version)
 
-        info_path = os.path.join(self.source_dir, 'activity', 'activity.info')
+        if dist_dir:
-        f = open(info_path,'r')
+            self.dist_dir = dist_dir
-        info = f.read()
+        else:
-        f.close()
+            self.dist_dir = os.path.join(self.source_dir, 'dist')
-        match = re.search('^name\s*=\s*(.*)$', info, flags = re.MULTILINE)
+            
-        self.activity_name = match.group(1)
+        if dist_name:
+            self.xo_name = self.tar_name = dist_name
+        else:
+            self.xo_name = '%s-%d.xo' % (self.bundle_name, self.version)
+            self.tar_name = '%s-%d.tar.bz2' % (self.bundle_name, self.version)
 
 class Builder(object):
     def __init__(self, config):
@@ -101,54 +109,70 @@ class Builder(object):
 class Packager(object):
     def __init__(self, config):
         self.config = config
-        self.dist_dir = os.path.join(self.config.source_dir, 'dist')
         self.package_path = None
 
-        if not os.path.exists(self.dist_dir):
+        if not os.path.exists(self.config.dist_dir):
-            os.mkdir(self.dist_dir)
+            os.mkdir(self.config.dist_dir)
             
 
 class BuildPackager(Packager):
-    def __init__(self, config):
-        Packager.__init__(self, config)
-        self.build_dir = self.config.source_dir
-
     def get_files(self):
-        return list_files(self.build_dir,
+        return self.config.bundle.get_files()
-                          ignore_dirs=['po', 'dist', '.git'],
+    
-                          ignore_files=['.gitignore'])
+    def _list_useful_files(self):
+        ignore_dirs = ['dist', '.git'],
+        ignore_files = ['.gitignore', 'MANIFEST', '*.pyc', '*~', '*.bak']
+        
+        return list_files(self.config.source_dir, ignore_dirs, ignore_files)
+        
+    def fix_manifest(self):
+        manifest = self.config.bundle.manifest
+        
+        allfiles = self._list_useful_files()        
+        for path in allfiles:
+            if path not in manifest:
+                manifest.append(path)
+        
+        f = open(os.path.join(self.config.source_dir, "MANIFEST"), "wb")
+        for line in manifest:
+            f.write(line + "\n")
 
 class XOPackager(BuildPackager):
     def __init__(self, config):
         BuildPackager.__init__(self, config)
-        self.package_path = os.path.join(self.dist_dir, self.config.xo_name)
+        self.package_path = os.path.join(self.config.dist_dir,
+                                         self.config.xo_name)
 
     def package(self):
         bundle_zip = zipfile.ZipFile(self.package_path, 'w',
                                      zipfile.ZIP_DEFLATED)
         
         for f in self.get_files():
-            bundle_zip.write(os.path.join(self.build_dir, f),
+            bundle_zip.write(os.path.join(self.config.source_dir, f),
                              os.path.join(self.config.bundle_root_dir, f))
 
         bundle_zip.close()
 
-class SourcePackager(Packager):
+class SourcePackager(BuildPackager):
     def __init__(self, config):
-        Packager.__init__(self, config)
+        BuildPackager.__init__(self, config)
-        self.package_path = os.path.join(self.dist_dir,
+        self.package_path = os.path.join(self.config.dist_dir,
-                                         self.config.tarball_name)
+                                         self.config.tar_name)
 
     def get_files(self):
-        return list_files(self.config.source_dir,
+        git_ls = subprocess.Popen('git-ls-files', stdout=subprocess.PIPE, 
-                          ignore_dirs=['locale', 'dist', '.git'],
+                                  cwd=self.config.source_dir)
-                          ignore_files=['.gitignore'])
+        if git_ls.wait():
+            # Fall back to filtered list
+            return self._list_useful_files()
+        
+        return [path.strip() for path in git_ls.stdout.readlines()]
 
     def package(self):
-        tar = tarfile.open(self.package_path, "w:bz2")
+        tar = tarfile.open(self.package_path, 'w:bz2')
         for f in self.get_files():
             tar.add(os.path.join(self.config.source_dir, f),
-                    os.path.join(self.config.tarball_root_dir, f))
+                    os.path.join(self.config.tar_root_dir, f))
         tar.close()
 
 def cmd_help(config, options, args):
@@ -168,7 +192,7 @@ def cmd_dev(config, options, args):
     bundle_path = env.get_user_activities_path()
     if not os.path.isdir(bundle_path):
         os.mkdir(bundle_path)
-    bundle_path = os.path.join(bundle_path, config.bundle_root_dir)
+    bundle_path = os.path.join(bundle_path, config.bundle_top_dir)
     try:
         os.symlink(config.source_dir, bundle_path)
     except OSError:
@@ -331,11 +355,13 @@ def cmd_build(config, options, args):
     builder = Builder(config)
     builder.build()
 
-def start(bundle_name):
+def start(bundle_name=None):
+    if bundle_name:
+        logging.warn("bundle_name deprecated, now comes from activity.info")
     parser = OptionParser()
     (options, args) = parser.parse_args()
 
-    config = Config(bundle_name)
+    config = Config()
 
     try:
         globals()['cmd_' + args[0]](config, options, args[1:])

src/sugar/bundle/activitybundle.py

@@ -56,7 +56,7 @@ class ActivityBundle(Bundle):
         self._show_launcher = True
         self._activity_version = 0
 
-        info_file = self._get_file('activity/activity.info')
+        info_file = self.get_file('activity/activity.info')
         if info_file is None:
             raise MalformedBundleException('No activity.info file')
         self._parse_info(info_file)
@@ -65,6 +65,66 @@ class ActivityBundle(Bundle):
         if linfo_file:
             self._parse_linfo(linfo_file)
 
+        self.manifest = None #This should be replaced by following function
+        self.read_manifest()
+
+    def _raw_manifest(self):
+        f = self.get_file("MANIFEST")
+        if not f:
+            logging.warning("Activity directory lacks a MANIFEST file.")
+            return []
+        
+        ret = [line.strip() for line in f.readlines()] 
+        f.close()
+        return ret
+        
+    def read_manifest(self):
+        """read_manifest: sets self.manifest to list of lines in MANIFEST, 
+        with invalid lines replaced by empty lines.
+        
+        Since absolute order carries information on file history, it should 
+        be preserved. For instance, when renaming a file, you should leave
+        the new name on the same line as the old one.
+        """
+        lines = self._raw_manifest()
+
+        # Remove trailing newlines, they do not help keep absolute position.
+        while lines and lines[-1] == "":
+            lines = lines[:-1]
+
+        for num, line in enumerate(lines):
+            if not line:
+                continue
+
+            # Remove duplicates
+            if line in lines[0:num]:
+                lines[num] = ""
+                logging.warning("Bundle %s: duplicate entry in MANIFEST: %s"
+                                % (self._name,line))
+                continue
+            
+            # Remove MANIFEST
+            if line == "MANIFEST":
+                lines[num] = ""
+                logging.warning("Bundle %s: MANIFEST includes itself: %s"
+                                % (self._name,line))
+                
+            # Remove invalid files
+            if not self.is_file(line):
+                lines[num] = ""
+                logging.warning("Bundle %s: invalid entry in MANIFEST: %s"
+                                % (self._name,line))
+
+        self.manifest = lines
+    
+    def get_files(self, manifest = None):
+        files = [line for line in (manifest or self.manifest) if line]
+
+        if self.is_file('MANIFEST'):
+            files.append('MANIFEST')
+
+        return files
+      
     def _parse_info(self, info_file):
         cp = ConfigParser()
         cp.readfp(info_file)
@@ -123,12 +183,12 @@ class ActivityBundle(Bundle):
             return None
 
         linfo_path = os.path.join('locale', lang, 'activity.linfo')
-        linfo_file = self._get_file(linfo_path)
+        linfo_file = self.get_file(linfo_path)
         if linfo_file is not None:
             return linfo_file
 
         linfo_path = os.path.join('locale', lang[:2], 'activity.linfo')
-        linfo_file = self._get_file(linfo_path)
+        linfo_file = self.get_file(linfo_path)
         if linfo_file is not None:
             return linfo_file
 
@@ -180,7 +240,7 @@ class ActivityBundle(Bundle):
         if self._unpacked:
             return os.path.join(self._path, icon_path)
         else:
-            icon_data = self._get_file(icon_path).read()
+            icon_data = self.get_file(icon_path).read()
             temp_file, temp_file_path = tempfile.mkstemp(self._icon)
             os.write(temp_file, icon_data)
             os.close(temp_file)
@@ -221,17 +281,38 @@ class ActivityBundle(Bundle):
         else:
             return False
     
-    def install(self):
+    def unpack(self, install_dir, strict_manifest=False):
-        activities_path = env.get_user_activities_path()
-        act = activity.get_registry().get_activity(self._bundle_id)
-        if act is not None and act.path.startswith(activities_path):
-            raise AlreadyInstalledException
-
-        install_dir = env.get_user_activities_path()
         self._unzip(install_dir)
 
         install_path = os.path.join(install_dir, self._zip_root_dir)
         
+        # List installed files
+        manifestfiles = self.get_files(self._raw_manifest())
+        paths  = []
+        for root, dirs, files in os.walk(install_path):
+            rel_path = root[len(install_path) + 1:]
+            for f in files:
+                paths.append(os.path.join(rel_path, f))
+                
+        # Check the list against the MANIFEST
+        for path in paths:
+            if path in manifestfiles:
+                manifestfiles.remove(path)
+            elif path != "MANIFEST":
+                logging.warning("Bundle %s: %s not in MANIFEST"%
+                                (self._name,path))
+                if strict_manifest:
+                    os.remove(os.path.join(install_path, path))
+                    
+        # Is anything in MANIFEST left over after accounting for all files?
+        if manifestfiles:
+            err = ("Bundle %s: files in MANIFEST not included: %s"%
+                   (self._name,str(manifestfiles)))
+            if strict_manifest:
+                raise MalformedBundleException(err)
+            else:
+                logging.warning(err)
+
         xdg_data_home = os.getenv('XDG_DATA_HOME',
                                   os.path.expanduser('~/.local/share'))
 
@@ -267,6 +348,16 @@ class ActivityBundle(Bundle):
                     os.symlink(info_file,
                                os.path.join(installed_icons_dir,
                                             os.path.basename(info_file)))
+        return install_path
+
+    def install(self):
+        activities_path = env.get_user_activities_path()
+        act = activity.get_registry().get_activity(self._bundle_id)
+        if act is not None and act.path.startswith(activities_path):
+            raise AlreadyInstalledException
+
+        install_dir = env.get_user_activities_path()
+        install_path = self.unpack(install_dir)
         
         if not activity.get_registry().add_bundle(install_path):
             raise RegistrationException

src/sugar/bundle/bundle.py

@@ -96,13 +96,15 @@ class Bundle:
                     'All files in the bundle must be inside a single ' +
                     'top-level directory')
 
-    def _get_file(self, filename):
+    def get_file(self, filename):
         f = None
 
         if self._unpacked:
             path = os.path.join(self._path, filename)
-            if os.path.isfile(path):
+            try:
-                f = open(path)
+                f = open(path,"rb")
+            except IOError:
+                return None
         else:
             zip_file = zipfile.ZipFile(self._path)
             path = os.path.join(self._zip_root_dir, filename)
@@ -115,6 +117,23 @@ class Bundle:
 
         return f
     
+    def is_file(self, filename):
+        if self._unpacked:
+            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)
+            except KeyError:
+                return False
+            finally:
+                zip_file.close()
+
+            return True
+                
+
     def get_path(self):
         """Get the bundle path."""
         return self._path