Merge branch 'samthursfield/stratum-build-depends'

Reviewed-By: Lars Wirzenius <lars.wirzenius@codethink.co.uk>

7 files changed, 285 insertions, 73 deletions

diff --git a/morphlib/buildcommand.py b/morphlib/buildcommand.py
index 95f3812e..cd7cbc26 100644
--- a/morphlib/buildcommand.py
+++ b/morphlib/buildcommand.py
@@ -102,8 +102,14 @@ class BuildCommand(object):
         return order
 
     def _validate_cross_morphology_references(self, srcpool):
+        '''Perform validation across all morphologies involved in the build'''
+
+        stratum_names = []
+
         for src in srcpool:
             kind = src.morphology['kind']
+
+            # Verify that chunks pointed to by strata really are chunks, etc.
             method_name = '_validate_cross_refs_for_%s' % kind
             if hasattr(self, method_name):
                 logging.debug('Calling %s' % method_name)
@@ -111,6 +117,19 @@ class BuildCommand(object):
             else:
                 logging.warning('No %s' % method_name)
 
+            # Verify stratum build-depends agree with the system's contents.
+            # It's not an error to build-depend on a stratum that isn't
+            # included in the target system, but if it is included, the repo
+            # and ref fields must match.
+            if src.morphology['kind'] == 'stratum':
+                name = src.morphology['name']
+                if name in stratum_names:
+                    raise morphlib.Error(
+                        "Conflicting versions of stratum '%s' appear in the "
+                        "build. Check the contents of the system against the "
+                        "build-depends of the strata." % name)
+                stratum_names.append(name)
+
     def _validate_cross_refs_for_system(self, src, srcpool):
         self._validate_cross_refs_for_xxx(
             src, srcpool, src.morphology['strata'], 'stratum')
diff --git a/morphlib/plugins/branch_and_merge_plugin.py b/morphlib/plugins/branch_and_merge_plugin.py
index 24c76ace..bba0afe4 100644
--- a/morphlib/plugins/branch_and_merge_plugin.py
+++ b/morphlib/plugins/branch_and_merge_plugin.py
@@ -623,45 +623,25 @@ class BranchAndMergePlugin(cliapp.Plugin):
                                       (' '.join(command), repo, error))
         return repo_dir
 
-    def edit_stratum(self, system_branch, branch_dir, branch_root,
-                     branch_root_dir, stratum):
-        if stratum['repo'] == branch_root:
-            stratum_repo_dir = branch_root_dir
-            if stratum['ref'] != system_branch:
-                # We need to bring the morphology forwards from its ref to the
-                # current HEAD
-                repo = self.lrc.get_repo(branch_root)
-                stratum_morphology = repo.load_morphology(
-                    stratum['ref'], stratum['morph'])
-                self.save_morphology(branch_root_dir,
-                    stratum['morph'], stratum_morphology)
-                self.log_change(
-                    stratum['repo'],
+    def make_available(self, spec, branch, branch_dir, root_repo,
+                       root_repo_dir):
+        '''Check out the morphology that 'spec' refers to, for editing'''
+
+        if spec['repo'] == root_repo:
+            # This is only possible for stratum morphologies
+            repo_dir = root_repo_dir
+            if spec['ref'] != branch:
+                # 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.log_change(spec['repo'],
                     '"%s" copied from "%s" to "%s"' %
-                    (stratum['morph'], stratum['ref'], system_branch))
+                    (spec['morph'], spec['ref'], branch))
         else:
-            # Make sure the stratum repository is available
-            stratum_repo_dir = self.checkout_repository(
-                branch_dir, stratum['repo'], system_branch,
-                parent_ref=stratum['ref'])
-
-        stratum['ref'] = system_branch
-        return stratum_repo_dir
-
-    def edit_chunk(self, system_branch, branch_dir, stratum_repo_dir, chunk):
-        if 'unpetrify-ref' in chunk:
-            chunk['ref'] = chunk['unpetrify-ref']
-            del chunk['unpetrify-ref']
-
-        # Make the chunk repository and the ref available locally.
-        chunk_repo_dir = self.checkout_repository(
-                branch_dir, chunk['repo'], system_branch,
-                parent_ref=chunk['ref'])
-
-        # Check if we need to update anything at all.
-        if chunk['ref'] != system_branch:
-            # Update the reference to the chunk in the stratum morphology.
-            chunk['ref'] = system_branch
+            repo_dir = self.checkout_repository(
+                branch_dir, spec['repo'], branch, parent_ref=spec['ref'])
+        return repo_dir
 
     def edit(self, args):
         '''Edit a component in a system branch.'''
