From 9cdeacaea8528c1bc090046715374882e9be0e15 Mon Sep 17 00:00:00 2001
From: Sam Thursfield <sam.thursfield@codethink.co.uk>
Date: Thu, 19 Feb 2015 17:14:15 +0000
Subject: sourceresolver: Fix LsTreeError raised when constructing build graph

If you had a repo that was not in your local or remote cache in your
definitions, Morph might raise LsTreeError and abort.

The code was assuming the repo would already be cached in the local
cache by the time it got to detecting build systems, but that's not
always true -- if the (reponame, ref) pair was already in the
'resolved_trees' cache but the repo had been deleted from the local
Git cache, to name one possibility.

Also, the remoterepocache.ls_tree operation can fail if the repo is
not hosted in the Trove, so we shouldn't abort the program in that case.

Finally, this patch removes an unused variable.
---
 morphlib/sourceresolver.py       | 26 +++++++++++++++++++-------
 morphlib/sourceresolver_tests.py | 23 +++++++++++++++++++++--
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/morphlib/sourceresolver.py b/morphlib/sourceresolver.py
index 29069d7d..0c2d1a0a 100644
--- a/morphlib/sourceresolver.py
+++ b/morphlib/sourceresolver.py
@@ -233,6 +233,7 @@ class SourceResolver(object):
 
 
         loader = morphlib.morphloader.MorphologyLoader()
+        morph = None
         if defs_filename and os.path.exists(defs_filename): # pragma: no cover
             morph = loader.load_from_file(defs_filename)
         elif self.lrc.has_repo(reponame):
@@ -245,7 +246,6 @@ class SourceResolver(object):
                 morph = loader.load_from_string(text)
             except IOError:
                 morph = None
-                file_list = repo.list_files(ref=sha1, recurse=False)
         elif self.rrc is not None:
             self.status(msg="Looking for %(reponame)s:%(filename)s in the "
                             "remote repo cache.",
@@ -280,16 +280,28 @@ class SourceResolver(object):
                         "chunk morph from repo's build system" %
                     expected_filename, chatty=True)
 
+        file_list = None
+
         if self.lrc.has_repo(reponame):
             repo = self.lrc.get_repo(reponame)
             file_list = repo.list_files(ref=sha1, recurse=False)
         elif self.rrc is not None:
-            file_list = self.rrc.ls_tree(reponame, sha1)
-        else:
-            # We assume that _resolve_ref() must have already been called and
-            # so the repo in question would have been made available already
-            # if it had been possible.
-            raise NotcachedError(reponame)
+            try:
+                # This may or may not succeed; if the is repo not
+                # hosted on the same Git server as the cache server then
+                # it'll definitely fail.
+                file_list = self.rrc.ls_tree(reponame, sha1)
+            except morphlib.remoterepocache.LsTreeError:
+                pass
+        if not file_list:
+            if self.update:
+                self.status(msg='Caching git repository %(reponame)s',
+                            reponame=reponame)
+                repo = self.lrc.cache_repo(reponame)
+                repo.update()
+                file_list = repo.list_files(ref=sha1, recurse=False)
+            else:  # pragma: no cover
+                raise NotcachedError(reponame)
 
         buildsystem = morphlib.buildsystem.detect_build_system(file_list)
 
diff --git a/morphlib/sourceresolver_tests.py b/morphlib/sourceresolver_tests.py
index 2410218a..6d6a83fa 100644
--- a/morphlib/sourceresolver_tests.py
+++ b/morphlib/sourceresolver_tests.py
@@ -24,7 +24,7 @@ from morphlib.sourceresolver import (SourceResolver,
                                      PickleCacheManager,
                                      MorphologyNotFoundError,
                                      NotcachedError)
-from morphlib.remoterepocache import CatFileError
+from morphlib.remoterepocache import CatFileError, LsTreeError
 
 
 class FakeRemoteRepoCache(object):
@@ -135,6 +135,9 @@ class FakeLocalRepo(object):
     def list_files(self, ref, recurse):
         return self.morphologies.keys()
 
+    def update(self):
+        pass
+
 
 class FakeLocalRepoCache(object):
 
@@ -147,6 +150,9 @@ class FakeLocalRepoCache(object):
     def get_repo(self, reponame):
         return self.lr
 
+    def cache_repo(self, reponame):
+        return self.lr
+
 
 class SourceResolverTests(unittest.TestCase):
 
