Merge lp:~james-w/linaro-image-tools/split-package-fetcher into lp:linaro-image-tools/11.11

[James Westby]

Hi,

This split PackageFetcher in to two, adding IsolatedAptCache. This
is useful if you want that functionality, but without aiming to download
packages. For instance, I'll want this in a minute in some tests.

It leaves prepare() and cleanup() of PackageFetcher fairly untested, but
I don't want to test the behaviour of those methods on IsolatedAptCache
twice.

Thanks,

James

[Zygmunt Krynicki (zyga)]

I did not follow hwpack development from the start but this seems like a simple refactoring. I ran the tests, read the diff and cannot point at anything broken.

Approve

review: Approve

=== modified file 'hwpack/packages.py'
--- hwpack/packages.py	2010-09-10 20:53:40 +0000
+++ hwpack/packages.py	2010-09-10 20:53:40 +0000
@@ -108,14 +108,15 @@
             (self.name, self.version, self.filename, self.size, self.md5))
 
 
-class PackageFetcher(object):
-    """A class to fetch packages from a defined list of sources."""
+class IsolatedAptCache(object):
+    """A apt.cache.Cache wrapper that isolates it from the system it runs on.
+
+    :ivar cache: the isolated cache.
+    :type cache: apt.cache.Cache
+    """
 
     def __init__(self, sources, architecture=None):
-        """Create a PackageFetcher.
-
-        Once created a PackageFetcher should have its `prepare` method
-        called before use.
+        """Create an IsolatedAptCache.
 
         :param sources: a list of sources such that they can be prefixed
             with "deb " and fed to apt.
@@ -128,7 +129,7 @@
         self.tempdir = None
 
     def prepare(self):
-        """Prepare a PackageFetcher for use.
+        """Prepare the IsolatedAptCache for use.
 
         Should be called before use, and after any modification to the list
         of sources.
@@ -166,7 +167,7 @@
     def cleanup(self):
         """Cleanup any remaining artefacts.
 
-        Should be called on all PackageFetchers when they are finished
+        Should be called on all IsolatedAptCache when they are finished
         with.
         """
         if self.tempdir is not None and os.path.exists(self.tempdir):
@@ -176,6 +177,46 @@
         self.cleanup()
         return False
 
+
+class PackageFetcher(object):
+    """A class to fetch packages from a defined list of sources."""
+
+    def __init__(self, sources, architecture=None):
+        """Create a PackageFetcher.
+
+        Once created a PackageFetcher should have its `prepare` method
+        called before use.
+
+        :param sources: a list of sources such that they can be prefixed
+            with "deb " and fed to apt.
+        :type sources: an iterable of str
+        :param architecture: the architecture to fetch packages for.
+        :type architecture: str
+        """
+        self.cache = IsolatedAptCache(sources, architecture=architecture)
+
+    def prepare(self):
+        """Prepare the PackageFetcher for use.
+
+        Should be called before use.
+        """
+        self.cache.prepare()
+        return self
+
+    __enter__ = prepare
+
+    def cleanup(self):
+        """Cleanup any remaining artefacts.
+
+        Should be called on all PackageFetchers when they are finished
+        with.
+        """
+        self.cache.cleanup()
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        self.cleanup()
+        return False
+
     def fetch_packages(self, packages):
         """Fetch the files for the given list of package names.
 
@@ -189,9 +230,9 @@
         """
         results = []
         for package in packages:
-            candidate = self.cache[package].candidate
+            candidate = self.cache.cache[package].candidate
             base = os.path.basename(candidate.filename)
