Merge lp:~james-w/linaro-image-tools/fetch-packages into lp:linaro-image-tools/11.11

[James Westby]

Hi,

Here's the result of my investigations in to how we can fetch packages.

It's small code which I like. It's currently limited to fetching just the
specified packages, as we don't really know what we want there yet. I'm
assuming that if we need to then python-apt can allow us to resolve
dependencies too.

I have two issues with this change, the first being the possible licensing
impact of using python-apt. It is GPL2+ licensed, which I believe would
mean that this code would need to be too, which means getting a TSC
exemption, unless that has already been done for linaro-image-tools.

The second is the growing amount of code in testing.py with no tests.
Should I be testing that code too?

Thanks,

James

[lp:~james-w/linaro-image-tools/fetch-packages]

84. By James Westby on 2010-09-01

Use apt rather than bzrlib, with our own method to avoid the stdout output.

85. By James Westby on 2010-09-01

Remove the unneeded ugly code.

86. By James Westby on 2010-09-02

Merged create-hwpack-skeleton into fetch-packages.

87. By James Westby on 2010-09-02

Use a class as the return value such that we can return name, version and filename.

[Michael Hudson-Doyle (mwhudson)]

FWIW about the licensing, http://bazaar.launchpad.net/~linaro-maintainers/linaro-image-tools/linaro-image-tools/revision/44 put linaro-media-create under GPLv3.

[James Westby (james-w)]

> FWIW about the licensing, http://bazaar.launchpad.net/~linaro-maintainers
> /linaro-image-tools/linaro-image-tools/revision/44 put linaro-media-create
> under GPLv3.

I just saw that too. I guess that means we will have no problem, but I will
check with Loïc.

Thanks,

James

[Michael Hudson-Doyle (mwhudson)]

As for the code, I think it's by and large fine, bearing in mind that I don't know how python-apt works.

I don't know if the stuff in testing.py needs tests, but it certainly needs documentation. I also wonder if AptSource should be called AptSourceFixture or something.

DummyProgress could use a docstring. After all that effort going to silencing python-apt, some kind of progress is likely to be desired, I think... I guess this can be carefully added back in later.

fetch_packages doesn't document its return type correctly any more.

All else looks good.

review: Approve

[lp:~james-w/linaro-image-tools/fetch-packages]

88. By James Westby on 2010-09-02

Add some documentation for the Package object.

89. By James Westby on 2010-09-02

Make Package in to DummyFetchedPackage to share an interface.

90. By James Westby on 2010-09-02

Add more documentation to testing.py and rename AptSource to AptSourceFixture

91. By James Westby on 2010-09-02

Tweak docstrings in hwpack.packages. Thanks Michael.