@@ -672,11 +652,11 @@ class BranchAndMergePlugin(cliapp.Plugin):
                 'or a system, a stratum and a chunk as arguments')
 
         workspace = self.deduce_workspace()
-        system_branch, branch_dir = self.deduce_system_branch()
+        branch, branch_dir = self.deduce_system_branch()
 
         # Find out which repository we branched off from.
-        branch_root = self.get_branch_config(branch_dir, 'branch.root')
-        branch_root_dir = self.find_repository(branch_dir, branch_root)
+        root_repo = self.get_branch_config(branch_dir, 'branch.root')
+        root_repo_dir = self.find_repository(branch_dir, root_repo)
 
         system_name = args[0]
         stratum_name = args[1]
@@ -684,32 +664,61 @@ class BranchAndMergePlugin(cliapp.Plugin):
 
         self.lrc, self.rrc = morphlib.util.new_repo_caches(self.app)
 
-        # Load the system morphology and find out which repo and ref
-        # we need to edit the stratum.
-        system_morphology = self.load_morphology(branch_root_dir, system_name)
-        stratum = self.get_edit_info(system_name, system_morphology,
-                                     stratum_name, collection='strata')
-        stratum_repo_dir = self.edit_stratum(
-            system_branch, branch_dir, branch_root, branch_root_dir, stratum)
-        self.save_morphology(branch_root_dir, system_name,
-                             system_morphology)
-        self.log_change(branch_root,
+        # We need to touch every stratum in the system, not just the target
+        # the user specified, because others may have build-depends that
+        # point to the edited stratum.
+        system_morphology = self.load_morphology(root_repo_dir, system_name)
+
+        # Test that the specified stratum exists in the system
+        self.get_edit_info(system_name, system_morphology, stratum_name)
+
+        for stratum_spec in system_morphology['strata']:
+            stratum_repo_dir = self.make_available(
+                stratum_spec, branch, branch_dir, root_repo, root_repo_dir)
+            stratum_morphology = self.load_morphology(
+                stratum_repo_dir, stratum_spec['morph'])
+            changed = False
+
+            if stratum_spec['morph'] == stratum_name:
+                if chunk_name is not None:
+                    # Change the stratum's ref to the chunk
+                    chunk_spec = self.get_edit_info(
+                        stratum_name, stratum_morphology, chunk_name,
+                        collection='chunks')
+
+                    if 'unpetrify-ref' in chunk_spec:
+                        chunk_spec['ref'] = chunk_spec['unpetrify-ref']
+                        del chunk_spec['unpetrify-ref']
+
+                    self.make_available(
+                        chunk_spec, branch, branch_dir, root_repo,
+                        root_repo_dir)
+
+                    if chunk_spec['ref'] != branch:
+                        chunk_spec['ref'] = branch
+
+                    self.log_change(stratum_spec['repo'],
                         '"%s" now includes "%s" from "%s"' %
-                        (system_name, stratum_name, system_branch))
-
-        if chunk_name:
-            # Load the stratum morphology and find out which repo and ref
-            # we need to edit the chunk.
-            stratum_morphology = self.load_morphology(stratum_repo_dir,
-                                                      stratum_name)
-            chunk = self.get_edit_info(stratum_name, stratum_morphology,
-                                       chunk_name, collection='chunks')
-            self.edit_chunk(system_branch, branch_dir, stratum_repo_dir, chunk)
-            self.save_morphology(stratum_repo_dir, stratum_name,
-                                 stratum_morphology)
-            self.log_change(stratum['repo'],
-                            '"%s" now includes "%s" from "%s"' %
-                            (stratum_name, chunk_name, system_branch))
+                        (stratum_name, chunk_name, branch))
+                changed = True
+            else:
+                # Update build-depends in other strata that point to this one
+                if stratum_morphology['build-depends'] is not None:
+                    for bd_spec in stratum_morphology['build-depends']:
+                        if bd_spec['morph'] == stratum_name:
+                            bd_spec['ref'] = branch
+                            changed = True
+                            break
+
+            if changed:
+                stratum_spec['ref'] = branch
+                self.save_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.print_changelog('The following changes were made but have not '
                              'been committed')
