Almost complete the unification

Change-Id: Ifc29f9cd22f2d8cc609ee178bb22a4580386431c

10 files changed, 141 insertions, 113 deletions

diff --git a/morphlib/buildcommand.py b/morphlib/buildcommand.py
index 0bd6c6cb..e185a808 100644
--- a/morphlib/buildcommand.py
+++ b/morphlib/buildcommand.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# Copyright (C) 2011-2015  Codethink Limited
+# Copyright (C) 2011-2016  Codethink Limited
 # Copyright © 2015  Richard Ipsum
 #
 # This program is free software; you can redistribute it and/or modify
@@ -47,7 +47,7 @@ class BuildCommand(object):
     def __init__(self, app, build_env = None):
         self.app = app
         self.lac, self.rac = self.new_artifact_caches()
-        self.repo_cache = morphlib.util.repo_cache(self.app)
+        self.repo_cache = morphlib.util.new_repo_cache(self.app)
 
     def build(self, repo_name, ref, filename, original_ref=None):
         '''Build a given system morphology.'''
@@ -93,9 +93,7 @@ class BuildCommand(object):
         self.app.status(msg='Creating source pool', chatty=True)
         srcpool = morphlib.sourceresolver.create_source_pool(
             self.repo_cache, repo_name, ref, filenames,
-            cachedir=self.app.settings['cachedir'],
             original_ref=original_ref,
-            update_repos=not self.app.settings['no-git-update'],
             status_cb=self.app.status)
         return srcpool
 
@@ -391,7 +389,8 @@ class BuildCommand(object):
         '''Update the local git repository cache with the sources.'''
 
         repo_name = source.repo_name
-        source.repo = self.repo_cache.get_updated_repo(repo_name, ref=source.sha1)
+        source.repo = self.repo_cache.get_updated_repo(repo_name,
+                                                       ref=source.sha1)
         self.repo_cache.ensure_submodules(source.repo, source.sha1)
 
     def cache_artifacts_locally(self, artifacts):
diff --git a/morphlib/builder.py b/morphlib/builder.py
index 2d0a4bd4..5db9b39d 100644
--- a/morphlib/builder.py
+++ b/morphlib/builder.py
@@ -45,7 +45,7 @@ def extract_sources(app, repo_cache, repo, sha1, srcdir): #pragma: no cover
 
         morphlib.gitdir.checkout_from_cached_repo(repo, sha1, destdir)
         morphlib.git.reset_workdir(app.runcmd, destdir)
-        submodules = morphlib.git.Submodules(app, repo.dirname, sha1)
+        submodules = morphlib.git.Submodules(repo.path, sha1, app.runcmd)
         try:
             submodules.load()
         except morphlib.git.NoModulesFileError:
diff --git a/morphlib/definitions_repo.py b/morphlib/definitions_repo.py
index 8e44a191..8b022867 100644
--- a/morphlib/definitions_repo.py
+++ b/morphlib/definitions_repo.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2015  Codethink Limited
+# Copyright (C) 2015-2016  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
diff --git a/morphlib/plugins/artifact_inspection_plugin.py b/morphlib/plugins/artifact_inspection_plugin.py
index ee3ab907..4cd8fcad 100644
--- a/morphlib/plugins/artifact_inspection_plugin.py
+++ b/morphlib/plugins/artifact_inspection_plugin.py
@@ -133,7 +133,7 @@ class VersionGuesser(object):
         self.app = app
         self.repo_cache = morphlib.util.new_repo_cache(app)
         self.guessers = [
-            AutotoolsVersionGuesser(app, repo_cache)
+            AutotoolsVersionGuesser(app, self.repo_cache)
         ]
 
     def guess_version(self, repo, ref):
