Merge lp:~james-w/linaro-image-tools/extra-text-in-packages-files into lp:linaro-image-tools/11.11

[James Westby]

Hi,

Another small change to allow specifying extra text for the Packages file.

The motivation for this is that the /var/lib/dpkg/status "database" of
installed packages uses this format with an extra line per-stanza.

I'm not particularly keen on the implementation, but I'm also wary of
adding lots of attributes to the Package class to accomodate this sort
of thing.

If someone thinks we should do this in a different way then I will be
happy to do so.

Thanks,

James

[Zygmunt Krynicki (zyga)]

14 + :param extra_text: extra text to insert in to each stanza.
15 + Should not end with a newline.

rstrip() will simplify it for your users.

Other than that I agree. I had similar issues in that past (class growing too many attributes) and such solutions tend to look not very pretty but are effective in practice. If you start abusing this argument you can always revisit this problem and add an extra attribute.

One other method that you might consider would be a callback that is called with each package to return this text. This would allow you do have one "hook" that decides on the extra bits on the caller side.

review: Approve

[lp:~james-w/linaro-image-tools/extra-text-in-packages-files]

145. By James Westby on 2010-09-13

Merged set-content-mtime into extra-text-in-packages-files.

146. By James Westby on 2010-09-13

Merged set-content-mtime into extra-text-in-packages-files.

[James Westby (james-w)]

On Mon, 13 Sep 2010 21:21:00 -0000, Zygmunt Krynicki <email address hidden> wrote:
> Review: Approve
> 14 + :param extra_text: extra text to insert in to each stanza.
> 15 + Should not end with a newline.
>
> rstrip() will simplify it for your users.

I'm not sure I want to do that, as whitespace can be significant in this
file. I was more trying to set expectations that you specify a line
fragment, rather than a whole line. I can reword it if you like.

> Other than that I agree. I had similar issues in that past (class
> growing too many attributes) and such solutions tend to look not very
> pretty but are effective in practice. If you start abusing this
> argument you can always revisit this problem and add an extra
> attribute.
>
> One other method that you might consider would be a callback that is
> called with each package to return this text. This would allow you do
> have one "hook" that decides on the extra bits on the caller side.

I think that's overkill for this use, but would certainly be warranted
if we actually want to generate usable Packages files with this code.

Thanks,

James

=== modified file 'hwpack/packages.py'
--- hwpack/packages.py	2010-09-13 21:29:42 +0000
+++ hwpack/packages.py	2010-09-13 21:29:42 +0000
@@ -7,11 +7,14 @@
 import apt_pkg
 
 
-def get_packages_file(packages):
+def get_packages_file(packages, extra_text=None):
     """Get the Packages file contents indexing `packages`.
 
     :param packages: the packages to index.
     :type packages: an iterable of FetchedPackages.
+    :param extra_text: extra text to insert in to each stanza.
+         Should not end with a newline.
+    :type extra_text: str or None
     :return: the Packages file contents indexing `packages`.
     :rtype: str
     """
@@ -19,6 +22,8 @@
     for package in packages:
         parts = []
         parts.append('Package: %s' % package.name)
+        if extra_text is not None:
+            parts.append(extra_text)
         parts.append('Version: %s' % package.version)
         parts.append('Filename: %s' % package.filename)
         parts.append('Size: %d' % package.size)

=== modified file 'hwpack/tests/test_packages.py'
--- hwpack/tests/test_packages.py	2010-09-13 21:29:42 +0000
+++ hwpack/tests/test_packages.py	2010-09-13 21:29:42 +0000
@@ -81,6 +81,22 @@
             self.get_stanza(package, "Recommends: bar | baz\n"),
             get_packages_file([package]))
 
+    def test_with_extra_text(self):
+        package = DummyFetchedPackage("foo", "1.1")
+        self.assertEqual(textwrap.dedent("""\
+            Package: foo
+            Status: bar
+            Version: 1.1
+            Filename: %(filename)s
+            Size: %(size)d
+            Architecture: all
+            MD5sum: %(md5)s
+            \n""" % {
+                'filename': package.filename,
+                'size': package.size,
+                'md5': package.md5,
+            }), get_packages_file([package], extra_text="Status: bar"))
+
 
 class StringifyRelationshipTests(TestCaseWithFixtures):