-            destfile = os.path.join(self.tempdir, base)
+            destfile = os.path.join(self.cache.tempdir, base)
             acq = apt_pkg.Acquire(DummyProgress())
             acqfile = apt_pkg.AcquireFile(
                 acq, candidate.uri, candidate.md5, candidate.size,

=== modified file 'hwpack/tests/test_packages.py'
--- hwpack/tests/test_packages.py	2010-09-10 20:53:40 +0000
+++ hwpack/tests/test_packages.py	2010-09-10 20:53:40 +0000
@@ -5,6 +5,7 @@
 from testtools import TestCase
 
 from hwpack.packages import (
+    IsolatedAptCache,
     FetchedPackage,
     get_packages_file,
     PackageFetcher,
@@ -119,87 +120,98 @@
         self.assertEqual(hash(package1), hash(package2))
 
 
-class PackageFetcherTests(TestCaseWithFixtures):
+class AptCacheTests(TestCaseWithFixtures):
 
     def test_cleanup_removes_tempdir(self):
-        fetcher = PackageFetcher([])
-        fetcher.prepare()
-        tempdir = fetcher.tempdir
-        fetcher.cleanup()
+        cache = IsolatedAptCache([])
+        cache.prepare()
+        tempdir = cache.tempdir
+        cache.cleanup()
         self.assertFalse(os.path.exists(tempdir))
 
     def test_cleanup_ignores_missing_tempdir(self):
-        fetcher = PackageFetcher([])
-        fetcher.prepare()
-        tempdir = fetcher.tempdir
-        fetcher.cleanup()
+        cache = IsolatedAptCache([])
+        cache.prepare()
+        tempdir = cache.tempdir
+        cache.cleanup()
         # Check that there is no problem removing it again
-        fetcher.cleanup()
+        cache.cleanup()
 
     def test_cleanup_before_prepare(self):
-        fetcher = PackageFetcher([])
+        cache = IsolatedAptCache([])
         # Check that there is no problem cleaning up before we start
-        fetcher.cleanup()
+        cache.cleanup()
 
     def test_prepare_creates_tempdir(self):
-        fetcher = PackageFetcher([])
-        self.addCleanup(fetcher.cleanup)
-        fetcher.prepare()
-        self.assertTrue(os.path.isdir(fetcher.tempdir))
+        cache = IsolatedAptCache([])
+        self.addCleanup(cache.cleanup)
+        cache.prepare()
+        self.assertTrue(os.path.isdir(cache.tempdir))
 
     def test_prepare_creates_var_lib_dpkg_status_file(self):
-        fetcher = PackageFetcher([])
-        self.addCleanup(fetcher.cleanup)
-        fetcher.prepare()
+        cache = IsolatedAptCache([])
+        self.addCleanup(cache.cleanup)
+        cache.prepare()
         self.assertEqual(
             '',
             open(os.path.join(
-                fetcher.tempdir, "var", "lib", "dpkg", "status")).read())
+                cache.tempdir, "var", "lib", "dpkg", "status")).read())
 
     def test_prepare_creates_var_cache_apt_archives_partial_dir(self):
-        fetcher = PackageFetcher([])
-        self.addCleanup(fetcher.cleanup)
-        fetcher.prepare()
+        cache = IsolatedAptCache([])
+        self.addCleanup(cache.cleanup)
+        cache.prepare()
         self.assertTrue(
             os.path.isdir(os.path.join(
-                fetcher.tempdir, "var", "cache", "apt", "archives",
+                cache.tempdir, "var", "cache", "apt", "archives",
                 "partial")))
 
     def test_prepare_creates_var_lib_apt_lists_partial_dir(self):
-        fetcher = PackageFetcher([])
-        self.addCleanup(fetcher.cleanup)
-        fetcher.prepare()
+        cache = IsolatedAptCache([])
+        self.addCleanup(cache.cleanup)
+        cache.prepare()
         self.assertTrue(
             os.path.isdir(os.path.join(
-                fetcher.tempdir, "var", "lib", "apt", "lists", "partial")))
+                cache.tempdir, "var", "lib", "apt", "lists", "partial")))
 
     def test_prepare_creates_etc_apt_sources_list_file(self):
         source1 = self.useFixture(AptSourceFixture([]))
         source2 = self.useFixture(AptSourceFixture([]))
-        fetcher = PackageFetcher(
+        cache = IsolatedAptCache(
             [source1.sources_entry, source2.sources_entry])
-        self.addCleanup(fetcher.cleanup)
-        fetcher.prepare()
+        self.addCleanup(cache.cleanup)
+        cache.prepare()
         self.assertEqual(
             "deb %s\ndeb %s\n" % (
                 source1.sources_entry, source2.sources_entry),
             open(os.path.join(
-                fetcher.tempdir, "etc", "apt", "sources.list")).read())
+                cache.tempdir, "etc", "apt", "sources.list")).read())
 
     def test_prepare_with_arch_creates_etc_apt_apt_conf(self):
-        fetcher = PackageFetcher([], architecture="arch")
-        self.addCleanup(fetcher.cleanup)
-        fetcher.prepare()
+        cache = IsolatedAptCache([], architecture="arch")
+        self.addCleanup(cache.cleanup)
+        cache.prepare()
         self.assertEqual(
             'Apt {\nArchitecture "arch";\n}\n',
             open(os.path.join(
-                fetcher.tempdir, "etc", "apt", "apt.conf")).read())
+                cache.tempdir, "etc", "apt", "apt.conf")).read())
+
+    def test_context_manager(self):
+        # A smoketest that IsolatedAptCache can be used as a context
+        # manager
+        with IsolatedAptCache([]) as cache:
+            tempdir = cache.tempdir
+            self.assertTrue(os.path.isdir(tempdir))
+        self.assertFalse(os.path.exists(tempdir))
+
+
+class PackageFetcherTests(TestCaseWithFixtures):
 
     def test_context_manager(self):
         # A smoketest that PackageFetcher can be used as a context
         # manager
         with PackageFetcher([]) as fetcher:
-            tempdir = fetcher.tempdir
+            tempdir = fetcher.cache.tempdir
             self.assertTrue(os.path.isdir(tempdir))
         self.assertFalse(os.path.exists(tempdir))