Merge lp:~james-w/linaro-image-tools/add-relations-to-packages-file into lp:linaro-image-tools/11.11

[James Westby]

Hi,

This adds "depends" to FetchedPackage, corresponding to the "Depends"
that should be put in the Packages file.

First it allows you to set it on the object, and works it in to __eq__,
__hash__ etc.

It then adds code the Packages file creation to write out the dependencies
in the file.

Then it adds a "from_apt" constructor to FetchedPackage that allows you
to create one from a python-apt apt.package.Version (meaning a single
version of a package) instance. This centralises that code, and allows
us to encapsulate the tedious process of going from apt's object definition
of dependencies to the string version that we want.

The tests for that use IsolatedAptCache from the previous branch to
get an actual python-apt object to use.

We should do the same for other dependencies (Pre-Depends and Conflicts being
the most important), but I'll put that in a separate branch.

Thanks,

James

[Zygmunt Krynicki (zyga)]

Hi.

I'm not familiar with this API but this method seems quite magic.
You could add a docstring explaining what it does.

+ @classmethod
+ def from_apt(cls, pkg, filename, content):
+ depends = None
+ pkg_dependencies = pkg.get_dependencies("Depends")
+ if pkg_dependencies:
+ depends_list = []
+ for or_dep in pkg_dependencies:
+ or_list = []
+ for or_alternative in or_dep.or_dependencies:
+ suffix = ""
+ if or_alternative.relation:
+ suffix = " (%s %s)" % (
+ or_alternative.relation, or_alternative.version)
+ or_list.append("%s%s" % (or_alternative.name, suffix))
+ depends_list.append(" | ".join(or_list))
+ depends = ", ".join(depends_list)
+ return cls(
+ pkg.package.name, pkg.version, filename, content, pkg.size,
+ pkg.md5, pkg.architecture, depends=depends)

[lp:~james-w/linaro-image-tools/add-relations-to-packages-file]

134. By James Westby on 2010-09-10

Add a docstring to from_apt. Thanks Zygmunt.

[James Westby (james-w)]

> Hi.
>
> I'm not familiar with this API but this method seems quite magic.
> You could add a docstring explaining what it does.

Done, thanks.

The next branch refactors this and explains more of the intent
of that odd code in the docstring of the new function. I can
pull that in to this branch if you like.

Thanks,

James

[Zygmunt Krynicki (zyga)]

> > I'm not familiar with this API but this method seems quite magic.
> > You could add a docstring explaining what it does.
>
> Done, thanks.

> The next branch refactors this and explains more of the intent
> of that odd code in the docstring of the new function. I can
> pull that in to this branch if you like.

I read the next branch, it's much better.
You don't need to merge it here, just go ahead and merge one by one.

Zygmunt

review: Approve

[James Westby (james-w)]

On Tue, 14 Sep 2010 15:09:40 -0000, Zygmunt Krynicki <email address hidden> wrote:
> Review: Approve
> > > I'm not familiar with this API but this method seems quite magic.
> > > You could add a docstring explaining what it does.
> >
> > Done, thanks.
>
> > The next branch refactors this and explains more of the intent
> > of that odd code in the docstring of the new function. I can
> > pull that in to this branch if you like.
>
> I read the next branch, it's much better.
> You don't need to merge it here, just go ahead and merge one by one.

Ok, thanks.

James

=== modified file 'hwpack/packages.py'
--- hwpack/packages.py	2010-09-10 21:53:40 +0000
+++ hwpack/packages.py	2010-09-10 21:53:40 +0000
@@ -22,8 +22,9 @@
         parts.append('Version: %s' % package.version)
         parts.append('Filename: %s' % package.filename)
         parts.append('Size: %d' % package.size)
-        # TODO: architecture support
         parts.append('Architecture: %s' % package.architecture)
+        if package.depends:
+            parts.append('Depends: %s' % package.depends)
         parts.append('MD5sum: %s' % package.md5)
         content += "\n".join(parts)
         content += "\n\n"
@@ -78,10 +79,14 @@
     :ivar architecture: the architecture that the package is for, may be
         'all'.
     :type architecture: str
+    :ivar depends: the depends string that the package has, i.e. the
+        dependencies as specified in debian/control. May be None if the
+        package has none.
+    :type depends: str or None
     """
 
     def __init__(self, name, version, filename, content, size, md5,
-                 architecture):
+                 architecture, depends=None):
         """Create a FetchedPackage.
 
         See the instance variables for the arguments.
@@ -93,6 +98,41 @@
         self.size = size
         self.md5 = md5
         self.architecture = architecture
