Don't require chunks in a stratum to appear before their dependencies

Currently Morph enforces that chunk A must be defined before anything
that build-depends on it. YBD doesn't enforce that.

Definitions format at <http://wiki.baserock.org/definitions/current/>
doesn't mention ordering currently. I propose that we make Morph be
permissive about ordering, like YBD is, and update the spec to mandate
no restrictions on ordering.

Since behaviour was previously undefined, making Morph be more
permissive about this shouldn't require a new version number of the
definitions format.

I still think we need to make sure stratum .morph files are ordered
logically, but that is in the realm of 'code style', it shouldn't be
being enforced by a build tool.

Change-Id: I425f2e5b9dfb62e4a26ed11f5c50e3978a0dd1a6

2 files changed, 68 insertions, 26 deletions

diff --git a/morphlib/artifactresolver.py b/morphlib/artifactresolver.py
index b49c1905..f3936df1 100644
--- a/morphlib/artifactresolver.py
+++ b/morphlib/artifactresolver.py
@@ -27,13 +27,14 @@ class MutualDependencyError(cliapp.AppException):
             self, 'Cyclic dependency between %s and %s detected' % (a, b))
 
 
-class DependencyOrderError(cliapp.AppException):
+class UnknownDependencyError(cliapp.AppException):
 
     def __init__(self, stratum_source, chunk, dependency_name):
         cliapp.AppException.__init__(
-            self, 'In stratum %s, chunk %s references its dependency %s '
-            'before it is defined' %
-            (stratum_source, chunk, dependency_name))
+            self, 'In stratum %s, chunk %s references a dependency %s '
+            'that is not defined anywhere in that stratum' %
+            (stratum_source.name, chunk, dependency_name))
+
 
 
 class ArtifactResolver(object):
@@ -173,20 +174,32 @@ class ArtifactResolver(object):
         # 'name' here is the chunk artifact name
         name_to_processed_artifacts = {}
 
-        for info in source.morphology['chunks']:
+        def lookup_chunk_ref(chunk_ref):
             filename = morphlib.util.sanitise_morphology_path(
-                info.get('morph', info['name']))
-            chunk_source = self._source_pool.lookup(
-                info['repo'],
-                info['ref'],
-                filename)[0]
+                chunk_ref.get('morph', chunk_ref['name']))
+            source = self._source_pool.lookup(
+                chunk_ref['repo'], chunk_ref['ref'], filename)[0]
+            return source
 
+        # First, create a lookup table of chunk name -> Artifact instance.
+        for info in source.morphology['chunks']:
+            chunk_source = lookup_chunk_ref(info)
             chunk_name = chunk_source.name
 
             # Resolve now to avoid a search for the parent morphology later
             chunk_source.build_mode = info['build-mode']
             chunk_source.prefix = info['prefix']
 
+            # Add these chunks to the processed artifacts, so other
+            # chunks may refer to them.
+            name_to_processed_artifacts[info['name']] = \
+                [chunk_source.artifacts[n] for n
+                 in chunk_source.split_rules.artifacts]
+
+        # Now work out the build dependencies for the chunks in this stratum.
+        for info in source.morphology['chunks']:
+            chunk_source = lookup_chunk_ref(info)
+            chunk_name = chunk_source.name
             build_depends = info.get('build-depends', None)
 
             # Add our stratum's build depends as dependencies of this chunk
@@ -195,10 +208,11 @@ class ArtifactResolver(object):
 
             # Add dependencies between chunks mentioned in this stratum
             if build_depends is not None:
-                for name in build_depends: # pragma: no cover
+                for name in build_depends:
                     if name not in name_to_processed_artifacts:
-                        raise DependencyOrderError(
+                        raise UnknownDependencyError(
                             source, info['name'], name)
+
                     other_artifacts = name_to_processed_artifacts[name]
                     for other_artifact in other_artifacts:
                         chunk_source.add_dependency(other_artifact)
@@ -215,10 +229,4 @@ class ArtifactResolver(object):
                 if chunk_artifact not in artifacts:
                     artifacts.append(chunk_artifact)
 
-            # Add these chunks to the processed artifacts, so other
-            # chunks may refer to them.
-            name_to_processed_artifacts[info['name']] = \
-                [chunk_source.artifacts[n] for n
-                 in chunk_source.split_rules.artifacts]
-
         return artifacts
diff --git a/morphlib/artifactresolver_tests.py b/morphlib/artifactresolver_tests.py
index 4bf42a93..30f949c8 100644
--- a/morphlib/artifactresolver_tests.py
+++ b/morphlib/artifactresolver_tests.py
@@ -223,11 +223,9 @@ class ArtifactResolverTests(unittest.TestCase):
 
         artifacts = self.resolver._resolve_artifacts(pool)
 
-        self.assertEqual(
-            set(artifacts),
-            set(itertools.chain.from_iterable(
-                    s.artifacts.itervalues()
-                    for s in pool)))
+        all_artifacts_in_source_pool = itertools.chain.from_iterable(
+            source.artifacts.itervalues() for source in pool)
+        self.assertEqual(set(artifacts), set(all_artifacts_in_source_pool))
 
         stratum_artifacts = set(a for a in artifacts
                                 if a.source in stratum_sources)
@@ -287,7 +285,7 @@ class ArtifactResolverTests(unittest.TestCase):
         self.assertRaises(morphlib.artifactresolver.MutualDependencyError,
                           self.resolver._resolve_artifacts, pool)
 
-    def test_detection_of_chunk_dependencies_in_invalid_order(self):
+    def test_handles_chunk_dependencies_out_of_invalid_order(self):
         pool = morphlib.sourcepool.SourcePool()
 
         loader = morphlib.morphloader.MorphologyLoader()
@@ -329,9 +327,45 @@ class ArtifactResolverTests(unittest.TestCase):
         for chunk2 in sources:
             pool.add(chunk2)
 
-        self.assertRaises(morphlib.artifactresolver.DependencyOrderError,
-                          self.resolver._resolve_artifacts, pool)
+        artifacts = self.resolver._resolve_artifacts(pool)
+
+        all_artifacts_in_source_pool = itertools.chain.from_iterable(
+            source.artifacts.itervalues() for source in pool)
+        self.assertEqual(set(artifacts), set(all_artifacts_in_source_pool))
+
+    def test_handles_invalid_chunk_dependencies(self):
+        pool = morphlib.sourcepool.SourcePool()
+
+        loader = morphlib.morphloader.MorphologyLoader()
+        morph = loader.load_from_string(
+            '''
+                name: stratum
+                kind: stratum
+                build-depends: []
+                chunks:
+                    - name: chunk1
+                      repo: repo
+                      ref: original/ref
+                      build-system: manual
+                      build-depends:
+                          - undefined-chunk
+            ''')
+        sources = morphlib.source.make_sources(
+            'repo', 'original/ref', 'stratum.morph', 'sha1', 'tree', morph,
+            default_split_rules=default_split_rules)
+        for stratum in sources:
+            pool.add(stratum)
+
+        morph = get_chunk_morphology('chunk1')
+        sources = morphlib.source.make_sources(
+            'repo', 'original/ref', 'chunk1.morph', 'sha1', 'tree', morph,
+            default_split_rules=default_split_rules)
+        for chunk1 in sources:
+            pool.add(chunk1)
 
+        with self.assertRaises(
+                morphlib.artifactresolver.UnknownDependencyError):
+            artifacts = self.resolver._resolve_artifacts(pool)
 
 # TODO: Expand test suite to include better dependency checking, many
 #       tests were removed due to the fundamental change in how artifacts