Merge lp:~mac9416/unwrapt/modularization into lp:unwrapt

[mac9416]

I tried to modularize a couple of things, and in the process hacked the existing code to pieces. There's still a lot of work to do, so feel free to do the same to mine. :-)

[mac9416 (mac9416)]

lp:unwrapt was out of date. I've updated it to lp:~excid3/keryx/unwrapt. Resubmitting.

[Chris Oliver (excid3)]

Yeah I did not update it because I wanted to make sure recent changes were tested in my personal branch before pushing. I think it was all good and I forgot to push. :P

[Chris Oliver (excid3)]

review: Approve

=== modified file 'example.py'
--- example.py	2010-08-01 19:34:08 +0000
+++ example.py	2010-08-13 17:34:44 +0000
@@ -29,7 +29,8 @@
 #apt.set_proxy({"http": "http://192.168.1.100:3128"}, "username", "password")
 
 # Configure the apt client
-apt.set_architecture("amd64")
+#apt.set_architecture("amd64")
+apt.set_architecture("i386")
 
 apt.set_status("/var/lib/dpkg/status")
 
@@ -51,7 +52,10 @@
 
 print "%i available packages" % len(apt.get_available_package_names())
 
-#apt.update(directory="/var/lib/apt/lists", download=False)
+#FIXME: /var/lib/apt is not the download directory. We should have the ability
+#       to individually specify the download directories.
+#apt.set_download_directory("/var/lib/apt")
+#apt.update(download=False)
 
 apt.update()
 

=== modified file 'unwrapt/DefinitionBase.py'
--- unwrapt/DefinitionBase.py	2010-08-01 19:34:08 +0000
+++ unwrapt/DefinitionBase.py	2010-08-13 17:34:44 +0000
@@ -90,6 +90,7 @@
             
             - directory is the location of the 
         """
+        #FIXME: that docstring.
         
         self.download_directory = os.path.abspath(os.path.expanduser(directory))
 
@@ -163,6 +164,7 @@
         
         pass
 
+
     @callback        
     def update(self, reporthook=None, directory=None, download=True):
         """
@@ -202,6 +204,7 @@
         
         pass
         
+
     @callback
     def get_latest_binary(self, package):
         """
@@ -268,7 +271,8 @@
             
             For example:
             
-            client.mark_package("firefox")
+            package = client.get_latest_binary("firefox")
+            client.mark_package(package)
         """
         
         pass
@@ -289,7 +293,6 @@
             For example:
             
             client.apply_changes()
