Preserve sort order of morphologies, so they can be edited by Morph

8 files changed, 174 insertions, 110 deletions

diff --git a/morphlib/morph2.py b/morphlib/morph2.py
index d99e4a9e..0eca6fed 100644
--- a/morphlib/morph2.py
+++ b/morphlib/morph2.py
@@ -14,7 +14,10 @@
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
 
+import collections
+import copy
 import json
+import re
 
 
 class Morphology(object):
@@ -50,8 +53,15 @@ class Morphology(object):
         ]
     }
 
+    @staticmethod
+    def _order_keys(pairs):
+        result = collections.OrderedDict()
+        for k,v in pairs:
+            result[k] = v
+        return result
+
     def __init__(self, text):
-        self._dict = json.loads(text)
+        self._dict = json.loads(text, object_pairs_hook=self._order_keys)
         self._set_defaults()
         self._validate_children()
 
@@ -105,19 +115,8 @@ class Morphology(object):
 
         if 'disk-size' in self:
             size = self['disk-size']
-            if isinstance(size, basestring):
-                size = size.lower()
-                if size.endswith('g'):
-                    size = int(size[:-1]) * 1024 ** 3
-                elif size.endswith('m'):  # pragma: no cover
-                    size = int(size[:-1]) * 1024 ** 2
-                elif size.endswith('k'):  # pragma: no cover
-                    size = int(size[:-1]) * 1024
-                else:  # pragma: no cover
-                    size = int(size)
-            else: # pragma: no cover
-                size = int(size)
-            self._dict['disk-size'] = size
+            self._dict['_disk-size'] = size
+            self._dict['disk-size'] = self._parse_size(size)
 
         for name, value in self.static_defaults[self['kind']]:
             if name not in self._dict:
@@ -134,3 +133,38 @@ class Morphology(object):
                 source['morph'] = source['name']
             if 'build-depends' not in source:
                 source['build-depends'] = None
+
+    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)
+
+    def write_to_file(self, f):
+        # Recreate dict without the empty default values, with a few kind
+        # specific hacks to try and edit standard morphologies as
+        # non-destructively as possible
+        as_dict = collections.OrderedDict()
+        for key in self.keys():
+            if self['kind'] == 'stratum' and key == 'chunks':
+                value = copy.copy(self[key])
+                for chunk in value:
+                    if chunk["morph"] == chunk["name"]:
+                        del chunk["morph"]
+            if self['kind'] == 'system' and key == 'disk-size':
+                # Use human-readable value (assumes we never programmatically
+                # change this value within morph)
+                value = self['_disk-size']
+            else:
+                value = self[key]
+            if value and key[0] != '_':
+                as_dict[key] = value
+        text = json.dumps(as_dict, indent=4)
+        text = re.sub(" \n", "\n", text)
+        f.write(text)
+        f.write('\n')
diff --git a/morphlib/morph2_tests.py b/morphlib/morph2_tests.py
index 60917537..142b5949 100644
--- a/morphlib/morph2_tests.py
+++ b/morphlib/morph2_tests.py
@@ -14,6 +14,7 @@
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
 
+import StringIO
 import unittest
 
 from morphlib.morph2 import Morphology
@@ -197,3 +198,78 @@ class MorphologyTests(unittest.TestCase):
         self.assertRaises(ValueError,
                           Morphology,
                           text)
