Merge lp:~james-w/linaro-image-tools/add-packages-to-hwpack into lp:linaro-image-tools/11.11
- add-packages-to-hwpack
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Guilherme Salgado |
Approved revision: | 104 |
Merged at revision: | 65 |
Proposed branch: | lp:~james-w/linaro-image-tools/add-packages-to-hwpack |
Merge into: | lp:linaro-image-tools/11.11 |
Prerequisite: | lp:~james-w/linaro-image-tools/fetch-packages |
Diff against target: |
369 lines (+205/-17) 5 files modified
hwpack/hardwarepack.py (+50/-3) hwpack/packages.py (+23/-0) hwpack/testing.py (+2/-9) hwpack/tests/test_hardwarepack.py (+98/-0) hwpack/tests/test_packages.py (+32/-5) |
To merge this branch: | bzr merge lp:~james-w/linaro-image-tools/add-packages-to-hwpack |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Guilherme Salgado (community) | Approve | ||
Review via email: mp+34373@code.launchpad.net |
Commit message
Description of the change
Hi,
This starts to glue things together such that we can actually have
useful hardware packs created. It adds two methods on the HardwarePack
class, add_packages and add_sources, which allow you to add sources
entries and .debs to the hwpack.
There is little new code required to implement these, and the majority
of the changes are consolidation of existing code to make it more general,
which is a great sign I think.
The hwpack requires a Packages file pre-generated inside it. Because of
the way that we plan to install hwpacks, this isn't one that a user will
ever see if they use a package manager to look at available or installed
packages. Therefore I decided to re-use the code I added in the previous
branch for adding a sparse Packages file that apt understands just fine,
but which isn't appropriate for end users.
In addition, I made the hwpack.
the new FetchedPackage class, and implemented __eq__, which makes some
tests easier to write. Now the testing one is just one with dummy
content that we control more directly, the superclass is fine for
use in the real world.
Thanks,
James
- 98. By James Westby
-
Merge fetch-packges.
- 99. By James Westby
-
Add a docstring to get_packages_file.
James Westby (james-w) wrote : | # |
On Thu, 02 Sep 2010 21:14:41 -0000, Guilherme Salgado <email address hidden> wrote:
> > + def add_sources(self, sources):
> > + """Add sources to the hardware pack.
>
> Maybe """Add apt sources ..."""?
Yes, and I will change the method name too, we might need to add source
packages at some stage.
> > + Arguments passed to this method will overwrite any that were
> > + passed in previous calls with the same identifier, but will
> > + be used in addition to any with different identifiers.
>
> I don't quite understand what this means. Do you mean that this will
> overwrite existing sources with the same identifier and add the ones with
> unseen identifiers?
Yes, the implementation is {}.update(), so it will overwrite anything
existing. Suggestions for how to phrase it better welcome.
> I wonder if it'd be a problem to use '"%s=%s" % (name, version)' here so that
> we could feed such lines directly to apt. AIUI, not all packages will
> define a version, so we might need to do this only when a version is
> specified, but it'd simplify things when installing the packages. What do you
> think?
One of the use cases in the spec is just using the hwpack as as source
of package names and apt sources. In that case you want to discard any
version there, so I went with the format that was easy to put together,
rather than the slightly trickier to split, even though using versions
will be more common.
We can change the spec if you would prefer to implement it the other way
around.
> Wouldn't it be slightly simpler to append strings to 'parts' and then have
> just one "\n".join() here? Like
>
> parts.append(
> ...
> content += "\n".join(parts)
>
> No big deal, though; just suggesting because the nested join()s make it a bit
> less obvious what we'll get in the end, at least to me.
Yes, that's reasonable.
>
> typo: packges
Thanks.
> Minor nitpick: you could use textwrap.dedent() in order to not affect the code
> flow with the unindented multi-line string.
Thanks, I didn't know about that one. It's a shame that it involves a
backslash though.
> typo: packges
Thanks.
Thanks for the review. I will make the changes requested.
James
Guilherme Salgado (salgado) wrote : | # |
On Thu, 2010-09-02 at 21:45 +0000, James Westby wrote:
> On Thu, 02 Sep 2010 21:14:41 -0000, Guilherme Salgado <email address hidden> wrote:
> > > + def add_sources(self, sources):
> > > + """Add sources to the hardware pack.
> >
> > Maybe """Add apt sources ..."""?
>
> Yes, and I will change the method name too, we might need to add source
> packages at some stage.
>
Cool.
> > > + Arguments passed to this method will overwrite any that were
> > > + passed in previous calls with the same identifier, but will
> > > + be used in addition to any with different identifiers.
> >
> > I don't quite understand what this means. Do you mean that this will
> > overwrite existing sources with the same identifier and add the ones with
> > unseen identifiers?
>
> Yes, the implementation is {}.update(), so it will overwrite anything
> existing. Suggestions for how to phrase it better welcome.
I think I was confused by 'Arguments', as this method only takes one, so
maybe just changing that to 'Sources'.
>
> > I wonder if it'd be a problem to use '"%s=%s" % (name, version)' here so that
> > we could feed such lines directly to apt. AIUI, not all packages will
> > define a version, so we might need to do this only when a version is
> > specified, but it'd simplify things when installing the packages. What do you
> > think?
>
> One of the use cases in the spec is just using the hwpack as as source
> of package names and apt sources. In that case you want to discard any
> version there, so I went with the format that was easy to put together,
> rather than the slightly trickier to split, even though using versions
> will be more common.
>
> We can change the spec if you would prefer to implement it the other way
> around.
If using versions really is going to be more common, then I'd be in
favour of changing as it should be quite easy to do so here, right?
>
> > Wouldn't it be slightly simpler to append strings to 'parts' and then have
> > just one "\n".join() here? Like
> >
> > parts.append(
> > ...
> > content += "\n".join(parts)
> >
> > No big deal, though; just suggesting because the nested join()s make it a bit
> > less obvious what we'll get in the end, at least to me.
>
> Yes, that's reasonable.
>
> >
> > typo: packges
>
> Thanks.
>
> > Minor nitpick: you could use textwrap.dedent() in order to not affect the code
> > flow with the unindented multi-line string.
>
> Thanks, I didn't know about that one. It's a shame that it involves a
> backslash though.
Why is that? You can still use the triple quotes and a multi-line
string, the only difference being that you'll indent it:
self.
Packages: foo
Version: bar
...
""" % replacements,
--
Guilherme Salgado <https:/
- 100. By James Westby
-
Rename add_sources to add_apt_sources and update the docstring.
Thanks Guilherme.
- 101. By James Westby
-
Avoid a double join by storing strings not tuples.
Thanks Guilherme.
- 102. By James Westby
-
Fix typos. Thanks Guilherme.
- 103. By James Westby
-
Correct another typo. Thanks Guilherme.
- 104. By James Westby
-
Use textwrap to avoid a multiline string breaking formatting.
Thanks Guilherme.
James Westby (james-w) wrote : | # |
On Thu, 02 Sep 2010 22:01:39 -0000, Guilherme Salgado <email address hidden> wrote:
> I think I was confused by 'Arguments', as this method only takes one, so
> maybe just changing that to 'Sources'.
I changed the wording to just be a bit simpler, let me know what you think.
> If using versions really is going to be more common, then I'd be in
> favour of changing as it should be quite easy to do so here, right?
Yeah, really easy to do here.
> Why is that? You can still use the triple quotes and a multi-line
> string, the only difference being that you'll indent it:
>
> self.assertEqua
> Packages: foo
> Version: bar
> ...
> """ % replacements,
> get_packages_
That gives an extra newline at the start doesn't it?
/me tries...
Yes, the test fails with that arrangement due to the start of the string
not matching.
Thanks,
James
Guilherme Salgado (salgado) wrote : | # |
On Thu, 2010-09-02 at 23:07 +0000, James Westby wrote:
> On Thu, 02 Sep 2010 22:01:39 -0000, Guilherme Salgado <email address hidden> wrote:
> > I think I was confused by 'Arguments', as this method only takes one, so
> > maybe just changing that to 'Sources'.
>
> I changed the wording to just be a bit simpler, let me know what you think.
It looks good, thanks.
>
> > If using versions really is going to be more common, then I'd be in
> > favour of changing as it should be quite easy to do so here, right?
>
> Yeah, really easy to do here.
>
Cool.
> > Why is that? You can still use the triple quotes and a multi-line
> > string, the only difference being that you'll indent it:
> >
> > self.assertEqua
> > Packages: foo
> > Version: bar
> > ...
> > """ % replacements,
> > get_packages_
>
> That gives an extra newline at the start doesn't it?
>
> /me tries...
>
> Yes, the test fails with that arrangement due to the start of the string
> not matching.
You're right; it preserves newlines.
status approved
--
Guilherme Salgado <https:/
Preview Diff
1 | === modified file 'hwpack/hardwarepack.py' |
2 | --- hwpack/hardwarepack.py 2010-09-01 23:50:25 +0000 |
3 | +++ hwpack/hardwarepack.py 2010-09-02 22:03:43 +0000 |
4 | @@ -1,4 +1,5 @@ |
5 | from hwpack.better_tarfile import writeable_tarfile |
6 | +from hwpack.packages import get_packages_file |
7 | |
8 | |
9 | class Metadata(object): |
10 | @@ -75,6 +76,8 @@ |
11 | :type metadata: Metadata |
12 | """ |
13 | self.metadata = metadata |
14 | + self.sources = {} |
15 | + self.packages = [] |
16 | |
17 | def filename(self): |
18 | """The filename that this hardware pack should have. |
19 | @@ -92,6 +95,37 @@ |
20 | return "hwpack_%s_%s%s.tar.gz" % ( |
21 | self.metadata.name, self.metadata.version, support_suffix) |
22 | |
23 | + def add_apt_sources(self, sources): |
24 | + """Add APT sources to the hardware pack. |
25 | + |
26 | + Given a dict of names and the source lines this will add |
27 | + them to the hardware pack. |
28 | + |
29 | + The names should be an identifier for the source, and the |
30 | + source lines should be what is put in sources.list for that |
31 | + source, minus the "deb" part. |
32 | + |
33 | + If you pass an identifier that has already been passed to this |
34 | + method, then the previous value will be replaced with the new |
35 | + value. |
36 | + |
37 | + :param sources: the sources to use as a dict mapping identifiers |
38 | + to sources entries. |
39 | + :type sources: a dict mapping str to str |
40 | + """ |
41 | + self.sources.update(sources) |
42 | + |
43 | + def add_packages(self, packages): |
44 | + """Add packages to the hardware pack. |
45 | + |
46 | + Given a list of packages this will add them to the hardware |
47 | + pack. |
48 | + |
49 | + :param packages: the packages to add |
50 | + :type packages: FetchedPackage |
51 | + """ |
52 | + self.packages += packages |
53 | + |
54 | def to_file(self, fileobj): |
55 | """Write the hwpack to a file object. |
56 | |
57 | @@ -112,9 +146,22 @@ |
58 | self.FORMAT_FILENAME, self.FORMAT + "\n") |
59 | tf.create_file_from_string( |
60 | self.METADATA_FILENAME, str(self.metadata)) |
61 | - # TODO: include packages and sources etc. |
62 | - tf.create_file_from_string(self.MANIFEST_FILENAME, "") |
63 | tf.create_dir(self.PACKAGES_DIRNAME) |
64 | - tf.create_file_from_string(self.PACKAGES_FILENAME, "") |
65 | + manifest_content = "" |
66 | + for package in self.packages: |
67 | + tf.create_file_from_string( |
68 | + self.PACKAGES_DIRNAME + "/" + package.filename, |
69 | + package.content.read()) |
70 | + manifest_content += "%s %s\n" % ( |
71 | + package.name, package.version) |
72 | + tf.create_file_from_string( |
73 | + self.MANIFEST_FILENAME, manifest_content) |
74 | + tf.create_file_from_string( |
75 | + self.PACKAGES_FILENAME, get_packages_file(self.packages)) |
76 | tf.create_dir(self.SOURCES_LIST_DIRNAME) |
77 | + for source_name, source_info in self.sources.items(): |
78 | + tf.create_file_from_string( |
79 | + self.SOURCES_LIST_DIRNAME + "/" + source_name, |
80 | + "deb " + source_info + "\n") |
81 | + # TODO: include sources keys etc. |
82 | tf.create_dir(self.SOURCES_LIST_GPG_DIRNAME) |
83 | |
84 | === modified file 'hwpack/packages.py' |
85 | --- hwpack/packages.py 2010-09-02 22:03:43 +0000 |
86 | +++ hwpack/packages.py 2010-09-02 22:03:43 +0000 |
87 | @@ -7,6 +7,29 @@ |
88 | import apt_pkg |
89 | |
90 | |
91 | +def get_packages_file(packages): |
92 | + """Get the Packages file contents indexing `packages`. |
93 | + |
94 | + :param packages: the packages to index. |
95 | + :type packages: an iterable of FetchedPackages. |
96 | + :return: the Packages file contents indexing `packages`. |
97 | + :rtype: str |
98 | + """ |
99 | + content = "" |
100 | + for package in packages: |
101 | + parts = [] |
102 | + parts.append('Package: %s' % package.name) |
103 | + parts.append('Version: %s' % package.version) |
104 | + parts.append('Filename: %s' % package.filename) |
105 | + parts.append('Size: %d' % package.size) |
106 | + # TODO: architecture support |
107 | + parts.append('Architecture: all') |
108 | + parts.append('MD5sum: %s' % package.md5) |
109 | + content += "\n".join(parts) |
110 | + content += "\n\n" |
111 | + return content |
112 | + |
113 | + |
114 | class DummyProgress(object): |
115 | """An AcquireProgress that silences all output. |
116 | |
117 | |
118 | === modified file 'hwpack/testing.py' |
119 | --- hwpack/testing.py 2010-09-02 22:03:43 +0000 |
120 | +++ hwpack/testing.py 2010-09-02 22:03:43 +0000 |
121 | @@ -9,7 +9,7 @@ |
122 | from testtools import TestCase |
123 | |
124 | from hwpack.better_tarfile import writeable_tarfile |
125 | -from hwpack.packages import FetchedPackage |
126 | +from hwpack.packages import get_packages_file, FetchedPackage |
127 | |
128 | |
129 | @contextmanager |
130 | @@ -109,14 +109,7 @@ |
131 | os.path.join(self.rootdir, package.filename), 'wb') as f: |
132 | f.write(package.content.read()) |
133 | with open(os.path.join(self.rootdir, "Packages"), 'wb') as f: |
134 | - for package in self.packages: |
135 | - f.write('Package: %s\n' % package.name) |
136 | - f.write('Version: %s\n' % package.version) |
137 | - f.write('Filename: %s\n' % package.filename) |
138 | - f.write('Size: %d\n' % package.size) |
139 | - f.write('Architecture: all\n') |
140 | - f.write('MD5sum: %s\n' % package.md5) |
141 | - f.write('\n') |
142 | + f.write(get_packages_file(self.packages)) |
143 | |
144 | def tearDown(self): |
145 | if os.path.exists(self.rootdir): |
146 | |
147 | === modified file 'hwpack/tests/test_hardwarepack.py' |
148 | --- hwpack/tests/test_hardwarepack.py 2010-09-02 00:01:18 +0000 |
149 | +++ hwpack/tests/test_hardwarepack.py 2010-09-02 22:03:43 +0000 |
150 | @@ -4,7 +4,9 @@ |
151 | from testtools import TestCase |
152 | |
153 | from hwpack.hardwarepack import HardwarePack, Metadata |
154 | +from hwpack.packages import get_packages_file |
155 | from hwpack.tarfile_matchers import TarfileHasFile |
156 | +from hwpack.testing import DummyFetchedPackage |
157 | |
158 | |
159 | class MetadataTests(TestCase): |
160 | @@ -159,11 +161,62 @@ |
161 | tf = self.get_tarfile(hwpack) |
162 | self.assertThat(tf, HardwarePackHasFile("manifest", content="")) |
163 | |
164 | + def test_manifest_contains_package_info(self): |
165 | + package1 = DummyFetchedPackage("foo", "1.1") |
166 | + package2 = DummyFetchedPackage("bar", "1.2") |
167 | + hwpack = HardwarePack(self.metadata) |
168 | + hwpack.add_packages([package1, package2]) |
169 | + tf = self.get_tarfile(hwpack) |
170 | + self.assertThat( |
171 | + tf, |
172 | + HardwarePackHasFile("manifest", content="foo 1.1\nbar 1.2\n")) |
173 | + |
174 | def test_creates_pkgs_dir(self): |
175 | hwpack = HardwarePack(self.metadata) |
176 | tf = self.get_tarfile(hwpack) |
177 | self.assertThat(tf, HardwarePackHasFile("pkgs", type=tarfile.DIRTYPE)) |
178 | |
179 | + def test_adds_packages(self): |
180 | + package = DummyFetchedPackage("foo", "1.1") |
181 | + hwpack = HardwarePack(self.metadata) |
182 | + hwpack.add_packages([package]) |
183 | + tf = self.get_tarfile(hwpack) |
184 | + self.assertThat( |
185 | + tf, |
186 | + HardwarePackHasFile("pkgs/%s" % package.filename, |
187 | + content=package.content.read())) |
188 | + |
189 | + def test_adds_multiple_packages_at_once(self): |
190 | + package1 = DummyFetchedPackage("foo", "1.1") |
191 | + package2 = DummyFetchedPackage("bar", "1.1") |
192 | + hwpack = HardwarePack(self.metadata) |
193 | + hwpack.add_packages([package1, package2]) |
194 | + tf = self.get_tarfile(hwpack) |
195 | + self.assertThat( |
196 | + tf, |
197 | + HardwarePackHasFile("pkgs/%s" % package1.filename, |
198 | + content=package1.content.read())) |
199 | + self.assertThat( |
200 | + tf, |
201 | + HardwarePackHasFile("pkgs/%s" % package2.filename, |
202 | + content=package2.content.read())) |
203 | + |
204 | + def test_adds_multiple_in_multiple_steps(self): |
205 | + package1 = DummyFetchedPackage("foo", "1.1") |
206 | + package2 = DummyFetchedPackage("bar", "1.1") |
207 | + hwpack = HardwarePack(self.metadata) |
208 | + hwpack.add_packages([package1]) |
209 | + hwpack.add_packages([package2]) |
210 | + tf = self.get_tarfile(hwpack) |
211 | + self.assertThat( |
212 | + tf, |
213 | + HardwarePackHasFile("pkgs/%s" % package1.filename, |
214 | + content=package1.content.read())) |
215 | + self.assertThat( |
216 | + tf, |
217 | + HardwarePackHasFile("pkgs/%s" % package2.filename, |
218 | + content=package2.content.read())) |
219 | + |
220 | def test_creates_Packages_file(self): |
221 | hwpack = HardwarePack(self.metadata) |
222 | tf = self.get_tarfile(hwpack) |
223 | @@ -174,12 +227,57 @@ |
224 | tf = self.get_tarfile(hwpack) |
225 | self.assertThat(tf, HardwarePackHasFile("pkgs/Packages", content="")) |
226 | |
227 | + def test_Packages_file_correct_contents_with_packages(self): |
228 | + package1 = DummyFetchedPackage("foo", "1.1") |
229 | + package2 = DummyFetchedPackage("bar", "1.1") |
230 | + hwpack = HardwarePack(self.metadata) |
231 | + hwpack.add_packages([package1, package2]) |
232 | + tf = self.get_tarfile(hwpack) |
233 | + self.assertThat( |
234 | + tf, |
235 | + HardwarePackHasFile( |
236 | + "pkgs/Packages", |
237 | + content=get_packages_file([package1, package2]))) |
238 | + |
239 | def test_creates_sources_list_dir(self): |
240 | hwpack = HardwarePack(self.metadata) |
241 | tf = self.get_tarfile(hwpack) |
242 | self.assertThat( |
243 | tf, HardwarePackHasFile("sources.list.d", type=tarfile.DIRTYPE)) |
244 | |
245 | + def test_adds_sources_list_file(self): |
246 | + hwpack = HardwarePack(self.metadata) |
247 | + source = 'http://example.org/ ubuntu' |
248 | + hwpack.add_apt_sources({'ubuntu': source}) |
249 | + tf = self.get_tarfile(hwpack) |
250 | + self.assertThat( |
251 | + tf, HardwarePackHasFile("sources.list.d/ubuntu", |
252 | + content="deb " + source + "\n")) |
253 | + |
254 | + def test_adds_multiple_sources_list_files(self): |
255 | + hwpack = HardwarePack(self.metadata) |
256 | + source1 = 'http://example.org/ ubuntu main universe' |
257 | + source2 = 'http://example.org/ linaro' |
258 | + hwpack.add_apt_sources({'ubuntu': source1, 'linaro': source2}) |
259 | + tf = self.get_tarfile(hwpack) |
260 | + self.assertThat( |
261 | + tf, HardwarePackHasFile("sources.list.d/ubuntu", |
262 | + content="deb " + source1 + "\n")) |
263 | + self.assertThat( |
264 | + tf, HardwarePackHasFile("sources.list.d/linaro", |
265 | + content="deb " + source2 + "\n")) |
266 | + |
267 | + def test_overwrites_sources_list_file(self): |
268 | + hwpack = HardwarePack(self.metadata) |
269 | + old_source = 'http://example.org/ ubuntu' |
270 | + hwpack.add_apt_sources({'ubuntu': old_source}) |
271 | + new_source = 'http://example.org/ ubuntu main universe' |
272 | + hwpack.add_apt_sources({'ubuntu': new_source}) |
273 | + tf = self.get_tarfile(hwpack) |
274 | + self.assertThat( |
275 | + tf, HardwarePackHasFile("sources.list.d/ubuntu", |
276 | + content="deb " + new_source + "\n")) |
277 | + |
278 | def test_creates_sources_list_gpg_dir(self): |
279 | hwpack = HardwarePack(self.metadata) |
280 | tf = self.get_tarfile(hwpack) |
281 | |
282 | === modified file 'hwpack/tests/test_packages.py' |
283 | --- hwpack/tests/test_packages.py 2010-09-02 22:03:43 +0000 |
284 | +++ hwpack/tests/test_packages.py 2010-09-02 22:03:43 +0000 |
285 | @@ -1,10 +1,12 @@ |
286 | import os |
287 | from StringIO import StringIO |
288 | +import textwrap |
289 | |
290 | from testtools import TestCase |
291 | |
292 | from hwpack.packages import ( |
293 | FetchedPackage, |
294 | + get_packages_file, |
295 | PackageFetcher, |
296 | ) |
297 | from hwpack.testing import ( |
298 | @@ -14,6 +16,31 @@ |
299 | ) |
300 | |
301 | |
302 | +class GetPackagesFileTests(TestCase): |
303 | + |
304 | + def test_single_stanza(self): |
305 | + package = DummyFetchedPackage("foo", "1.1") |
306 | + self.assertEqual(textwrap.dedent("""\ |
307 | + Package: foo |
308 | + Version: 1.1 |
309 | + Filename: %(filename)s |
310 | + Size: %(size)d |
311 | + Architecture: all |
312 | + MD5sum: %(md5)s |
313 | + \n""" % { |
314 | + 'filename': package.filename, |
315 | + 'size': package.size, |
316 | + 'md5': package.md5, |
317 | + }), get_packages_file([package])) |
318 | + |
319 | + def test_two_stanzas(self): |
320 | + package1 = DummyFetchedPackage("foo", "1.1") |
321 | + package2 = DummyFetchedPackage("bar", "1.2") |
322 | + self.assertEqual( |
323 | + get_packages_file([package1]) + get_packages_file([package2]), |
324 | + get_packages_file([package1, package2])) |
325 | + |
326 | + |
327 | class FetchedPackageTests(TestCase): |
328 | |
329 | def test_attributes(self): |
330 | @@ -173,26 +200,26 @@ |
331 | self.assertRaises( |
332 | KeyError, fetcher.fetch_packages, ["foo", "nothere"]) |
333 | |
334 | - def test_fetch_packges_fetches_no_packages(self): |
335 | + def test_fetch_packages_fetches_no_packages(self): |
336 | available_package = DummyFetchedPackage("foo", "1.0") |
337 | source = self.useFixture(AptSourceFixture([available_package])) |
338 | fetcher = self.get_fetcher([source]) |
339 | self.assertEqual(0, len(fetcher.fetch_packages([]))) |
340 | |
341 | - def test_fetch_packges_fetches_single_package(self): |
342 | + def test_fetch_packages_fetches_single_package(self): |
343 | available_package = DummyFetchedPackage("foo", "1.0") |
344 | source = self.useFixture(AptSourceFixture([available_package])) |
345 | fetcher = self.get_fetcher([source]) |
346 | self.assertEqual(1, len(fetcher.fetch_packages(["foo"]))) |
347 | |
348 | - def test_fetch_packges_fetches_correct_packge(self): |
349 | + def test_fetch_packages_fetches_correct_package(self): |
350 | available_package = DummyFetchedPackage("foo", "1.0") |
351 | source = self.useFixture(AptSourceFixture([available_package])) |
352 | fetcher = self.get_fetcher([source]) |
353 | self.assertEqual( |
354 | available_package, fetcher.fetch_packages(["foo"])[0]) |
355 | |
356 | - def test_fetch_packges_fetches_multiple_packages(self): |
357 | + def test_fetch_packages_fetches_multiple_packages(self): |
358 | available_packages = [ |
359 | DummyFetchedPackage("bar", "1.0"), |
360 | DummyFetchedPackage("foo", "1.0"), |
361 | @@ -201,7 +228,7 @@ |
362 | fetcher = self.get_fetcher([source]) |
363 | self.assertEqual(2, len(fetcher.fetch_packages(["foo", "bar"]))) |
364 | |
365 | - def test_fetch_packges_fetches_multiple_packages_correctly(self): |
366 | + def test_fetch_packages_fetches_multiple_packages_correctly(self): |
367 | available_packages = [ |
368 | DummyFetchedPackage("foo", "1.0"), |
369 | DummyFetchedPackage("bar", "1.0"), |
Hi James,
This one looks good as well; I have just a few minor suggestions.
> === modified file 'hwpack/ hardwarepack. py' hardwarepack. py 2010-09-01 23:50:25 +0000 hardwarepack. py 2010-09-02 19:12:45 +0000 better_ tarfile import writeable_tarfile %s_%s%s. tar.gz" % ( version, support_suffix)
> --- hwpack/
> +++ hwpack/
> @@ -1,4 +1,5 @@
> from hwpack.
> +from hwpack.packages import get_packages_file
>
>
> class Metadata(object):
> @@ -75,6 +76,8 @@
> :type metadata: Metadata
> """
> self.metadata = metadata
> + self.sources = {}
> + self.packages = []
>
> def filename(self):
> """The filename that this hardware pack should have.
> @@ -92,6 +95,37 @@
> return "hwpack_
> self.metadata.name, self.metadata.
>
> + def add_sources(self, sources):
> + """Add sources to the hardware pack.
Maybe """Add apt sources ..."""?
> +
> + Given a dict of names and the source lines this will add
> + them to the hardware pack.
> +
> + The names should be an identifier for the source, and the
> + source lines should be what is put in sources.list for that
> + source, minus the "deb" part.
> +
> + Arguments passed to this method will overwrite any that were
> + passed in previous calls with the same identifier, but will
> + be used in addition to any with different identifiers.
I don't quite understand what this means. Do you mean that this will
overwrite existing sources with the same identifier and add the ones with
unseen identifiers?
> + update( sources) FILENAME, self.FORMAT + "\n") file_from_ string( FILENAME, str(self.metadata)) file_from_ string( self.MANIFEST_ FILENAME, "") dir(self. PACKAGES_ DIRNAME) file_from_ string( self.PACKAGES_ FILENAME, "") file_from_ string( DIRNAME + "/" + package.filename, content. read())
> + :param sources: the sources to use as a dict mapping identifiers
> + to sources entries.
> + :type sources: a dict mapping str to str
> + """
> + self.sources.
> +
> + def add_packages(self, packages):
> + """Add packages to the hardware pack.
> +
> + Given a list of packages this will add them to the hardware
> + pack.
> +
> + :param packages: the packages to add
> + :type packages: FetchedPackage
> + """
> + self.packages += packages
> +
> def to_file(self, fileobj):
> """Write the hwpack to a file object.
>
> @@ -112,9 +146,22 @@
> self.FORMAT_
> tf.create_
> self.METADATA_
> - # TODO: include packages and sources etc.
> - tf.create_
> tf.create_
> - tf.create_
> + manifest_content = ""
> + for package in self.packages:
> + tf.create_
> + self.PACKAGES_
> + package.
> + manifest_content += "%s %s\n" % (
> + package.name, package.version)
I wonder if it'd be a problem to use '"%s=%s" % (name, version)' here so that
we could feed such lines directly to apt. AIUI, not all packages will
define a version, s...