Merge lp:~james-w/linaro-image-tools/make-package-content-optional into lp:linaro-image-tools/11.11

[James Westby]

Hi,

The specification calls for it to be optional whether debs are included
in the hardware pack or not.

To that end we need a way of specifying to the hwpack a list of packages
with name+version, but for which we don't want to include the content, and
shouldn't bother downloading it.

To that end here is a branch to make FetchedPackage.content optional, as
it has everything else we need, and so making a new object would be overkill
IMO.

The next branch will actually look at the include-debs config option etc.,
this is just groundwork.

Thanks,

James

[Zygmunt Krynicki (zyga)]

I don't see any bugs but some things are IMHO odd:

Why is this needed, can't you just have content = None in __init__() and be done with it? What am I missing? In addition a content= setter would be better than a method that you might not call since content is a public attribute.

64 + self.content = None
65 +
66 + def set_content(self, content):
67 + """Set the content of the package.
68 +
69 + :param content: the content of the package
70 + :type content: a file-like object
71 + """
72 + self.content = content

I'm not sure I like this part. It seems like we need to reinvent the operators module to get it to play with assertThat. I would extend testtools to have assertThatNot instead (it's cleaner) or make it work well with regular python operators (if it's returning some sort of assertion object we can have __not__ and all the usual suspects)

176 +class Not:
177 + """Inverts a matcher."""
178 +
179 + def __init__(self, matcher):
180 + self.matcher = matcher
181 +
182 + def __str__(self):
183 + return 'Not(%s)' % (self.matcher,)
184 +
185 + def match(self, other):
186 + mismatch = self.matcher.match(other)
187 + if mismatch is None:
188 + return MatchedUnexpectedly(self.matcher, other)
189 + else:
190 + return None

review: Needs Fixing

[James Westby (james-w)]

On Tue, 14 Sep 2010 20:51:48 -0000, Zygmunt Krynicki <email address hidden> wrote:
> Review: Needs Fixing
> I don't see any bugs but some things are IMHO odd:
>
>
> Why is this needed, can't you just have content = None in __init__()
> and be done with it? What am I missing? In addition a content= setter
> would be better than a method that you might not call since content is
> a public attribute.

Agreed.

> I'm not sure I like this part. It seems like we need to reinvent the
> operators module to get it to play with assertThat. I would extend
> testtools to have assertThatNot instead (it's cleaner) or make it work
> well with regular python operators (if it's returning some sort of
> assertion object we can have __not__ and all the usual suspects)

This is taken from testtools, so it's the way they decided to go, I'm
just backporting it so that we don't require a too-new version of it.

Feel free to file a bug on testtools to discuss some of your ideas.

Thanks,

James

[lp:~james-w/linaro-image-tools/make-package-content-optional]

149. By James Westby on 2010-09-14

Remove set_content as it is not needed.

[Zygmunt Krynicki (zyga)]

> > I'm not sure I like this part. It seems like we need to reinvent the
> > operators module to get it to play with assertThat. I would extend
> > testtools to have assertThatNot instead (it's cleaner) or make it work
> > well with regular python operators (if it's returning some sort of
> > assertion object we can have __not__ and all the usual suspects)
>
> This is taken from testtools, so it's the way they decided to go, I'm
> just backporting it so that we don't require a too-new version of it.
>
> Feel free to file a bug on testtools to discuss some of your ideas.

Understood, if you are just backporting a future API then it makes sense.
I would recommend though to keep the API in a separate file and mark it clearly as something that you copied from another project. This will make it possible to revisit and remove it in the future and keep things clear license-wise (although I believe both projects use the same license here)

Thanks
Zygmunt

[James Westby (james-w)]

On Tue, 14 Sep 2010 20:51:48 -0000, Zygmunt Krynicki <email address hidden> wrote:
> Review: Needs Fixing
> I don't see any bugs but some things are IMHO odd:
>
>
> Why is this needed, can't you just have content = None in __init__()
> and be done with it?

Done, thanks.

James

[Zygmunt Krynicki (zyga)]

Thanks, looks good now.

Approved

review: Approve

=== modified file 'hwpack/hardwarepack.py'
--- hwpack/hardwarepack.py	2010-09-14 21:03:45 +0000
+++ hwpack/hardwarepack.py	2010-09-14 21:03:45 +0000
@@ -177,15 +177,18 @@
             tf.create_dir(self.PACKAGES_DIRNAME)
             manifest_content = ""
             for package in self.packages:
-                tf.create_file_from_string(
-                    self.PACKAGES_DIRNAME + "/" + package.filename,
-                    package.content.read())
+                if package.content is not None:
+                    tf.create_file_from_string(
+                        self.PACKAGES_DIRNAME + "/" + package.filename,
+                        package.content.read())
                 manifest_content += "%s=%s\n" % (
                     package.name, package.version)
             tf.create_file_from_string(
                 self.MANIFEST_FILENAME, manifest_content)
             tf.create_file_from_string(
-                self.PACKAGES_FILENAME, get_packages_file(self.packages))
+                self.PACKAGES_FILENAME,
+                get_packages_file(
+                    [p for p in self.packages if p.content is not None]))
             tf.create_dir(self.SOURCES_LIST_DIRNAME)
             for source_name, source_info in self.sources.items():
                 tf.create_file_from_string(

=== modified file 'hwpack/packages.py'
--- hwpack/packages.py	2010-09-14 21:03:45 +0000
+++ hwpack/packages.py	2010-09-14 21:03:45 +0000
@@ -114,8 +114,9 @@
     :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 content: a file that the content of the package can be read from,
+        or None if the content is not known.
+    :type content: a file-like object or None
     :ivar size: the size of the package
     :type size: int
     :ivar md5: the hex representation of the md5sum of the contents of
@@ -142,7 +143,7 @@
     :type recommends: str or None
     """
 
-    def __init__(self, name, version, filename, content, size, md5,
+    def __init__(self, name, version, filename, size, md5,
                  architecture, depends=None, pre_depends=None,
                  conflicts=None, recommends=None):
         """Create a FetchedPackage.
@@ -152,7 +153,6 @@
         self.name = name
         self.version = version
         self.filename = filename
-        self.content = content
         self.size = size
         self.md5 = md5
         self.architecture = architecture
@@ -160,9 +160,10 @@
         self.pre_depends = pre_depends
         self.conflicts = conflicts
         self.recommends = recommends
+        self.content = None
 
     @classmethod
-    def from_apt(cls, pkg, filename, content):
+    def from_apt(cls, pkg, filename, content=None):
         """Create a FetchedPackage from a python-apt Version (package).
 
         This is an alternative constructor for FetchedPackages that
@@ -181,17 +182,24 @@
         pre_depends = stringify_relationship(pkg, "PreDepends")
         conflicts = stringify_relationship(pkg, "Conflicts")
         recommends = stringify_relationship(pkg, "Recommends")
-        return cls(
-            pkg.package.name, pkg.version, filename, content, pkg.size,
+        pkg = cls(
+            pkg.package.name, pkg.version, filename, pkg.size,
             pkg.md5, pkg.architecture, depends=depends,
             pre_depends=pre_depends, conflicts=conflicts,
             recommends=recommends)
+        if content is not None:
+            pkg.content = content
+        return pkg
 
     def __eq__(self, other):
+        def get_content(pkg):
+            return pkg.content and pkg.content.read()
+        content = get_content(self)
+        other_content = get_content(other)
         return (self.name == other.name
                 and self.version == other.version
                 and self.filename == other.filename
-                and self.content.read() == other.content.read()
+                and content == other_content
                 and self.size == other.size
                 and self.md5 == other.md5
                 and self.architecture == other.architecture
@@ -204,12 +212,14 @@
         return not self.__eq__(other)
 
     def __repr__(self):
+        has_content = self.content and "yes" or "no"
         return (
             '<%s name=%s version=%s size=%s md5=%s architecture=%s '
-            'depends="%s" pre_depends="%s" conflicts="%s" recommends="%s">'
+            'depends="%s" pre_depends="%s" conflicts="%s" recommends="%s" '
+            'has_content=%s>'
             % (self.__class__.__name__, self.name, self.version, self.size,
                 self.md5, self.architecture, self.depends, self.pre_depends,
-                self.conflicts, self.recommends))
+                self.conflicts, self.recommends, has_content))
 
 
 class IsolatedAptCache(object):

=== modified file 'hwpack/testing.py'
--- hwpack/testing.py	2010-09-14 21:03:45 +0000
+++ hwpack/testing.py	2010-09-14 21:03:45 +0000
@@ -56,7 +56,8 @@
     """
 
     def __init__(self, name, version, architecture="all", depends=None,
-                 pre_depends=None, conflicts=None, recommends=None):
+                 pre_depends=None, conflicts=None, recommends=None,
+                 no_content=False):
         self.name = name
         self.version = version
         self.architecture = architecture
@@ -64,23 +65,29 @@
         self.pre_depends = pre_depends
         self.conflicts = conflicts
         self.recommends = recommends
+        self._no_content = no_content
 
     @property
     def filename(self):
         return "%s_%s_all.deb" % (self.name, self.version)
 
+    def _content_str(self):
+        return "Content of %s" % self.filename
+
     @property
     def content(self):
-        return StringIO("Content of %s" % self.filename)
+        if self._no_content:
+            return None
+        return StringIO(self._content_str())
 
     @property
     def size(self):
-        return len(self.content.read())
+        return len(self._content_str())
 
     @property
     def md5(self):
         md5sum = hashlib.md5()
-        md5sum.update(self.content.read())
+        md5sum.update(self._content_str())
         return md5sum.hexdigest()
 
 
@@ -217,6 +224,34 @@
             return None
 
 
+class Not:
+    """Inverts a matcher."""
+
+    def __init__(self, matcher):
+        self.matcher = matcher
+
+    def __str__(self):
+        return 'Not(%s)' % (self.matcher,)
+
+    def match(self, other):
+        mismatch = self.matcher.match(other)
+        if mismatch is None:
+            return MatchedUnexpectedly(self.matcher, other)
+        else:
+            return None
+
+
+class MatchedUnexpectedly:
+    """A thing matched when it wasn't supposed to."""
+
+    def __init__(self, matcher, other):
+        self.matcher = matcher
+        self.other = other
+
+    def describe(self):
+        return "%r matches %s" % (self.other, self.matcher)
+
+
 class HardwarePackHasFile(TarfileHasFile):
     """A subclass of TarfileHasFile specific to hardware packs.
 
@@ -265,10 +300,12 @@
 
 class IsHardwarePack(Matcher):
 
-    def __init__(self, metadata, packages, sources):
+    def __init__(self, metadata, packages, sources,
+                 packages_without_content=None):
         self.metadata = metadata
         self.packages = packages
         self.sources = sources
+        self.packages_without_content = packages_without_content or []
 
     def match(self, path):
         tf = tarfile.open(name=path, mode="r:gz")
@@ -282,12 +319,15 @@
                 manifest += "%s=%s\n" % (package.name, package.version)
             matchers.append(HardwarePackHasFile("manifest", content=manifest))
             matchers.append(HardwarePackHasFile("pkgs", type=tarfile.DIRTYPE))
-            for package in self.packages:
+            packages_with_content = [p for p in self.packages
+                if p not in self.packages_without_content]
+            for package in packages_with_content:
                 matchers.append(HardwarePackHasFile(
                     "pkgs/%s" % package.filename,
                     content=package.content.read()))
             matchers.append(HardwarePackHasFile(
-                "pkgs/Packages", content=get_packages_file(self.packages)))
+                "pkgs/Packages",
+                content=get_packages_file(packages_with_content)))
             matchers.append(HardwarePackHasFile(
                 "sources.list.d", type=tarfile.DIRTYPE))
             for source_id, sources_entry in self.sources.items():

=== modified file 'hwpack/tests/test_hardwarepack.py'
--- hwpack/tests/test_hardwarepack.py	2010-09-14 17:17:43 +0000
+++ hwpack/tests/test_hardwarepack.py	2010-09-14 21:03:45 +0000
@@ -5,7 +5,7 @@
 
 from hwpack.hardwarepack import HardwarePack, Metadata
 from hwpack.packages import get_packages_file
-from hwpack.testing import DummyFetchedPackage, HardwarePackHasFile
+from hwpack.testing import DummyFetchedPackage, HardwarePackHasFile, Not
 
 
 class MetadataTests(TestCase):
@@ -199,6 +199,15 @@
             HardwarePackHasFile("pkgs/%s" % package2.filename,
                 content=package2.content.read()))
 
+    def test_add_packages_without_content_leaves_out_debs(self):
+        package1 = DummyFetchedPackage("foo", "1.1", no_content=True)
+        hwpack = HardwarePack(self.metadata)
+        hwpack.add_packages([package1])
+        tf = self.get_tarfile(hwpack)
+        self.assertThat(
+            tf,
+            Not(HardwarePackHasFile("pkgs/%s" % package1.filename)))
+
     def test_creates_Packages_file(self):
         hwpack = HardwarePack(self.metadata)
         tf = self.get_tarfile(hwpack)
@@ -221,6 +230,28 @@
                 "pkgs/Packages",
                 content=get_packages_file([package1, package2])))
 
+    def test_Packages_file_empty_with_no_deb_content(self):
+        package1 = DummyFetchedPackage("foo", "1.1", no_content=True)
+        package2 = DummyFetchedPackage("bar", "1.1", no_content=True)
+        hwpack = HardwarePack(self.metadata)
+        hwpack.add_packages([package1, package2])
+        tf = self.get_tarfile(hwpack)
+        self.assertThat(
+            tf,
+            HardwarePackHasFile("pkgs/Packages", content=""))
+
+    def test_Packages_file_correct_content_with_some_deb_content(self):
+        package1 = DummyFetchedPackage("foo", "1.1", no_content=True)
+        package2 = DummyFetchedPackage("bar", "1.1")
+        hwpack = HardwarePack(self.metadata)
+        hwpack.add_packages([package1, package2])
+        tf = self.get_tarfile(hwpack)
+        self.assertThat(
+            tf,
+            HardwarePackHasFile(
+                "pkgs/Packages",
+                content=get_packages_file([package2])))
+
     def test_creates_sources_list_dir(self):
         hwpack = HardwarePack(self.metadata)
         tf = self.get_tarfile(hwpack)

=== modified file 'hwpack/tests/test_packages.py'
--- hwpack/tests/test_packages.py	2010-09-14 21:03:45 +0000
+++ hwpack/tests/test_packages.py	2010-09-14 21:03:45 +0000
@@ -147,179 +147,188 @@
 
     def test_attributes(self):
         package = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa",
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa",
             "armel")
         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(None, package.content)
         self.assertEqual(4, package.size)
         self.assertEqual("aaaa", package.md5)
         self.assertEqual("armel", package.architecture)
 
     def test_equal(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
         self.assertEqual(package1, package2)
         self.assertFalse(package1 != package2)
 
     def test_not_equal_different_name(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
         package2 = FetchedPackage(
-            "bar", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel")
+            "bar", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
         self.assertNotEqual(package1, package2)
 
     def test_not_equal_different_version(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
         package2 = FetchedPackage(
-            "foo", "1.2", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel")
+            "foo", "1.2", "foo_1.1.deb", 4, "aaaa", "armel")
         self.assertNotEqual(package1, package2)
 
     def test_not_equal_different_filename(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel")
-        package2 = FetchedPackage(
-            "foo", "1.1", "afoo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel")
-        self.assertNotEqual(package1, package2)
-
-    def test_not_equal_different_content(self):
-        package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel")
-        package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("yyyy"), 4, "aaaa", "armel")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
+        package2 = FetchedPackage(
+            "foo", "1.1", "afoo_1.1.deb", 4, "aaaa", "armel")
         self.assertNotEqual(package1, package2)
 
     def test_not_equal_different_size(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 5, "aaaa", "armel")
+            "foo", "1.1", "foo_1.1.deb", 5, "aaaa", "armel")
         self.assertNotEqual(package1, package2)
 
     def test_not_equal_different_md5(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "bbbb", "armel")
+            "foo", "1.1", "foo_1.1.deb", 4, "bbbb", "armel")
         self.assertNotEqual(package1, package2)
 
     def test_not_equal_different_architecture(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "i386")
+            "foo", "1.1", "foo_1.1.deb", 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")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel", depends="bar")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
-            depends="baz")
+            "foo", "1.1", "foo_1.1.deb", 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")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel", depends="bar")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
-            depends=None)
+            "foo", "1.1", "foo_1.1.deb", 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")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel", depends="bar")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
-            depends="bar")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel", depends="bar")
         self.assertEqual(package1, package2)
 
     def test_not_equal_different_pre_depends(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel",
             pre_depends="bar")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel",
             pre_depends="baz")
         self.assertNotEqual(package1, package2)
 
     def test_not_equal_different_pre_depends_one_None(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel",
             pre_depends="bar")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel",
             pre_depends=None)
         self.assertNotEqual(package1, package2)
 
     def test_equal_same_pre_depends(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel",
             pre_depends="bar")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel",
             pre_depends="bar")
         self.assertEqual(package1, package2)
 
     def test_not_equal_different_conflicts(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
-            conflicts="bar")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel", conflicts="bar")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
-            conflicts="baz")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel", conflicts="baz")
         self.assertNotEqual(package1, package2)
 
     def test_not_equal_different_conflicts_one_None(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel",
             conflicts="bar")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel",
             conflicts=None)
         self.assertNotEqual(package1, package2)
 
     def test_equal_same_conflicts(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
-            conflicts="bar")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel", conflicts="bar")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
-            conflicts="bar")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel", conflicts="bar")
         self.assertEqual(package1, package2)
 
     def test_not_equal_different_recommends(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel",
             recommends="bar")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel",
             recommends="baz")
         self.assertNotEqual(package1, package2)
 
     def test_not_equal_different_recommends_one_None(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel",
             recommends="bar")
         package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel",
             recommends=None)
         self.assertNotEqual(package1, package2)
 
     def test_equal_same_recommends(self):
         package1 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
-            recommends="bar")
-        package2 = FetchedPackage(
-            "foo", "1.1", "foo_1.1.deb", StringIO("xxxx"), 4, "aaaa", "armel",
-            recommends="bar")
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel",
+            recommends="bar")
+        package2 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel",
+            recommends="bar")
+        self.assertEqual(package1, package2)
+
+    def test_not_equal_different_contents(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
+        package1.content = StringIO("xxxx")
+        package2 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
+        package2.content = StringIO("yyyy")
+        self.assertNotEqual(package1, package2)
+
+    def test_not_equal_different_contents_one_unknown(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
+        package1.content = StringIO("xxxx")
+        package2 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
+        self.assertNotEqual(package1, package2)
+
+    def test_equal_same_contents(self):
+        package1 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
+        package1.content = StringIO("xxxx")
+        package2 = FetchedPackage(
+            "foo", "1.1", "foo_1.1.deb", 4, "aaaa", "armel")
+        package2.content = StringIO("xxxx")
         self.assertEqual(package1, package2)
 
     def test_from_apt(self):
@@ -328,7 +337,8 @@
         with IsolatedAptCache([source.sources_entry]) as cache:
             candidate = cache.cache['foo'].candidate
             created_package = FetchedPackage.from_apt(
-                candidate, target_package.filename, target_package.content)
+                candidate, target_package.filename,
+                content=target_package.content)
             self.assertEqual(target_package, created_package)
 
     def assert_from_apt_translates_relationship(self, relationship):
@@ -339,7 +349,8 @@
         with IsolatedAptCache([source.sources_entry]) as cache:
             candidate = cache.cache['foo'].candidate
             created_package = FetchedPackage.from_apt(
-                candidate, target_package.filename, target_package.content)
+                candidate, target_package.filename,
+                content=target_package.content)
             self.assertEqual(target_package, created_package)
 
     def test_from_apt_with_depends(self):
@@ -354,6 +365,15 @@
     def test_from_apt_with_recommends(self):
         self.assert_from_apt_translates_relationship('recommends')
 
+    def test_from_apt_without_content(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)
+            self.assertEqual(None, created_package.content)
+
 
 class AptCacheTests(TestCaseWithFixtures):
 
@@ -554,7 +574,7 @@
         fetched = fetcher.fetch_packages(["bar"])
         self.assertEqual(new_source_packages[0], fetched[0])
 
-    def test_fetch_package_records_correct_architecture(self):
+    def test_fetch_packages_records_correct_architecture(self):
         available_package = DummyFetchedPackage(
             "foo", "1.0", architecture="nonexistant")
         source = self.useFixture(AptSourceFixture([available_package]))
@@ -562,7 +582,7 @@
         self.assertEqual(
             "nonexistant", fetcher.fetch_packages(["foo"])[0].architecture)
 
-    def test_fetch_package_fetches_from_correct_architecture(self):
+    def test_fetch_packages_fetches_from_correct_architecture(self):
         wanted_package = DummyFetchedPackage(
             "foo", "1.0", architecture="arch1")
         unwanted_package = DummyFetchedPackage(
@@ -573,7 +593,7 @@
         self.assertEqual(
             wanted_package, fetcher.fetch_packages(["foo"])[0])
 
-    def test_fetch_package_fetches_with_relationships(self):
+    def test_fetch_packages_fetches_with_relationships(self):
         depends = "foo"
         pre_depends = "bar (>= 1.0)"
         conflicts = "baz | zap"