Merge branch 'sam/build-fix-2'

Reviewed-By: Richard Ipsum <richard.ipsum@codethink.co.uk>
Reviewed-By: Richard Maw <richard.maw@codethink.co.uk>
Reviewed-By: Daniel Silverstone <daniel.silverstone@codethink.co.uk>

5 files changed, 49 insertions, 26 deletions

diff --git a/morphlib/app.py b/morphlib/app.py
index 9ab102b3..c3c9c970 100644
--- a/morphlib/app.py
+++ b/morphlib/app.py
@@ -290,7 +290,8 @@ class Morph(cliapp.Application):
                    morphlib.util.sanitise_morphology_path(args[2]))
             args = args[3:]
 
-    def create_source_pool(self, lrc, rrc, repo, ref, filename):
+    def create_source_pool(self, lrc, rrc, repo, ref, filename,
+                           original_ref=None):
         pool = morphlib.sourcepool.SourcePool()
 
         def add_to_pool(reponame, ref, filename, absref, tree, morphology):
@@ -302,7 +303,8 @@ class Morph(cliapp.Application):
 
         self.traverse_morphs(repo, ref, [filename], lrc, rrc,
                              update=not self.settings['no-git-update'],
-                             visit=add_to_pool)
+                             visit=add_to_pool,
+                             definitions_original_ref=original_ref)
         return pool
 
     def resolve_ref(self, lrc, rrc, reponame, ref, update=True):
@@ -346,7 +348,8 @@ class Morph(cliapp.Application):
 
     def traverse_morphs(self, definitions_repo, definitions_ref,
                         system_filenames, lrc, rrc, update=True,
-                        visit=lambda rn, rf, fn, arf, m: None):
+                        visit=lambda rn, rf, fn, arf, m: None,
+                        definitions_original_ref=None):
         morph_factory = morphlib.morphologyfactory.MorphologyFactory(lrc, rrc,
                                                                      self)
         definitions_queue = collections.deque(system_filenames)
@@ -359,6 +362,9 @@ class Morph(cliapp.Application):
         definitions_absref, definitions_tree = self.resolve_ref(
             lrc, rrc, definitions_repo, definitions_ref, update)
 
+        if definitions_original_ref:
+            definitions_ref = definitions_original_ref
+
         while definitions_queue:
             filename = definitions_queue.popleft()
 
diff --git a/morphlib/buildbranch.py b/morphlib/buildbranch.py
index 638350e3..cfc4a67f 100644
--- a/morphlib/buildbranch.py
+++ b/morphlib/buildbranch.py
@@ -254,6 +254,10 @@ class BuildBranch(object):
         return self._sb.get_config('branch.name')
 
     @property
+    def root_commit(self):
+        return self._root.resolve_ref_to_commit(self.root_ref)
+
+    @property
     def root_local_repo_url(self):
         return urlparse.urljoin('file://', self._root.dirname)
 