@@ -765,9 +774,9 @@ class BranchAndMergePlugin(cliapp.Plugin):
             self.save_morphology(root_repo_dir, name, morphology)
 
         for (repo, morph), ref in strata.iteritems():
-            repo_dir = self.edit_stratum(
-                branch, branch_path, root_repo, root_repo_dir,
-                { 'repo': repo, 'ref': ref, 'morph': morph})
+            repo_dir = self.make_available(
+                { 'repo': repo, 'ref': ref, 'morph': morph},
+                branch, branch_path, root_repo, root_repo_dir)
 
             stratum = self.load_morphology(repo_dir, morph)
 
@@ -808,9 +817,10 @@ class BranchAndMergePlugin(cliapp.Plugin):
                 continue
 
             for stratum_info in morphology['strata']:
-                repo_dir = self.edit_stratum(
-                    branch, branch_path, root_repo, root_repo_dir,
-                    stratum_info)
+                repo_dir = self.make_available(
+                    stratum_info, branch, branch_path, root_repo,
+                    root_repo_dir)
+                stratum_info['ref'] = branch
 
                 stratum = self.load_morphology(repo_dir, stratum_info['morph'])
 
@@ -947,8 +957,9 @@ class BranchAndMergePlugin(cliapp.Plugin):
         for info in strata:
             # Obtain the commit SHA1 this stratum would be built from.
             commit, tree = self.resolve_info(info, resolved_refs)
-            stratum_repo_dir = self.edit_stratum(
-                    branch, branch_dir, repo, repo_dir, info)
+            stratum_repo_dir = self.make_available(
+                    info, branch, branch_dir, repo, repo_dir)
+            info['ref'] = branch
 
             # Load the stratum morphology and petrify it recursively if
             # that hasn't happened yet.
@@ -1442,6 +1453,9 @@ class BranchAndMergePlugin(cliapp.Plugin):
             for info in morphology['strata']:
                 inject_build_ref(info)
         elif morphology['kind'] == 'stratum':
+            if morphology['build-depends'] is not None:
+                for info in morphology['build-depends']:
+                    inject_build_ref(info)
             for info in morphology['chunks']:
                 inject_build_ref(info)
 
