Merge remote-tracking branch 'origin/danielfirth/RT189'

Changed the error (exception) to list all obsolete fields.

8 files changed, 41 insertions, 124 deletions

diff --git a/morphlib/morph2.py b/morphlib/morph2.py
index 6975e699..a10dda8d 100644
--- a/morphlib/morph2.py
+++ b/morphlib/morph2.py
@@ -58,7 +58,6 @@ class Morphology(object):
             ('strata', []),
             ('description', ''),
             ('arch', None),
-            ('system-kind', None),
             ('configuration-extensions', []),
         ],
         'cluster': []
@@ -169,10 +168,6 @@ class Morphology(object):
             self._set_default_value(self._dict, 'max-jobs',
                                     int(self['max-jobs']))
 
-        if 'disk-size' in self:
-            self._set_default_value(self._dict, 'disk-size',
-                                    self._parse_size(self['disk-size']))
-
         for name, value in self.static_defaults[self['kind']]:
             if name not in self._dict:
                 self._set_default_value(self._dict, name, value)
@@ -207,17 +202,6 @@ class Morphology(object):
                                             'deploy',
                                             dict())
 
-    def _parse_size(self, size):
-        if isinstance(size, basestring):
-            size = size.lower()
-            if size.endswith('g'):
-                return int(size[:-1]) * 1024 ** 3
-            elif size.endswith('m'):  # pragma: no cover
-                return int(size[:-1]) * 1024 ** 2
-            elif size.endswith('k'):  # pragma: no cover
-                return int(size[:-1]) * 1024
-        return int(size) # pragma: no cover
-
     def lookup_child_by_name(self, name):
         '''Find child reference by its name.
 
@@ -285,10 +269,7 @@ class Morphology(object):
             # Simple values. Use original value unless it has been changed from
             # the default in memmory.
             if live_dict[key] == live_dict.get('_orig_' + key, None):
-                if key in original_dict:
-                    result = original_dict[key]
-                else:
-                    result = None
+                result = original_dict.get(key, None)
             else:
                 result = live_dict[key]
         return result
diff --git a/morphlib/morph2_tests.py b/morphlib/morph2_tests.py
index bf32d3c2..aaa1d1cc 100644
--- a/morphlib/morph2_tests.py
+++ b/morphlib/morph2_tests.py
@@ -96,29 +96,16 @@ class MorphologyTests(unittest.TestCase):
         self.assertEqual(m['chunks'][0]['morph'], 'le-chunk')
         self.assertEqual(m['chunks'][0]['build-depends'], None)
 
-    def test_parses_system_disk_size(self):
-        m = Morphology('''
-            {
-                "name": "foo",
-                "kind": "system",
-                "disk-size": "1g"
-            }
-        ''')
-
-        self.assertEqual(m['disk-size'], 1024 ** 3)
-
     def test_returns_dict_keys(self):
         m = Morphology('''
             {
                 "name": "foo",
                 "kind": "system",
-                "disk-size": "1g"
             }
         ''')
 
         self.assertTrue('name' in m.keys())
         self.assertTrue('kind' in m.keys())
-        self.assertTrue('disk-size' in m.keys())
 
     def test_system_indexes_strata(self):
         m = Morphology('''
@@ -307,32 +294,9 @@ class MorphologyTests(unittest.TestCase):
 
     system_text = '''{
     "kind": "system",
-    "disk-size": "1g",
     "arch": "x86_64",
-    "system-kind": "rootfs-tarball"
 }'''
 
-    def test_writing_preserves_disk_size(self):
-        text_lines = self.system_text.splitlines()
-        morphology = Morphology(self.system_text)
-
-        output = StringIO.StringIO()
-        morphology.update_text(self.system_text, output)
-        output_lines = output.getvalue().splitlines()
-        self.assertEqual(text_lines, output_lines)
-
-    def test_writing_updates_disk_size(self):
-        text_lines = self.system_text.splitlines()
-        text_lines[2] = '    "disk-size": 512,'
-
-        morphology = Morphology(self.system_text)
-        morphology._dict['disk-size'] = 512
-
-        output = StringIO.StringIO()
-        morphology.update_text(self.system_text, output)
-        output_lines = output.getvalue().splitlines()
-        self.assertEqual(text_lines, output_lines)
-
     def test_nested_dict(self):
         # Real morphologies don't trigger this code path, so we test manually
         original_dict = {
diff --git a/morphlib/morphloader.py b/morphlib/morphloader.py
index c94078f9..8c20d9ed 100644
--- a/morphlib/morphloader.py
+++ b/morphlib/morphloader.py
@@ -54,6 +54,12 @@ class InvalidFieldError(morphlib.Error):
         self.msg = (
             'Field %s not allowed in morphology %s' % (field, morphology))
 
+class ObsoleteFieldsError(morphlib.Error):
+
+    def __init__(self, fields, morphology):
+        self.msg = (
+           'Morphology %s uses obsolete fields: %s' % 
+           (morphology, ' '.join(fields)))
 
 class UnknownArchitectureError(morphlib.Error):
 
@@ -62,14 +68,6 @@ class UnknownArchitectureError(morphlib.Error):
             'Unknown architecture %s in morphology %s' % (arch, morphology))
 
 
