diff options
author | Sam Thursfield <sam.thursfield@codethink.co.uk> | 2013-02-07 15:20:26 +0000 |
---|---|---|
committer | Sam Thursfield <sam.thursfield@codethink.co.uk> | 2013-02-18 11:30:43 +0000 |
commit | 4ab560d0b8d8243b941a343f4984112112cacbbd (patch) | |
tree | 6bf2f9d4110a34244560637872eb573f941fc74f | |
parent | 8875864676a8d8b5fe08a861eb8662947f8fe115 (diff) | |
download | morph-4ab560d0b8d8243b941a343f4984112112cacbbd.tar.gz |
Make writing morphologies back out properly non-destructive
Remove the special case hacks we had and do a proper comparison
between original and new in-memory dict when writing updates to
user morphologies.
-rw-r--r-- | morphlib/morph2.py | 188 | ||||
-rw-r--r-- | morphlib/morph2_tests.py | 116 | ||||
-rw-r--r-- | morphlib/plugins/branch_and_merge_plugin.py | 102 |
3 files changed, 252 insertions, 154 deletions
diff --git a/morphlib/morph2.py b/morphlib/morph2.py index ae720cfb..488b7d3b 100644 --- a/morphlib/morph2.py +++ b/morphlib/morph2.py @@ -66,20 +66,7 @@ class Morphology(object): f.write('\n') def __init__(self, text): - # Load as JSON first, then try YAML, so morphologies - # that read as JSON are dumped as JSON, likewise with YAML. - try: - self._dict = self._load_json(text) - self._dumper = self._dump_json - except ValueError as e: # pragma: no cover - self._dict = morphlib.yamlparse.load(text) - self._dumper = morphlib.yamlparse.dump - - if data is None: - raise morphlib.YAMLError("Morphology is empty") - if type(data) not in [dict, OrderedDict]: - raise morphlib.YAMLError("Morphology did not parse as a dict") - + self._dict, self._dumper = self._load_morphology_dict(text) self._set_defaults() self._validate_children() @@ -92,6 +79,23 @@ class Morphology(object): def keys(self): return self._dict.keys() + def _load_morphology_dict(self, text): + '''Load morphology, identifying whether it is JSON or YAML''' + + try: + data = self._load_json(text) + dumper = self._dump_json + except ValueError as e: # pragma: no cover + data = morphlib.yamlparse.load(text) + dumper = morphlib.yamlparse.dump + + if data is None: + raise morphlib.YAMLError("Morphology is empty") + if type(data) not in [dict, OrderedDict]: + raise morphlib.YAMLError("Morphology did not parse as a dict") + + return data, dumper + def _validate_children(self): if self['kind'] == 'system': names = set() @@ -108,37 +112,30 @@ class Morphology(object): raise ValueError('Duplicate chunk "%s"' % name) names.add(name) - def lookup_child_by_name(self, name): - '''Find child reference by its name. + def _set_default_value(self, target_dict, key, value): + '''Change a value in the in-memory representation of the morphology - This lookup honors aliases. + Record the default value separately, so that when writing out the + morphology we can determine whether the change from the on-disk value + was done at load time, or later on (we want to only write back out + the later, deliberate changes). ''' - - if self['kind'] == 'system': - for info in self['strata']: - source_name = info.get('alias', info['morph']) - if source_name == name: - return info - elif self['kind'] == 'stratum': - for info in self['chunks']: - source_name = info.get('alias', info['morph']) - if source_name == name: - return info - raise KeyError('"%s" not found' % name) + target_dict[key] = value + target_dict['_orig_' + key] = value def _set_defaults(self): if 'max-jobs' in self: - self._dict['max-jobs'] = int(self['max-jobs']) + self._set_default_value(self._dict, 'max-jobs', + int(self['max-jobs'])) if 'disk-size' in self: - size = self['disk-size'] - self._dict['_disk-size'] = size - self._dict['disk-size'] = self._parse_size(size) + 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._dict[name] = value + self._set_default_value(self._dict, name, value) if self['kind'] == 'stratum': self._set_stratum_defaults() @@ -146,11 +143,11 @@ class Morphology(object): def _set_stratum_defaults(self): for source in self['chunks']: if 'repo' not in source: - source['repo'] = source['name'] + self._set_default_value(source, 'repo', source['name']) if 'morph' not in source: - source['morph'] = source['name'] + self._set_default_value(source, 'morph', source['name']) if 'build-depends' not in source: - source['build-depends'] = None + self._set_default_value(source, 'build-depends', None) def _parse_size(self, size): if isinstance(size, basestring): @@ -163,23 +160,102 @@ class Morphology(object): return int(size[:-1]) * 1024 return int(size) # pragma: no cover - def write_to_file(self, f): # pragma: no cover - # 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 = 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'] + def lookup_child_by_name(self, name): + '''Find child reference by its name. + + This lookup honors aliases. + + ''' + + if self['kind'] == 'system': + for info in self['strata']: + source_name = info.get('alias', info['morph']) + if source_name == name: + return info + elif self['kind'] == 'stratum': + for info in self['chunks']: + source_name = info.get('alias', info['morph']) + if source_name == name: + return info + raise KeyError('"%s" not found' % name) + + def _apply_changes(self, live_dict, original_dict): + '''Returns a new dict updated with changes from the in-memory object + + This allows us to write out a morphology including only the changes + that were done after the morphology was loaded -- not the changes done + to set default values during construction. + + ''' + output_dict = OrderedDict() + + for key in live_dict.keys(): + if key.startswith('_orig_'): + continue + + value = self._apply_changes_for_key(key, live_dict, original_dict) + if value is not None: + output_dict[key] = value + return output_dict + + def _apply_changes_for_key(self, key, live_dict, original_dict): + '''Return value to write out for one key, recursing if necessary''' + + live_value = live_dict.get(key, None) + orig_value = original_dict.get(key, None) + + if type(live_value) in [dict, OrderedDict] and orig_value is not None: + # Recursively apply changes for dict + result = self._apply_changes(live_value, orig_value) + elif type(live_value) is list and orig_value is not None: + # Recursively apply changes for list (existing, then new items). + result = [] + for i in range(0, min(len(orig_value), len(live_value))): + if type(live_value[i]) in [dict, OrderedDict]: + item = self._apply_changes(live_value[i], orig_value[i]) + else: + item = live_value[i] + result.append(item) + for i in range(len(orig_value), len(live_value)): + if type(live_value[i]) in [dict, OrderedDict]: + item = self._apply_changes(live_value[i], {}) + else: + item = live_value[i] + result.append(item) + else: + # 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 else: - value = self[key] - if value and key[0] != '_': - as_dict[key] = value - self._dumper(as_dict, f) + result = live_dict[key] + return result + + def update_text(self, text, output_fd): + '''Write out in-memory changes to loaded morphology text + + Similar in function to update_file(). + + ''' + original_dict, dumper = self._load_morphology_dict(text) + + output_dict = self._apply_changes(self._dict, original_dict) + + dumper(output_dict, output_fd) + + def update_file(self, filename, output_fd=None): # pragma: no cover + '''Write out in-memory changes to on-disk morphology file + + This function reads the original morphology text from 'filename', so + that it can avoid writing out properties that are set in memory + to their default value but weren't specified by the user at all. + + ''' + with open(filename, 'r') as f: + text = f.read() + + with output_fd or morphlib.savefile.SaveFile(filename, 'w') as f: + self.update_text(text, f) diff --git a/morphlib/morph2_tests.py b/morphlib/morph2_tests.py index 61e1744b..d2973d5c 100644 --- a/morphlib/morph2_tests.py +++ b/morphlib/morph2_tests.py @@ -14,6 +14,8 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +import copy +import json import StringIO import unittest @@ -224,38 +226,9 @@ class MorphologyTests(unittest.TestCase): 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) + ## Writing tests - 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 = '''{ + stratum_text = '''{ "kind": "stratum", "chunks": [ { @@ -268,33 +241,90 @@ class MorphologyTests(unittest.TestCase): "name": "bar", "repo": "morphs", "ref": "ref", - "build-depends": [] + "build-depends": [ + "foo" + ] } ] }''' - morphology = Morphology(text) + + def test_writing_preserves_chunk_order(self): + text_lines = self.stratum_text.splitlines() + text_lines[6] = ' "ref": "new-ref",' + + # Change one of the fields + morphology = Morphology(self.stratum_text) + morphology['chunks'][0]['ref'] = 'new-ref' + output = StringIO.StringIO() - morphology.write_to_file(output) + morphology.update_text(self.stratum_text, output) + output_lines = output.getvalue().splitlines() + self.assertEqual(text_lines, output_lines) - text_lines = text.splitlines() + def test_writing_handles_added_chunks(self): + text_lines = self.stratum_text.splitlines() + text_lines = text_lines[0:16] + text_lines[8:17] + text_lines[17:] + text_lines[18] = ' "name": "baz",' + + # Add a new chunk to the list + morphology = Morphology(self.stratum_text) + morphology['chunks'].append(copy.copy(morphology['chunks'][1])) + morphology['chunks'][2]['name'] = 'baz' + + output = StringIO.StringIO() + morphology.update_text(self.stratum_text, output) output_lines = output.getvalue().splitlines() + self.assertEqual(text_lines, output_lines) + + def test_writing_handles_deleted_chunks(self): + text_lines = self.stratum_text.splitlines() + text_lines = text_lines[0:3] + text_lines[9:] + + # Delete a chunk + morphology = Morphology(self.stratum_text) + del morphology['chunks'][0] - # Verify that input and output are equal. + output = StringIO.StringIO() + morphology.update_text(self.stratum_text, output) + output_lines = output.getvalue().splitlines() self.assertEqual(text_lines, output_lines) - def test_writing_preserves_disk_size(self): - text = '''{ + system_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() + 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 - # Verify that in- and output are the same. + 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 = { + 'dict': { '1': 'fee', '2': 'fie', '3': 'foe', '4': 'foo' } + } + live_dict = copy.deepcopy(original_dict) + live_dict['_orig_dict'] = live_dict['dict'] + + dummy = Morphology(self.stratum_text) + output_dict = dummy._apply_changes(live_dict, original_dict) + self.assertEqual(original_dict, output_dict) diff --git a/morphlib/plugins/branch_and_merge_plugin.py b/morphlib/plugins/branch_and_merge_plugin.py index e17ef740..f9595f98 100644 --- a/morphlib/plugins/branch_and_merge_plugin.py +++ b/morphlib/plugins/branch_and_merge_plugin.py @@ -361,7 +361,7 @@ class BranchAndMergePlugin(cliapp.Plugin): known = required[kind] + also_known[kind] for field in morphology.keys(): - if field not in known: + if field not in known and not field.startswith('_orig_'): msg = 'Unknown field "%s" in %s' % (field, basename) logging.warning(msg) self.app.status(msg=msg) @@ -378,18 +378,13 @@ class BranchAndMergePlugin(cliapp.Plugin): (repo_dir, error)) @staticmethod - def save_morphology(repo_dir, name, morphology): + def update_morphology(repo_dir, name, morphology, output_fd=None): if not name.endswith('.morph'): name = '%s.morph' % name - if os.path.isabs(name): - filename = name - else: - filename = os.path.join(repo_dir, name) filename = os.path.join(repo_dir, '%s' % name) - with morphlib.savefile.SaveFile(filename, 'w') as f: - morphology.write_to_file(f) + morphology.update_file(filename, output_fd=output_fd) - if name != morphology['name']: + if name != morphology['name'] + '.morph': logging.warning('%s: morphology "name" should match filename' % filename) @@ -657,7 +652,7 @@ class BranchAndMergePlugin(cliapp.Plugin): # Bring the morphology forward from its ref to the current HEAD repo = self.lrc.get_repo(root_repo) m = repo.load_morphology(spec['ref'], spec['morph']) - self.save_morphology(root_repo_dir, spec['morph'], m) + self.update_morphology(root_repo_dir, spec['morph'], m) self.log_change(spec['repo'], '"%s" copied from "%s" to "%s"' % (spec['morph'], spec['ref'], branch)) @@ -731,8 +726,8 @@ class BranchAndMergePlugin(cliapp.Plugin): stratum_spec['morph'])) # Correct the System Morphology's reference stratum_spec['ref'] = branch - self.save_morphology(stratum_repo_dir, stratum_spec['morph'], - stratum_morphology) + self.update_morphology(stratum_repo_dir, stratum_spec['morph'], + stratum_morphology) self.log_change(root_repo, '"%s" now includes "%s" from "%s"' % (system_name, stratum_name, branch)) @@ -767,14 +762,14 @@ class BranchAndMergePlugin(cliapp.Plugin): # Update the System morphology to use # the modified version of the Stratum stratum_spec['ref'] = branch - self.save_morphology(stratum_repo_dir, - stratum_spec['morph'], - stratum_morphology) + self.update_morphology(stratum_repo_dir, + stratum_spec['morph'], + stratum_morphology) self.log_change(root_repo, '"%s" now includes "%s" from "%s"' % (system_name, stratum_name, branch)) - self.save_morphology(root_repo_dir, system_name, system_morphology) + self.update_morphology(root_repo_dir, system_name, system_morphology) self.print_changelog('The following changes were made but have not ' 'been committed') @@ -921,7 +916,7 @@ class BranchAndMergePlugin(cliapp.Plugin): else: strata[key] = stratum_info['ref'] stratum_info['ref'] = branch - self.save_morphology(root_repo_dir, name, morphology) + self.update_morphology(root_repo_dir, name, morphology) for (repo, morph), ref in strata.iteritems(): repo_dir = self.make_available( @@ -939,7 +934,7 @@ class BranchAndMergePlugin(cliapp.Plugin): update=not self.app.settings['no-git-update']) chunk_info['unpetrify-ref'] = chunk_info['ref'] chunk_info['ref'] = commit_sha1 - self.save_morphology(repo_dir, morph, stratum) + self.update_morphology(repo_dir, morph, stratum) self.print_changelog('The following changes were made but have not ' 'been committed') @@ -978,9 +973,10 @@ class BranchAndMergePlugin(cliapp.Plugin): if 'unpetrify-ref' in chunk_info: chunk_info['ref'] = chunk_info['unpetrify-ref'] del chunk_info['unpetrify-ref'] - self.save_morphology(repo_dir, stratum_info['morph'], stratum) + self.update_morphology(repo_dir, stratum_info['morph'], + stratum) - self.save_morphology(root_repo_dir, name, morphology) + self.update_morphology(root_repo_dir, name, morphology) self.print_changelog('The following changes were made but have not ' 'been committed') @@ -1149,27 +1145,25 @@ class BranchAndMergePlugin(cliapp.Plugin): # Write the petrified morphology to a temporary file in the # branch root repository for inclusion in the tag commit. - handle, tmpfile = tempfile.mkstemp(suffix='.morph') - self.save_morphology(branch_root_dir, tmpfile, morphology) - - # Hash the petrified morphology and add it to the index - # for the tag commit. - sha1 = self.app.runcmd( - ['git', 'hash-object', '-t', 'blob', '-w', tmpfile], - cwd=branch_root_dir, env=env) - self.app.runcmd( - ['git', 'update-index', '--add', '--cacheinfo', - '100644', sha1, '%s.morph' % name], - cwd=branch_root_dir, env=env) - - # Update the working tree if requested. This can be done with - # git-checkout-index, but we still have the file, so use that - if update_working_tree: - shutil.copy(tmpfile, - os.path.join(branch_root_dir, '%s.morph' % name)) - - # Delete the temporary file again. - os.remove(tmpfile) + with tempfile.NamedTemporaryFile(suffix='.morph') as f: + self.update_morphology( + repo_dir, name, morphology, output_fd=f.file) + + # Hash the petrified morphology and add it to the index + # for the tag commit. + sha1 = self.app.runcmd( + ['git', 'hash-object', '-t', 'blob', '-w', f.name], + cwd=branch_root_dir, env=env) + self.app.runcmd( + ['git', 'update-index', '--add', '--cacheinfo', + '100644', sha1, '%s.morph' % name], + cwd=branch_root_dir, env=env) + + # Update the working tree if requested. This can be done with + # git-checkout-index, but we still have the file, so use that + if update_working_tree: + shutil.copy(f.name, + os.path.join(branch_root_dir, '%s.morph' % name)) def resolve_info(self, info, resolved_refs): '''Takes a morphology info and resolves its ref with cache support.''' @@ -1414,7 +1408,7 @@ class BranchAndMergePlugin(cliapp.Plugin): ci['ref'] = old_ci['ref'] merge_chunk(path, old_ci, ci) if changed: - self.save_morphology(to_repo_dir, si['morph'], to_morph) + self.update_morphology(to_repo_dir, si['morph'], to_morph) self.app.runcmd(['git', 'add', si['morph'] + '.morph'], cwd=to_repo_dir) @@ -1447,7 +1441,7 @@ class BranchAndMergePlugin(cliapp.Plugin): si['ref'] = old_si['ref'] merge_stratum(name, old_si, si) if changed: - self.save_morphology(to_root_dir, name, to_morph) + self.update_morphology(to_root_dir, name, to_morph) self.app.runcmd(['git', 'add', f], cwd=to_root_dir) merged_repos = {} @@ -1684,20 +1678,18 @@ class BranchAndMergePlugin(cliapp.Plugin): # Inject temporary refs in the right places in each morphology. morphology = self.load_morphology(repo_dir, filename) self.inject_build_refs(morphology, build_repos, will_push) - handle, tmpfile = tempfile.mkstemp(suffix='.morph') - self.save_morphology(repo_dir, tmpfile, morphology) - - morphology_sha1 = self.app.runcmd( - ['git', 'hash-object', '-t', 'blob', '-w', tmpfile], - cwd=repo_dir, env=env) + with tempfile.NamedTemporaryFile(suffix='.morph') as f: + self.update_morphology( + repo_dir, filename, morphology, output_fd=f.file) - self.app.runcmd( - ['git', 'update-index', '--cacheinfo', - '100644', morphology_sha1, '%s.morph' % filename], - cwd=repo_dir, env=env) + morphology_sha1 = self.app.runcmd( + ['git', 'hash-object', '-t', 'blob', '-w', f.name], + cwd=repo_dir, env=env) - # Remove the temporary morphology file. - os.remove(tmpfile) + self.app.runcmd( + ['git', 'update-index', '--cacheinfo', + '100644', morphology_sha1, '%s.morph' % filename], + cwd=repo_dir, env=env) # Create a commit message including the build UUID. This allows us # to collect all commits of a build across repositories and thereby |