@@ -291,7 +295,13 @@ def pushed_build_branch(bb, loader, changes_need_pushing, name, email,
         unpushed = any(bb.get_unpushed_branches())
 
         if not changes_made and not unpushed:
-            yield bb.root_repo_url, bb.root_ref
+            # We resolve the system branch ref to the commit SHA1 here, so that
+            # the build uses whatever commit the user's copy of the root repo
+            # considers the head of that branch to be. If we returned a named
+            # ref, we risk building what the remote considers the head of that
+            # branch to be instead, and we also trigger a needless update in
+            # the cached copy of the root repo.
+            yield bb.root_repo_url, bb.root_commit, bb.root_ref
             return
 
         def report_inject(gd):
@@ -318,6 +328,6 @@ def pushed_build_branch(bb, loader, changes_need_pushing, name, email,
                            remote=remote.get_push_url(), chatty=True)
             bb.push_build_branches(push_cb=report_push)
 
-            yield bb.root_repo_url, bb.root_build_ref
+            yield bb.root_repo_url, bb.root_build_ref, bb.root_build_ref
         else:
-            yield bb.root_local_repo_url, bb.root_build_ref
+            yield bb.root_local_repo_url, bb.root_build_ref, bb.root_build_ref
diff --git a/morphlib/buildcommand.py b/morphlib/buildcommand.py
index 7eb6c0ab..544d88d8 100644
--- a/morphlib/buildcommand.py
+++ b/morphlib/buildcommand.py
@@ -50,21 +50,24 @@ class BuildCommand(object):
         self.lac, self.rac = self.new_artifact_caches()
         self.lrc, self.rrc = self.new_repo_caches()
 
-    def build(self, args):
-        '''Build triplets specified on command line.'''
+    def build(self, repo_name, ref, filename, original_ref=None):
+        '''Build a given system morphology.'''
 
-        self.app.status(msg='Build starts', chatty=True)
+        self.app.status(
+            msg='Building %(repo_name)s %(ref)s %(filename)s',
+            repo_name=repo_name, ref=ref, filename=filename)
 
-        for repo_name, ref, filename in self.app.itertriplets(args):
-            self.app.status(msg='Building %(repo_name)s %(ref)s %(filename)s',
-                            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.build_in_order(root_artifact)
+        self.app.status(msg='Deciding on task order')
+        srcpool = self.create_source_pool(
+            repo_name, ref, filename, original_ref)
+        self.validate_sources(srcpool)
+        root_artifact = self.resolve_artifacts(srcpool)
+        self.build_in_order(root_artifact)
 
-        self.app.status(msg='Build ends successfully')
+        self.app.status(
+            msg='Build of %(repo_name)s %(ref)s %(filename)s ended '
+                'successfully',
+            repo_name=repo_name, ref=ref, filename=filename)
 
     def new_artifact_caches(self):
         '''Create interfaces for the build artifact caches.
@@ -82,7 +85,7 @@ class BuildCommand(object):
         return morphlib.buildenvironment.BuildEnvironment(self.app.settings,
                                                           arch)
 
-    def create_source_pool(self, repo_name, ref, filename):
+    def create_source_pool(self, repo_name, ref, filename, original_ref=None):
         '''Find the source objects required for building a the given artifact
 
         The SourcePool will contain every stratum and chunk dependency of the
@@ -92,7 +95,8 @@ class BuildCommand(object):
         '''
         self.app.status(msg='Creating source pool', chatty=True)
         srcpool = self.app.create_source_pool(
-            self.lrc, self.rrc, repo_name, ref, filename)
+            self.lrc, self.rrc, repo_name, ref, filename,
+            original_ref=original_ref)
 
         return srcpool
 
diff --git a/morphlib/plugins/build_plugin.py b/morphlib/plugins/build_plugin.py
index 64630c2b..218bd819 100644
--- a/morphlib/plugins/build_plugin.py
+++ b/morphlib/plugins/build_plugin.py
@@ -56,7 +56,8 @@ class BuildPlugin(cliapp.Plugin):
 
         build_command = morphlib.buildcommand.InitiatorBuildCommand(
             self.app, addr, port)
-        build_command.build(args)
+        for repo_name, ref, filename in self.app.itertriplets(args):
+            build_command.build(repo_name, ref, filename)
 
     def distbuild(self, args):
         '''Distbuild a system image in the current system branch
@@ -116,7 +117,8 @@ class BuildPlugin(cliapp.Plugin):
             self.app.settings['cachedir-min-space'])
 
         build_command = morphlib.buildcommand.BuildCommand(self.app)
-        build_command.build(args)
+        for repo_name, ref, filename in self.app.itertriplets(args):
+            build_command.build(repo_name, ref, filename)
 
     def build(self, args):
         '''Build a system image in the current system branch
@@ -189,5 +191,6 @@ class BuildPlugin(cliapp.Plugin):
                 bb, loader=loader, changes_need_pushing=push,
                 name=name, email=email, build_uuid=build_uuid,
                 status=self.app.status)
-        with pbb as (repo, ref):
-            build_command.build([repo, ref, system_filename])
+        with pbb as (repo, commit, original_ref):
+            build_command.build(repo, commit, system_filename,
+                                original_ref=original_ref)
diff --git a/morphlib/plugins/deploy_plugin.py b/morphlib/plugins/deploy_plugin.py
index 2bc53a0d..e795e637 100644
--- a/morphlib/plugins/deploy_plugin.py
+++ b/morphlib/plugins/deploy_plugin.py
@@ -328,14 +328,14 @@ class DeployPlugin(cliapp.Plugin):
                 bb, loader=loader, changes_need_pushing=False,
                 name=name, email=email, build_uuid=build_uuid,
                 status=self.app.status)
-        with pbb as (repo, ref):
+        with pbb as (repo, commit, original_ref):
             # Create a tempdir for this deployment to work in
             deploy_tempdir = tempfile.mkdtemp(
                 dir=os.path.join(self.app.settings['tempdir'], 'deployments'))
             try:
                 for system in cluster_morphology['systems']:
                     self.deploy_system(build_command, deploy_tempdir,
-                                       root_repo_dir, repo, ref, system,
+                                       root_repo_dir, repo, commit, system,
                                        env_vars, deployments,
                                        parent_location='')
             finally: