Merge lp:~james-w/linaro-image-tools/architectures-from-config into lp:linaro-image-tools/11.11

Proposed by James Westby
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: 108
Merged at revision: 66
Proposed branch: lp:~james-w/linaro-image-tools/architectures-from-config
Merge into: lp:linaro-image-tools/11.11
Prerequisite: lp:~james-w/linaro-image-tools/architecture-support
Diff against target: 178 lines (+108/-6)
2 files modified
hwpack/config.py (+47/-1)
hwpack/tests/test_config.py (+61/-5)
To merge this branch: bzr merge lp:~james-w/linaro-image-tools/architectures-from-config
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle (community) Approve
Review via email: mp+34467@code.launchpad.net

Description of the change

Hi,

Here's a small branch that adds architectures to the configuration file.

I decided that we want the configuration to at least state which
architecture(s) it is intended for. We can add ways to limit/expand this
list at build time later if we want.

Firstly this branch adds the code and tests to ensure that the architectures
list is present and has at least one entry.

Then it adds a property for getting the list of architectures.

Lastly it also adds a property to get the sources. I folded this in as it is
very small, and along the same lines. There are already tests for the validity
of the sources specification.

Thanks,

James

To post a comment you must log in.
108. By James Westby

Merged architecture-support into architectures-from-config.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

It all looks fine.

I'm not sure of the motivation for filtering out duplicates in the accessor methods -- wouldn't complaining during validation make just as much sense?

review: Approve
Revision history for this message
James Westby (james-w) wrote :

On Fri, 03 Sep 2010 05:05:54 -0000, Michael Hudson <email address hidden> wrote:
> Review: Approve
> It all looks fine.
>
> I'm not sure of the motivation for filtering out duplicates in the
> accessor methods -- wouldn't complaining during validation make just
> as much sense?

Yes, it would, but I figured that we should just accept the data as it's
understandable, if wrong. I can make the change if you want.

Thanks,

James

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

On Sat, 04 Sep 2010 10:36:44 -0000, James Westby <email address hidden> wrote:
> On Fri, 03 Sep 2010 05:05:54 -0000, Michael Hudson <email address hidden> wrote:
> > Review: Approve
> > It all looks fine.
> >
> > I'm not sure of the motivation for filtering out duplicates in the
> > accessor methods -- wouldn't complaining during validation make just
> > as much sense?
>
> Yes, it would, but I figured that we should just accept the data as it's
> understandable, if wrong. I can make the change if you want.

No, that sounds fair enough.