-class InvalidSystemKindError(morphlib.Error):
-
-    def __init__(self, system_kind, morphology):
-        self.msg = (
-            'system-kind %s not allowed (must be rootfs-tarball), in %s' %
-                (system_kind, morphology))
-
-
 class NoBuildDependenciesError(morphlib.Error):
 
     def __init__(self, stratum_name, chunk_name, morphology):
@@ -115,6 +113,13 @@ class MorphologyLoader(object):
         ],
     }
 
+    _obsolete_fields = {
+        'system': [
+            'system-kind',
+            'disk-size',
+        ],
+    }
+
     _static_defaults = {
         'chunk': {
             'description': '',
@@ -144,9 +149,7 @@ class MorphologyLoader(object):
             'strata': [],
             'description': '',
             'arch': None,
-            'system-kind': 'rootfs-tarball',
             'configuration-extensions': [],
-            'disk-size': '1G',
         },
         'cluster': {},
     }
@@ -229,8 +232,10 @@ class MorphologyLoader(object):
             raise UnknownKindError(morph['kind'], morph.filename)
 
         required = ['kind'] + self._required_fields[kind]
+        obsolete = self._obsolete_fields.get(kind, [])
         allowed = self._static_defaults[kind].keys()
         self._require_fields(required, morph)
+        self._deny_obsolete_fields(obsolete, morph)
         self._deny_unknown_fields(required + allowed, morph)
 
         if kind == 'system':
@@ -260,12 +265,6 @@ class MorphologyLoader(object):
         if morph['arch'] not in morphlib.valid_archs:
             raise UnknownArchitectureError(morph['arch'], morph.filename)
 
-        # If system-kind is present, it must be rootfs-tarball.
-        if 'system-kind' in morph:
-            if morph['system-kind'] not in (None, 'rootfs-tarball'):
-                raise InvalidSystemKindError(
-                    morph['system-kind'], morph.filename)
-
     def _validate_stratum(self, morph):
         # Require at least one chunk.
         if len(morph.get('chunks', [])) == 0:
@@ -308,6 +307,11 @@ class MorphologyLoader(object):
         for field in fields:
             self._require_field(field, morphology)
 
+    def _deny_obsolete_fields(self, fields, morphology):
+        obsolete_ones = [x for x in morphology if x in fields]
+        if obsolete_ones:
+            raise ObsoleteFieldsError(obsolete_ones, morphology.filename)
+
     def _deny_unknown_fields(self, allowed, morphology):
         for field in morphology:
             if field not in allowed:
diff --git a/morphlib/morphloader_tests.py b/morphlib/morphloader_tests.py
index ac0fef53..f38d58e8 100644
--- a/morphlib/morphloader_tests.py
+++ b/morphlib/morphloader_tests.py
@@ -94,6 +94,26 @@ build-system: dummy
         self.assertRaises(
             morphlib.morphloader.InvalidFieldError, self.loader.validate, m)
 