@@ -188,6 +194,9 @@ class SourceResolverTests(unittest.TestCase):
     def noremotefile(self, *args):
         raise CatFileError('reponame', 'ref', 'filename')
 
+    def noremoterepo(self, *args):
+        raise LsTreeError('reponame', 'ref')
+
     def localmorph(self, *args):
         return ['chunk.morph']
 
@@ -241,6 +250,15 @@ class SourceResolverTests(unittest.TestCase):
                                             'assumed-local.morph')
         self.assertEqual('autotools', name)
 
+    def test_cache_repo_if_not_in_either_cache(self):
+        self.lrc.has_repo = self.doesnothaverepo
+        self.lr.read_file = self.nolocalmorph
+        self.lr.list_files = self.autotoolsbuildsystem
+        self.rrc.ls_tree = self.noremoterepo
+        name = self.sr._detect_build_system('reponame', 'sha1',
+                                            'assumed-local.morph')
+        self.assertEqual('autotools', name)
+
     def test_autodetects_remote_morphology(self):
         self.lrc.has_repo = self.doesnothaverepo
         self.rrc.cat_file = self.noremotemorph
@@ -262,7 +280,8 @@ class SourceResolverTests(unittest.TestCase):
 
     def test_raises_error_when_repo_does_not_exist(self):
         self.lrc.has_repo = self.doesnothaverepo