diff --git a/morphlib/plugins/list_artifacts_plugin.py b/morphlib/plugins/list_artifacts_plugin.py
index 7b92efa7..2c098c2a 100644
--- a/morphlib/plugins/list_artifacts_plugin.py
+++ b/morphlib/plugins/list_artifacts_plugin.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2014-2015  Codethink Limited
+# Copyright (C) 2014-2016  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
diff --git a/morphlib/plugins/show_dependencies_plugin.py b/morphlib/plugins/show_dependencies_plugin.py
index 880e7925..bfe4d6c2 100644
--- a/morphlib/plugins/show_dependencies_plugin.py
+++ b/morphlib/plugins/show_dependencies_plugin.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2012-2015  Codethink Limited
+# Copyright (C) 2012-2016  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
diff --git a/morphlib/plugins/system_manifests_plugin.py b/morphlib/plugins/system_manifests_plugin.py
index b178e4b2..923a27b0 100644
--- a/morphlib/plugins/system_manifests_plugin.py
+++ b/morphlib/plugins/system_manifests_plugin.py
@@ -133,7 +133,8 @@ class SystemManifestsPlugin(cliapp.Plugin):
         except IndexError:
             trove_id = None
         with morphlib.util.temp_dir(dir=self.app.settings['tempdir']) as td:
