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

[James Westby]

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

[lp:~james-w/linaro-image-tools/architectures-from-config]

108. By James Westby on 2010-09-02

Merged architecture-support into architectures-from-config.

[Michael Hudson-Doyle (mwhudson)]

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

[James Westby (james-w)]

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

[Michael Hudson-Doyle (mwhudson)]

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

=== 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 @@
     PACKAGE_REGEX = NAME_REGEX
     ORIGIN_KEY = "origin"
     MAINTAINER_KEY = "maintainer"
+    ARCHITECTURES_KEY = "architectures"
 
     def __init__(self, fp):
         """Create a Config.
@@ -39,6 +40,7 @@
         self._validate_include_debs()
         self._validate_support()
         self._validate_packages()
+        self._validate_architectures()
         self._validate_sections()
 
     @property
@@ -109,7 +111,44 @@
         if raw_packages is None:
             return []
         packages = re.split("\s+", raw_packages)
-        return packages
+        filtered_packages = []
+        for package in packages:
+            if package not in filtered_packages:
+                filtered_packages.append(package)
+        return filtered_packages
+
+    @property
+    def architectures(self):
+        """The architectures to build the hwpack for.
+
+        A list of str.
+        """
+        raw_architectures = self._get_option_from_main_section(
+            self.ARCHITECTURES_KEY)
+        if raw_architectures is None:
+            return []
+        architectures = re.split("\s+", raw_architectures)
+        filtered_architectures = []
+        for architecture in architectures:
+            if architecture not in filtered_architectures:
+                filtered_architectures.append(architecture)
+        return filtered_architectures
+
+    @property
+    def sources(self):
+        """The sources defined in the configuration.
+
+        A dict mapping source identifiers to sources entries.
+        """
+        sources = {}
+        sections = self.parser.sections()
+        found = False
+        for section_name in sections:
+            if section_name == self.MAIN_SECTION:
+                continue
+            sources[section_name] = self.parser.get(
+                section_name, self.SOURCES_ENTRY_KEY)
+        return sources
 
     def _validate_name(self):
         try:
@@ -148,6 +187,13 @@
                     "Invalid value in %s in the [%s] section: %s"
                     % (self.PACKAGES_KEY, self.MAIN_SECTION, package))
 
+    def _validate_architectures(self):
+        architectures = self.architectures
+        if not architectures:
+            raise HwpackConfigError(
+                "No %s in the [%s] section"
+                % (self.ARCHITECTURES_KEY, self.MAIN_SECTION))
+
     def _validate_section_sources_entry(self, section_name):
         try:
             sources_entry = self.parser.get(

=== 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 @@
 
 class ConfigTests(TestCase):
 
-    valid_start = "[hwpack]\nname = ahwpack\npackages = foo\n"
+    valid_start = (
+        "[hwpack]\nname = ahwpack\npackages = foo\narchitectures = armel\n")
 
     def test_create(self):
         config = Config(StringIO())
@@ -70,6 +71,19 @@
             "Invalid value in packages in the [hwpack] section: ~~",
             config)
 
+    def test_validate_no_architectures(self):
+        config = self.get_config(
+                "[hwpack]\nname = ahwpack\npackages = foo\n")
+        self.assertValidationError(
+            "No architectures in the [hwpack] section", config)
+
+    def test_validate_empty_architectures(self):
+        config = self.get_config(
+                "[hwpack]\nname = ahwpack\npackages = foo\n"
+                "architectures = \n")
+        self.assertValidationError(
+            "No architectures in the [hwpack] section", config)
+
     def test_validate_no_other_sections(self):
         config = self.get_config(self.valid_start + "\n")
         self.assertValidationError(
@@ -119,7 +133,9 @@
         self.assertEqual(None, config.validate())
 
     def test_name(self):
-        config = self.get_config("[hwpack]\nname = ahwpack\npackages = foo\n")
+        config = self.get_config(
+            "[hwpack]\nname = ahwpack\npackages = foo\n"
+            "architectures = armel\n")
         self.assertEqual("ahwpack", config.name)
 
     def test_include_debs(self):
@@ -179,10 +195,50 @@
 
     def test_packages(self):
         config = self.get_config(
-            "[hwpack]\nname=ahwpack\npackages=foo  bar\n")
+            "[hwpack]\nname=ahwpack\npackages=foo  bar\n"
+            "architectures=armel\n")
         self.assertEqual(["foo", "bar"], config.packages)
 
     def test_packages_with_newline(self):
         config = self.get_config(
-            "[hwpack]\nname=ahwpack\npackages=foo\n bar\n")
-        self.assertEqual(["foo", "bar"], config.packages)
+            "[hwpack]\nname=ahwpack\npackages=foo\n bar\n"
+            "architectures=armel\n")
+        self.assertEqual(["foo", "bar"], config.packages)
+
+    def test_packages_filters_duplicates(self):
+        config = self.get_config(
+            "[hwpack]\nname=ahwpack\npackages=foo bar foo\n"
+            "architectures=armel\n")
+        self.assertEqual(["foo", "bar"], config.packages)
+
+    def test_sources_single(self):
+        config = self.get_config(
+            self.valid_start
+            + "\n[ubuntu]\nsources-entry = http://example.org foo\n")
+        self.assertEqual({"ubuntu": "http://example.org foo"}, config.sources)
+
+    def test_sources_multiple(self):
+        config = self.get_config(
+            self.valid_start
+            + "\n[ubuntu]\nsources-entry = http://example.org foo\n"
+            + "\n[linaro]\nsources-entry = http://example.org bar\n")
+        self.assertEqual(
+            {"ubuntu": "http://example.org foo",
+             "linaro": "http://example.org bar"},
+            config.sources)
+
+    def test_architectures(self):
+        config = self.get_config(
+            "[hwpack]\nname=ahwpack\npackages=foo\narchitectures=foo  bar\n")
+        self.assertEqual(["foo", "bar"], config.architectures)
+
+    def test_architectures_with_newline(self):
+        config = self.get_config(
+            "[hwpack]\nname=ahwpack\npackages=foo\narchitectures=foo\n bar\n")
+        self.assertEqual(["foo", "bar"], config.architectures)
+
+    def test_architectures_filters_duplicates(self):
+        config = self.get_config(
+            "[hwpack]\nname=ahwpack\npackages=foo\n"
+            "architectures=foo bar foo\n")
+        self.assertEqual(["foo", "bar"], config.architectures)