From 5eafbe6c70910b5662c941f9f7a3c9befc61feea Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Thu, 28 Nov 2013 13:03:57 +0000 Subject: yarns: Correctly allow run_morph to output stderr on failure Set -e meant that the stderr could never be re-output, catching the return code and re-outputting was not sufficient. --- yarns/morph.shell-lib | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/yarns/morph.shell-lib b/yarns/morph.shell-lib index 2981e6d9..66abd076 100644 --- a/yarns/morph.shell-lib +++ b/yarns/morph.shell-lib @@ -30,13 +30,16 @@ run_morph() { - "${SRCDIR:-.}"/morph \ - --cachedir-min-space=0 --tempdir-min-space=0 \ - --no-default-config --config "$DATADIR/morph.conf" "$@" \ - 2> "$DATADIR/result-$1" - local exit_code="$?" - cat "$DATADIR/result-$1" >&2 - return "$exit_code" + { + set +e + "${SRCDIR:-.}"/morph \ + --cachedir-min-space=0 --tempdir-min-space=0 \ + --no-default-config --config "$DATADIR/morph.conf" "$@" \ + 2> "$DATADIR/result-$1" + local exit_code="$?" + cat "$DATADIR/result-$1" >&2 + return "$exit_code" + } } -- cgit v1.2.1 From 5ceb033caf279016876643315a66ab2aad3095d1 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Thu, 28 Nov 2013 15:07:41 +0000 Subject: morphloader: Don't use ValueError exception Dedicated exceptions allow more fine-grained exception catching and can have extra data. --- morphlib/morphloader.py | 24 ++++++++++++++++++++++-- morphlib/morphloader_tests.py | 6 ++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/morphlib/morphloader.py b/morphlib/morphloader.py index 702a330c..769d8c8e 100644 --- a/morphlib/morphloader.py +++ b/morphlib/morphloader.py @@ -92,6 +92,26 @@ class EmptyStratumError(morphlib.Error): (stratum_name, morphology)) +class DuplicateChunkError(morphlib.Error): + + def __init__(self, stratum_name, chunk_name): + self.stratum_name = stratum_name + self.chunk_name = chunk_name + morphlib.Error.__init__( + self, 'Duplicate chunk %(chunk_name)s '\ + 'in stratum %(stratum_name)s' % locals()) + + +class DuplicateStratumError(morphlib.Error): + + def __init__(self, system_name, stratum_name): + self.system_name = system_name + self.stratum_name = stratum_name + morphlib.Error.__init__( + self, 'Duplicate stratum %(stratum_name)s '\ + 'in system %(system_name)s' % locals()) + + class MorphologyLoader(object): '''Load morphologies from disk, or save them back to disk.''' @@ -255,7 +275,7 @@ class MorphologyLoader(object): for spec in morph['strata']: name = spec.get('alias', spec['morph']) if name in names: - raise ValueError('Duplicate stratum "%s"' % name) + raise DuplicateStratumError(morph['name'], name) names.add(name) # We allow the ARMv7 little-endian architecture to be specified @@ -277,7 +297,7 @@ class MorphologyLoader(object): for spec in morph['chunks']: name = spec.get('alias', spec['name']) if name in names: - raise ValueError('Duplicate chunk "%s"' % name) + raise DuplicateChunkError(morph['name'], name) names.add(name) # Require build-dependencies for the stratum itself, unless diff --git a/morphlib/morphloader_tests.py b/morphlib/morphloader_tests.py index f38d58e8..2711db20 100644 --- a/morphlib/morphloader_tests.py +++ b/morphlib/morphloader_tests.py @@ -157,7 +157,8 @@ build-system: dummy } ] }) - self.assertRaises(ValueError, self.loader.validate, m) + self.assertRaises(morphlib.morphloader.DuplicateStratumError, + self.loader.validate, m) def test_validate_requires_unique_chunk_names_within_a_stratum(self): m = morphlib.morph3.Morphology( @@ -177,7 +178,8 @@ build-system: dummy } ] }) - self.assertRaises(ValueError, self.loader.validate, m) + self.assertRaises(morphlib.morphloader.DuplicateChunkError, + self.loader.validate, m) def test_validate_requires_a_valid_architecture(self): m = morphlib.morph3.Morphology( -- cgit v1.2.1 From 6e30db8033160fedbf864db08e98fd18b92a0d08 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 22 Nov 2013 16:49:12 +0000 Subject: morphloader: Set default values for cluster morphs This was omittted from the MorphologyLoader due to cluster morphologies being added at about the same time. This bug escaped detection since the MorphologyLoader was not required to deploy. It soon will be. --- morphlib/morphloader.py | 22 +++++++++++++++++++--- morphlib/morphloader_tests.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/morphlib/morphloader.py b/morphlib/morphloader.py index 769d8c8e..9ac248da 100644 --- a/morphlib/morphloader.py +++ b/morphlib/morphloader.py @@ -352,7 +352,9 @@ class MorphologyLoader(object): if key not in morphology: morphology[key] = defaults[key] - if kind == 'system': + if kind == 'cluster': + self._set_cluster_defaults(morphology) + elif kind == 'system': self._set_system_defaults(morphology) elif kind == 'stratum': self._set_stratum_defaults(morphology) @@ -372,8 +374,22 @@ class MorphologyLoader(object): if key in morphology and morphology[key] == defaults[key]: del morphology[key] - if kind == 'stratum': - self._unset_stratum_defaults(morphology) + if kind in ('stratum', 'cluster'): + getattr(self, '_unset_%s_defaults' % kind)(morphology) + + def _set_cluster_defaults(self, morph): + for system in morph.get('systems', []): + if 'deploy-defaults' not in system: + system['deploy-defaults'] = {} + if 'deploy' not in system: + system['deploy'] = {} + + def _unset_cluster_defaults(self, morph): + for system in morph.get('systems', []): + if 'deploy-defaults' in system and system['deploy-defaults'] == {}: + del system['deploy-defaults'] + if 'deploy' in system and system['deploy'] == {}: + del system['deploy'] def _set_system_defaults(self, morph): pass diff --git a/morphlib/morphloader_tests.py b/morphlib/morphloader_tests.py index 2711db20..b6ebff6a 100644 --- a/morphlib/morphloader_tests.py +++ b/morphlib/morphloader_tests.py @@ -497,6 +497,41 @@ name: foo 'arch': 'x86_64', }) + def test_sets_defaults_for_cluster(self): + m = morphlib.morph3.Morphology( + name='foo', + kind='cluster', + systems=[ + {'morph': 'foo'}, + {'morph': 'bar'}]) + self.loader.set_defaults(m) + self.loader.validate(m) + self.assertEqual(m['systems'], + [{'morph': 'foo', + 'deploy-defaults': {}, + 'deploy': {}}, + {'morph': 'bar', + 'deploy-defaults': {}, + 'deploy': {}}]) + + def test_unsets_defaults_for_cluster(self): + m = morphlib.morph3.Morphology( + name='foo', + kind='cluster', + description='', + systems=[ + {'morph': 'foo', + 'deploy-defaults': {}, + 'deploy': {}}, + {'morph': 'bar', + 'deploy-defaults': {}, + 'deploy': {}}]) + self.loader.unset_defaults(m) + self.assertNotIn('description', m) + self.assertEqual(m['systems'], + [{'morph': 'foo'}, + {'morph': 'bar'}]) + def test_sets_stratum_chunks_repo_and_morph_from_name(self): m = morphlib.morph3.Morphology( { -- cgit v1.2.1 From f5c1a50c9f35450801846a0309aa571e9893946a Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 22 Nov 2013 18:28:12 +0000 Subject: morphloader: Require systems have at least one stratum It doesn't currently make sense to build a system which contains no strata. We may later add other fields, such as initramfs to contribute to the system's artifact, but until then it's another bug to trip over. This uses collections.Sequence for checking the type of the systems entry in the morphology as a style choice, though it allows more flexibility if the types in the parsed morphology change. --- morphlib/morphloader.py | 46 ++++++++++++++- morphlib/morphloader_tests.py | 128 +++++++++++++++++++++++++++++++----------- yarns/implementations.yarn | 25 +++++---- yarns/regression.yarn | 4 +- 4 files changed, 155 insertions(+), 48 deletions(-) diff --git a/morphlib/morphloader.py b/morphlib/morphloader.py index 9ac248da..fdc3b62f 100644 --- a/morphlib/morphloader.py +++ b/morphlib/morphloader.py @@ -16,6 +16,7 @@ # =*= License: GPL-2 =*= +import collections import logging import yaml @@ -102,6 +103,16 @@ class DuplicateChunkError(morphlib.Error): 'in stratum %(stratum_name)s' % locals()) +class SystemStrataNotListError(morphlib.Error): + + def __init__(self, system_name, strata_type): + self.system_name = system_name + self.strata_type = strata_type + typename = strata_type.__name__ + morphlib.Error.__init__( + self, 'System %(system_name)s has the wrong type for its strata: '\ + '%(typename)s, expected list' % locals()) + class DuplicateStratumError(morphlib.Error): def __init__(self, system_name, stratum_name): @@ -112,6 +123,23 @@ class DuplicateStratumError(morphlib.Error): 'in system %(system_name)s' % locals()) +class SystemStratumSpecsNotMappingError(morphlib.Error): + + def __init__(self, system_name, strata): + self.system_name = system_name + self.strata = strata + morphlib.Error.__init__( + self, 'System %(system_name)s has stratum specs '\ + 'that are not mappings.' % locals()) + + +class EmptySystemError(morphlib.Error): + + def __init__(self, system_name): + morphlib.Error.__init__( + self, 'System %(system_name)s has no strata.' % locals()) + + class MorphologyLoader(object): '''Load morphologies from disk, or save them back to disk.''' @@ -126,6 +154,7 @@ class MorphologyLoader(object): 'system': [ 'name', 'arch', + 'strata', ], 'cluster': [ 'name', @@ -166,7 +195,6 @@ class MorphologyLoader(object): 'build-depends': [], }, 'system': { - 'strata': [], 'description': '', 'arch': None, 'configuration-extensions': [], @@ -270,9 +298,23 @@ class MorphologyLoader(object): assert kind == 'cluster' def _validate_system(self, morph): + # A system must contain at least one stratum + strata = morph['strata'] + if (not isinstance(strata, collections.Iterable) + or isinstance(strata, collections.Mapping)): + + raise SystemStrataNotListError(morph['name'], + type(strata)) + + if not strata: + raise EmptySystemError(morph['name']) + + if not all(isinstance(o, collections.Mapping) for o in strata): + raise SystemStratumSpecsNotMappingError(morph['name'], strata) + # All stratum names should be unique within a system. names = set() - for spec in morph['strata']: + for spec in strata: name = spec.get('alias', spec['morph']) if name in names: raise DuplicateStratumError(morph['name'], name) diff --git a/morphlib/morphloader_tests.py b/morphlib/morphloader_tests.py index b6ebff6a..8b87467a 100644 --- a/morphlib/morphloader_tests.py +++ b/morphlib/morphloader_tests.py @@ -99,6 +99,9 @@ build-system: dummy 'kind': 'system', 'name': 'foo', 'arch': 'x86_64', + 'strata': [ + {'morph': 'bar'}, + ], 'system-kind': 'foo', }) self.assertRaises( @@ -109,6 +112,9 @@ build-system: dummy 'kind': 'system', 'name': 'foo', 'arch': 'x86_64', + 'strata': [ + {'morph': 'bar'}, + ], 'disk-size': 'over 9000', }) self.assertRaises( @@ -122,12 +128,14 @@ build-system: dummy morphlib.morphloader.MissingFieldError, self.loader.validate, m) def test_fails_to_validate_system_with_invalid_field(self): - m = morphlib.morph3.Morphology({ - 'kind': 'system', - 'name': 'name', - 'arch': 'x86_64', - 'invalid': 'field', - }) + m = morphlib.morph3.Morphology( + kind="system", + name="foo", + arch="blah", + strata=[ + {'morph': 'bar'}, + ], + invalid='field') self.assertRaises( morphlib.morphloader.InvalidFieldError, self.loader.validate, m) @@ -183,24 +191,24 @@ build-system: dummy def test_validate_requires_a_valid_architecture(self): m = morphlib.morph3.Morphology( - { - "kind": "system", - "name": "foo", - "arch": "blah", - "strata": [], - }) + kind="system", + name="foo", + arch="blah", + strata=[ + {'morph': 'bar'}, + ]) self.assertRaises( morphlib.morphloader.UnknownArchitectureError, self.loader.validate, m) def test_validate_normalises_architecture_armv7_to_armv7l(self): m = morphlib.morph3.Morphology( - { - "kind": "system", - "name": "foo", - "arch": "armv7", - "strata": [], - }) + kind="system", + name="foo", + arch="armv7", + strata=[ + {'morph': 'bar'}, + ]) self.loader.validate(m) self.assertEqual(m['arch'], 'armv7l') @@ -276,6 +284,50 @@ build-system: dummy morphlib.morphloader.EmptyStratumError, self.loader.validate, m) + def test_validate_requires_strata_in_system(self): + m = morphlib.morph3.Morphology( + name='system', + kind='system', + arch='testarch') + self.assertRaises( + morphlib.morphloader.MissingFieldError, + self.loader.validate, m) + + def test_validate_requires_list_of_strata_in_system(self): + for v in (None, {}): + m = morphlib.morph3.Morphology( + name='system', + kind='system', + arch='testarch', + strata=v) + with self.assertRaises( + morphlib.morphloader.SystemStrataNotListError) as cm: + + self.loader.validate(m) + self.assertEqual(cm.exception.strata_type, type(v)) + + def test_validate_requires_non_empty_strata_in_system(self): + m = morphlib.morph3.Morphology( + name='system', + kind='system', + arch='testarch', + strata=[]) + self.assertRaises( + morphlib.morphloader.EmptySystemError, + self.loader.validate, m) + + def test_validate_requires_stratum_specs_in_system(self): + m = morphlib.morph3.Morphology( + name='system', + kind='system', + arch='testarch', + strata=["foo"]) + with self.assertRaises( + morphlib.morphloader.SystemStratumSpecsNotMappingError) as cm: + + self.loader.validate(m) + self.assertEqual(cm.exception.strata, ["foo"]) + def test_loads_yaml_from_string(self): string = '''\ name: foo @@ -463,11 +515,13 @@ name: foo test_dict) def test_sets_defaults_for_system(self): - m = morphlib.morph3.Morphology({ - 'kind': 'system', - 'name': 'foo', - 'arch': 'x86_64', - }) + m = morphlib.morph3.Morphology( + kind='system', + name='foo', + arch='testarch', + strata=[ + {'morph': 'bar'}, + ]) self.loader.set_defaults(m) self.loader.validate(m) self.assertEqual( @@ -476,25 +530,35 @@ name: foo 'kind': 'system', 'name': 'foo', 'description': '', - 'arch': 'x86_64', - 'strata': [], + 'arch': 'testarch', + 'strata': [ + {'morph': 'bar'}, + ], 'configuration-extensions': [], }) def test_unsets_defaults_for_system(self): - m = morphlib.morph3.Morphology({ - 'kind': 'system', - 'name': 'foo', - 'arch': 'x86_64', - 'strata': [], - }) + m = morphlib.morph3.Morphology( + { + 'description': '', + 'kind': 'system', + 'name': 'foo', + 'arch': 'testarch', + 'strata': [ + {'morph': 'bar'}, + ], + 'configuration-extensions': [], + }) self.loader.unset_defaults(m) self.assertEqual( dict(m), { 'kind': 'system', 'name': 'foo', - 'arch': 'x86_64', + 'arch': 'testarch', + 'strata': [ + {'morph': 'bar'}, + ], }) def test_sets_defaults_for_cluster(self): diff --git a/yarns/implementations.yarn b/yarns/implementations.yarn index 6491b38e..98955f48 100644 --- a/yarns/implementations.yarn +++ b/yarns/implementations.yarn @@ -75,13 +75,6 @@ another to hold a chunk. morph: test-stratum EOF - cat << EOF > "$DATADIR/gits/morphs/simple-system.morph" - name: simple-system - kind: system - arch: $arch - strata: [] - EOF - cat << EOF > "$DATADIR/gits/morphs/test-stratum.morph" name: test-stratum kind: stratum @@ -134,7 +127,11 @@ another to hold a chunk. description: A system called $MATCH_1 for architectures $MATCH_2 kind: system name: $MATCH_1 - strata: [] + strata: + - name: test-stratum + repo: test:morphs + ref: master + morph: test-stratum EOF run_in "$DATADIR/gits/morphs" git add "$MATCH_1.morph" run_in "$DATADIR/gits/morphs" git commit -m "Added $MATCH_1 morphology." @@ -275,7 +272,11 @@ Editing morphologies with `morph edit`. description: A system called $MATCH_1 for architectures $MATCH_2 kind: system name: $MATCH_1 - strata: [] + strata: + - name: test-stratum + repo: test:morphs + ref: master + morph: test-stratum EOF Reporting status of checked out repositories: @@ -425,7 +426,7 @@ Generating a manifest. > "$DATADIR/baserock/hello_world.meta" { "artifact-name": "hello_world", - "cache-key": + "cache-key": "ab8d00a80298a842446ce23507cea6b4d0e34c7ddfa05c67f460318b04d21308", "kind": "chunk", "morphology": "hello_world.morph", @@ -440,7 +441,7 @@ Generating a manifest. IMPLEMENTS WHEN morph generates a manifest run_morph generate-manifest "$DATADIR/artifact.tar" > "$DATADIR/manifest" - + IMPLEMENTS THEN the manifest is generated # Generated manifest should contain the name of the repository @@ -554,4 +555,4 @@ Implementations for building systems IMPLEMENTS THEN morph build the system (\S+) of the (branch|tag) (\S+) of the repo (\S+) cd "$DATADIR/workspace/$MATCH_3/$MATCH_4" - run_morph build "$MATCH_1" + run_morph build "$MATCH_1" diff --git a/yarns/regression.yarn b/yarns/regression.yarn index a17d2f87..582ebb08 100644 --- a/yarns/regression.yarn +++ b/yarns/regression.yarn @@ -10,13 +10,13 @@ Testing if we can build after checking out from a tag. GIVEN a workspace AND a git server WHEN the user checks out the system tag called test-tag - THEN morph build the system simple-system of the tag test-tag of the repo test:morphs + THEN morph build the system test-system of the tag test-tag of the repo test:morphs Running `morph branch` when the branch directory exists doesn't remove the existing directory. - SCENARIO re-running 'morph branch' fails, original branch untouched + SCENARIO re-running 'morph branch' fails, original branch untouched GIVEN a workspace AND a git server WHEN the user creates a system branch called foo -- cgit v1.2.1 From 03e724f1fb690e9ef95b082e4f1ad96799ec18c1 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Thu, 28 Nov 2013 12:49:41 +0000 Subject: validation: Require there be non-bootstrap chunks in systems Bootstrap chunks don't make it into the final system, so there needs to be an extra check for empty systems after the sources have been collected. This was complicated slightly by the fact that if you try to build a chunk directly you will have no strata in your sources, hence no non-bootstrap chunks, but validation for having been told to build a chunk is best handled later. This amends the old yarns that depended on building a bootstrap chunk and adds a new one that explicitly builds a system with bootstrap chunks. --- morphlib/buildcommand.py | 46 ++++++++++++++++++++++++++++++++++++++++------ yarns/implementations.yarn | 2 +- yarns/regression.yarn | 46 +++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 86 insertions(+), 8 deletions(-) diff --git a/morphlib/buildcommand.py b/morphlib/buildcommand.py index 8ad893a9..4b3b2108 100644 --- a/morphlib/buildcommand.py +++ b/morphlib/buildcommand.py @@ -49,8 +49,8 @@ class BuildCommand(object): repo_name=repo_name, ref=ref, filename=filename) self.app.status(msg='Deciding on task order') srcpool = self.create_source_pool(repo_name, ref, filename) + self.validate_sources(srcpool) root_artifact = self.resolve_artifacts(srcpool) - self._validate_architecture(root_artifact) self.build_in_order(root_artifact) self.app.status(msg='Build ends successfully') @@ -83,11 +83,27 @@ class BuildCommand(object): srcpool = self.app.create_source_pool( self.lrc, self.rrc, (repo_name, ref, filename)) + return srcpool + + def validate_sources(self, srcpool): self.app.status( msg='Validating cross-morphology references', chatty=True) self._validate_cross_morphology_references(srcpool) - return srcpool + self.app.status(msg='Validating for there being non-bootstrap chunks', + chatty=True) + self._validate_has_non_bootstrap_chunks(srcpool) + + def _validate_root_artifact(self, root_artifact): + self._validate_root_kind(root_artifact) + self._validate_architecture(root_artifact) + + @staticmethod + def _validate_root_kind(root_artifact): + root_kind = root_artifact.source.morphology['kind'] + if root_kind != 'system': + raise morphlib.Error( + 'Building a %s directly is not supported' % root_kind) def _validate_architecture(self, root_artifact): '''Perform the validation between root and target architectures.''' @@ -100,6 +116,22 @@ class BuildCommand(object): 'Host architecture is %s but target is %s' % (host_arch, root_arch)) + @staticmethod + def _validate_has_non_bootstrap_chunks(srcpool): + stratum_sources = [src for src in srcpool + if src.morphology['kind'] == 'stratum'] + # any will return true for an empty iterable, which will give + # a false positive when there are no strata. + # This is an error by itself, but the source of this error can + # be better diagnosed later, so we abort validating here. + if not stratum_sources: + return + + if not any(spec.get('build-mode', 'staging') != 'bootstrap' + for src in stratum_sources + for spec in src.morphology['chunks']): + raise morphlib.Error('No non-bootstrap chunks found.') + def resolve_artifacts(self, srcpool): '''Resolve the artifacts that will be built for a set of sources''' @@ -112,10 +144,12 @@ class BuildCommand(object): self.app.status(msg='Computing build order', chatty=True) root_artifact = self._find_root_artifact(artifacts) - root_kind = root_artifact.source.morphology['kind'] - if root_kind != 'system': - raise morphlib.Error( - 'Building a %s directly is not supported' % root_kind) + # Validate the root artifact here, since it's a costly function + # to finalise it, so any pre finalisation validation is better + # done before that happens, but we also don't want to expose + # the root artifact until it's finalised. + self.app.status(msg='Validating root artifact', chatty=True) + self._validate_root_artifact(root_artifact) arch = root_artifact.source.morphology['arch'] self.app.status(msg='Creating build environment for %(arch)s', diff --git a/yarns/implementations.yarn b/yarns/implementations.yarn index 98955f48..65ac1283 100644 --- a/yarns/implementations.yarn +++ b/yarns/implementations.yarn @@ -83,7 +83,7 @@ another to hold a chunk. repo: test:test-chunk ref: master morph: test-chunk - build-mode: bootstrap + build-mode: test build-depends: [] EOF diff --git a/yarns/regression.yarn b/yarns/regression.yarn index 582ebb08..eae01343 100644 --- a/yarns/regression.yarn +++ b/yarns/regression.yarn @@ -27,8 +27,52 @@ The branch is checked out correctly, now it should fail if the user executes WHEN the user attempts to create a system branch called foo THEN morph failed - AND the branch error message includes the string "File exists" + AND the branch error message includes the string "File exists" The branch still checked out. AND the system branch foo is checked out + + +It doesn't make much sense to be able to build a system with only +bootstrap chunks, since they will have been constructed without a staging +area, hence their results cannot be trusted. + + SCENARIO building a system with only bootstrap chunks fails + GIVEN a workspace + AND a git server + AND a system containing only bootstrap chunks called bootstrap-system + WHEN the user checks out the system branch called master + AND the user attempts to build the system bootstrap-system in branch master + THEN the build error message includes the string "No non-bootstrap chunks found" + + +Implementations +--------------- + + IMPLEMENTS GIVEN a system containing only bootstrap chunks called (\S+) + arch=$(run_morph print-architecture) + cat <"$DATADIR/gits/morphs/$MATCH_1.morph" + name: $MATCH_1 + kind: system + arch: $arch + strata: + - morph: bootstrap-stratum + repo: test:morphs + ref: master + EOF + + cat << EOF > "$DATADIR/gits/morphs/bootstrap-stratum.morph" + name: bootstrap-stratum + kind: stratum + chunks: + - name: bootstrap-chunk + repo: test:test-chunk + ref: master + morph: test-chunk + build-mode: bootstrap + build-depends: [] + EOF + + run_in "$DATADIR/gits/morphs" git add . + run_in "$DATADIR/gits/morphs" git commit -m "Add bootstrap-system" -- cgit v1.2.1 From 5ecc3651d4cab13d244e394ab63e45d79294f62d Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Tue, 26 Nov 2013 12:37:33 +0000 Subject: morphloader: use getattr for validate, set defaults It saves some boilerplate. --- morphlib/morphloader.py | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/morphlib/morphloader.py b/morphlib/morphloader.py index fdc3b62f..e7c1d9ff 100644 --- a/morphlib/morphloader.py +++ b/morphlib/morphloader.py @@ -288,14 +288,10 @@ class MorphologyLoader(object): self._deny_obsolete_fields(obsolete, morph) self._deny_unknown_fields(required + allowed, morph) - if kind == 'system': - self._validate_system(morph) - elif kind == 'stratum': - self._validate_stratum(morph) - elif kind == 'chunk': - self._validate_chunk(morph) - else: - assert kind == 'cluster' + getattr(self, '_validate_%s' % kind)(morph) + + def _validate_cluster(self, morph): + pass def _validate_system(self, morph): # A system must contain at least one stratum @@ -394,14 +390,7 @@ class MorphologyLoader(object): if key not in morphology: morphology[key] = defaults[key] - if kind == 'cluster': - self._set_cluster_defaults(morphology) - elif kind == 'system': - self._set_system_defaults(morphology) - elif kind == 'stratum': - self._set_stratum_defaults(morphology) - elif kind == 'chunk': - self._set_chunk_defaults(morphology) + getattr(self, '_set_%s_defaults' % kind)(morphology) def unset_defaults(self, morphology): '''If a field is equal to its default, delete it. -- cgit v1.2.1 From 21d7299e927696c2536e2170f77eef9b25f80172 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Mon, 25 Nov 2013 11:50:03 +0000 Subject: yarns: un-parameterise architecture in system morphologies It doesn't make sense to be able to specify an architecture from the IMPLEMENTS name, since you either need your architecture for something to build, or testarch for something that consistently doesn't build. --- yarns/architecture.yarn | 2 +- yarns/building.yarn | 4 ++-- yarns/implementations.yarn | 22 +++++++++++++++------- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/yarns/architecture.yarn b/yarns/architecture.yarn index 038492cd..521575a3 100644 --- a/yarns/architecture.yarn +++ b/yarns/architecture.yarn @@ -4,7 +4,7 @@ Morph Cross-Building Tests SCENARIO building a system for a different architecture GIVEN a workspace AND a git server - AND a system called base-system-testarch for architecture testarch in the git server + AND a system called base-system-testarch for the test architecture in the git server WHEN the user checks out the system branch called master AND the user attempts to build the system base-system-testarch in branch master THEN morph failed diff --git a/yarns/building.yarn b/yarns/building.yarn index b78c69cd..6dad426e 100644 --- a/yarns/building.yarn +++ b/yarns/building.yarn @@ -5,7 +5,7 @@ Morph Building Tests GIVEN a workspace AND a git server WHEN the user checks out the system branch called master - AND the user creates an uncommitted system morphology called base-system-testarch for architecture testarch in system branch master - AND the user attempts to build the system base-system-testarch in branch master + AND the user creates an uncommitted system morphology called base-system for our architecture in system branch master + AND the user attempts to build the system base-system in branch master THEN morph failed AND the build error message includes the string "Did you forget to commit it?" diff --git a/yarns/implementations.yarn b/yarns/implementations.yarn index 65ac1283..132ce9b3 100644 --- a/yarns/implementations.yarn +++ b/yarns/implementations.yarn @@ -120,11 +120,14 @@ another to hold a chunk. mkdir "$DATADIR/cache" mkdir "$DATADIR/tmp" - IMPLEMENTS GIVEN a system called (\S+) for architecture (\S+) in the git server +We need a consistent value for the architecture in some tests, so we +have a morphology using the test architecture. + + IMPLEMENTS GIVEN a system called (\S+) for the test architecture in the git server cat << EOF > "$DATADIR/gits/morphs/$MATCH_1.morph" - arch: $MATCH_2 + arch: testarch configuration-extensions: [] - description: A system called $MATCH_1 for architectures $MATCH_2 + description: A system called $MATCH_1 for test architecture kind: system name: $MATCH_1 strata: @@ -265,11 +268,16 @@ Editing morphologies with `morph edit`. cd "$DATADIR/workspace/$MATCH_3" attempt_morph edit "$MATCH_2" "$MATCH_1" - IMPLEMENTS WHEN the user creates an uncommitted system morphology called (\S+) for architecture (\S+) in system branch (\S+) - cat << EOF > "$DATADIR/workspace/$MATCH_3/test:morphs/$MATCH_1.morph" - arch: $MATCH_2 +To produce buildable morphologies, we need them to be of the same +architecture as the machine doing the testing. This uses `morph +print-architecture` to get a value appropriate for morph. + + IMPLEMENTS WHEN the user creates an uncommitted system morphology called (\S+) for our architecture in system branch (\S+) + arch=$(morph print-architecture) + cat << EOF > "$DATADIR/workspace/$MATCH_2/test:morphs/$MATCH_1.morph" + arch: $arch configuration-extensions: [] - description: A system called $MATCH_1 for architectures $MATCH_2 + description: A system called $MATCH_1 for architectures $arch kind: system name: $MATCH_1 strata: -- cgit v1.2.1 From b38e47f413c6651c8953d2bebd99ae0bb80c07f9 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 22 Nov 2013 13:44:15 +0000 Subject: sysbranchdir: Move load_all_morphologies helper here This was previously a private method of the branch and merge plugin, but it's useful to other plugins, so has been moved to the SystemBranchDirectory class, where everything else can get to it. It has an unpleasant amount of coupling to other classes, but in a *good* object oriented design it would either be a tiny module on its own, or not exist and leave all its users to re-implement the same logic multiple ways, so we've opted for a less clean, but more useful design. It is left un-covered by the unit tests, since it requires a great deal of instrumentation to test, at which point it may be best to leave it to integration tests. --- morphlib/plugins/branch_and_merge_new_plugin.py | 11 ++--------- morphlib/sysbranchdir.py | 14 ++++++++++++++ morphlib/util.py | 1 - 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/morphlib/plugins/branch_and_merge_new_plugin.py b/morphlib/plugins/branch_and_merge_new_plugin.py index 8ad9effd..3a3c1d1b 100644 --- a/morphlib/plugins/branch_and_merge_new_plugin.py +++ b/morphlib/plugins/branch_and_merge_new_plugin.py @@ -615,15 +615,8 @@ class SimpleBranchAndMergePlugin(cliapp.Plugin): '''Read in all the morphologies in the root repository.''' self.app.status(msg='Loading in all morphologies') morphs = morphlib.morphset.MorphologySet() - mf = morphlib.morphologyfinder.MorphologyFinder( - morphlib.gitdir.GitDirectory( - sb.get_git_directory_name(sb.root_repository_url))) - for morph in mf.list_morphologies(): - text, filename = mf.read_morphology(morph) - m = loader.load_from_string(text, filename=filename) - m.repo_url = sb.root_repository_url - m.ref = sb.system_branch_name - morphs.add_morphology(m) + for morph in sb.load_all_morphologies(loader): + morphs.add_morphology(morph) return morphs def petrify(self, args): diff --git a/morphlib/sysbranchdir.py b/morphlib/sysbranchdir.py index 73a07d5e..1a8b898a 100644 --- a/morphlib/sysbranchdir.py +++ b/morphlib/sysbranchdir.py @@ -161,6 +161,20 @@ class SystemBranchDirectory(object): for dirname in morphlib.util.find_leaves(self.root_directory, '.git')) + # Not covered by unit tests, since testing the functionality spans + # multiple modules and only tests useful output with a full system + # branch, so it is instead covered by integration tests. + def load_all_morphologies(self, loader): # pragma: no cover + gd_name = self.get_git_directory_name(self.root_repository_url) + gd = morphlib.gitdir.GitDirectory(gd_name) + mf = morphlib.morphologyfinder.MorphologyFinder(gd) + for morph in mf.list_morphologies(): + text, filename = mf.read_morphology(morph) + m = loader.load_from_string(text, filename=filename) + m.repo_url = self.root_repository_url + m.ref = self.system_branch_name + yield m + def create(root_directory, root_repository_url, system_branch_name): '''Create a new system branch directory on disk. diff --git a/morphlib/util.py b/morphlib/util.py index 7382e40c..16e56366 100644 --- a/morphlib/util.py +++ b/morphlib/util.py @@ -18,7 +18,6 @@ import os import re import morphlib -import logging '''Utility functions for morph.''' -- cgit v1.2.1 From 3eb2b658b1f3a612c78c14f3d2cba2a1f0b1333f Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Wed, 27 Nov 2013 14:26:31 +0000 Subject: branchmanager: Allow deferred and optional cleanup on success. Now it will optionally clean up on success based on a constructor parameter. It can be later cleaned up explicitly by calling close(). It is called close, rather than something more obvious, like cleanup(), since it means the manager can be re-used with contextlib.closing(). This now means that using the Managers without a context manager is less ugly, since you can explicitly call .close() in a finally block. --- morphlib/branchmanager.py | 94 ++++++++++++++++++++++++++++++++------ morphlib/branchmanager_tests.py | 99 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 179 insertions(+), 14 deletions(-) diff --git a/morphlib/branchmanager.py b/morphlib/branchmanager.py index 87a75ddc..a33b4ccb 100644 --- a/morphlib/branchmanager.py +++ b/morphlib/branchmanager.py @@ -34,23 +34,58 @@ class RefCleanupError(cliapp.AppException): class LocalRefManager(object): '''Provide atomic update over a set of refs in a set of repositories. - Any ref changes made with the update, add and delete methods will - be reversed after the end of the with statement the LocalRefManager - is used in, if an exception is raised in the aforesaid with statement. + When used in a with statement, if an exception is raised in the + body, then any ref changes are reverted, so deletes get replaced, + new branches get deleted and ref changes are changed back to the + value before the LocalRefManager was created. + + By default, changes are kept after the with statement ends. This can + be overridden to revert after the manager exits by passing True to + the construcor. + + with LocalRefManager(True) as lrm: + # Update refs with lrm.update, lrm.add or lrm.delete + # Use changed refs + # refs are back to their previous value + + There is also an explicit .close() method to clean up after the + context has exited like so: + + with LocalRefManager() as lrm: + # update refs + # Do something with altered refs + lrm.close() # Explicitly clean up + + The name .close() was chosen for the cleanup method, so the + LocalRefManager object may also be used again in a second with + statement using contextlib.closing(). + + with LocalRefManager() as lrm: + # update refs + with contextlib.closing(lrm) as lrm: + # Do something with pushed refs and clean up if there is an + # exception + + This is also useful if the LocalRefManager is nested in another + object, since the .close() method can be called in that object's + cleanup method. ''' - def __init__(self): - self._cleanup = None + def __init__(self, cleanup_on_success=False): + self._cleanup_on_success = cleanup_on_success + self._cleanup = collections.deque() def __enter__(self): - self._cleanup = collections.deque() return self def __exit__(self, etype, evalue, estack): # No exception was raised, so no cleanup is required - if (etype, evalue, estack) == (None, None, None): + if not self._cleanup_on_success and evalue is None: return + self.close(evalue) + + def close(self, primary=None): exceptions = [] d = self._cleanup while d: @@ -60,7 +95,7 @@ class LocalRefManager(object): except Exception, e: exceptions.append((op, args, e)) if exceptions: - raise RefCleanupError(evalue, exceptions) + raise RefCleanupError(primary, exceptions) def update(self, gd, ref, commit, old_commit, message=None): '''Update a git repository's ref, reverting it on failure. @@ -116,19 +151,50 @@ class LocalRefManager(object): class RemoteRefManager(object): '''Provide temporary pushes to remote repositories. - Any ref changes made with the push method will be reversed after - the end of the with statement the RemoteRefManager is used in. + When used in a with statement, if an exception is raised in the body, + then any pushed refs are reverted, so deletes get replaced and new + branches get deleted. + + By default it will also undo pushed refs when an exception is not + raised, this can be overridden by passing False to the constructor. + + There is also an explicit .close() method to clean up after the + context has exited like so: + + with RemoteRefManager(False) as rrm: + # push refs with rrm.push(...) + # Do something with pushed refs + rrm.close() # Explicitly clean up + + The name .close() was chosen for the cleanup method, so the + RemoteRefManager object may also be used again in a second with + statement using contextlib.closing(). + + with RemoteRefManager(False) as rrm: + rrm.push(...) + with contextlib.closing(rrm) as rrm: + # Do something with pushed refs and clean up if there is an + # exception + + This is also useful if the RemoteRefManager is nested in another + object, since the .close() method can be called in that object's + cleanup method. ''' - def __init__(self): - self._cleanup = None + def __init__(self, cleanup_on_success=True): + self._cleanup_on_success = cleanup_on_success + self._cleanup = collections.deque() def __enter__(self): - self._cleanup = collections.deque() return self def __exit__(self, etype, evalue, estack): + if not self._cleanup_on_success and evalue is None: + return + self.close(evalue) + + def close(self, primary=None): exceptions = [] d = self._cleanup while d: @@ -138,7 +204,7 @@ class RemoteRefManager(object): except Exception, e: exceptions.append((remote, refspecs, e)) if exceptions: - raise RefCleanupError(evalue, exceptions) + raise RefCleanupError(primary, exceptions) def push(self, remote, *refspecs): '''Push refspecs to remote and revert on failure. diff --git a/morphlib/branchmanager_tests.py b/morphlib/branchmanager_tests.py index 9bba7f2e..a7988c96 100644 --- a/morphlib/branchmanager_tests.py +++ b/morphlib/branchmanager_tests.py @@ -74,6 +74,25 @@ class LocalRefManagerTests(unittest.TestCase): with self.assertRaises(morphlib.gitdir.InvalidRefError): gd.resolve_ref_to_commit('refs/heads/create%d' % i) + def test_add_rollback_on_success(self): + with self.lrm(True) as lrm: + for i, gd in enumerate(self.repos): + commit = gd.resolve_ref_to_commit('refs/heads/master') + lrm.add(gd, 'refs/heads/create%d' % i, commit) + for i, gd in enumerate(self.repos): + with self.assertRaises(morphlib.gitdir.InvalidRefError): + gd.resolve_ref_to_commit('refs/heads/create%d' % i) + + def test_add_rollback_deferred(self): + with self.lrm(False) as lrm: + for i, gd in enumerate(self.repos): + commit = gd.resolve_ref_to_commit('refs/heads/master') + lrm.add(gd, 'refs/heads/create%d' % i, commit) + lrm.close() + for i, gd in enumerate(self.repos): + with self.assertRaises(morphlib.gitdir.InvalidRefError): + gd.resolve_ref_to_commit('refs/heads/create%d' % i) + def test_add_rollback_failure(self): failure_exception = Exception() with self.assertRaises(morphlib.branchmanager.RefCleanupError) as cm: @@ -117,6 +136,31 @@ class LocalRefManagerTests(unittest.TestCase): self.assertEqual(gd.resolve_ref_to_commit('refs/heads/master'), refinfo[i]) + def test_update_rollback_on_success(self): + refinfo = [] + with self.lrm(True) as lrm: + for i, gd in enumerate(self.repos): + old_master = gd.resolve_ref_to_commit('refs/heads/master') + commit = gd.resolve_ref_to_commit('refs/heads/dev-branch') + refinfo.append(old_master) + lrm.update(gd, 'refs/heads/master', commit, old_master) + for i, gd in enumerate(self.repos): + self.assertEqual(gd.resolve_ref_to_commit('refs/heads/master'), + refinfo[i]) + + def test_update_rollback_deferred(self): + refinfo = [] + with self.lrm(False) as lrm: + for i, gd in enumerate(self.repos): + old_master = gd.resolve_ref_to_commit('refs/heads/master') + commit = gd.resolve_ref_to_commit('refs/heads/dev-branch') + refinfo.append(old_master) + lrm.update(gd, 'refs/heads/master', commit, old_master) + lrm.close() + for i, gd in enumerate(self.repos): + self.assertEqual(gd.resolve_ref_to_commit('refs/heads/master'), + refinfo[i]) + def test_update_rollback_failure(self): failure_exception = Exception() with self.assertRaises(morphlib.branchmanager.RefCleanupError) as cm: @@ -154,6 +198,29 @@ class LocalRefManagerTests(unittest.TestCase): self.assertEqual(gd.resolve_ref_to_commit('refs/heads/master'), refinfo[i]) + def test_delete_rollback_on_success(self): + refinfo = [] + with self.lrm(True) as lrm: + for i, gd in enumerate(self.repos): + commit = gd.resolve_ref_to_commit('refs/heads/master') + refinfo.append(commit) + lrm.delete(gd, 'refs/heads/master', commit) + for i, gd in enumerate(self.repos): + self.assertEqual(gd.resolve_ref_to_commit('refs/heads/master'), + refinfo[i]) + + def test_delete_rollback_deferred(self): + refinfo = [] + with self.lrm(False) as lrm: + for i, gd in enumerate(self.repos): + commit = gd.resolve_ref_to_commit('refs/heads/master') + refinfo.append(commit) + lrm.delete(gd, 'refs/heads/master', commit) + lrm.close() + for i, gd in enumerate(self.repos): + self.assertEqual(gd.resolve_ref_to_commit('refs/heads/master'), + refinfo[i]) + def test_delete_rollback_failure(self): failure_exception = Exception() with self.assertRaises(morphlib.branchmanager.RefCleanupError) as cm: @@ -250,6 +317,17 @@ class RemoteRefManagerTests(unittest.TestCase): self.assert_remote_branches() self.assert_no_remote_branches() + def test_keep_after_create_success(self): + with morphlib.branchmanager.RemoteRefManager(False) as rrm: + self.push_creates(rrm) + self.assert_remote_branches() + + def test_deferred_rollback_after_create_success(self): + with morphlib.branchmanager.RemoteRefManager(False) as rrm: + self.push_creates(rrm) + rrm.close() + self.assert_no_remote_branches() + def test_rollback_after_create_failure(self): failure_exception = Exception() with self.assertRaises(Exception) as cm: @@ -292,6 +370,27 @@ class RemoteRefManagerTests(unittest.TestCase): self.assert_no_remote_branches() self.assert_remote_branches() + def test_keep_after_deletes_success(self): + for name, dirname, gd in self.remotes: + self.sgd.get_remote(name).push( + morphlib.gitdir.RefSpec('master'), + morphlib.gitdir.RefSpec('dev-branch')) + self.assert_remote_branches() + with morphlib.branchmanager.RemoteRefManager(False) as rrm: + self.push_deletes(rrm) + self.assert_no_remote_branches() + + def test_deferred_rollback_after_deletes_success(self): + for name, dirname, gd in self.remotes: + self.sgd.get_remote(name).push( + morphlib.gitdir.RefSpec('master'), + morphlib.gitdir.RefSpec('dev-branch')) + self.assert_remote_branches() + with morphlib.branchmanager.RemoteRefManager(False) as rrm: + self.push_deletes(rrm) + rrm.close() + self.assert_remote_branches() + def test_rollback_after_deletes_failure(self): failure_exception = Exception() for name, dirname, gd in self.remotes: -- cgit v1.2.1 From 198b59056058be476cb95e44fbe839f09258527c Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Wed, 27 Nov 2013 14:33:22 +0000 Subject: morphlib: Add BuildBranch abstraction This is an abstraction on top of SystemBranchDirectories, providing the ability to add uncommitted changes to the temporary build branch, push temporary build branches and retrieve the correct repository URI and ref to build the system. --- morphlib/__init__.py | 1 + morphlib/buildbranch.py | 260 ++++++++++++++++++++++++++++++++++++++++++++++++ without-test-modules | 2 + 3 files changed, 263 insertions(+) create mode 100644 morphlib/buildbranch.py diff --git a/morphlib/__init__.py b/morphlib/__init__.py index ad90bd4d..8f39cb30 100644 --- a/morphlib/__init__.py +++ b/morphlib/__init__.py @@ -50,6 +50,7 @@ import artifactcachereference import artifactresolver import branchmanager import bins +import buildbranch import buildcommand import buildenvironment import buildsystem diff --git a/morphlib/buildbranch.py b/morphlib/buildbranch.py new file mode 100644 index 00000000..d4426afb --- /dev/null +++ b/morphlib/buildbranch.py @@ -0,0 +1,260 @@ +# Copyright (C) 2013 Codethink Limited +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + + +import collections +import os +import urlparse + +import cliapp +import fs.tempfs + +import morphlib + + +class BuildBranchCleanupError(cliapp.AppException): + def __init__(self, bb, exceptions): + self.bb = bb + self.exceptions = exceptions + ex_nr = len(exceptions) + cliapp.AppException.__init__( + self, '%(ex_nr)d exceptions caught when cleaning up build branch' + % locals()) + + +class BuildBranch(object): + '''Represent the sources modified in a system branch. + + This is an abstraction on top of SystemBranchDirectories, providing + the ability to add uncommitted changes to the temporary build branch, + push temporary build branches and retrieve the correct repository + URI and ref to build the system. + + ''' + + # TODO: This currently always uses the temporary build ref. It + # would be better to not use local repositories and temporary refs, + # so building from a workspace appears to be identical to using + # `morph build-morphology` + def __init__(self, sb, build_ref_prefix, push_temporary): + + self._sb = sb + self._push_temporary = push_temporary + + self._cleanup = collections.deque() + self._to_push = {} + self._td = fs.tempfs.TempFS() + self._register_cleanup(self._td.close) + + self._branch_root = sb.get_config('branch.root') + branch_uuid = sb.get_config('branch.uuid') + + for gd in sb.list_git_directories(): + try: + repo_uuid = gd.get_config('morph.uuid') + except cliapp.AppException: + # Not a repository cloned by morph, ignore + break + build_ref = os.path.join('refs/heads', build_ref_prefix, + branch_uuid, repo_uuid) + # index is commit of workspace + uncommitted changes may want + # to change to use user's index instead of user's commit, + # so they can add new files first + index = gd.get_index(self._td.getsyspath(repo_uuid)) + index.set_to_tree(gd.resolve_ref_to_tree(gd.HEAD)) + self._to_push[gd] = (build_ref, index) + + rootinfo, = ((gd, index) for gd, (build_ref, index) + in self._to_push.iteritems() + if gd.get_config('morph.repository') == self._branch_root) + self._root, self._root_index = rootinfo + + def _register_cleanup(self, func, *args, **kwargs): + self._cleanup.append((func, args, kwargs)) + + def add_uncommitted_changes(self): + '''Add any uncommitted changes to temporary build GitIndexes''' + for gd, (build_ref, index) in self._to_push.iteritems(): + changed = [to_path for code, to_path, from_path + in index.get_uncommitted_changes()] + if not changed: + continue + yield gd, build_ref + index.add_files_from_working_tree(changed) + + @staticmethod + def _hash_morphologies(gd, morphologies, loader): + '''Hash morphologies and return object info''' + for morphology in morphologies: + loader.unset_defaults(morphology) + sha1 = gd.store_blob(loader.save_to_string(morphology)) + yield 0100644, sha1, morphology.filename + + def inject_build_refs(self, loader): + '''Update system and stratum morphologies to point to our branch. + + For all edited repositories, this alter the temporary GitIndex + of the morphs repositories to point their temporary build branch + versions. + + `loader` is a MorphologyLoader that is used to convert morphology + files into their in-memory representations and back again. + + ''' + root_repo = self._root.get_config('morph.repository') + root_ref = self._root.HEAD + morphs = morphlib.morphset.MorphologySet() + for morph in self._sb.load_all_morphologies(loader): + morphs.add_morphology(morph) + + sb_info = {} + for gd, (build_ref, index) in self._to_push.iteritems(): + repo, ref = gd.get_config('morph.repository'), gd.HEAD + sb_info[repo, ref] = (gd, build_ref) + + def filter(m, kind, spec): + return (spec['repo'], spec['ref']) in sb_info + def process(m, kind, spec): + repo, ref = spec['repo'], spec['ref'] + gd, build_ref = sb_info[repo, ref] + if (repo, ref) == (root_repo, root_ref): + spec['repo'] = None + spec['ref'] = None + return True + if not self._push_temporary: + spec['repo'] = urlparse.urljoin('file://', gd.dirname) + spec['ref'] = build_ref + return True + + morphs.traverse_specs(process, filter) + + if any(m.dirty for m in morphs.morphologies): + yield self._root + + self._root_index.add_files_from_index_info( + self._hash_morphologies(self._root, morphs.morphologies, loader)) + + def update_build_refs(self, name, email, uuid): + '''Commit changes in temporary GitIndexes to temporary branches. + + `name` and `email` are required to construct the commit author info. + `uuid` is used to identify each build uniquely and is included + in the commit message. + + A new commit is added to the temporary build branch of each of + the repositories in the SystemBranch with: + 1. The tree of anything currently in the temporary GitIndex. + This is the same as the current commit on HEAD unless + `add_uncommitted_changes` or `inject_build_refs` have + been called. + 2. the parent of the previous temporary commit, or the last + commit of the working tree if there has been no previous + commits + 3. author and committer email as specified by `email`, author + name of `name` and committer name of 'Morph (on behalf of + `name`)' + 4. commit message describing the current build using `uuid` + + ''' + commit_message = 'Morph build %s\n\nSystem branch: %s\n' % \ + (uuid, self._sb.system_branch_name) + author_name = name + committer_name = 'Morph (on behalf of %s)' % name + author_email = committer_email = email + + with morphlib.branchmanager.LocalRefManager() as lrm: + for gd, (build_ref, index) in self._to_push.iteritems(): + yield gd, build_ref + tree = index.write_tree() + try: + parent = gd.resolve_ref_to_commit(build_ref) + except morphlib.gitdir.InvalidRefError: + parent = gd.resolve_ref_to_commit(gd.HEAD) + + commit = gd.commit_tree(tree, parent=parent, + committer_name=committer_name, + committer_email=committer_email, + author_name=author_name, + author_email=author_email, + message=commit_message) + try: + old_commit = gd.resolve_ref_to_commit(build_ref) + except morphlib.gitdir.InvalidRefError: + lrm.add(gd, build_ref, commit) + else: + # NOTE: This will fail if build_ref pointed to a tag, + # due to resolve_ref_to_commit returning the + # commit id of tags, but since it's only morph + # that touches those refs, it should not be + # a problem. + lrm.update(gd, build_ref, commit, old_commit) + + def push_build_branches(self): + '''Push all temporary build branches to the remote repositories. + + This is a no-op if the BuildBranch was constructed with + `push_temporary` as False, so that the code flow for the user of + the BuildBranch can be the same when it can be pushed as when + it can't. + + ''' + # TODO: When BuildBranches become more context aware, if there + # are no uncommitted changes and the local versions are pushed + # we can skip pushing even if push_temporary is set. + # No uncommitted changes isn't sufficient reason to push the + # current HEAD + if self._push_temporary: + with morphlib.branchmanager.RemoteRefManager(False) as rrm: + for gd, build_ref in self._to_push.iterkeys(): + remote = gd.get_remote('origin') + yield gd, build_ref, remote + refspec = morphlib.gitdir.RefSpec(build_ref) + rrm.push(remote, refspec) + self._register_cleanup(rrm.close) + + @property + def root_repo_url(self): + '''URI of the repository that systems may be found in.''' + # TODO: When BuildBranches become more context aware, we only + # have to use the file:// URI when there's uncommitted changes + # and we can't push; or HEAD is not pushed and we can't push. + # All other times we can use the pushed branch + return (self._sb.get_config('branch.root') if self._push_temporary + else urlparse.urljoin('file://', self._root.dirname)) + + @property + def root_ref(self): + '''Name of the ref of the repository that systems may be found in.''' + # TODO: When BuildBranches become more context aware, this can be + # HEAD when there's no uncommitted changes and we're not pushing; + # or we are pushing and there's no uncommitted changes and HEAD + # has been pushed. + build_ref, index = self._to_push[self._root] + return build_ref + + def close(self): + '''Clean up any resources acquired during operation.''' + # TODO: This is a common pattern for our context managers, + # we could do with a way to share the common code. I suggest the + # ExitStack from python3.4 or the contextlib2 module. + exceptions = [] + while self._cleanup: + func, args, kwargs = self._cleanup.pop() + try: + func(*args, **kwargs) + except Exception, e: + exceptions.append(e) + if exceptions: + raise BuildBranchCleanupError(self, exceptions) diff --git a/without-test-modules b/without-test-modules index 4efcdb40..c34ba59c 100644 --- a/without-test-modules +++ b/without-test-modules @@ -28,3 +28,5 @@ morphlib/plugins/trovectl_plugin.py morphlib/plugins/gc_plugin.py morphlib/plugins/branch_and_merge_new_plugin.py morphlib/plugins/print_architecture_plugin.py +# Not unit tested, since it needs a full system branch +morphlib/buildbranch.py -- cgit v1.2.1 From 93c77a63f13b7197ee623603a8b7abbf2893a3e8 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 22 Nov 2013 15:37:17 +0000 Subject: plugins: Add new build command This uses all the new APIs, so the code is shared across morphlib and unit tested rather than everything being in one massive plugin that is only black-box tested. --- morphlib/plugins/build_plugin.py | 92 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/morphlib/plugins/build_plugin.py b/morphlib/plugins/build_plugin.py index e9555888..f304e59a 100644 --- a/morphlib/plugins/build_plugin.py +++ b/morphlib/plugins/build_plugin.py @@ -15,6 +15,8 @@ import cliapp +import contextlib +import uuid import morphlib @@ -24,6 +26,8 @@ class BuildPlugin(cliapp.Plugin): def enable(self): self.app.add_subcommand('build-morphology', self.build_morphology, arg_synopsis='(REPO REF FILENAME)...') + self.app.add_subcommand('new-build', self.build, + arg_synopsis='SYSTEM') def disable(self): pass @@ -64,3 +68,91 @@ class BuildPlugin(cliapp.Plugin): build_command = self.app.hookmgr.call('new-build-command', build_command) build_command.build(args) + + def build(self, args): + '''Build a system image in the current system branch + + Command line arguments: + + * `SYSTEM` is the name of the system to build. + + This builds a system image, and any of its components that + need building. The system name is the basename of the system + morphology, in the root repository of the current system branch, + without the `.morph` suffix in the filename. + + The location of the resulting system image artifact is printed + at the end of the build output. + + You do not need to commit your changes before building, Morph + does that for you, in a temporary branch for each build. However, + note that Morph does not untracked files to the temporary branch, + only uncommitted changes to files git already knows about. You + need to `git add` and commit each new file yourself. + + Example: + + morph build devel-system-x86_64-generic + + ''' + + if len(args) != 1: + raise cliapp.AppException('morph build expects exactly one ' + 'parameter: the system to build') + + # Raise an exception if there is not enough space + morphlib.util.check_disk_available( + self.app.settings['tempdir'], + self.app.settings['tempdir-min-space'], + self.app.settings['cachedir'], + self.app.settings['cachedir-min-space']) + + system_name = args[0] + + ws = morphlib.workspace.open('.') + sb = morphlib.sysbranchdir.open_from_within('.') + + build_uuid = uuid.uuid4().hex + + build_command = morphlib.buildcommand.BuildCommand(self.app) + build_command = self.app.hookmgr.call('new-build-command', + build_command) + loader = morphlib.morphloader.MorphologyLoader() + push = self.app.settings['push-build-branches'] + name = morphlib.git.get_user_name(self.app.runcmd) + email = morphlib.git.get_user_email(self.app.runcmd) + build_ref_prefix = self.app.settings['build-ref-prefix'] + + self.app.status(msg='Starting build %(uuid)s', uuid=build_uuid) + self.app.status(msg='Collecting morphologies involved in ' + 'building %(system)s from %(branch)s', + system=system_name, branch=sb.system_branch_name) + + bb = morphlib.buildbranch.BuildBranch(sb, build_ref_prefix, + push_temporary=push) + with contextlib.closing(bb) as bb: + + for gd, build_ref in bb.add_uncommitted_changes(): + self.app.status(msg='Adding uncommitted changes '\ + 'in %(dirname)s to %(ref)s', + dirname=gd.dirname, ref=build_ref, chatty=True) + + for gd in bb.inject_build_refs(loader): + self.app.status(msg='Injecting temporary build refs '\ + 'into morphologies in %(dirname)s', + dirname=gd.dirname, chatty=True) + + for gd, build_ref in bb.update_build_refs(name, email, build_uuid): + self.app.status(msg='Committing changes in %(dirname)s '\ + 'to %(ref)s', + dirname=gd.dirname, ref=build_ref, chatty=True) + + for gd, build_ref, remote in bb.push_build_branches(): + self.app.status(msg='Pushing %(ref)s in %(dirname)s '\ + 'to %(remote)s', + ref=build_ref, dirname=gd.dirname, + remote=remote.get_push_url(), chatty=True) + + build_command.build([bb.root_repo_url, + bb.root_ref, + system_name]) -- cgit v1.2.1 From e9944a7d4115a4d314e3e0655762384a68722aef Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 22 Nov 2013 17:05:28 +0000 Subject: plugins: Use new build command as default The old build is still around for comparison. --- morphlib/plugins/branch_and_merge_plugin.py | 2 +- morphlib/plugins/build_plugin.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/morphlib/plugins/branch_and_merge_plugin.py b/morphlib/plugins/branch_and_merge_plugin.py index 454caade..4666ea96 100644 --- a/morphlib/plugins/branch_and_merge_plugin.py +++ b/morphlib/plugins/branch_and_merge_plugin.py @@ -65,7 +65,7 @@ class BranchAndMergePlugin(cliapp.Plugin): self.app.add_subcommand('old-unpetrify', self.unpetrify) self.app.add_subcommand( 'tag', self.tag, arg_synopsis='TAG-NAME -- [GIT-COMMIT-ARG...]') - self.app.add_subcommand('build', self.build, + self.app.add_subcommand('old-build', self.build, arg_synopsis='SYSTEM') self.app.add_subcommand('old-status', self.status) self.app.add_subcommand('old-branch-from-image', diff --git a/morphlib/plugins/build_plugin.py b/morphlib/plugins/build_plugin.py index f304e59a..8e04d0b3 100644 --- a/morphlib/plugins/build_plugin.py +++ b/morphlib/plugins/build_plugin.py @@ -26,7 +26,7 @@ class BuildPlugin(cliapp.Plugin): def enable(self): self.app.add_subcommand('build-morphology', self.build_morphology, arg_synopsis='(REPO REF FILENAME)...') - self.app.add_subcommand('new-build', self.build, + self.app.add_subcommand('build', self.build, arg_synopsis='SYSTEM') def disable(self): -- cgit v1.2.1 From de230e7475a0cdd412990398679f23e40db23376 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Mon, 25 Nov 2013 12:00:15 +0000 Subject: yarns: Change expected result of building uncommitted morphologies The new build code uses `git update-index --add`, which means it can use morphologies that haven't previously been added. --- yarns/building.yarn | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/yarns/building.yarn b/yarns/building.yarn index 6dad426e..5b6b29a0 100644 --- a/yarns/building.yarn +++ b/yarns/building.yarn @@ -6,6 +6,4 @@ Morph Building Tests AND a git server WHEN the user checks out the system branch called master AND the user creates an uncommitted system morphology called base-system for our architecture in system branch master - AND the user attempts to build the system base-system in branch master - THEN morph failed - AND the build error message includes the string "Did you forget to commit it?" + THEN morph build the system base-system of the branch master of the repo test:morphs -- cgit v1.2.1 From 400427729b882566d0e394805a306481f0810145 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 22 Nov 2013 13:46:05 +0000 Subject: plugins: Convert deploy to new classes. It now does not push branches as this is not necessary to locate the artifact. It still makes temporary build branches, since it is assumed that if you have changes in your workspace, it's preferable for the deploy to fail, rather than think you've deployed something you haven't. --- morphlib/plugins/deploy_plugin.py | 155 +++++++++++++++++--------------------- 1 file changed, 70 insertions(+), 85 deletions(-) diff --git a/morphlib/plugins/deploy_plugin.py b/morphlib/plugins/deploy_plugin.py index 825b0124..09405aa4 100644 --- a/morphlib/plugins/deploy_plugin.py +++ b/morphlib/plugins/deploy_plugin.py @@ -15,13 +15,12 @@ import cliapp -import gzip +import contextlib import os import shutil import stat import tarfile import tempfile -import urlparse import uuid import morphlib @@ -266,26 +265,74 @@ class DeployPlugin(cliapp.Plugin): self.app.settings['tempdir-min-space'], '/', 0) - cluster = args[0] + cluster_name = args[0] env_vars = args[1:] - branch_dir = self.other.deduce_system_branch()[1] - root_repo = self.other.get_branch_config(branch_dir, 'branch.root') - root_repo_dir = self.other.find_repository(branch_dir, root_repo) - data = self.other.load_morphology(root_repo_dir, cluster) + ws = morphlib.workspace.open('.') + sb = morphlib.sysbranchdir.open_from_within('.') - if data['kind'] != 'cluster': + build_uuid = uuid.uuid4().hex + + build_command = morphlib.buildcommand.BuildCommand(self.app) + build_command = self.app.hookmgr.call('new-build-command', + build_command) + loader = morphlib.morphloader.MorphologyLoader() + name = morphlib.git.get_user_name(self.app.runcmd) + email = morphlib.git.get_user_email(self.app.runcmd) + build_ref_prefix = self.app.settings['build-ref-prefix'] + + root_repo_dir = morphlib.gitdir.GitDirectory( + sb.get_git_directory_name(sb.root_repository_url)) + mf = morphlib.morphologyfinder.MorphologyFinder(root_repo_dir) + cluster_text, cluster_filename = mf.read_morphology(cluster_name) + cluster_morphology = loader.load_from_string(cluster_text, + filename=cluster_filename) + + if cluster_morphology['kind'] != 'cluster': raise cliapp.AppException( "Error: morph deploy is only supported for cluster" " morphologies.") - for system in data['systems']: - self.deploy_system(system, env_vars) - def deploy_system(self, system, env_vars): + bb = morphlib.buildbranch.BuildBranch(sb, build_ref_prefix, + push_temporary=False) + with contextlib.closing(bb) as bb: + + for gd, build_ref in bb.add_uncommitted_changes(): + self.app.status(msg='Adding uncommitted changes '\ + 'in %(dirname)s to %(ref)s', + dirname=gd.dirname, ref=build_ref, chatty=True) + + for gd in bb.inject_build_refs(loader): + self.app.status(msg='Injecting temporary build refs '\ + 'into morphologies in %(dirname)s', + dirname=gd.dirname, chatty=True) + + for gd, build_ref in bb.update_build_refs(name, email, build_uuid): + self.app.status(msg='Committing changes in %(dirname)s '\ + 'to %(ref)s', + dirname=gd.dirname, ref=build_ref, chatty=True) + + for gd, build_ref, remote in bb.push_build_branches(): + self.app.status(msg='Pushing %(ref)s in %(dirname)s '\ + 'to %(remote)s', + ref=build_ref, dirname=gd.dirname, + remote=remote.get_push_url(), chatty=True) + + for system in cluster_morphology['systems']: + self.deploy_system(build_command, root_repo_dir, + bb.root_repo_url, bb.root_ref, + system, env_vars) + + def deploy_system(self, build_command, root_repo_dir, build_repo, ref, + system, env_vars): + # Find the artifact to build morph = system['morph'] + srcpool = build_command.create_source_pool(build_repo, ref, + morph + '.morph') + artifact = build_command.resolve_artifacts(srcpool) + deploy_defaults = system['deploy-defaults'] deployments = system['deploy'] - for system_id, deploy_params in deployments.iteritems(): user_env = morphlib.util.parse_environment_pairs( os.environ, @@ -308,64 +355,11 @@ class DeployPlugin(cliapp.Plugin): 'for system "%s"' % system_id) morphlib.util.sanitize_environment(final_env) - self.do_deploy(morph, deployment_type, location, final_env) - - def do_deploy(self, system_name, deployment_type, location, env): - # Deduce workspace and system branch and branch root repository. - workspace = self.other.deduce_workspace() - branch, branch_dir = self.other.deduce_system_branch() - branch_root = self.other.get_branch_config(branch_dir, 'branch.root') - branch_uuid = self.other.get_branch_config(branch_dir, 'branch.uuid') - - # Generate a UUID for the build. - build_uuid = uuid.uuid4().hex - - build_command = morphlib.buildcommand.BuildCommand(self.app) - build_command = self.app.hookmgr.call('new-build-command', - build_command) - push = self.app.settings['push-build-branches'] - - self.app.status(msg='Starting build %(uuid)s', uuid=build_uuid) + self.do_deploy(build_command, root_repo_dir, ref, artifact, + deployment_type, location, final_env) - self.app.status(msg='Collecting morphologies involved in ' - 'building %(system)s from %(branch)s', - system=system_name, branch=branch) - - # Find system branch root repository on the local disk. - root_repo = self.other.get_branch_config(branch_dir, 'branch.root') - root_repo_dir = self.other.find_repository(branch_dir, root_repo) - - # Get repositories of morphologies involved in building this system - # from the current system branch. - build_repos = self.other.get_system_build_repos( - branch, branch_dir, branch_root, system_name) - - # Generate temporary build ref names for all these repositories. - self.other.generate_build_ref_names(build_repos, branch_uuid) - - # Create the build refs for all these repositories and commit - # all uncommitted changes to them, updating all references - # to system branch refs to point to the build refs instead. - self.other.update_build_refs(build_repos, branch, build_uuid, push) - - if push: - self.other.push_build_refs(build_repos) - build_branch_root = branch_root - else: - dirname = build_repos[branch_root]['dirname'] - build_branch_root = urlparse.urljoin('file://', dirname) - - # Run the build. - build_ref = build_repos[branch_root]['build-ref'] - srcpool = build_command.create_source_pool( - build_branch_root, - build_ref, - system_name + '.morph') - artifact = build_command.resolve_artifacts(srcpool) - - if push: - self.other.delete_remote_build_refs(build_repos) - + def do_deploy(self, build_command, root_repo_dir, ref, artifact, + deployment_type, location, env): # Create a tempdir for this deployment to work in deploy_tempdir = tempfile.mkdtemp( @@ -405,7 +399,7 @@ class DeployPlugin(cliapp.Plugin): for name in names: self._run_extension( root_repo_dir, - build_ref, + ref, name, '.configure', [system_tree], @@ -415,7 +409,7 @@ class DeployPlugin(cliapp.Plugin): self.app.status(msg='Writing to device') self._run_extension( root_repo_dir, - build_ref, + ref, deployment_type, '.write', [system_tree, location], @@ -428,7 +422,7 @@ class DeployPlugin(cliapp.Plugin): self.app.status(msg='Finished deployment') - def _run_extension(self, repo_dir, ref, name, kind, args, env): + def _run_extension(self, gd, ref, name, kind, args, env): '''Run an extension. The ``kind`` should be either ``.configure`` or ``.write``, @@ -440,8 +434,9 @@ class DeployPlugin(cliapp.Plugin): ''' # Look for extension in the system morphology's repository. - ext = self._cat_file(repo_dir, ref, name + kind) - if ext is None: + try: + ext = gd.get_file_from_ref(ref, name + kind) + except cliapp.AppException: # Not found: look for it in the Morph code. code_dir = os.path.dirname(morphlib.__file__) ext_filename = os.path.join(code_dir, 'exts', name + kind) @@ -465,7 +460,7 @@ class DeployPlugin(cliapp.Plugin): self.app.runcmd( [ext_filename] + args, ['sh', '-c', 'while read l; do echo `date "+%F %T"` $l; done'], - cwd=repo_dir, env=env, stdout=None, stderr=None) + cwd=gd.dirname, env=env, stdout=None, stderr=None) if delete_ext: os.remove(ext_filename) @@ -474,13 +469,3 @@ class DeployPlugin(cliapp.Plugin): st = os.stat(filename) mask = stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH return (stat.S_IMODE(st.st_mode) & mask) != 0 - - def _cat_file(self, repo_dir, ref, pathname): - '''Retrieve contents of a file from a git repository.''' - - argv = ['git', 'cat-file', 'blob', '%s:%s' % (ref, pathname)] - try: - return self.app.runcmd(argv, cwd=repo_dir) - except cliapp.AppException: - return None - -- cgit v1.2.1