-            lorries = get_lorry_repos(td, self.repo_cache, self.app.status, trove_id,
+            lorries = get_lorry_repos(td, self.repo_cache, self.app.status,
+                                      trove_id,
                                       self.app.settings['trove-host'])
             manifest = Manifest(system_artifact.name, td, self.app.status,
                                 self.repo_cache)
@@ -242,7 +243,7 @@ def get_lorry_repos(tempdir, repo_cache, status, trove_id, trove_host):
         baserock_lorrydir = checkout_repo(repo_cache, baserock_lorry_repo,
                                           lorrydir)
         lorries.extend(load_lorries(lorrydir))
-    except morphlib.localrepocache.NoRemote as e:
+    except morphlib.repocache.NoRemote as e:
         status(msg="WARNING: Could not find lorries from git.baserock.org, "
                    "expected to find them on %(trove)s at %(reponame)s",
                trove=trove_host, reponame = e.reponame)
diff --git a/morphlib/repocache.py b/morphlib/repocache.py
index 86002f3b..58e05129 100644
--- a/morphlib/repocache.py
+++ b/morphlib/repocache.py
@@ -128,19 +128,25 @@ class RepoCache(object):
     git_resolve_cache_url parameter enables this feature. Baserock 'Trove'
     systems run 'morph-cache-server' by default.
 
+    The 'custom_fs' parameter takes a PyFilesystem instance, which you can use
+    to override where 'cachedir' is stored. This should probably only be used
+    for testing.
+
     '''
     def __init__(self, cachedir, resolver, tarball_base_url=None,
                  git_resolve_cache_url=None,
                  update_gits=True,
                  runcmd_cb=cliapp.runcmd, status_cb=lambda **kwargs: None,
-                 verbose=False, debug=False):
-        morphlib.util.ensure_directory_exists(cachedir)
+                 verbose=False, debug=False,
+                 custom_fs=None):
+        self.fs = custom_fs or fs.osfs.OSFS('/')
+
+        self.fs.makedir(cachedir, recursive=True, allow_recreate=True)
 
-        self.fs = fs.osfs.OSFS('/')
         self.cachedir = cachedir
         self._resolver = resolver
         if tarball_base_url and not tarball_base_url.endswith('/'):
-            tarball_base_url += '/'  # pragma: no cover
+            tarball_base_url += '/'
         self._tarball_base_url = tarball_base_url
         self._cached_repo_objects = {}
 
@@ -152,7 +158,7 @@ class RepoCache(object):
         self.verbose = verbose
         self.debug = debug
 
-        if git_resolve_cache_url:
+        if git_resolve_cache_url:  # pragma: no cover
             self.remote_cache = RemoteRepoCache(git_resolve_cache_url,
                                                 resolver)
         else:
@@ -197,7 +203,7 @@ class RepoCache(object):
         This method is meant to be overridden by unit tests.
 
         '''
-        return tempfile.mkdtemp(dir=dirname)
+        return tempfile.mkdtemp(dir=self.fs.getsyspath(dirname))
 
     def _escape(self, url):
         '''Escape a URL so it can be used as a basename in a file.'''
@@ -231,7 +237,7 @@ class RepoCache(object):
             self._git(['config', 'remote.origin.mirror', 'true'], cwd=path)
             self._git(['config', 'remote.origin.fetch', '+refs/*:refs/*'],
                       cwd=path)
-        except BaseException as e:  # pragma: no cover
+        except BaseException as e:
             if self.fs.exists(path):
                 self.fs.removedir(path, force=True)
             return False, 'Unable to extract tarball %s: %s' % (
@@ -268,7 +274,7 @@ class RepoCache(object):
             errors.append('Unable to clone from %s to %s: %s' %
                           (repourl, target, e))
             if self.fs.exists(target):
-                self.fs.removedir(target, recursive=True, force=True)
+                self.fs.removedir(target, force=True)
             raise NoRemote(reponame, errors)
 
         self.fs.rename(target, path)
@@ -303,7 +309,7 @@ class RepoCache(object):
             raise UpdateError(self)
 
     def get_updated_repo(self, repo_name,
-                         ref=None, refs=None):  # pragma: no cover
+                         ref=None, refs=None):
         '''Return object representing cached repository.
 
         If all the specified refs in 'ref' or 'refs' point to SHA1s that are
@@ -322,13 +328,15 @@ class RepoCache(object):
 
         if ref is not None and refs is None:
             refs = (ref,)
+        else:
+            refs = list(refs)
 
         if self.has_repo(repo_name):
             repo = self._get_repo(repo_name)
             if refs:
                 required_refs = set(refs)
                 missing_refs = set()
-                for required_ref in required_refs:
+                for required_ref in required_refs:  # pragma: no cover
                     if morphlib.git.is_valid_sha1(required_ref):
                         try:
                             repo.resolve_ref_to_commit(required_ref)
@@ -337,7 +345,7 @@ class RepoCache(object):
                             pass
                     missing_refs.add(required_ref)
 
-                if not missing_refs:
+                if not missing_refs:  # pragma: no cover
                     self.status_cb(
                         msg='Not updating git repository %(repo_name)s '
                             'because it already contains %(sha1s)s',
@@ -345,12 +353,17 @@ class RepoCache(object):
                         sha1s=_word_join_list(tuple(required_refs)))
                     return repo
 
-            self.status_cb(msg='Updating %(repo_name)s', repo_name=repo_name)
+            if ref:
+                ref_str = 'ref %s' % ref
+            else:
+                ref_str = '%i refs' % len(refs)
+            self.status_cb(msg='Updating %(repo_name)s for %(ref_str)s',
+                           repo_name=repo_name, ref_str=ref_str)
             self._update_repo(repo)
             return repo
         else:
             self.status_cb(msg='Cloning %(repo_name)s', repo_name=repo_name)
-            return self._cache_repo(repo_name)
+            return self._get_repo(repo_name)
 
     def ensure_submodules(self, toplevel_repo,
                           toplevel_ref):  # pragma: no cover
@@ -378,7 +391,7 @@ class RepoCache(object):
                 if submod not in done:
                     subs_to_process.append(submod)
 
-    def resolve_ref_to_commit_and_tree(self, repo_name, ref):
+    def resolve_ref_to_commit_and_tree(self, repo_name, ref):  # pragma: no cover
         absref = None
         tree = None
 
@@ -400,38 +413,38 @@ class RepoCache(object):
 
         if absref is None:
             # As a last resort, clone the repo to resolve the ref.
-            repo = self.get_updated_repo(reponame, ref)
+            repo = self.get_updated_repo(repo_name, ref)
             absref = repo.resolve_ref_to_commit(ref)
             tree = repo.resolve_ref_to_tree(absref)
 
         return absref, ref
 
 
-class ResolveRefError(cliapp.AppException):
+class RemoteResolveRefError(cliapp.AppException):
 
     def __init__(self, repo_name, ref):
         cliapp.AppException.__init__(
-            self, 'Failed to resolve ref %s for repo %s' %
+            self, 'Failed to resolve ref %s for repo %s from remote cache' %
             (ref, repo_name))
 
 
-class CatFileError(cliapp.AppException):
+class RemoteCatFileError(cliapp.AppException):  # pragma: no cover
 
     def __init__(self, repo_name, ref, filename):
         cliapp.AppException.__init__(
-            self, 'Failed to cat file %s in ref %s of repo %s' %
-            (filename, ref, repo_name))
+            self, 'Failed to cat file %s in ref %s of repo %s, from remote '
+                  'cache' % (filename, ref, repo_name))
 
 
-class LsTreeError(cliapp.AppException):
+class RemoteLsTreeError(cliapp.AppException):  # pragma: no cover
 
     def __init__(self, repo_name, ref):
         cliapp.AppException.__init__(
-            self, 'Failed to list tree in ref %s of repo %s' %
-            (ref, repo_name))
+            self, 'Failed to list tree in ref %s of repo %s, from remote'
+                  'cache' % (ref, repo_name))
 
 
-class RemoteRepoCache(object):
+class RemoteRepoCache(object):  # pragma: no cover
 
     def __init__(self, server_url, resolver):
         self.server_url = server_url
@@ -443,7 +456,7 @@ class RemoteRepoCache(object):
             return self._resolve_ref_for_repo_url(repo_url, ref)
         except BaseException as e:
             logging.error('Caught exception: %s' % str(e))
-            raise ResolveRefError(repo_name, ref)
+            raise RemoteResolveRefError(repo_name, ref)
 
     def cat_file(self, repo_name, ref, filename):
         repo_url = self._resolver.pull_url(repo_name)
@@ -452,7 +465,7 @@ class RemoteRepoCache(object):
         except urllib2.HTTPError as e:
             logging.error('Caught exception: %s' % str(e))
             if e.code == 404:
-                raise CatFileError(repo_name, ref, filename)
+                raise RemoteCatFileError(repo_name, ref, filename)
             raise  # pragma: no cover
 
     def ls_tree(self, repo_name, ref):
@@ -462,7 +475,7 @@ class RemoteRepoCache(object):
             return info['tree'].keys()
         except BaseException as e:
             logging.error('Caught exception: %s' % str(e))
-            raise LsTreeError(repo_name, ref)
+            raise RemoteLsTreeError(repo_name, ref)
 
     def _resolve_ref_for_repo_url(self, repo_url, ref):  # pragma: no cover
         data = self._make_request(
diff --git a/morphlib/repocache_tests.py b/morphlib/repocache_tests.py
index 91fdb216..32af4591 100644
--- a/morphlib/repocache_tests.py
+++ b/morphlib/repocache_tests.py
@@ -19,54 +19,56 @@ import os
 
 import cliapp
 import fs.memoryfs
+import tempfile
 
 import morphlib
 import morphlib.gitdir_tests
 
 
-class FakeApplication(object):
+class TestableRepoCache(morphlib.repocache.RepoCache):
+    '''Adapts the RepoCache class for unit testing.
 
-    def __init__(self):
-        self.settings = {
-            'debug': True,
-            'verbose': True,
-            'no-git-update': False,
-        }
+    All Git operations are stubbed out. You can track what Git operations have
+    taken place by looking at the 'remotes' dict -- any 'clone' operations will
+    set an entry in there. The 'tarballs_fetched' list tracks what tarballs
+    of Git repos would have been downloaded.
 
-    def status(self, **kwargs):
-        pass
+    There is a single repo alias, 'example' which expands to
+    git://example.com/.
 
+    '''
+    def __init__(self, update_gits=True):
+        aliases = ['example=git://example.com/#example.com:%s.git']
+        repo_resolver = morphlib.repoaliasresolver.RepoAliasResolver(aliases)
+        tarball_base_url = 'http://lorry.example.com/tarballs'
+        cachedir = '/cache/gits/'
+        memoryfs = fs.memoryfs.MemoryFS()
 
-class LocalRepoCacheTests(unittest.TestCase):
+        morphlib.repocache.RepoCache.__init__(
+            self, cachedir, repo_resolver, tarball_base_url=tarball_base_url,
+            custom_fs=memoryfs, update_gits=update_gits)
 
-    def setUp(self):
-        aliases = ['upstream=git://example.com/#example.com:%s.git']
-        repo_resolver = morphlib.repoaliasresolver.RepoAliasResolver(aliases)
-        tarball_base_url = 'http://lorry.example.com/tarballs/'
-        self.reponame = 'upstream:reponame'
-        self.repourl = 'git://example.com/reponame'
-        escaped_url = 'git___example_com_reponame'
-        self.tarball_url = '%s%s.tar' % (tarball_base_url, escaped_url)
-        self.cachedir = '/cache/dir'
-        self.cache_path = '%s/%s' % (self.cachedir, escaped_url)
         self.remotes = {}
-        self.fetched = []
-        self.lrc = morphlib.localrepocache.LocalRepoCache(
-            FakeApplication(), self.cachedir, repo_resolver, tarball_base_url)
-        self.lrc.fs = fs.memoryfs.MemoryFS()
-        self.lrc._git = self.fake_git
-        self.lrc._fetch = self.not_found
-        self.lrc._mkdtemp = self.fake_mkdtemp
-        self.lrc._update_repo = lambda *args: None
+        self.tarballs_fetched = []
+
         self._mkdtemp_count = 0
 
-    def fake_git(self, args, **kwargs):
+    def _mkdtemp(self, dirname):
+        thing = "foo"+str(self._mkdtemp_count)
+        self._mkdtemp_count += 1
+        self.fs.makedir(dirname+"/"+thing)
+        return thing
+
+    def _fetch(self, url, path):
+        self.tarballs_fetched.append(url)
+
+    def _git(self, args, **kwargs):
         if args[0] == 'clone':
-            self.assertEqual(len(args), 5)
+            assert len(args) == 5
             remote = args[3]
             local = args[4]
             self.remotes['origin'] = {'url': remote, 'updates': 0}
-            self.lrc.fs.makedir(local, recursive=True)
+            self.fs.makedir(local, recursive=True)
         elif args[0:2] == ['remote', 'set-url']:
             remote = args[2]
             url = args[3]
@@ -82,68 +84,80 @@ class LocalRepoCacheTests(unittest.TestCase):
         else:
             raise NotImplementedError()
 
-    def fake_mkdtemp(self, dirname):
-        thing = "foo"+str(self._mkdtemp_count)
-        self._mkdtemp_count += 1
-        self.lrc.fs.makedir(dirname+"/"+thing)
-        return thing
-
-    def not_found(self, url, path):
-        raise cliapp.AppException('Not found')
-
-    def test_has_not_got_shortened_repo_initially(self):
-        self.assertFalse(self.lrc.has_repo(self.reponame))
-
-    def test_has_not_got_absolute_repo_initially(self):
-        self.assertFalse(self.lrc.has_repo(self.repourl))
+    def _new_cached_repo_instance(self, *args, **kwargs):
+        with morphlib.gitdir_tests.allow_nonexistant_git_repos():
+            repo = morphlib.cachedrepo.CachedRepo(*args, **kwargs)
+            repo.update = lambda: None
+            return repo
 
-    def test_cachedir_does_not_exist_initially(self):
-        self.assertFalse(self.lrc.fs.exists(self.cachedir))
+class RepoCacheTests(unittest.TestCase):
 
-    def test_creates_cachedir_if_missing(self):
-        with morphlib.gitdir_tests.allow_nonexistant_git_repos():
-            self.lrc.get_updated_repo(self.repourl, ref='master')
-        self.assertTrue(self.lrc.fs.exists(self.cachedir))
+    def test_has_not_got_repo_initially(self):
+        repo_cache = TestableRepoCache()
+        self.assertFalse(repo_cache.has_repo('example:repo'))
+        self.assertFalse(repo_cache.has_repo('git://example.com/repo'))
 
     def test_happily_caches_same_repo_twice(self):
+        repo_cache = TestableRepoCache()
         with morphlib.gitdir_tests.allow_nonexistant_git_repos():
-            self.lrc.get_updated_repo(self.repourl, ref='master')
-            self.lrc.get_updated_repo(self.repourl, ref='master')
+            repo_cache.get_updated_repo('example:repo', ref='master')
+            repo_cache.get_updated_repo('example:repo', ref='master')
 
     def test_fails_to_cache_when_remote_does_not_exist(self):
-        def fail(args, **kwargs):
-            self.lrc.fs.makedir(args[4])
+        repo_cache = TestableRepoCache()
+
+        def clone_fails(args, **kwargs):
+            repo_cache.fs.makedir(args[4])
             raise cliapp.AppException('')
-        self.lrc._git = fail
-        self.assertRaises(morphlib.localrepocache.NoRemote,
-                          self.lrc.get_updated_repo, self.repourl, 'master')
+        repo_cache._git = clone_fails
+
+        with self.assertRaises(morphlib.repocache.NoRemote):
+            repo_cache.get_updated_repo('example:repo', 'master')
 
     def test_does_not_mind_a_missing_tarball(self):
+        repo_cache = TestableRepoCache()
+
+        def no_tarball(*args, **kwargs):
+            raise cliapp.AppException('Not found')
+        repo_cache._fetch = no_tarball
+
         with morphlib.gitdir_tests.allow_nonexistant_git_repos():
-            self.lrc.get_updated_repo(self.repourl, ref='master')
-        self.assertEqual(self.fetched, [])
+            repo_cache.get_updated_repo('example:repo', ref='master')
+        self.assertEqual(repo_cache.tarballs_fetched, [])
 
     def test_fetches_tarball_when_it_exists(self):
-        self.lrc._fetch = lambda url, path: self.fetched.append(url)
+        repo_url = 'git://example.com/reponame'
+        repo_cache = TestableRepoCache()
 
         with morphlib.gitdir_tests.allow_nonexistant_git_repos():
-            self.lrc.get_updated_repo(self.repourl, ref='master')
+            repo_cache.get_updated_repo(repo_url, ref='master')
+
+        tarball_url = '%s%s.tar' % (repo_cache._tarball_base_url,
+                                    repo_cache._escape(repo_url))
+        self.assertEqual(repo_cache.tarballs_fetched, [tarball_url])
 
-        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)
+        # Check that the cache updated the repo after fetching the tarball.
+        self.assertEqual(repo_cache.remotes['origin']['url'], repo_url)
 
     def test_escapes_repourl_as_filename(self):
-        escaped = self.lrc._escape(self.repourl)
+        repo_cache = TestableRepoCache()
+        escaped = repo_cache._escape('git://example.com/reponame')
         self.assertFalse('/' in escaped)
 
     def test_noremote_error_message_contains_repo_name(self):
-        e = morphlib.localrepocache.NoRemote(self.repourl, [])
-        self.assertTrue(self.repourl in str(e))
+        repo_url = 'git://example.com/reponame'
+        e = morphlib.repocache.NoRemote(repo_url, [])
+        self.assertTrue(repo_url in str(e))
 
     def test_avoids_caching_local_repo(self):
-        self.lrc.fs.makedir('/local/repo', recursive=True)
-        with morphlib.gitdir_tests.allow_nonexistant_git_repos():
-            cached = self.lrc.get_updated_repo('file:///local/repo',
-                                               refs='master')
-        assert cached.dirname == '/local/repo'
+        repo_cache = TestableRepoCache()
+
+        repo_cache.fs.makedir('/local/repo', recursive=True)
+        cached = repo_cache.get_updated_repo('file:///local/repo', refs='master')
+        assert cached.path == '/local/repo'
+
+    def test_no_git_update_setting(self):
+        repo_cache = TestableRepoCache(update_gits=False)
+
+        with self.assertRaises(morphlib.repocache.NotCached):
+            repo_cache.get_updated_repo('example:repo', ref='master')
diff --git a/morphlib/sourceresolver.py b/morphlib/sourceresolver.py
index 97697be2..a9acaba0 100644
--- a/morphlib/sourceresolver.py
+++ b/morphlib/sourceresolver.py
@@ -195,7 +195,8 @@ class SourceResolver(object):
 
         logging.debug('tree (%s, %s) not in cache', reponame, ref)
 
-        absref, tree = self.repo_cache.resolve_ref_to_commit_and_tree(ref)
+        absref, tree = self.repo_cache.resolve_ref_to_commit_and_tree(reponame,
+                                                                      ref)
 
         logging.debug('Writing tree to cache with ref (%s, %s)',
                       reponame, absref)