From e8a67a7d12d2defbf975d707e7513837403d93a2 Mon Sep 17 00:00:00 2001 From: Sam Thursfield Date: Thu, 3 Mar 2016 10:35:52 +0000 Subject: Hide get_repo() and cache_repo() functions, always use get_updated_repo() This allows us to simplify a couple of places. I cannot think of a single situation where'd you want to get the cached copy of a repo, but not update it. Think about it -- the repo might be *years* behind the upstream remote. Change-Id: I60340c7fb33e7bfe871ad30c0a9322a7202548e2 --- morphlib/builder.py | 2 +- morphlib/localrepocache.py | 18 +++++----- morphlib/localrepocache_tests.py | 49 ++++++++------------------ morphlib/plugins/anchor_plugin.py | 3 -- morphlib/plugins/artifact_inspection_plugin.py | 8 ++--- morphlib/plugins/certify_plugin.py | 8 ++--- morphlib/plugins/system_manifests_plugin.py | 12 ++----- morphlib/sourceresolver.py | 6 +--- 8 files changed, 33 insertions(+), 73 deletions(-) diff --git a/morphlib/builder.py b/morphlib/builder.py index d8050637..6a5ad184 100644 --- a/morphlib/builder.py +++ b/morphlib/builder.py @@ -53,7 +53,7 @@ def extract_sources(app, repo_cache, repo, sha1, srcdir): #pragma: no cover else: tuples = [] for sub in submodules: - cached_repo = repo_cache.get_repo(sub.url) + cached_repo = repo_cache.get_updated_repo(sub.url, sub.commit) sub_dir = os.path.join(destdir, sub.path) tuples.append((cached_repo, sub.commit, sub_dir)) return tuples diff --git a/morphlib/localrepocache.py b/morphlib/localrepocache.py index 26c516ce..a7c5d1ec 100644 --- a/morphlib/localrepocache.py +++ b/morphlib/localrepocache.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 @@ -181,7 +181,7 @@ class LocalRepoCache(object): return True, None - def cache_repo(self, reponame): + def _cache_repo(self, reponame): '''Clone the given repo into the cache. If the repo is already cloned, do nothing. @@ -192,7 +192,7 @@ class LocalRepoCache(object): self.fs.makedir(self._cachedir, recursive=True) try: - return self.get_repo(reponame) + return self._get_repo(reponame) except NotCached as e: pass @@ -201,7 +201,7 @@ class LocalRepoCache(object): if self._tarball_base_url: ok, error = self._clone_with_tarball(repourl, path) if ok: - repo = self.get_repo(reponame) + repo = self._get_repo(reponame) repo.update() return repo else: @@ -223,7 +223,7 @@ class LocalRepoCache(object): self.fs.rename(target, path) - repo = self.get_repo(reponame) + repo = self._get_repo(reponame) repo.already_updated = True return repo @@ -232,7 +232,7 @@ class LocalRepoCache(object): return morphlib.cachedrepo.CachedRepo( self._app, reponame, repourl, path) - def get_repo(self, reponame): + def _get_repo(self, reponame): '''Return an object representing a cached repository.''' if reponame in self._cached_repo_objects: @@ -262,13 +262,13 @@ class LocalRepoCache(object): 'because of no-git-update being set', chatty=True, repo_name=repo_name) - return self.get_repo(repo_name) + return self._get_repo(repo_name) if ref is not None and refs is None: refs = (ref,) if self.has_repo(repo_name): - repo = self.get_repo(repo_name) + repo = self._get_repo(repo_name) if refs: required_refs = set(refs) missing_refs = set() @@ -296,7 +296,7 @@ class LocalRepoCache(object): else: self._app.status(msg='Cloning %(repo_name)s', repo_name=repo_name) - return self.cache_repo(repo_name) + return self._cache_repo(repo_name) def ensure_submodules(self, toplevel_repo, toplevel_ref): # pragma: no cover diff --git a/morphlib/localrepocache_tests.py b/morphlib/localrepocache_tests.py index c4ccdbab..ea56bdf2 100644 --- a/morphlib/localrepocache_tests.py +++ b/morphlib/localrepocache_tests.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 @@ -29,10 +29,11 @@ class FakeApplication(object): def __init__(self): self.settings = { 'debug': True, - 'verbose': True + 'verbose': True, + 'no-git-update': False, } - def status(self, msg): + def status(self, **kwargs): pass @@ -89,8 +90,10 @@ class LocalRepoCacheTests(unittest.TestCase): def new_cached_repo_instance(self, *args): with morphlib.gitdir_tests.allow_nonexistant_git_repos(): - return morphlib.cachedrepo.CachedRepo( + repo = morphlib.cachedrepo.CachedRepo( FakeApplication(), *args) + repo.update = lambda: None + return repo def not_found(self, url, path): raise cliapp.AppException('Not found') @@ -101,26 +104,16 @@ class LocalRepoCacheTests(unittest.TestCase): def test_has_not_got_absolute_repo_initially(self): self.assertFalse(self.lrc.has_repo(self.repourl)) - def test_caches_shortened_repository_on_request(self): - self.lrc.cache_repo(self.reponame) - self.assertTrue(self.lrc.has_repo(self.reponame)) - self.assertTrue(self.lrc.has_repo(self.repourl)) - - def test_caches_absolute_repository_on_request(self): - self.lrc.cache_repo(self.repourl) - self.assertTrue(self.lrc.has_repo(self.reponame)) - self.assertTrue(self.lrc.has_repo(self.repourl)) - def test_cachedir_does_not_exist_initially(self): self.assertFalse(self.lrc.fs.exists(self.cachedir)) def test_creates_cachedir_if_missing(self): - self.lrc.cache_repo(self.repourl) + self.lrc.get_updated_repo(self.repourl, ref='master') self.assertTrue(self.lrc.fs.exists(self.cachedir)) def test_happily_caches_same_repo_twice(self): - self.lrc.cache_repo(self.repourl) - self.lrc.cache_repo(self.repourl) + self.lrc.get_updated_repo(self.repourl, ref='master') + self.lrc.get_updated_repo(self.repourl, ref='master') def test_fails_to_cache_when_remote_does_not_exist(self): def fail(args, **kwargs): @@ -128,10 +121,10 @@ class LocalRepoCacheTests(unittest.TestCase): raise cliapp.AppException('') self.lrc._git = fail self.assertRaises(morphlib.localrepocache.NoRemote, - self.lrc.cache_repo, self.repourl) + self.lrc.get_updated_repo, self.repourl, 'master') def test_does_not_mind_a_missing_tarball(self): - self.lrc.cache_repo(self.repourl) + self.lrc.get_updated_repo(self.repourl, ref='master') self.assertEqual(self.fetched, []) def test_fetches_tarball_when_it_exists(self): @@ -139,25 +132,12 @@ class LocalRepoCacheTests(unittest.TestCase): with morphlib.gitdir_tests.monkeypatch( morphlib.cachedrepo.CachedRepo, 'update', lambda self: None): - self.lrc.cache_repo(self.repourl) + self.lrc.get_updated_repo(self.repourl, ref='master') 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) - def test_gets_cached_shortened_repo(self): - self.lrc.cache_repo(self.reponame) - cached = self.lrc.get_repo(self.reponame) - self.assertTrue(cached is not None) - - def test_gets_cached_absolute_repo(self): - self.lrc.cache_repo(self.repourl) - cached = self.lrc.get_repo(self.repourl) - self.assertTrue(cached is not None) - - def test_get_repo_raises_exception_if_repo_is_not_cached(self): - self.assertRaises(Exception, self.lrc.get_repo, self.repourl) - def test_escapes_repourl_as_filename(self): escaped = self.lrc._escape(self.repourl) self.assertFalse('/' in escaped) @@ -168,6 +148,5 @@ class LocalRepoCacheTests(unittest.TestCase): def test_avoids_caching_local_repo(self): self.lrc.fs.makedir('/local/repo', recursive=True) - self.lrc.cache_repo('file:///local/repo') - cached = self.lrc.get_repo('file:///local/repo') + cached = self.lrc.get_updated_repo('file:///local/repo', refs='master') assert cached.path == '/local/repo' diff --git a/morphlib/plugins/anchor_plugin.py b/morphlib/plugins/anchor_plugin.py index 40cd4c48..62c66c15 100644 --- a/morphlib/plugins/anchor_plugin.py +++ b/morphlib/plugins/anchor_plugin.py @@ -137,9 +137,6 @@ class AnchorPlugin(cliapp.Plugin): for reponame, sources in sources_by_reponame.iteritems(): # UGLY HACK we need to push *FROM* our local repo cache to # avoid cloning everything multiple times. - # This uses get_updated_repo rather than get_repo because the - # BuildCommand.create_source_pool won't cache the repositories - # locally if it can use a remote cache instead. repo = bc.lrc.get_updated_repo(reponame, refs=(s.original_ref for s in sources)) diff --git a/morphlib/plugins/artifact_inspection_plugin.py b/morphlib/plugins/artifact_inspection_plugin.py index fc433a01..413a0072 100644 --- a/morphlib/plugins/artifact_inspection_plugin.py +++ b/morphlib/plugins/artifact_inspection_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 @@ -46,7 +46,7 @@ class ProjectVersionGuesser(object): filenames = [x for x in self.interesting_files if x in tree] if filenames: if self.lrc.has_repo(repo): - repository = self.lrc.get_repo(repo) + repository = self.lrc.get_updated_repo(repo, ref) for filename in filenames: yield filename, repository.read_file(filename, ref) elif self.rrc: @@ -147,9 +147,7 @@ class VersionGuesser(object): version = None try: if self.lrc.has_repo(repo): - repository = self.lrc.get_repo(repo) - if not self.app.settings['no-git-update']: - repository.update() + repository = self.lrc.get_updated_repo(repo, ref) tree = repository.list_files(ref=ref, recurse=False) elif self.rrc: repository = None diff --git a/morphlib/plugins/certify_plugin.py b/morphlib/plugins/certify_plugin.py index 8228be4d..735d0332 100644 --- a/morphlib/plugins/certify_plugin.py +++ b/morphlib/plugins/certify_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 @@ -115,11 +115,7 @@ class CertifyPlugin(cliapp.Plugin): .format(name, ref)) certified = False - # Ensure we have a cache of the repo - if not self.lrc.has_repo(source.repo_name): - self.lrc.cache_repo(source.repo_name) - - cached = self.lrc.get_repo(source.repo_name) + cached = self.lrc.get_updated_repo(source.repo_name, ref) # Test that sha1 ref is anchored in a tag or branch, # and thus not a candidate for removal on `git gc`. diff --git a/morphlib/plugins/system_manifests_plugin.py b/morphlib/plugins/system_manifests_plugin.py index 8e14d2eb..4444ecb3 100644 --- a/morphlib/plugins/system_manifests_plugin.py +++ b/morphlib/plugins/system_manifests_plugin.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 @@ -150,11 +150,7 @@ class SystemManifestsPlugin(cliapp.Plugin): name = source.morphology['name'] ref = source.original_ref - # Ensure we have a cache of the repo - if not self.lrc.has_repo(source.repo_name): - self.lrc.cache_repo(source.repo_name) - - cached = self.lrc.get_repo(source.repo_name) + cached = self.lrc.get_updated_repo(source.repo_name, ref) new_prefix = '[%d/%d][%s] ' % (i, len(sources), name) self.app.status_prefix = old_prefix + new_prefix @@ -174,9 +170,7 @@ def run_licensecheck(filename): return output[len(filename) + 2:].strip() def checkout_repo(lrc, repo, dest, ref='master'): - if not lrc.has_repo(repo): - lrc.cache_repo(repo) - cached = lrc.get_repo(repo) + cached = lrc.get_updated_repo(repo, ref) if not os.path.exists(dest): cached.checkout(ref, dest) diff --git a/morphlib/sourceresolver.py b/morphlib/sourceresolver.py index 204fdb7d..c6f77cf9 100644 --- a/morphlib/sourceresolver.py +++ b/morphlib/sourceresolver.py @@ -200,11 +200,7 @@ class SourceResolver(object): absref = None if self.lrc.has_repo(reponame): - repo = self.lrc.get_repo(reponame) - if self.update and repo.requires_update_for_ref(ref): - self.status(msg='Updating cached git repository %(reponame)s ' - 'for ref %(ref)s', reponame=reponame, ref=ref) - repo.update() + repo = self.lrc.get_updated_repo(reponame, ref) # If the user passed --no-git-update, and the ref is a SHA1 not # available locally, this call will raise an exception. absref = repo.resolve_ref_to_commit(ref) -- cgit v1.2.1