-        self.assertRaises(NotcachedError, self.lsr._detect_build_system,
+        self.assertRaises(MorphologyNotFoundError,
+                          self.lsr._detect_build_system,
                           'reponame', 'sha1', 'non-existent.morph')
 
     def test_raises_error_when_failed_to_detect_build_system(self):
-- 
cgit v1.2.1


From afcb1146a2ed1a09244266f4e302ee78d8625b92 Mon Sep 17 00:00:00 2001
From: Sam Thursfield <sam.thursfield@codethink.co.uk>
Date: Fri, 20 Feb 2015 10:23:11 +0000
Subject: sourceresolver: Always try to read chunk morph files from definitions

This fixes the 'Morph ignores the chunk morph file I added to
definitions' regression introduced in the recent build-graph speedups
branch.

The new 'detected-chunk-buildsystems' cache associates a (chunk repo,
chunk ref) pair with the auto-detected build system. When calculating
the build graph, if the is an auto-detected build system known already,
that will be used instead of looking for a chunk morphology.

There's a flaw here: if the user added or changed the chunk morphology
in the definitions repo, the *chunk* ref won't necessarily have changed
-- so Morph will ignore the user's changes, if it had already cached an
autodetected buildsystem for that repo/ref pair.

This doesn't cost much in terms of speed because we're just reading a
file from disk. We can still avoid looking in the chunk repo for a chunk
morph, because the chunk ref would be different if something had changed
in the chunk repo.
---
 morphlib/sourceresolver.py | 69 +++++++++++++++++++++++++++-------------------
 1 file changed, 40 insertions(+), 29 deletions(-)

diff --git a/morphlib/sourceresolver.py b/morphlib/sourceresolver.py
index 0c2d1a0a..832266dd 100644
--- a/morphlib/sourceresolver.py
+++ b/morphlib/sourceresolver.py
@@ -214,29 +214,12 @@ class SourceResolver(object):
 
         return absref, tree
 
-    def _get_morphology(self, reponame, sha1, filename):
-        '''Read the morphology at the specified location.
-
-        Returns None if the file does not exist in the specified commit.
-
-        '''
-        key = (reponame, sha1, filename)
-        if key in self._resolved_morphologies:
-            return self._resolved_morphologies[key]
+    def _get_morphology_from_definitions(self, loader, filename):
+        return loader.load_from_file(filename)
 
-        if reponame == self._definitions_repo and \
-                sha1 == self._definitions_absref: # pragma: no cover
-            defs_filename = os.path.join(self._definitions_checkout_dir,
-                                         filename)
-        else:
-            defs_filename = None
-
-
-        loader = morphlib.morphloader.MorphologyLoader()
-        morph = None
-        if defs_filename and os.path.exists(defs_filename): # pragma: no cover
-            morph = loader.load_from_file(defs_filename)
-        elif self.lrc.has_repo(reponame):
+    def _get_morphology_from_chunk_repo(self, loader, reponame, sha1,
+                                        filename):
+        if self.lrc.has_repo(reponame):
             self.status(msg="Looking for %(reponame)s:%(filename)s in the "
                             "local repo cache.",
                         reponame=reponame, filename=filename, chatty=True)
@@ -261,6 +244,34 @@ class SourceResolver(object):
             # if it had been possible.
             raise NotcachedError(reponame)
 
+        return morph
+
+    def _get_morphology(self, reponame, sha1, filename,
+                        look_in_chunk_repo=True):
+        '''Read the morphology at the specified location.
+
+        Returns None if the file does not exist in the specified commit.
+
+        '''
+        key = (reponame, sha1, filename)
+        if key in self._resolved_morphologies:
+            return self._resolved_morphologies[key]
+
+        if reponame == self._definitions_repo and \
+                sha1 == self._definitions_absref: # pragma: no cover
+            defs_filename = os.path.join(self._definitions_checkout_dir,
+                                         filename)
+        else:
+            defs_filename = None
+
+
+        loader = morphlib.morphloader.MorphologyLoader()
+        morph = None
+        if defs_filename and os.path.exists(defs_filename): # pragma: no cover
+            morph = self._get_morphology_from_definitions(loader, defs_filename)
+        elif look_in_chunk_repo:
+            morph = self._get_morphology_from_chunk_repo(loader, reponame, sha1, filename)
+
         if morph is None:
             return None
         else:
@@ -378,18 +389,18 @@ class SourceResolver(object):
 
         morph_name = os.path.splitext(os.path.basename(filename))[0]
 
-        morphology = None
+        morphology = self._get_morphology(
+            *definition_key, look_in_chunk_repo=False)
         buildsystem = None
 
         if chunk_key in self._resolved_buildsystems:
             buildsystem = self._resolved_buildsystems[chunk_key]
 
-        if buildsystem is None:
-            # The morphologies aren't locally cached, so a morphology
-            # for a chunk kept in the chunk repo will be read every time.
-            # So, always keep your chunk morphs in your definitions repo,
-            # not in the chunk repo!
-            morphology = self._get_morphology(*definition_key)
+        if morphology is None and buildsystem is None:
+            # This is a slow operation (looking for a file in Git repo may
+            # potentially require cloning the whole thing).
+            morphology = self._get_morphology(
+                *definition_key, look_in_chunk_repo=True)
 
         if morphology is None:
             if buildsystem is None:
-- 
cgit v1.2.1


From 0b0f6631fc410b281bca45d30bf6f41105b0b0f8 Mon Sep 17 00:00:00 2001
From: Sam Thursfield <sam.thursfield@codethink.co.uk>
Date: Fri, 20 Feb 2015 10:37:30 +0000
Subject: sourceresolver: Fix InvalidRefError when local git cache is out of
 date

Code in sourceresolver.py assumed that resolve_ref() would be called for
the chunk repo before get_morphology() was called, so the repo would
always be up to date. This wasn't actually true.

If your local repo cache was out of date, you might see the following
sort of error:

    InvalidRefError: Git directory
    /src/cache/gits/git___git_baserock_org_delta_usbutils has no commit at
    ref c37f146eb2c6642c600f1b025a6d56996b0697ff^{tree}.
---
 morphlib/sourceresolver.py | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/morphlib/sourceresolver.py b/morphlib/sourceresolver.py
index 832266dd..de80515e 100644
--- a/morphlib/sourceresolver.py
+++ b/morphlib/sourceresolver.py
@@ -384,8 +384,11 @@ class SourceResolver(object):
 
     def process_chunk(self, definition_repo, definition_ref, chunk_repo,
                       chunk_ref, filename, visit): # pragma: no cover
+        absref = None
+        tree = None
+
         definition_key = (definition_repo, definition_ref, filename)
-        chunk_key = (chunk_repo, chunk_ref, filename)
+        chunk_key = None
 
         morph_name = os.path.splitext(os.path.basename(filename))[0]
 
@@ -399,8 +402,10 @@ class SourceResolver(object):
         if morphology is None and buildsystem is None:
             # This is a slow operation (looking for a file in Git repo may
             # potentially require cloning the whole thing).
+            absref, tree = self._resolve_ref(chunk_repo, chunk_ref)
+            chunk_key = (chunk_repo, absref, filename)
             morphology = self._get_morphology(
-                *definition_key, look_in_chunk_repo=True)
+                *chunk_key, look_in_chunk_repo=True)
 
         if morphology is None:
             if buildsystem is None:
@@ -413,7 +418,9 @@ class SourceResolver(object):
                     buildsystem, morph_name)
                 self._resolved_morphologies[definition_key] = morphology
 
-        absref, tree = self._resolve_ref(chunk_repo, chunk_ref)
+        if not absref or not tree:
+            absref, tree = self._resolve_ref(chunk_repo, chunk_ref)
+
         visit(chunk_repo, chunk_ref, filename, absref, tree, morphology)
 
     def traverse_morphs(self, definitions_repo, definitions_ref,
@@ -452,10 +459,7 @@ class SourceResolver(object):
                     system_filenames, definitions_repo, definitions_ref,
                     definitions_absref, definitions_tree, visit)
 
-            # Now process all the chunks involved in the build. First those
-            # with morphologies in definitions.git, and then (for compatibility
-            # reasons only) those with the morphology in the chunk's source
-            # repository.
+            # Now process all the chunks involved in the build.
             for repo, ref, filename in chunk_in_definitions_repo_queue:
                 self.process_chunk(definitions_repo, definitions_absref, repo,
                                    ref, filename, visit)
-- 
cgit v1.2.1


From 9c53c8b12a2ff92e850daf3def87aa489dfafecf Mon Sep 17 00:00:00 2001
From: Sam Thursfield <sam.thursfield@codethink.co.uk>
Date: Fri, 20 Feb 2015 10:50:35 +0000
Subject: sourceresolver: Simplify some code paths

It turns out that always looking for the chunk morph in the definitions
repo allows us to make the code a little less nasty. Bonus!
---
 morphlib/sourceresolver.py       | 68 +++++++++++++++++++++-------------------
 morphlib/sourceresolver_tests.py |  8 +++--
 2 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/morphlib/sourceresolver.py b/morphlib/sourceresolver.py
index de80515e..2d2d02f1 100644
--- a/morphlib/sourceresolver.py
+++ b/morphlib/sourceresolver.py
@@ -214,11 +214,14 @@ class SourceResolver(object):
 
         return absref, tree
 
-    def _get_morphology_from_definitions(self, loader, filename):
-        return loader.load_from_file(filename)
+    def _get_morphology_from_definitions(self, loader,
+                                         filename):  # pragma: no cover
+        if os.path.exists(filename):
+            return loader.load_from_file(filename)
+        else:
+            return None
 