-            
         """
         
         pass

=== modified file 'unwrapt/definitions/aptdef/__init__.py'
--- unwrapt/definitions/aptdef/__init__.py	2010-08-01 19:34:08 +0000
+++ unwrapt/definitions/aptdef/__init__.py	2010-08-13 17:34:44 +0000
@@ -51,14 +51,12 @@
 
 
 #TODO: Move this code to proper library location
-def url_join(first, last):
+def url_join(*args):
     """ Returns full URL """
-    if first.endswith('/'):
-        if last.startswith('/'): return first + last[1:]
-        else:                    return first + last
-    else:
-        if last.startswith('/'): return first + last
-        else:                    return first + '/' + last
+    # Strip any leading or trailing slashes from the parts.
+    args = [x.strip("/") for x in args]
+
+    return "/".join(args)
 
 
 #class Repository(Base):
@@ -108,10 +106,15 @@
 
 
 def to_url(repository, architecture, format):
-    return url_join(repository["url"], url_join(architecture, format))
+    return url_join(repository["url"], architecture, format)
 
 
 def to_filename(directory, url):
+    """
+       Forms a full filename from a directory and url.
+       i.e. Strips the url of the protocol prefix, replaces all slashes with 
+       underscores, and appends it to directory.
+    """
     return os.path.join(directory, url.split("//")[1].replace("/", "_"))
 
             
@@ -147,8 +150,13 @@
     supported = ["amd64", "armel", "i386", "ia64", "powerpc", "sparc"]
     status_properties = ["Package", "Version", "Status", "Provides"]
     binary_dependencies = ["Pre-Depends", "Depends", "Recommends"]
-    supported_statuses = ["install ok installed", "to be installed", "to be downloaded"]
+    supported_statuses = ["install ok installed", 
+                          "to be downloaded",  
+                          "dependency to be downloaded",
+                          "to be installed", 
+                          "dependency to be installed"]
 
+    #FIXME: This seems redundant. Could it be moved to DefinitionBase?
     def on_set_proxy(self, proxy, username=None, password=None):
         self.proxy = {"proxy": proxy,
                       "user": username,
@@ -184,7 +192,7 @@
                 self.repositories[count]["url"] = url
                 self.repositories[count]["dist"] = dist
                 self.repositories[count]["section"] = section
-                self.repositories[count]["url"] = url_join(url, url_join("dists", url_join(dist, section)))
+                self.repositories[count]["url"] = url_join(url, "dists", dist, section)
 
                 count += 1
 
@@ -218,13 +226,22 @@
         """
             This is a missing docstring ZOMG!
         """
+
+        if download:
+            self.on_download_lists(reporthook)
+
+        # Read the newly-downloaded lists.
+        self.on_read_lists()
+
+
+    def on_download_lists(self, reporthook=None):
         
         directory = os.path.join(self.download_directory, "lists")
 
-        #TODO: This function obviously needs to be split up and modularized :)
+        # If the download directory does not exist, create it
+        if not os.path.exists(directory):
+            os.makedirs(directory)
 
-        # This is a list of files we downloaded and now need to parse
-        downloaded = []
         for repo in self.__iter_repositories():
 
             # Build the strings
@@ -232,31 +249,31 @@
             filename = to_filename(directory, url)
             display_name = "Repository => %s / %s" % (repo["dist"], repo["section"])
 
-            # If the download directory does not exist, create it
-            if not os.path.exists(directory):
-                os.makedirs(directory)
-
             # Download
             #TODO: pass proxy information and catch exceptions
             #TODO: Support bz2 and unarchived Packages files
             filename = "%s.gz" % filename
-            if download:
-                download_url("%s.gz" % url, filename, display_name, proxy=self.proxy["proxy"], username=self.proxy["user"], password=self.proxy["pass"])
-                downloaded.append((repo, filename))
-                
-            #TODO: Improve this. For now we are just opening local files in 
-            # unextracted format (what you find in /var/lib/apt/lists) since
-            # that's an easy way to do things. This won't open the gz files
-            # that Unwrapt downloads however
-            else: # Files that are pre-downloaded
-                downloaded.append((repo, filename[:-3]))
-            
+            download_url("%s.gz" % url, filename, display_name, proxy=self.proxy["proxy"], username=self.proxy["user"], password=self.proxy["pass"])
+
+
+    def on_read_lists(self):
+        
+        directory = os.path.join(self.download_directory, "lists")
+
+        lists = []
+        for repo in self.__iter_repositories():
+
+            # Build the strings
+            url = to_url(repo, self.architecture, "Packages")
+            filename = to_filename(directory, url)
+            filename = "%s.gz" % filename  # Works only if the index files are gz
+            lists.append((repo, filename))
             
         self.packages = {}
  
-        total = len(downloaded)
+        total = len(lists)
         # Now parse each file, extracting as necessary
-        for i, value in enumerate(downloaded):
+        for i, value in enumerate(lists):
             repo, filename = value
 
             # Display percent read            
@@ -264,17 +281,19 @@
             sys.stdout.write("\rReading package lists... %3i%%" % frac)
             sys.stdout.flush()
 
-            # Parse packages into dictionary
+            # Attempt to open the package list.
             try:
                 if filename.endswith(".gz"):
                     f = gzip.open(filename, "rb")
                 else:
-                    f = open(filename, "rb")    
-                    
-                self.__parse(repo, f)
-                f.close()
-            except:
-                logging.error("\nPackage list does not exist: %s" % filename)
+                    f = open(filename, "rb")   
+            except:  #FIXME: specify exception.
+                logging.error("\nPackage list does not exist: %s" % filename) 
+                continue
+
+            # Parse packages into dictionary
+            self.__parse(repo, f)
+            f.close()
             
             #TODO: Insert items into database
 
@@ -282,6 +301,7 @@
         sys.stdout.write("\n")
         
         logging.info("%i packages available" % len(self.packages))
+        
 
     def __parse(self, repo, f):
         """