diff --git a/tests.as-root/build-handles-stratum-build-depends.script b/tests.as-root/build-handles-stratum-build-depends.script
new file mode 100755
index 00000000..5df90bfc
--- /dev/null
+++ b/tests.as-root/build-handles-stratum-build-depends.script
@@ -0,0 +1,41 @@
+#!/bin/bash
+# Copyright (C) 2012  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.
+
+## "morph build" should update the build-depends fields of strata correctly.
+
+set -eu
+
+# Disable test on versions of Python before 2.7.
+. "$SRCDIR/scripts/python-check"
+
+. "$SRCDIR/tests.as-root/setup-build"
+
+cd "$DATADIR/workspace"
+"$SRCDIR/scripts/test-morph" branch test:morphs test/stratum-build-depends
+
+cd test/stratum-build-depends/test:morphs
+
+# Both 'linux-system' and the build-depends field of 'linux-stratum' need to
+# be updated here. If they are not, the build command will fail.
+"$SRCDIR/scripts/test-morph" edit linux-system hello-stratum
+
+# Likewise, this command must update build-depends or the 'repo' field will
+# not be changed in the temporary build branch, leading to:
+#
+#   ERROR: Ref test/stratum-build-depends is an invalid reference for
+#   repo file://TMP/morphs
+#
+"$SRCDIR/scripts/test-morph" build linux-system
diff --git a/tests.as-root/setup b/tests.as-root/setup
index a69dd94e..d3ba7ef4 100755
--- a/tests.as-root/setup
+++ b/tests.as-root/setup
@@ -129,6 +129,13 @@ cat <<EOF > linux-stratum.morph
 {
     "name": "linux-stratum",
     "kind": "stratum",
+    "build-depends": [
+        {
+            "morph": "hello-stratum",
+            "repo": "test:morphs",
+            "ref": "master"
+        }
+    ],
     "chunks": [
         {
             "name": "linux",
diff --git a/tests.as-root/tarball-image-is-sensible.setup b/tests.as-root/tarball-image-is-sensible.setup
index 322038ba..c1c88993 100755
--- a/tests.as-root/tarball-image-is-sensible.setup
+++ b/tests.as-root/tarball-image-is-sensible.setup
@@ -69,6 +69,12 @@ cat <<EOF > hello-tarball.morph
 EOF
 git add hello-tarball.morph
 
+# Change build-depends ref of hello-stratum from master to tarball-links
+sed -i linux-stratum.morph \
+    -e '/"build-depends": \[/,/\]/ s/"ref": "master"/"ref": "tarball-links"/'
+
+git add linux-stratum.morph
+
 cat <<EOF > link-stratum.morph
 {
     "name": "link-stratum",
diff --git a/tests.branching/edit-updates-stratum-build-depends.script b/tests.branching/edit-updates-stratum-build-depends.script
new file mode 100755
index 00000000..525822c9
--- /dev/null
+++ b/tests.branching/edit-updates-stratum-build-depends.script
@@ -0,0 +1,77 @@
+#!/bin/sh
+#
+# Copyright (C) 2012  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.
+
+
+## Verify that "morph edit" clones a chunk repository into a system branch.
+
+set -eu
+
+# Disable test on versions of Python before 2.7.
+. "$SRCDIR/scripts/python-check"
+
+# Add a second stratum that build-depends on the first. We can ignore the fact
+# that both strata contain the same chunk for this test.
+cd "$DATADIR/morphs"
+
+cat <<EOF > xyzzy-stratum.morph
+{
+    "name": "xyzzy-stratum",
+    "kind": "stratum",
+    "build-depends": [
+        {
+            "morph": "hello-stratum",
+            "repo": "test:morphs",
+            "ref": "master"
+        }
+    ],
+    "chunks": [
+        {
+            "name": "hello",
+            "repo": "test:hello",
+            "ref": "master",
+            "build-depends": []
+        }
+    ]
+}
+EOF
+
+cat hello-system.morph | head -n $(expr $(wc -l < hello-system.morph) - 3) \
+    > hello-system.morph
+cat <<EOF >> hello-system.morph
+         },
+         {
+            "morph": "xyzzy-stratum",
+            "repo": "test:morphs",
+            "ref": "master"
+         }
+    ]
+}
+EOF
+
+git add xyzzy-stratum.morph hello-system.morph
+git commit -q -m "Add 2nd stratum with a build dependency"
+
+# Create system branch.
+cd "$DATADIR/workspace"
+"$SRCDIR/scripts/test-morph" init
+"$SRCDIR/scripts/test-morph" branch test:morphs newbranch
+
+# Edit chunk.
+"$SRCDIR/scripts/test-morph" edit hello-system hello-stratum hello
+
+# See what effect the editing had.
+"$SRCDIR/scripts/run-git-in" "newbranch/test:morphs" diff
diff --git a/tests.branching/edit-updates-stratum-build-depends.stdout b/tests.branching/edit-updates-stratum-build-depends.stdout
new file mode 100644
index 00000000..00010f17
--- /dev/null
+++ b/tests.branching/edit-updates-stratum-build-depends.stdout
@@ -0,0 +1,48 @@
+diff --git a/hello-stratum.morph b/hello-stratum.morph
+index 3b7be17..febfa60 100644
+--- a/hello-stratum.morph
++++ b/hello-stratum.morph
+@@ -5,7 +5,7 @@
+         {
+             "name": "hello",
+             "repo": "test:hello",
+-            "ref": "master",
++            "ref": "newbranch",
+             "build-depends": []
+         }
+     ]
+diff --git a/hello-system.morph b/hello-system.morph
+index 87a47f0..75b0f90 100644
+--- a/hello-system.morph
++++ b/hello-system.morph
+@@ -8,12 +8,12 @@
+         {
+             "morph": "hello-stratum",
+             "repo": "test:morphs",
+-            "ref": "master"
+-         },
+-         {
++            "ref": "newbranch"
++        },
++        {
+             "morph": "xyzzy-stratum",
+             "repo": "test:morphs",
+-            "ref": "master"
+-         }
++            "ref": "newbranch"
++        }
+     ]
+ }
+diff --git a/xyzzy-stratum.morph b/xyzzy-stratum.morph
+index 8f83beb..e0a895a 100644
+--- a/xyzzy-stratum.morph
++++ b/xyzzy-stratum.morph
+@@ -5,7 +5,7 @@
+         {
+             "morph": "hello-stratum",
+             "repo": "test:morphs",
+-            "ref": "master"
++            "ref": "newbranch"
+         }
+     ],
+     "chunks": [