+
+    def test_writing_preserves_field_order(self):
+        text = '''{
+    "kind": "system",
+    "disk-size": 1073741824,
+    "description": "Some text",
+    "arch": "x86_64",
+    "system-kind": "syslinux-disk",
+    "strata": [
+        {
+            "morph": "foundation",
+            "repo": "morphs",
+            "ref": "ref"
+        },
+        {
+            "morph": "devel",
+            "repo": "morphs",
+            "ref": "ref"
+        }
+    ]
+}'''
+        morphology = Morphology(text)
+        output = StringIO.StringIO()
+        morphology.write_to_file(output)
+
+        text_lines = text.splitlines()
+        output_lines = output.getvalue().splitlines()
+
+        # Verify that input and output are equal.
+        self.assertEqual(text_lines, output_lines)
+
+    def test_writing_stratum_morphology_preserves_chunk_order(self):
+        text = '''{
+    "kind": "stratum",
+    "chunks": [
+        {
+            "name": "foo",
+            "repo": "morphs",
+            "ref": "ref",
+            "build-depends": []
+        },
+        {
+            "name": "bar",
+            "repo": "morphs",
+            "ref": "ref",
+            "build-depends": []
+        }
+    ]
+}'''
+        morphology = Morphology(text)
+        output = StringIO.StringIO()
+        morphology.write_to_file(output)
+
+        text_lines = text.splitlines()
+        output_lines = output.getvalue().splitlines()
+
+        # Verify that input and output are equal.
+        self.assertEqual(text_lines, output_lines)
+
+    def test_writing_preserves_disk_size(self):
+        text = '''{
+    "kind": "system",
+    "disk-size": "1g",
+    "arch": "x86_64",
+    "system-kind": "syslinux-disk"
+}'''
+        morphology = Morphology(text)
+        output = StringIO.StringIO()
+        morphology.write_to_file(output)
+
+        text_lines = text.splitlines()
+        output_lines = output.getvalue().splitlines()
+
+        # Verify that in- and output are the same.
+        self.assertEqual(text_lines, output_lines)
diff --git a/morphlib/plugins/branch_and_merge_plugin.py b/morphlib/plugins/branch_and_merge_plugin.py
index 5e5ce0c1..d6485c2b 100644
--- a/morphlib/plugins/branch_and_merge_plugin.py
+++ b/morphlib/plugins/branch_and_merge_plugin.py
@@ -234,14 +234,9 @@ class BranchAndMergePlugin(cliapp.Plugin):
             filename = name
         else:
             filename = os.path.join(repo_dir, name)
-        as_dict = {}
-        for key in morphology.keys():
-            value = morphology[key]
-            if value:
-                as_dict[key] = value
+        filename = os.path.join(repo_dir, '%s' % name)
         with morphlib.savefile.SaveFile(filename, 'w') as f:
-            json.dump(as_dict, fp=f, indent=4, sort_keys=True)
-            f.write('\n')
+            morphology.write_to_file(f)
 
     @staticmethod
     def get_edit_info(morphology_name, morphology, name, collection='strata'):
@@ -258,17 +253,6 @@ class BranchAndMergePlugin(cliapp.Plugin):
                         (name, morphology_name))
 
     @staticmethod
-    def write_morphology(filename, morphology):
-        as_dict = {}
-        for key in morphology.keys():
-            value = morphology[key]
-            if value:
-                as_dict[key] = value
-        with morphlib.savefile.SaveFile(filename, 'w') as f:
-            json.dump(as_dict, fp=f, indent=4, sort_keys=True)
-            f.write('\n')
-
-    @staticmethod
     def convert_uri_to_path(uri):
         parts = urlparse.urlparse(uri)
 
@@ -357,7 +341,8 @@ class BranchAndMergePlugin(cliapp.Plugin):
                 repo = cache.get_repo(reponame)
                 source['ref'], tree = repo.resolve_ref(ref)
 
-            self.write_morphology(filename, morph)
+            with morphlib.savefile.SaveFile(filename, 'w') as f:
+                morph.write_to_file(f)
 
     def init(self, args):
         '''Initialize a workspace directory.'''
diff --git a/tests.as-root/building-a-system-branch-multiple-times-doesnt-generate-new-artifacts.stdout b/tests.as-root/building-a-system-branch-multiple-times-doesnt-generate-new-artifacts.stdout
index aac9e454..323e99d3 100644
--- a/tests.as-root/building-a-system-branch-multiple-times-doesnt-generate-new-artifacts.stdout
+++ b/tests.as-root/building-a-system-branch-multiple-times-doesnt-generate-new-artifacts.stdout
@@ -1,30 +1,30 @@
 d .