@@ -328,7 +348,6 @@
             installed statuses.
         """
 
-
         f = open(status, "rb")
         
         self.status = {}
@@ -420,7 +439,7 @@
         return self.packages[package]
 
 
-    def on_mark_package(self, metadata):
+    def on_mark_package(self, metadata, dependency=False):
         """
             Get a list of dependencies based on package metadata
         """
@@ -431,25 +450,21 @@
         #TODO: This function obviously needs to be split up and modularized :)
 
         # First check if the package is installed already?
-        if metadata["Package"] in self.status:
-            raise AttributeError, "Package already set to status: %s" % \
-                self.status[metadata["Package"]]["Status"]
+        status = self.on_get_package_status(metadata["Package"])
+        if status != "not installed":
+            raise AttributeError, "Package already set to status: %s" % status
         
         # Mark the package itself
-        metadata["Status"] = "to be downloaded"
+        if not dependency: metadata["Status"] = "to be downloaded"
+        else: metadata["Status"] = "dependency to be downloaded"
         self.status[metadata["Package"]] = metadata
 
         logging.info("Finding dependencies for %s..." % metadata["Package"])
 
-        # Build a string of the necessary sections we need
-        depends = []
-        for section in self.binary_dependencies:
-            if section in metadata:
-                depends.append(metadata[section])
-        depends = ", ".join(depends)
+        depends = self.on_get_package_dependencies(metadata)
         
         # Do the dependency calculations
-        for dep in depends.split(", "):
+        for dep in depends:
             
             # In case we have some ORs
             options = dep.split(" | ")
@@ -470,7 +485,7 @@
                     # Test for compatible version just in case
                     if len(details) > 1:
                         comparison = details[1][1:] # strip the '('
-                        version = details [2][:-1] # strip the ')'
+                        version = details[2][:-1] # strip the ')'
                         
                         satisfied = DpkgVersion(self.status[name]["Version"]).compare_string(comparison, version)
                         
@@ -492,15 +507,26 @@
                 
                 # Mark sub-dependencies as well
                 if pkg:
-                    self.on_mark_package(pkg)
-                
-                
+                    self.on_mark_package(pkg, dependency=True)
+
+
+    def on_get_package_dependencies(self, metadata):
+
+        # Build a string of the necessary sections we need
+        depends = []
+        for section in self.binary_dependencies:
+            if section in metadata:
+                depends += metadata[section].split(", ")
+
+        return depends
+
+
     def on_apply_changes(self):
         
         directory = os.path.join(self.download_directory, "packages")
         
         # Build the list of package urls to download
-        downloads = [(key, value["Repository"]["url"].split("dists")[0] + value["Filename"]) for key, value in self.status.items() if value["Status"] == "to be downloaded"]
+        downloads = [(key, value["Repository"]["url"].split("dists")[0] + value["Filename"]) for key, value in self.status.items() if value["Status"] in ["to be downloaded", "dependency to be downloaded"]]
         
         #downloads = []
         #for key, value in self.status.items():
@@ -517,12 +543,16 @@
         for key, url in downloads:
             download_url(url, "%s/%s" % (directory, url.rsplit("/", 1)[1]), proxy=self.proxy["proxy"], username=self.proxy["user"], password=self.proxy["pass"])
             # Once it's downloaded, mark this package status to "to be installed"
-            self.status[key]["Status"] = "to be installed"
+            # or "dependency to be installed", depending on what it is now.
+            if self.status[key]["Status"] == "to be downloaded":
+                self.status[key]["Status"] = "to be installed"
+            elif self.status[key]["Status"] == "dependency to be downloaded":
+                self.status[key]["Status"] = "dependency to be installed"
         
         
     def on_save_changes(self, status):
     
-        # This will NOT create a staus file to override /var/lib/dpkg/status
+        # This will NOT create a status file to override /var/lib/dpkg/status
         # so DO NOT try to replace the system status file.
         # YOU HAVE BEEN WARNED
         
@@ -541,22 +571,19 @@
         
         
     def on_cancel_changes(self, downloads, installs):
-    
-        cancellations = []
         
         for key, value in self.status.items():
-            if downloads and value["Status"] == "to be downloaded" or \
-               installs and value["Status"] == "to be installed":
-               cancellations.append(key)
-               
-        for key in cancellations:
-            del self.status[key]
+            if downloads and value["Status"] in \
+                    ["to be downloaded", "dependency to be downloaded"] or \
+               installs and value["Status"] in \
+                    ["to be installed", "dependency to be installed"]:
+                del self.status[key]
         
         
     def on_get_changes_size(self):
     
         # Build list of packages to be downloaded
-        packages = [(value["Package"], value["Version"]) for key, value in self.status.items() if value["Status"] == "to be downloaded"]
+        packages = [(value["Package"], value["Version"]) for key, value in self.status.items() if value["Status"] in ["to be downloaded", "dependency to be downloaded"]]
 
         count = 0
         total = 0        
@@ -582,36 +609,36 @@
             We will take the approach of installing by copying the lists to
             /var/lib/apt/lists and the packages to /var/cache/apt/archives and
             calling apt-get update and then apt-get install on the packages 
-            which have the stats of "to be installed". This prevents tampering
+            which have the status of "to be installed". This prevents tampering
             with sources.list and works more or less the exact same if we made
             a local repository.
         """
         