+    def test_fails_to_validate_system_with_obsolete_system_kind_field(self):
+        m = morphlib.morph3.Morphology({
+            'kind': 'system',
+            'name': 'foo',
+            'arch': 'x86_64',
+            'system-kind': 'foo',
+        })
+        self.assertRaises(
+            morphlib.morphloader.ObsoleteFieldsError, self.loader.validate, m)
+
+    def test_fails_to_validate_system_with_obsolete_disk_size_field(self):
+        m = morphlib.morph3.Morphology({
+            'kind': 'system',
+            'name': 'foo',
+            'arch': 'x86_64',
+            'disk-size': 'over 9000',
+        })
+        self.assertRaises(
+            morphlib.morphloader.ObsoleteFieldsError, self.loader.validate, m)
+
     def test_fails_to_validate_system_with_no_fields(self):
         m = morphlib.morph3.Morphology({
             'kind': 'system',
@@ -182,22 +202,6 @@ build-system: dummy
         self.loader.validate(m)
         self.assertEqual(m['arch'], 'armv7l')
 
-    def test_validate_requires_system_kind_to_be_tarball_if_present(self):
-        m = morphlib.morph3.Morphology(
-            {
-                "kind": "system",
-                "name": "foo",
-                "arch": "armv7l",
-                "strata": [],
-                "system-kind": "blah",
-            })
-
-        self.assertRaises(
-            morphlib.morphloader.InvalidSystemKindError,
-            self.loader.validate, m)
-        m['system-kind'] = 'rootfs-tarball'
-        self.loader.validate(m)
-
     def test_validate_requires_build_deps_for_chunks_in_strata(self):
         m = morphlib.morph3.Morphology(
             {
@@ -468,13 +472,11 @@ name: foo
             dict(m),
             {
                 'kind': 'system',
-                'system-kind': 'rootfs-tarball',
                 'name': 'foo',
                 'description': '',
                 'arch': 'x86_64',
                 'strata': [],
                 'configuration-extensions': [],
-                'disk-size': '1G',
             })
 
     def test_unsets_defaults_for_system(self):
diff --git a/morphlib/morphologyfactory.py b/morphlib/morphologyfactory.py
index ae5a4332..5afafefb 100644
--- a/morphlib/morphologyfactory.py
+++ b/morphlib/morphologyfactory.py
@@ -140,19 +140,6 @@ class MorphologyFactory(object):
                                  (morphology['arch'],
                                   ', '.join(morphlib.valid_archs)))
 
-        kind = morphology['system-kind']
-        if kind == 'rootfs-tarball': # pragma: no cover
-            self._app.status(
-                msg='WARNING: Obsolete field system-kind used in morphology '
-                    '(it is harmless, but should be removed)')
-        elif kind:
-            raise morphlib.Error(
-                'System kind %s is not supported (anymore), '
-                'the whole system-kind field is deprecated. '
-                'Please remove system-kind from your system '
-                'morphologies and morph deploy to create '
-                'the desired output format.' % kind)
-
         name = morphology['name']
         morphology.builds_artifacts = [name + '-rootfs']
 
diff --git a/morphlib/morphologyfactory_tests.py b/morphlib/morphologyfactory_tests.py
index 06489085..6e1e67d3 100644
--- a/morphlib/morphologyfactory_tests.py
+++ b/morphlib/morphologyfactory_tests.py
@@ -118,7 +118,6 @@ class FakeLocalRepo(object):
         'system.morph': '''{
                 "name": "system",
                 "kind": "system",
-                "system-kind": "%(system_kind)s",
                 "arch": "%(arch)s"
             }''',
         'parse-error.morph': '''{ "name"''',
@@ -130,13 +129,11 @@ class FakeLocalRepo(object):
 
     def __init__(self):
         self.arch = 'x86_64'
-        self.system_kind = ''
 
     def cat(self, sha1, filename):
         if filename in self.morphologies:
             values = {
                 'arch': self.arch,
-                'system_kind': self.system_kind,
             }
             return self.morphologies[filename] % values
         elif filename.endswith('.morph'):
@@ -308,11 +305,6 @@ class MorphologyFactoryTests(unittest.TestCase):
         morph = self.mf.get_morphology('reponame', 'sha1', 'system.morph')
         self.assertEqual(morph['arch'], 'armv7l')
 
-    def test_fails_if_system_define_system_kind_that_is_not_tarball(self):
-        self.lr.system_kind = 'blahblah'
-        self.assertRaises(morphlib.Error, self.mf.get_morphology,
-                          'reponame', 'sha1', 'system.morph')
-
     def test_fails_on_parse_error(self):
         self.assertRaises(morphlib.Error, self.mf.get_morphology,
                           'reponame', 'sha1', 'parse-error.morph')
diff --git a/morphlib/plugins/branch_and_merge_new_plugin.py b/morphlib/plugins/branch_and_merge_new_plugin.py
index 3d0a71a5..0a43b918 100644
--- a/morphlib/plugins/branch_and_merge_new_plugin.py
+++ b/morphlib/plugins/branch_and_merge_new_plugin.py
@@ -434,16 +434,6 @@ class SimpleBranchAndMergePlugin(cliapp.Plugin):
         sb = morphlib.sysbranchdir.open_from_within('.')
         loader = morphlib.morphloader.MorphologyLoader()
 
-        # FIXME: The old "morph edit" code did its own morphology validation,
-        # which was much laxer than what MorphologyFactory does, or the
-        # new MorphologyLoader does. This new "morph edit" uses
-        # MorphologyLoader, and the stricter validation breaks the test
-        # suite. However, I want to keep the test suite as untouched as
-        # possible, until all the old code is gone (after which the test
-        # suite will be refactored). Thus, to work around the test suite
-        # breaking, we disable morphology validation for now.
-        loader.validate = lambda *args: None
-
         # Load the system morphology, and all stratum morphologies, including
         # all the strata that are being build-depended on.
 
diff --git a/morphlib/plugins/branch_and_merge_plugin.py b/morphlib/plugins/branch_and_merge_plugin.py
index 2f8560d0..fec16415 100644
--- a/morphlib/plugins/branch_and_merge_plugin.py
+++ b/morphlib/plugins/branch_and_merge_plugin.py
@@ -333,7 +333,6 @@ class BranchAndMergePlugin(cliapp.Plugin):
         required = {
             'system': [
                 'name',
-                'system-kind',
                 'arch',
                 'strata',
             ],
@@ -354,8 +353,6 @@ class BranchAndMergePlugin(cliapp.Plugin):
             'system': [
                 'kind',
                 'description',
-                'disk-size',
-                '_disk-size',
                 'configuration-extensions',
             ],
             'stratum': [