Cheers,
mwh

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'hwpack/config.py'
--- hwpack/config.py 2010-08-31 20:51:40 +0000
+++ hwpack/config.py 2010-09-02 23:07:47 +0000
@@ -19,6 +19,7 @@
19 PACKAGE_REGEX = NAME_REGEX19 PACKAGE_REGEX = NAME_REGEX
20 ORIGIN_KEY = "origin"20 ORIGIN_KEY = "origin"
21 MAINTAINER_KEY = "maintainer"21 MAINTAINER_KEY = "maintainer"
22 ARCHITECTURES_KEY = "architectures"
2223
23 def __init__(self, fp):24 def __init__(self, fp):
24 """Create a Config.25 """Create a Config.
@@ -39,6 +40,7 @@
39 self._validate_include_debs()40 self._validate_include_debs()
40 self._validate_support()41 self._validate_support()
41 self._validate_packages()42 self._validate_packages()
43 self._validate_architectures()
42 self._validate_sections()44 self._validate_sections()
4345
44 @property46 @property
@@ -109,7 +111,44 @@
109 if raw_packages is None:111 if raw_packages is None:
110 return []112 return []
111 packages = re.split("\s+", raw_packages)113 packages = re.split("\s+", raw_packages)
112 return packages114 filtered_packages = []
115 for package in packages:
116 if package not in filtered_packages:
117 filtered_packages.append(package)
118 return filtered_packages
119
120 @property
121 def architectures(self):
122 """The architectures to build the hwpack for.
123
124 A list of str.
125 """
126 raw_architectures = self._get_option_from_main_section(
127 self.ARCHITECTURES_KEY)
128 if raw_architectures is None:
129 return []
130 architectures = re.split("\s+", raw_architectures)
131 filtered_architectures = []
132 for architecture in architectures:
133 if architecture not in filtered_architectures:
134 filtered_architectures.append(architecture)
135 return filtered_architectures
136
137 @property
138 def sources(self):
139 """The sources defined in the configuration.
140
141 A dict mapping source identifiers to sources entries.
142 """
143 sources = {}
144 sections = self.parser.sections()
145 found = False
146 for section_name in sections:
147 if section_name == self.MAIN_SECTION:
148 continue
149 sources[section_name] = self.parser.get(
150 section_name, self.SOURCES_ENTRY_KEY)
151 return sources
113152
114 def _validate_name(self):153 def _validate_name(self):
115 try:154 try:
@@ -148,6 +187,13 @@
148 "Invalid value in %s in the [%s] section: %s"187 "Invalid value in %s in the [%s] section: %s"
149 % (self.PACKAGES_KEY, self.MAIN_SECTION, package))188 % (self.PACKAGES_KEY, self.MAIN_SECTION, package))
150189
190 def _validate_architectures(self):
191 architectures = self.architectures
192 if not architectures:
193 raise HwpackConfigError(
194 "No %s in the [%s] section"
195 % (self.ARCHITECTURES_KEY, self.MAIN_SECTION))
196
151 def _validate_section_sources_entry(self, section_name):197 def _validate_section_sources_entry(self, section_name):
152 try:198 try:
153 sources_entry = self.parser.get(199 sources_entry = self.parser.get(
154200
=== modified file 'hwpack/tests/test_config.py'
--- hwpack/tests/test_config.py 2010-08-31 20:51:40 +0000
+++ hwpack/tests/test_config.py 2010-09-02 23:07:47 +0000
@@ -7,7 +7,8 @@
77
8class ConfigTests(TestCase):8class ConfigTests(TestCase):
99
10 valid_start = "[hwpack]\nname = ahwpack\npackages = foo\n"10 valid_start = (
11 "[hwpack]\nname = ahwpack\npackages = foo\narchitectures = armel\n")
1112
12 def test_create(self):13 def test_create(self):
13 config = Config(StringIO())14 config = Config(StringIO())
@@ -70,6 +71,19 @@
70 "Invalid value in packages in the [hwpack] section: ~~",71 "Invalid value in packages in the [hwpack] section: ~~",
71 config)72 config)
7273
74 def test_validate_no_architectures(self):
75 config = self.get_config(
76 "[hwpack]\nname = ahwpack\npackages = foo\n")
77 self.assertValidationError(
78 "No architectures in the [hwpack] section", config)
79
80 def test_validate_empty_architectures(self):
81 config = self.get_config(
82 "[hwpack]\nname = ahwpack\npackages = foo\n"
83 "architectures = \n")
84 self.assertValidationError(
85 "No architectures in the [hwpack] section", config)
86
73 def test_validate_no_other_sections(self):87 def test_validate_no_other_sections(self):
74 config = self.get_config(self.valid_start + "\n")88 config = self.get_config(self.valid_start + "\n")
75 self.assertValidationError(89 self.assertValidationError(
@@ -119,7 +133,9 @@
119 self.assertEqual(None, config.validate())133 self.assertEqual(None, config.validate())
120134
121 def test_name(self):135 def test_name(self):
122 config = self.get_config("[hwpack]\nname = ahwpack\npackages = foo\n")136 config = self.get_config(
137 "[hwpack]\nname = ahwpack\npackages = foo\n"
138 "architectures = armel\n")
123 self.assertEqual("ahwpack", config.name)139 self.assertEqual("ahwpack", config.name)
124140
125 def test_include_debs(self):141 def test_include_debs(self):
@@ -179,10 +195,50 @@
179195
180 def test_packages(self):196 def test_packages(self):
181 config = self.get_config(197 config = self.get_config(
182 "[hwpack]\nname=ahwpack\npackages=foo bar\n")198 "[hwpack]\nname=ahwpack\npackages=foo bar\n"
199 "architectures=armel\n")
183 self.assertEqual(["foo", "bar"], config.packages)200 self.assertEqual(["foo", "bar"], config.packages)
184201
185 def test_packages_with_newline(self):202 def test_packages_with_newline(self):
186 config = self.get_config(203 config = self.get_config(
187 "[hwpack]\nname=ahwpack\npackages=foo\n bar\n")204 "[hwpack]\nname=ahwpack\npackages=foo\n bar\n"
188 self.assertEqual(["foo", "bar"], config.packages)205 "architectures=armel\n")
206 self.assertEqual(["foo", "bar"], config.packages)
207
208 def test_packages_filters_duplicates(self):
209 config = self.get_config(
210 "[hwpack]\nname=ahwpack\npackages=foo bar foo\n"
211 "architectures=armel\n")
212 self.assertEqual(["foo", "bar"], config.packages)
213
214 def test_sources_single(self):
215 config = self.get_config(
216 self.valid_start
217 + "\n[ubuntu]\nsources-entry = http://example.org foo\n")
218 self.assertEqual({"ubuntu": "http://example.org foo"}, config.sources)
219
220 def test_sources_multiple(self):
221 config = self.get_config(
222 self.valid_start
223 + "\n[ubuntu]\nsources-entry = http://example.org foo\n"
224 + "\n[linaro]\nsources-entry = http://example.org bar\n")
225 self.assertEqual(
226 {"ubuntu": "http://example.org foo",
227 "linaro": "http://example.org bar"},
228 config.sources)
229
230 def test_architectures(self):
231 config = self.get_config(
232 "[hwpack]\nname=ahwpack\npackages=foo\narchitectures=foo bar\n")
233 self.assertEqual(["foo", "bar"], config.architectures)
234
235 def test_architectures_with_newline(self):
236 config = self.get_config(
237 "[hwpack]\nname=ahwpack\npackages=foo\narchitectures=foo\n bar\n")
238 self.assertEqual(["foo", "bar"], config.architectures)
239
240 def test_architectures_filters_duplicates(self):
241 config = self.get_config(
242 "[hwpack]\nname=ahwpack\npackages=foo\n"
243 "architectures=foo bar foo\n")
244 self.assertEqual(["foo", "bar"], config.architectures)

Subscribers

People subscribed via source and target branches