-        if not os.geteuid()==0:    
+        if not os.geteuid() == 0:    
             raise PermissionsError, "You may only install as root"
         
         # Copy lists over
-        try:
-            for repo in self.__iter_repositories():
-                url = to_url(repo, self.architecture, "Packages")
-                filename = to_filename(os.path.join(self.download_directory, "lists"), url)
+        for repo in self.__iter_repositories():
+            url = to_url(repo, self.architecture, "Packages")
+            filename = to_filename(os.path.join(self.download_directory, "lists"), url)
 
+            try:
                 # Extract the gz
                 g = gzip.open("%s.gz" % filename, "rb")
                 f = open(os.path.join("/var/lib/apt/lists", os.path.basename(filename)), "wb")
                 f.write(g.read())
                 f.close()
                 g.close()
-        except IOError, e:
-            # We will just ignore this, it only trip out if the user did download=False on update()
-            pass
+            except IOError, e:
+                # We will just ignore this, it only trip out if the user did download=False on update()
+                pass
 
         
         # Copy packages over
         for key, value in self.status.items():
-            if value["Status"] == "to be installed":
+            if value["Status"] in ["to be installed", "dependency to be installed"]:
                 pkg_filename = self.get_binary_version(value["Package"], value["Version"])["Filename"].rsplit("/", 1)[1]
-                filename = os.path.join(self.download_directory, os.path.join("packages", pkg_filename))
+                filename = os.path.join(self.download_directory, "packages", pkg_filename)
                 dest = os.path.join("/var/cache/apt/archives", os.path.basename(filename))
                 shutil.copyfile(filename, dest)
 
@@ -619,7 +646,7 @@
         # Call apt-get install with the packages
         packages = [value["Package"] for key, value in self.status.items() if value["Status"] == "to be installed"]
         
-        subprocess.call("apt-get update", shell=True)
+        subprocess.call("apt-gcache gencaches", shell=True)
         subprocess.call("apt-get -y install %s" % " ".join(packages), shell=True)
         
         
@@ -642,5 +669,3 @@
 
         
         return upgrades
-        
-