-f ./4854734df8ff06a9fbaab8536be43a46b54c1e6bcb9545ccb8fc345b5fb17ebd.meta
-f ./4854734df8ff06a9fbaab8536be43a46b54c1e6bcb9545ccb8fc345b5fb17ebd.stratum.linux-stratum
-f ./4854734df8ff06a9fbaab8536be43a46b54c1e6bcb9545ccb8fc345b5fb17ebd.stratum.linux-stratum.meta
-f ./cda58b36cef0fe28c5ffaf080fed5ee85128f109f806c4c599c46b1f064ce028.meta
-f ./cda58b36cef0fe28c5ffaf080fed5ee85128f109f806c4c599c46b1f064ce028.system.linux-system-kernel
-f ./cda58b36cef0fe28c5ffaf080fed5ee85128f109f806c4c599c46b1f064ce028.system.linux-system-rootfs
+f ./2311765afd12c233a772a33d7f7e31e2450c191379a430790ea03b07938b9e14.meta
+f ./2311765afd12c233a772a33d7f7e31e2450c191379a430790ea03b07938b9e14.stratum.linux-stratum
+f ./2311765afd12c233a772a33d7f7e31e2450c191379a430790ea03b07938b9e14.stratum.linux-stratum.meta
+f ./a93235b001f0e456593d6ca4d686c968a66206b8b83dcdf8764f649afe1b0d19.meta
+f ./a93235b001f0e456593d6ca4d686c968a66206b8b83dcdf8764f649afe1b0d19.system.linux-system-kernel
+f ./a93235b001f0e456593d6ca4d686c968a66206b8b83dcdf8764f649afe1b0d19.system.linux-system-rootfs
 f ./e8d3ecb31babcb58516f1298ccc5f63b167186e6527d831af5a788309a648cd6.build-log
 f ./e8d3ecb31babcb58516f1298ccc5f63b167186e6527d831af5a788309a648cd6.chunk.linux
 f ./e8d3ecb31babcb58516f1298ccc5f63b167186e6527d831af5a788309a648cd6.meta
 d .
-f ./4854734df8ff06a9fbaab8536be43a46b54c1e6bcb9545ccb8fc345b5fb17ebd.meta
-f ./4854734df8ff06a9fbaab8536be43a46b54c1e6bcb9545ccb8fc345b5fb17ebd.stratum.linux-stratum
-f ./4854734df8ff06a9fbaab8536be43a46b54c1e6bcb9545ccb8fc345b5fb17ebd.stratum.linux-stratum.meta
-f ./cda58b36cef0fe28c5ffaf080fed5ee85128f109f806c4c599c46b1f064ce028.meta
-f ./cda58b36cef0fe28c5ffaf080fed5ee85128f109f806c4c599c46b1f064ce028.system.linux-system-kernel
-f ./cda58b36cef0fe28c5ffaf080fed5ee85128f109f806c4c599c46b1f064ce028.system.linux-system-rootfs
+f ./2311765afd12c233a772a33d7f7e31e2450c191379a430790ea03b07938b9e14.meta
+f ./2311765afd12c233a772a33d7f7e31e2450c191379a430790ea03b07938b9e14.stratum.linux-stratum
+f ./2311765afd12c233a772a33d7f7e31e2450c191379a430790ea03b07938b9e14.stratum.linux-stratum.meta
+f ./a93235b001f0e456593d6ca4d686c968a66206b8b83dcdf8764f649afe1b0d19.meta
+f ./a93235b001f0e456593d6ca4d686c968a66206b8b83dcdf8764f649afe1b0d19.system.linux-system-kernel
+f ./a93235b001f0e456593d6ca4d686c968a66206b8b83dcdf8764f649afe1b0d19.system.linux-system-rootfs
 f ./e8d3ecb31babcb58516f1298ccc5f63b167186e6527d831af5a788309a648cd6.build-log
 f ./e8d3ecb31babcb58516f1298ccc5f63b167186e6527d831af5a788309a648cd6.chunk.linux
 f ./e8d3ecb31babcb58516f1298ccc5f63b167186e6527d831af5a788309a648cd6.meta
 d .