-    def _get_morphology_from_chunk_repo(self, loader, reponame, sha1,
-                                        filename):
+    def _get_morphology_from_repo(self, loader, reponame, sha1, filename):
         if self.lrc.has_repo(reponame):
             self.status(msg="Looking for %(reponame)s:%(filename)s in the "
                             "local repo cache.",
@@ -246,8 +249,7 @@ class SourceResolver(object):
 
         return morph
 
-    def _get_morphology(self, reponame, sha1, filename,
-                        look_in_chunk_repo=True):
+    def _get_morphology(self, reponame, sha1, filename):
         '''Read the morphology at the specified location.
 
         Returns None if the file does not exist in the specified commit.
@@ -257,20 +259,20 @@ class SourceResolver(object):
         if key in self._resolved_morphologies:
             return self._resolved_morphologies[key]
 
+        loader = morphlib.morphloader.MorphologyLoader()
+        morph = None
+
         if reponame == self._definitions_repo and \
                 sha1 == self._definitions_absref: # pragma: no cover
+            # There is a temporary local checkout of the definitions repo which
+            # we can quickly read definitions files from.
             defs_filename = os.path.join(self._definitions_checkout_dir,
                                          filename)
+            morph = self._get_morphology_from_definitions(loader,
+                                                          defs_filename)
         else:
-            defs_filename = None
-
-
-        loader = morphlib.morphloader.MorphologyLoader()
-        morph = None
-        if defs_filename and os.path.exists(defs_filename): # pragma: no cover
-            morph = self._get_morphology_from_definitions(loader, defs_filename)
-        elif look_in_chunk_repo:
-            morph = self._get_morphology_from_chunk_repo(loader, reponame, sha1, filename)
+            morph = self._get_morphology_from_repo(loader, reponame, sha1,
+                                                   filename)
 
         if morph is None:
             return None
@@ -343,8 +345,7 @@ class SourceResolver(object):
                                            definitions_tree,
                                            visit): # pragma: no cover
         definitions_queue = collections.deque(system_filenames)
-        chunk_in_definitions_repo_queue = set()
-        chunk_in_source_repo_queue = set()
+        chunk_queue = set()
 
         while definitions_queue:
             filename = definitions_queue.popleft()
@@ -372,15 +373,22 @@ class SourceResolver(object):
                         for s in morphology['build-depends'])
                 for c in morphology['chunks']:
                     if 'morph' not in c:
+                        # Autodetect a path if one is not given. This is to
+                        # support the deprecated approach of putting the chunk
+                        # .morph file in the toplevel directory of the chunk
+                        # repo, instead of putting it in the definitions.git
+                        # repo.
+                        #
+                        # All users should be specifying a full path to the
+                        # chunk morph file, using the 'morph' field, and this
+                        # code path should be removed.
                         path = morphlib.util.sanitise_morphology_path(
                             c.get('morph', c['name']))
-                        chunk_in_source_repo_queue.add(
-                            (c['repo'], c['ref'], path))
-                        continue
-                    chunk_in_definitions_repo_queue.add(
-                        (c['repo'], c['ref'], c['morph']))
+                        chunk_queue.add((c['repo'], c['ref'], path))
+                    else:
+                        chunk_queue.add((c['repo'], c['ref'], c['morph']))
 
-        return chunk_in_definitions_repo_queue, chunk_in_source_repo_queue
+        return chunk_queue
 
     def process_chunk(self, definition_repo, definition_ref, chunk_repo,
                       chunk_ref, filename, visit): # pragma: no cover
@@ -392,8 +400,7 @@ class SourceResolver(object):
 
         morph_name = os.path.splitext(os.path.basename(filename))[0]
 
-        morphology = self._get_morphology(
-            *definition_key, look_in_chunk_repo=False)
+        morphology = self._get_morphology(*definition_key)
         buildsystem = None
 
         if chunk_key in self._resolved_buildsystems:
@@ -404,8 +411,7 @@ class SourceResolver(object):
             # potentially require cloning the whole thing).
             absref, tree = self._resolve_ref(chunk_repo, chunk_ref)
             chunk_key = (chunk_repo, absref, filename)
-            morphology = self._get_morphology(
-                *chunk_key, look_in_chunk_repo=True)
+            morphology = self._get_morphology(*chunk_key)
 
         if morphology is None:
             if buildsystem is None:
@@ -454,18 +460,14 @@ class SourceResolver(object):
             # First, process the system and its stratum morphologies. These
             # will all live in the same Git repository, and will point to
             # various chunk morphologies.
-            chunk_in_definitions_repo_queue, chunk_in_source_repo_queue = \
-                self._process_definitions_with_children(
+            chunk_queue = self._process_definitions_with_children(
                     system_filenames, definitions_repo, definitions_ref,
                     definitions_absref, definitions_tree, visit)
 
             # Now process all the chunks involved in the build.
-            for repo, ref, filename in chunk_in_definitions_repo_queue:
+            for repo, ref, filename in chunk_queue:
                 self.process_chunk(definitions_repo, definitions_absref, repo,
                                    ref, filename, visit)
-
-            for repo, ref, filename in chunk_in_source_repo_queue:
-                self.process_chunk(repo, ref, repo, ref, filename, visit)
         finally:
             shutil.rmtree(self._definitions_checkout_dir)
             self._definitions_checkout_dir = None
diff --git a/morphlib/sourceresolver_tests.py b/morphlib/sourceresolver_tests.py
index 6d6a83fa..75025a9a 100644
--- a/morphlib/sourceresolver_tests.py
+++ b/morphlib/sourceresolver_tests.py
@@ -308,10 +308,12 @@ class SourceResolverTests(unittest.TestCase):
                                             'assumed-local.morph')
         self.assertEqual('autotools', name)
 
-    def test_fails_when_local_not_cached_and_no_remote(self):
+    def test_succeeds_when_local_not_cached_and_no_remote(self):
         self.lrc.has_repo = self.doesnothaverepo
-        self.assertRaises(NotcachedError, self.lsr._get_morphology,
-                          'reponame', 'sha1', 'unreached.morph')
+        self.lr.list_files = self.localmorph
+        morph = self.sr._get_morphology('reponame', 'sha1',
+                                       'chunk.morph')
+        self.assertEqual('chunk', morph['name'])
 
     def test_arch_is_validated(self):
         self.lr.arch = 'unknown'
-- 
cgit v1.2.1


From 1d7e39b955f4da58230f9c5a06ff1b05f8bc020b Mon Sep 17 00:00:00 2001
From: Sam Thursfield <sam.thursfield@codethink.co.uk>
Date: Fri, 20 Feb 2015 11:13:24 +0000
Subject: sourceresolver: Factor out 'cache repo locally' code into a function

Also, move the repo.update() call into the 'fetch from tarball' code
path in the localrepocache module -- there's no need to update straight
after doing a `git clone`, but we do need to do one if we got the repo
from a `git archive` tarball that may be out of date.
---
 morphlib/localrepocache.py       |  6 ++++--
 morphlib/localrepocache_tests.py |  8 ++++++--
 morphlib/sourceresolver.py       | 44 +++++++++++++++++-----------------------
 morphlib/sourceresolver_tests.py |  3 +--
 4 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/morphlib/localrepocache.py b/morphlib/localrepocache.py
index 9bccb20b..39fbd200 100644
--- a/morphlib/localrepocache.py
+++ b/morphlib/localrepocache.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2012-2014  Codethink Limited
+# Copyright (C) 2012-2015  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
@@ -204,7 +204,9 @@ class LocalRepoCache(object):
         if self._tarball_base_url:
             ok, error = self._clone_with_tarball(repourl, path)
             if ok:
-                return self.get_repo(reponame)
+                repo = self.get_repo(reponame)
+                repo.update()
+                return repo
             else:
                 errors.append(error)
                 self._app.status(
diff --git a/morphlib/localrepocache_tests.py b/morphlib/localrepocache_tests.py
index ab6e71fd..aeb32961 100644
--- a/morphlib/localrepocache_tests.py
+++ b/morphlib/localrepocache_tests.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2012-2014  Codethink Limited
+# Copyright (C) 2012-2015 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
@@ -139,7 +139,11 @@ class LocalRepoCacheTests(unittest.TestCase):
         self.lrc._fetch = lambda url, path: self.fetched.append(url)
         self.unpacked_tar = ""
         self.mkdir_path = ""
-        self.lrc.cache_repo(self.repourl)
+
+        with morphlib.gitdir_tests.monkeypatch(
+                morphlib.cachedrepo.CachedRepo, 'update', lambda self: None):
+            self.lrc.cache_repo(self.repourl)
+
         self.assertEqual(self.fetched, [self.tarball_url])
         self.assertFalse(self.lrc.fs.exists(self.cache_path + '.tar'))
         self.assertEqual(self.remotes['origin']['url'], self.repourl)
diff --git a/morphlib/sourceresolver.py b/morphlib/sourceresolver.py
index 2d2d02f1..e84a6cd2 100644
--- a/morphlib/sourceresolver.py
+++ b/morphlib/sourceresolver.py
@@ -90,13 +90,6 @@ class MorphologyNotFoundError(SourceResolverError): # pragma: no cover
             self, "Couldn't find morphology: %s" % filename)
 
 
-class NotcachedError(SourceResolverError):
-    def __init__(self, repo_name):
-        SourceResolverError.__init__(
-            self, "Repository %s is not cached locally and there is no "
-                  "remote cache specified" % repo_name)
-
-
 class SourceResolver(object):
     '''Provides a way of resolving the set of sources for a given system.
 
@@ -155,6 +148,18 @@ class SourceResolver(object):
 
         self._definitions_checkout_dir = None
 
+    def cache_repo_locally(self, reponame):
+        if self.update:
+            self.status(msg='Caching git repository %(reponame)s',
+                        reponame=reponame)
+            repo = self.lrc.cache_repo(reponame)
+        else:  # pragma: no cover
+            # This is likely to raise a morphlib.localrepocache.NotCached
+            # exception, because the caller should have checked if the
+            # localrepocache already had the repo. But we may as well try.
+            repo = self.lrc.get_repo(reponame)
+        return repo
+
     def _resolve_ref(self, reponame, ref): # pragma: no cover
         '''Resolves commit and tree sha1s of the ref in a repo and returns it.
 
@@ -195,16 +200,9 @@ class SourceResolver(object):
                                 chatty=True)
             except BaseException, e:
                 logging.warning('Caught (and ignored) exception: %s' % str(e))
+
         if absref is None:
-            if self.update:
-                self.status(msg='Caching git repository %(reponame)s',
-                            reponame=reponame)
-                repo = self.lrc.cache_repo(reponame)
-                repo.update()
-            else:
-                # This is likely to raise an exception, because if the local
-                # repo cache had the repo we'd have already resolved the ref.
-                repo = self.lrc.get_repo(reponame)
+            repo = self.cache_repo_locally(reponame)
             absref = repo.resolve_ref_to_commit(ref)
             tree = repo.resolve_ref_to_tree(absref)
 
@@ -306,15 +304,10 @@ class SourceResolver(object):
                 file_list = self.rrc.ls_tree(reponame, sha1)
             except morphlib.remoterepocache.LsTreeError:
                 pass
+
         if not file_list:
-            if self.update:
-                self.status(msg='Caching git repository %(reponame)s',
-                            reponame=reponame)
-                repo = self.lrc.cache_repo(reponame)
-                repo.update()
-                file_list = repo.list_files(ref=sha1, recurse=False)
-            else:  # pragma: no cover
-                raise NotcachedError(reponame)
+            repo = self.cache_repo_locally(reponame)
+            file_list = repo.list_files(ref=sha1, recurse=False)
 
         buildsystem = morphlib.buildsystem.detect_build_system(file_list)
 
@@ -453,7 +446,8 @@ class SourceResolver(object):
             try:
                 definitions_cached_repo = self.lrc.get_repo(definitions_repo)
             except morphlib.localrepocache.NotCached:
-                definitions_cached_repo = self.lrc.cache_repo(definitions_repo)
+                definitions_cached_repo = self.cache_repo_locally(
+                    definitions_repo)
             definitions_cached_repo.extract_commit(
                 definitions_absref, self._definitions_checkout_dir)
 
diff --git a/morphlib/sourceresolver_tests.py b/morphlib/sourceresolver_tests.py
index 75025a9a..638f593f 100644
--- a/morphlib/sourceresolver_tests.py
+++ b/morphlib/sourceresolver_tests.py
@@ -22,8 +22,7 @@ import unittest
 import morphlib
 from morphlib.sourceresolver import (SourceResolver,
                                      PickleCacheManager,
-                                     MorphologyNotFoundError,
-                                     NotcachedError)
+                                     MorphologyNotFoundError)
 from morphlib.remoterepocache import CatFileError, LsTreeError
 
 
-- 
cgit v1.2.1


From 532af11d11003d52fc79cd4719f2b3353640a38b Mon Sep 17 00:00:00 2001
From: Sam Thursfield <sam.thursfield@codethink.co.uk>
Date: Fri, 20 Feb 2015 11:33:21 +0000
Subject: sourceresolver: Never assume that a given ref is present locally

Checking that a given ref exists using `git rev-parse --verify
1234^{commit}` is a reasonably quick operation. As a rough guide, 1000
invocations took 1.6 seconds on my PC.

The code is too fragile and hard to reason about if we assume that one
function has been called before another so the repo will already be up
to date.

This should fix any spurious InvalidRefError exceptions that the
build graph speedups branch has introduced.
---
 morphlib/sourceresolver.py | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/morphlib/sourceresolver.py b/morphlib/sourceresolver.py
index e84a6cd2..22e643d2 100644
--- a/morphlib/sourceresolver.py
+++ b/morphlib/sourceresolver.py
@@ -239,11 +239,10 @@ class SourceResolver(object):
                 morph = loader.load_from_string(text)
             except morphlib.remoterepocache.CatFileError:
                 morph = None
-        else:
-            # We assume that _resolve_ref() must have already been called and
-            # so the repo in question would have been made available already
-            # if it had been possible.
-            raise NotcachedError(reponame)
+        else:  # pragma: no cover
+            repo = self.cache_repo_locally(reponame)
+            text = repo.read_file(filename, sha1)
+            morph = loader.load_from_string(text)
 
         return morph
 
@@ -295,7 +294,10 @@ class SourceResolver(object):
 
         if self.lrc.has_repo(reponame):
             repo = self.lrc.get_repo(reponame)
-            file_list = repo.list_files(ref=sha1, recurse=False)
+            try:
+                file_list = repo.list_files(ref=sha1, recurse=False)
+            except morphlib.gitdir.InvalidRefError:  # pragma: no cover
+                pass
         elif self.rrc is not None:
             try:
                 # This may or may not succeed; if the is repo not
-- 
cgit v1.2.1