=== added file 'hwpack/packages.py'
--- hwpack/packages.py	1970-01-01 00:00:00 +0000
+++ hwpack/packages.py	2010-09-02 15:37:46 +0000
@@ -0,0 +1,165 @@
+import os
+import shutil
+import tempfile
+
+from apt.cache import Cache
+from apt.package import FetchError
+import apt_pkg
+
+
+class DummyProgress(object):
+    """An AcquireProgress that silences all output.
+
+    This can be used to ensure that apt produces no output
+    when fetching files.
+    """
+
+    def start(self):
+        pass
+
+    def ims_hit(self, item):
+        pass
+
+    def fail(self, item):
+        pass
+
+    def fetch(self, item):
+        pass
+
+    def pulse(self, owner):
+        return True
+
+    def media_change(self):
+        return False
+
+    def stop(self):
+        pass
+
+
+class FetchedPackage(object):
+    """The result of fetching packages.
+
+    :ivar name: the name of the fetched package.
+    :type name: str
+    :ivar version: the version of the fetched package.
+    :type version: str
+    :ivar filename: the filename that the package has.
+    :type filename: str
+    :ivar content: a file that the content of the package can be read from.
+    :type content: a file-like object
+    :ivar size: the size of the package
+    :type size: int
+    :ivar md5: the hex representation of the md5sum of the contents of
+        the package.
+    :type md5: str
+    """
+
+    def __init__(self, name, version, filename, content, size, md5):
+        """Create a FetchedPackage.
+
+        See the instance variables for the arguments.
+        """
+        self.name = name
+        self.version = version
+        self.filename = filename
+        self.content = content
+        self.size = size
+        self.md5 = md5
+
+    def __eq__(self, other):
+        return (self.name == other.name
+                and self.version == other.version
+                and self.filename == other.filename
+                and self.content.read() == other.content.read()
+                and self.size == other.size
+                and self.md5 == other.md5)
+
+    def __hash__(self):
+        return hash(
+            (self.name, self.version, self.filename, self.size, self.md5))
+
+
+class PackageFetcher(object):
+    """A class to fetch packages from a defined list of sources."""
+
+    def __init__(self, sources):
+        """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
+        """
+        self.sources = sources
+        self.tempdir = None
+
+    def prepare(self):
+        """Prepare a PackageFetcher for use.
+
+        Should be called before use, and after any modification to the list
+        of sources.
+        """
+        self.cleanup()
+        self.tempdir = tempfile.mkdtemp(prefix="hwpack-apt-cache-")
+        files = ["var/lib/dpkg/status",
+                ]
+        dirs = ["var/lib/dpkg",
+                "etc/apt/",
+                "var/cache/apt/archives/partial",
+                "var/lib/apt/lists/partial",
+               ]
+        for d in dirs:
+            os.makedirs(os.path.join(self.tempdir, d))
+        for fn in files:
+            with open(os.path.join(self.tempdir, fn), 'w'):
+                pass
+        sources_list = os.path.join(
+            self.tempdir, "etc", "apt", "sources.list")
+        with open(sources_list, 'w') as f:
+            for source in self.sources:
+                f.write("deb %s\n" % source)
+        self.cache = Cache(rootdir=self.tempdir, memonly=True)
+        self.cache.update()
+        self.cache.open()
+
+    def cleanup(self):
+        """Cleanup any remaining artefacts.
+
+        Should be called on all PackageFetchers when they are finished
+        with.
+        """
+        if self.tempdir is not None and os.path.exists(self.tempdir):
+            shutil.rmtree(self.tempdir)
+
+    def fetch_packages(self, packages):
+        """Fetch the files for the given list of package names.
+
+        :param packages: a list of package names to install
+        :type packages: an iterable of str
+        :return: a list of the packages that were fetched, with relevant
+            metdata and the contents of the files available.
+        :rtype: an iterable of FetchedPackages.
+        :raises KeyError: if any of the package names in the list couldn't
+            be found.
+        """
+        results = []
+        for package in packages:
+            candidate = self.cache[package].candidate
+            base = os.path.basename(candidate.filename)
+            destfile = os.path.join(self.tempdir, base)
+            acq = apt_pkg.Acquire(DummyProgress())
+            acqfile = apt_pkg.AcquireFile(
+                acq, candidate.uri, candidate.md5, candidate.size,
+                base, destfile=destfile)
+            acq.run()
+            if acqfile.status != acqfile.STAT_DONE:
+                raise FetchError(
+                    "The item %r could not be fetched: %s" %
+                    (acqfile.destfile, acqfile.error_text))
+            result_package = FetchedPackage(
+                candidate.package.name, candidate.version, base,
+                open(destfile), candidate.size, candidate.md5)
+            results.append(result_package)
+        return results
 
=== modified file 'hwpack/testing.py'
--- hwpack/testing.py	2010-08-31 15:10:50 +0000
+++ hwpack/testing.py	2010-09-02 15:37:46 +0000
@@ -1,8 +1,15 @@
 from contextlib import contextmanager
+import hashlib
+import os
+import shutil
+import tempfile
 from StringIO import StringIO
 import tarfile
 
+from testtools import TestCase
+
 from hwpack.better_tarfile import writeable_tarfile
+from hwpack.packages import FetchedPackage
 
 
 @contextmanager