+        self.depends = depends
+
+    @classmethod
+    def from_apt(cls, pkg, filename, content):
+        """Create a FetchedPackage from a python-apt Version (package).
+
+        This is an alternative constructor for FetchedPackages that
+        takes most of the information from an apt.package.Version
+        object (i.e. a single version of a package), with some additional
+        information supplied by tha caller.
+
+        :param pkg: the python-apt package to take the information from.
+        :type pkg: apt.package.Version instance
+        :param filename: the filename that the package has.
+        :type filename: str
+        :param content: the content of the package.
+        :type content: file-like object
+        """
+        depends = None
+        pkg_dependencies = pkg.get_dependencies("Depends")
+        if pkg_dependencies:
+            depends_list = []
+            for or_dep in pkg_dependencies:
+                or_list = []
+                for or_alternative in or_dep.or_dependencies:
+                    suffix = ""
+                    if or_alternative.relation:
+                        suffix = " (%s %s)" % (
+                            or_alternative.relation, or_alternative.version)
+                    or_list.append("%s%s" % (or_alternative.name, suffix))
+                depends_list.append(" | ".join(or_list))
+            depends = ", ".join(depends_list)
+        return cls(
+            pkg.package.name, pkg.version, filename, content, pkg.size,
+            pkg.md5, pkg.architecture, depends=depends)
 
     def __eq__(self, other):
         return (self.name == other.name
@@ -101,11 +141,20 @@
                 and self.content.read() == other.content.read()
                 and self.size == other.size
                 and self.md5 == other.md5
-                and self.architecture == other.architecture)
+                and self.architecture == other.architecture
+                and self.depends == other.depends)
 
     def __hash__(self):
         return hash(
-            (self.name, self.version, self.filename, self.size, self.md5))
+            (self.name, self.version, self.filename, self.size, self.md5,
+             self.depends))
+
+    def __repr__(self):
+        return (
+            '<%s name=%s version=%s size=%s md5=%s architecture=%s '
+            'depends="%s">' % (self.__class__.__name__, self.name,
+                self.version, self.size, self.md5, self.architecture,
+                self.depends))
 
 
 class IsolatedAptCache(object):
@@ -242,9 +291,7 @@
                 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,
-                candidate.architecture)
+            result_package = FetchedPackage.from_apt(
+                candidate, base, open(destfile))
             results.append(result_package)
         return results

=== modified file 'hwpack/testing.py'
--- hwpack/testing.py	2010-09-10 21:53:40 +0000
+++ hwpack/testing.py	2010-09-10 21:53:40 +0000
@@ -54,10 +54,11 @@
     See FetchedPackage for the instance variables.
     """
 
-    def __init__(self, name, version, architecture="all"):
+    def __init__(self, name, version, architecture="all", depends=None):
         self.name = name
         self.version = version
         self.architecture = architecture
+        self.depends = depends
 
     @property
     def filename(self):

=== modified file 'hwpack/tests/test_packages.py'
--- hwpack/tests/test_packages.py	2010-09-10 21:53:40 +0000
+++ hwpack/tests/test_packages.py	2010-09-10 21:53:40 +0000
@@ -41,8 +41,24 @@
             get_packages_file([package1]) + get_packages_file([package2]),
             get_packages_file([package1, package2]))
 
-
-class FetchedPackageTests(TestCase):
+    def test_with_depends(self):
+        package = DummyFetchedPackage("foo", "1.1", depends="bar | baz")
+        self.assertEqual(textwrap.dedent("""\
+            Package: foo
+            Version: 1.1
+            Filename: %(filename)s
+            Size: %(size)d
+            Architecture: all
+            Depends: bar | baz
+            MD5sum: %(md5)s
+            \n""" % {
+                'filename': package.filename,
+                'size': package.size,
+                'md5': package.md5,
+            }), get_packages_file([package]))
+
+
+class FetchedPackageTests(TestCaseWithFixtures):
 
     def test_attributes(self):
         package = FetchedPackage(
@@ -112,6 +128,33 @@
             "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "i386")
         self.assertNotEqual(package1, package2)
 
+    def test_not_equal_different_depends(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            depends="bar")
+        package2 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            depends="baz")
+        self.assertNotEqual(package1, package2)
+
+    def test_not_equal_different_depends_one_None(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            depends="bar")
+        package2 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            depends=None)
+        self.assertNotEqual(package1, package2)
+
+    def test_equal_same_depends(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            depends="bar")
+        package2 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            depends="bar")
+        self.assertEqual(package1, package2)
+
     def test_equal_hash_equal(self):
         package1 = FetchedPackage(
             "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel")
@@ -119,6 +162,34 @@
             "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel")
         self.assertEqual(hash(package1), hash(package2))
 
+    def test_equal_hash_equal_with_depends(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            depends="bar")
+        package2 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            depends="bar")
+        self.assertEqual(hash(package1), hash(package2))
+
+    def test_from_apt(self):
+        target_package = DummyFetchedPackage("foo", "1.0")
+        source = self.useFixture(AptSourceFixture([target_package]))
+        with IsolatedAptCache([source.sources_entry]) as cache:
+            candidate = cache.cache['foo'].candidate
+            created_package = FetchedPackage.from_apt(
+                candidate, target_package.filename, target_package.content)
+            self.assertEqual(target_package, created_package)
+
+    def test_from_apt_with_depends(self):
+        target_package = DummyFetchedPackage(
+            "foo", "1.0", depends="bar | baz (>= 1.0), zap")
+        source = self.useFixture(AptSourceFixture([target_package]))
+        with IsolatedAptCache([source.sources_entry]) as cache:
+            candidate = cache.cache['foo'].candidate
+            created_package = FetchedPackage.from_apt(
+                candidate, target_package.filename, target_package.content)
+            self.assertEqual(target_package, created_package)
+
 
 class AptCacheTests(TestCaseWithFixtures):