-f ./4854734df8ff06a9fbaab8536be43a46b54c1e6bcb9545ccb8fc345b5fb17ebd.meta
-f ./4854734df8ff06a9fbaab8536be43a46b54c1e6bcb9545ccb8fc345b5fb17ebd.stratum.linux-stratum
-f ./4854734df8ff06a9fbaab8536be43a46b54c1e6bcb9545ccb8fc345b5fb17ebd.stratum.linux-stratum.meta
-f ./cda58b36cef0fe28c5ffaf080fed5ee85128f109f806c4c599c46b1f064ce028.meta
-f ./cda58b36cef0fe28c5ffaf080fed5ee85128f109f806c4c599c46b1f064ce028.system.linux-system-kernel
-f ./cda58b36cef0fe28c5ffaf080fed5ee85128f109f806c4c599c46b1f064ce028.system.linux-system-rootfs
+f ./2311765afd12c233a772a33d7f7e31e2450c191379a430790ea03b07938b9e14.meta
+f ./2311765afd12c233a772a33d7f7e31e2450c191379a430790ea03b07938b9e14.stratum.linux-stratum
+f ./2311765afd12c233a772a33d7f7e31e2450c191379a430790ea03b07938b9e14.stratum.linux-stratum.meta
+f ./a93235b001f0e456593d6ca4d686c968a66206b8b83dcdf8764f649afe1b0d19.meta
+f ./a93235b001f0e456593d6ca4d686c968a66206b8b83dcdf8764f649afe1b0d19.system.linux-system-kernel
+f ./a93235b001f0e456593d6ca4d686c968a66206b8b83dcdf8764f649afe1b0d19.system.linux-system-rootfs
 f ./e8d3ecb31babcb58516f1298ccc5f63b167186e6527d831af5a788309a648cd6.build-log
 f ./e8d3ecb31babcb58516f1298ccc5f63b167186e6527d831af5a788309a648cd6.chunk.linux
 f ./e8d3ecb31babcb58516f1298ccc5f63b167186e6527d831af5a788309a648cd6.meta
diff --git a/tests.as-root/building-a-system-branch-picks-up-uncommitted-changes.stdout b/tests.as-root/building-a-system-branch-picks-up-uncommitted-changes.stdout
index bd264b77..3cb8aee1 100644
--- a/tests.as-root/building-a-system-branch-picks-up-uncommitted-changes.stdout
+++ b/tests.as-root/building-a-system-branch-picks-up-uncommitted-changes.stdout
@@ -1,9 +1,9 @@
 Tree of morphs repo build branch after first build:
-116b36d01472c894812d4a7703d745aa90d17b3d
+101ce7d2f829c44c076c01aef4656ffa3a47e732
 Tree of kernel repo build branch after first build:
 83d99190fa36d14f1bd24d88ba0fbe2ce674dd7c
 
 Tree of morphs repo build branch after second build:
-116b36d01472c894812d4a7703d745aa90d17b3d
+101ce7d2f829c44c076c01aef4656ffa3a47e732
 Tree of kernel repo build branch after second build:
 5b015689415c96cdd290834ba105a64be28a3cfb
diff --git a/tests.as-root/building-a-system-branch-works-anywhere.stdout b/tests.as-root/building-a-system-branch-works-anywhere.stdout
index 68c29eae..82a95018 100644
--- a/tests.as-root/building-a-system-branch-works-anywhere.stdout
+++ b/tests.as-root/building-a-system-branch-works-anywhere.stdout
@@ -1,10 +1,10 @@
 d .
-f ./4854734df8ff06a9fbaab8536be43a46b54c1e6bcb9545ccb8fc345b5fb17ebd.meta
-f ./4854734df8ff06a9fbaab8536be43a46b54c1e6bcb9545ccb8fc345b5fb17ebd.stratum.linux-stratum
-f ./4854734df8ff06a9fbaab8536be43a46b54c1e6bcb9545ccb8fc345b5fb17ebd.stratum.linux-stratum.meta
-f ./cda58b36cef0fe28c5ffaf080fed5ee85128f109f806c4c599c46b1f064ce028.meta
-f ./cda58b36cef0fe28c5ffaf080fed5ee85128f109f806c4c599c46b1f064ce028.system.linux-system-kernel
-f ./cda58b36cef0fe28c5ffaf080fed5ee85128f109f806c4c599c46b1f064ce028.system.linux-system-rootfs
+f ./2311765afd12c233a772a33d7f7e31e2450c191379a430790ea03b07938b9e14.meta
+f ./2311765afd12c233a772a33d7f7e31e2450c191379a430790ea03b07938b9e14.stratum.linux-stratum
+f ./2311765afd12c233a772a33d7f7e31e2450c191379a430790ea03b07938b9e14.stratum.linux-stratum.meta
+f ./a93235b001f0e456593d6ca4d686c968a66206b8b83dcdf8764f649afe1b0d19.meta
+f ./a93235b001f0e456593d6ca4d686c968a66206b8b83dcdf8764f649afe1b0d19.system.linux-system-kernel
+f ./a93235b001f0e456593d6ca4d686c968a66206b8b83dcdf8764f649afe1b0d19.system.linux-system-rootfs
 f ./e8d3ecb31babcb58516f1298ccc5f63b167186e6527d831af5a788309a648cd6.build-log
 f ./e8d3ecb31babcb58516f1298ccc5f63b167186e6527d831af5a788309a648cd6.chunk.linux
 f ./e8d3ecb31babcb58516f1298ccc5f63b167186e6527d831af5a788309a648cd6.meta