@@ -37,3 +44,108 @@
         yield tf
     finally:
         tf.close()
+
+
+class DummyFetchedPackage(FetchedPackage):
+    """A FetchedPackage with dummy information.
+
+    See FetchedPackage for the instance variables.
+    """
+
+    def __init__(self, name, version):
+        """Create a DummyFetchedPackage.
+
+        :param name: the name of the package.
+        :type name: str
+        :param version: the version of the package.
+        :type version: str
+        """
+        self.name = name
+        self.version = version
+
+    @property
+    def filename(self):
+        return "%s_%s_all.deb" % (self.name, self.version)
+
+    @property
+    def content(self):
+        return StringIO("Content of %s" % self.filename)
+
+    @property
+    def size(self):
+        return len(self.content.read())
+
+    @property
+    def md5(self):
+        md5sum = hashlib.md5()
+        md5sum.update(self.content.read())
+        return md5sum.hexdigest()
+
+
+class AptSourceFixture(object):
+    """A fixture that provides an apt source, with packages and indices.
+
+    An apt source provides a set of package files, and a Packages file
+    that allows apt to determine the contents of the source.
+
+    :ivar sources_entry: the URI and suite to give to apt to view the
+        source (i.e. a sources.list line without the "deb" prefix
+    :type sources_entry: str
+    """
+
+    def __init__(self, packages):
+        """Create an AptSourceFixture.
+
+        :param packages: a list of packages to add to the source
+            and index.
+        :type packages: an iterable of FetchedPackages
+        """
+        self.packages = packages
+
+    def setUp(self):
+        self.rootdir = tempfile.mkdtemp(prefix="hwpack-apt-source-")
+        for package in self.packages:
+            with open(
+                os.path.join(self.rootdir, package.filename), 'wb') as f:
+                f.write(package.content.read())
+        with open(os.path.join(self.rootdir, "Packages"), 'wb') as f:
+            for package in self.packages:
+                f.write('Package: %s\n' % package.name)
+                f.write('Version: %s\n' % package.version)
+                f.write('Filename: %s\n' % package.filename)
+                f.write('Size: %d\n' % package.size)
+                f.write('Architecture: all\n')
+                f.write('MD5sum: %s\n' % package.md5)
+                f.write('\n')
+
+    def tearDown(self):
+        if os.path.exists(self.rootdir):
+            shutil.rmtree(self.rootdir)
+
+    @property
+    def sources_entry(self):
+        return "file:" + os.path.abspath(self.rootdir) +" ./"
+
+
+class TestCaseWithFixtures(TestCase):
+    """A TestCase with the ability to easily add 'fixtures'.
+
+    A fixture is an object which can be created and cleaned up, and
+    this test case knows how to manage them to ensure that they will
+    always be cleaned up at the end of the test.
+    """
+
+    def useFixture(self, fixture):
+        """Make use of a fixture, ensuring that it will be cleaned up.
+
+        Given a fixture, this method will run the `setUp` method of
+        the fixture, and ensure that its `tearDown` method will be
+        called at the end of the test, regardless of success or failure.
+
+        :param fixture: the fixture to use.
+        :type fixture: an object with setUp and tearDown methods.
+        :return: the fixture that was passed in.
+        """
+        self.addCleanup(fixture.tearDown)
+        fixture.setUp()
+        return fixture
 
=== modified file 'hwpack/tests/__init__.py'
--- hwpack/tests/__init__.py	2010-08-31 01:05:12 +0000
+++ hwpack/tests/__init__.py	2010-09-02 15:37:46 +0000
@@ -4,6 +4,7 @@
     module_names = ['hwpack.tests.test_config',
                     'hwpack.tests.test_better_tarfile',
                     'hwpack.tests.test_hardwarepack',
+                    'hwpack.tests.test_packages',
                     'hwpack.tests.test_tarfile_matchers',
                    ]
     loader = unittest.TestLoader()
 