diff --git a/tests.branching/edit-updates-stratum.stdout b/tests.branching/edit-updates-stratum.stdout
index a61be053..260ac9a6 100644
--- a/tests.branching/edit-updates-stratum.stdout
+++ b/tests.branching/edit-updates-stratum.stdout
@@ -1,55 +1,26 @@
 diff --git a/hello-stratum.morph b/hello-stratum.morph
-index 006a96c..ad8c08b 100644
+index 006a96c..68fcbff 100644
 --- a/hello-stratum.morph
 +++ b/hello-stratum.morph
-@@ -1,12 +1,14 @@
- {
--    "name": "hello-stratum",
--    "kind": "stratum",
-+    "build-system": "manual", 
-     "chunks": [
+@@ -5,7 +5,7 @@
          {
--            "name": "hello",
--            "repo": "baserock:hello",
+             "name": "hello",
+             "repo": "baserock:hello",
 -            "ref": "master",
--            "build-depends": []
-+            "build-depends": [], 
-+            "morph": "hello", 
-+            "name": "hello", 
-+            "ref": "newbranch", 
-+            "repo": "baserock:hello"
++            "ref": "newbranch",
+             "build-depends": []
          }
--    ]
-+    ], 
-+    "kind": "stratum", 
-+    "name": "hello-stratum"
- }
+     ]
 diff --git a/hello-system.morph b/hello-system.morph
-index 1a33ed6..d5aae46 100644
+index 1a33ed6..9cca1bf 100644
 --- a/hello-system.morph
 +++ b/hello-system.morph
-@@ -1,14 +1,15 @@
- {
--    "name": "hello-system",
--    "kind": "system",
--    "system-kind": "syslinux-disk",
--    "arch": "x86_64",
--    "disk-size": "1G",
-+    "arch": "x86_64", 
-+    "build-system": "manual", 
-+    "disk-size": 1073741824, 
-+    "kind": "system", 
-+    "name": "hello-system", 
-     "strata": [
+@@ -8,7 +8,7 @@
          {
--            "morph": "hello-stratum",
--            "repo": "baserock:morphs",
+             "morph": "hello-stratum",
+             "repo": "baserock:morphs",
 -            "ref": "master"
-+            "morph": "hello-stratum", 
-+            "ref": "newbranch", 
-+            "repo": "baserock:morphs"
++            "ref": "newbranch"
          }
--    ]
-+    ], 
-+    "system-kind": "syslinux-disk"
+     ]
  }
diff --git a/tests.branching/petrify.stdout b/tests.branching/petrify.stdout
index ea0fb795..eecd51ce 100644
--- a/tests.branching/petrify.stdout
+++ b/tests.branching/petrify.stdout
@@ -1,14 +1,12 @@
 {
-    "build-system": "manual", 
+    "name": "hello-stratum",
+    "kind": "stratum",
     "chunks": [
         {
-            "build-depends": [], 
-            "morph": "hello", 
-            "name": "hello", 
-            "ref": "f4d032b42c0134e67bdf19a43fa99072493667d7", 
-            "repo": "baserock:hello"
+            "name": "hello",
+            "repo": "baserock:hello",
+            "ref": "f4d032b42c0134e67bdf19a43fa99072493667d7",
+            "build-depends": []
         }
-    ], 
-    "kind": "stratum", 
-    "name": "hello-stratum"
+    ]
 }