=== added file 'hwpack/tests/test_packages.py'
--- hwpack/tests/test_packages.py	1970-01-01 00:00:00 +0000
+++ hwpack/tests/test_packages.py	2010-09-02 15:37:46 +0000
@@ -0,0 +1,232 @@
+import os
+from StringIO import StringIO
+
+from testtools import TestCase
+
+from hwpack.packages import (
+    FetchedPackage,
+    PackageFetcher,
+    )
+from hwpack.testing import (
+    AptSourceFixture,
+    DummyFetchedPackage,
+    TestCaseWithFixtures,
+    )
+
+
+class FetchedPackageTests(TestCase):
+
+    def test_attributes(self):
+        package = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa")
+        self.assertEqual("foo", package.name)
+        self.assertEqual("1.1", package.version)
+        self.assertEqual("foo_1.1.deb", package.filename)
+        self.assertEqual("xxxx", package.content.read())
+        self.assertEqual(4, package.size)
+        self.assertEqual("aaaa", package.md5)
+
+    def test_equal(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa")
+        package2 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa")
+        self.assertEqual(package1, package2)
+
+    def test_not_equal_different_name(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa")
+        package2 = FetchedPackage(
+            "bar", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa")
+        self.assertNotEqual(package1, package2)
+
+    def test_not_equal_different_version(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa")
+        package2 = FetchedPackage(
+            "foo", "1.2", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa")
+        self.assertNotEqual(package1, package2)
+
+    def test_not_equal_different_filename(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa")
+        package2 = FetchedPackage(
+            "foo", "1.1", "afoo_1.1.deb", StringIO("xxxx"), 4, "aaaa")
+        self.assertNotEqual(package1, package2)
+
+    def test_not_equal_different_content(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa")
+        package2 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("yyyy"), 4, "aaaa")
+        self.assertNotEqual(package1, package2)
+
+    def test_not_equal_different_size(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa")
+        package2 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 5, "aaaa")
+        self.assertNotEqual(package1, package2)
+
+    def test_not_equal_different_md5(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa")
+        package2 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "bbbb")
+        self.assertNotEqual(package1, package2)
+
+    def test_equal_hash_equal(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa")
+        package2 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa")
+        self.assertEqual(hash(package1), hash(package2))
+
+
+class PackageFetcherTests(TestCaseWithFixtures):
+
+    def test_cleanup_removes_tempdir(self):
+        fetcher = PackageFetcher([])
+        fetcher.prepare()
+        tempdir = fetcher.tempdir
+        fetcher.cleanup()
+        self.assertFalse(os.path.exists(tempdir))
+
+    def test_cleanup_ignores_missing_tempdir(self):
+        fetcher = PackageFetcher([])
+        fetcher.prepare()
+        tempdir = fetcher.tempdir
+        fetcher.cleanup()
+        # Check that there is no problem removing it again
+        fetcher.cleanup()
+
+    def test_cleanup_before_prepare(self):
+        fetcher = PackageFetcher([])
+        # Check that there is no problem cleaning up before we start
+        fetcher.cleanup()
+
+    def test_prepare_creates_tempdir(self):
+        fetcher = PackageFetcher([])
+        self.addCleanup(fetcher.cleanup)
+        fetcher.prepare()
+        self.assertTrue(os.path.isdir(fetcher.tempdir))
+
+    def test_prepare_creates_var_lib_dpkg_status_file(self):
+        fetcher = PackageFetcher([])
+        self.addCleanup(fetcher.cleanup)
+        fetcher.prepare()
+        self.assertEqual(
+            '',
+            open(os.path.join(
+                fetcher.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()
+        self.assertTrue(
+            os.path.isdir(os.path.join(
+                fetcher.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()
+        self.assertTrue(
+            os.path.isdir(os.path.join(
+                fetcher.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(
+            [source1.sources_entry, source2.sources_entry])
+        self.addCleanup(fetcher.cleanup)
+        fetcher.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())
+
+    def get_fetcher(self, sources):
+        fetcher = PackageFetcher([s.sources_entry for s in sources])
+        self.addCleanup(fetcher.cleanup)
+        fetcher.prepare()
+        return fetcher
+
+    def test_fetch_packages_not_found_because_no_sources(self):
+        fetcher = self.get_fetcher([])
+        self.assertRaises(KeyError, fetcher.fetch_packages, ["nothere"])
+
+    def test_fetch_packages_not_found_because_not_in_sources(self):
+        available_package = DummyFetchedPackage("foo", "1.0")
+        source = self.useFixture(AptSourceFixture([available_package]))
+        fetcher = self.get_fetcher([source])
+        self.assertRaises(KeyError, fetcher.fetch_packages, ["nothere"])
+
+    def test_fetch_packages_not_found_one_of_two_missing(self):
+        available_package = DummyFetchedPackage("foo", "1.0")
+        source = self.useFixture(AptSourceFixture([available_package]))
+        fetcher = self.get_fetcher([source])
+        self.assertRaises(
+            KeyError, fetcher.fetch_packages, ["foo", "nothere"])
+
+    def test_fetch_packges_fetches_no_packages(self):
+        available_package = DummyFetchedPackage("foo", "1.0")
+        source = self.useFixture(AptSourceFixture([available_package]))
+        fetcher = self.get_fetcher([source])
+        self.assertEqual(0, len(fetcher.fetch_packages([])))
+
+    def test_fetch_packges_fetches_single_package(self):
+        available_package = DummyFetchedPackage("foo", "1.0")
+        source = self.useFixture(AptSourceFixture([available_package]))
+        fetcher = self.get_fetcher([source])
+        self.assertEqual(1, len(fetcher.fetch_packages(["foo"])))
+
+    def test_fetch_packges_fetches_correct_packge(self):
+        available_package = DummyFetchedPackage("foo", "1.0")
+        source = self.useFixture(AptSourceFixture([available_package]))
+        fetcher = self.get_fetcher([source])
+        self.assertEqual(
+            available_package, fetcher.fetch_packages(["foo"])[0])
+
+    def test_fetch_packges_fetches_multiple_packages(self):
+        available_packages = [
+            DummyFetchedPackage("bar", "1.0"),
+            DummyFetchedPackage("foo", "1.0"),
+        ]
+        source = self.useFixture(AptSourceFixture(available_packages))
+        fetcher = self.get_fetcher([source])
+        self.assertEqual(2, len(fetcher.fetch_packages(["foo", "bar"])))
+
+    def test_fetch_packges_fetches_multiple_packages_correctly(self):
+        available_packages = [
+            DummyFetchedPackage("foo", "1.0"),
+            DummyFetchedPackage("bar", "1.0"),
+        ]
+        source = self.useFixture(AptSourceFixture(available_packages))
+        fetcher = self.get_fetcher([source])
+        fetched = fetcher.fetch_packages(["foo", "bar"])
+        self.assertEqual(available_packages[0], fetched[0])
+        self.assertEqual(available_packages[1], fetched[1])
+
+    def test_fetch_packages_fetches_newest(self):
+        available_packages = [
+            DummyFetchedPackage("bar", "1.0"),
+            DummyFetchedPackage("bar", "1.1"),
+        ]
+        source = self.useFixture(AptSourceFixture(available_packages))
+        fetcher = self.get_fetcher([source])
+        fetched = fetcher.fetch_packages(["bar"])
+        self.assertEqual(available_packages[1], fetched[0])
+
+    def test_fetch_packages_fetches_newest_from_multiple_sources(self):
+        old_source_packages = [DummyFetchedPackage("bar", "1.0")]
+        new_source_packages = [DummyFetchedPackage("bar", "1.1")]
+        old_source = self.useFixture(AptSourceFixture(old_source_packages))
+        new_source = self.useFixture(AptSourceFixture(new_source_packages))
+        fetcher = self.get_fetcher([old_source, new_source])
+        fetched = fetcher.fetch_packages(["bar"])
+        self.assertEqual(new_source_packages[0], fetched[0])