summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSam Thursfield <sam.thursfield@codethink.co.uk>2014-06-30 13:47:06 +0000
committerSam Thursfield <sam.thursfield@codethink.co.uk>2014-07-07 15:06:22 +0100
commitc029130068e3a08c51dd551fda88cc5302671d53 (patch)
tree185e204a874c085b625d2ca536593ceba3f26833
parent770a6cb434ac31238eb2eee526e235728ce07aff (diff)
downloadmorph-c029130068e3a08c51dd551fda88cc5302671d53.tar.gz
Fix Morph failing to update some cached git repos
I was getting the following error when running the 'do-release.py' script: ERROR:root:Command failed: morph --quiet --trove-host=git.baserock.org list-artifacts baserock:baserock/definitions sam/auto-release minimal-system-x86_32-generic ERROR: Ref d67a0e110187abd560a1de63fa172894a52839d5 is an invalid reference for repo git://git.baserock.org/delta/linux The commit that it wants did actually exist in git.baserock.org and the logs show that Morph hadn't done a `git remote update` to get the commit locally. This turned out to be because it'd had already looked up a different ref in linux.git. It hadn't needed to run 'git remote update', but it *had* added linux.git to a "don't need to update this repo again" list. Oops! I've moved the code in question to the cachedrepo module so that the repo object keeps that of whether it should be updated. The bug is now fixed. As a side effect this patch fixes the spurious 'Updating repo file:///...' messages, which were printed even though repos with file:/// URLs are never updated.
-rw-r--r--morphlib/app.py13
-rw-r--r--morphlib/cachedrepo.py28
-rw-r--r--morphlib/cachedrepo_tests.py18
3 files changed, 46 insertions, 13 deletions
diff --git a/morphlib/app.py b/morphlib/app.py
index 07e6348f..df8c360a 100644
--- a/morphlib/app.py
+++ b/morphlib/app.py
@@ -306,14 +306,9 @@ class Morph(cliapp.Application):
'''
absref = None
- def cached_repo_requires_update_for_ref(repo, ref):
- # Named refs that are valid SHA1s will confuse this code.
- ref_can_change = not morphlib.git.is_valid_sha1(ref)
- return (ref_can_change or not repo.ref_exists(ref))
-
if lrc.has_repo(reponame):
repo = lrc.get_repo(reponame)
- if update and cached_repo_requires_update_for_ref(repo, ref):
+ if 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()
@@ -347,23 +342,19 @@ class Morph(cliapp.Application):
morph_factory = morphlib.morphologyfactory.MorphologyFactory(lrc, rrc,
self)
queue = collections.deque(triplets)
- updated_repos = set()
resolved_refs = {}
resolved_morphologies = {}
while queue:
reponame, ref, filename = queue.popleft()
- update_repo = update and reponame not in updated_repos
# Resolve the (repo, ref) reference, cache result.
reference = (reponame, ref)
if not reference in resolved_refs:
resolved_refs[reference] = self.resolve_ref(
- lrc, rrc, reponame, ref, update_repo)
+ lrc, rrc, reponame, ref, update)
absref, tree = resolved_refs[reference]
- updated_repos.add(reponame)
-
# Fetch the (repo, ref, filename) morphology, cache result.
reference = (reponame, absref, filename)
if not reference in resolved_morphologies:
diff --git a/morphlib/cachedrepo.py b/morphlib/cachedrepo.py
index e85b0059..996b42f7 100644
--- a/morphlib/cachedrepo.py
+++ b/morphlib/cachedrepo.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2012-2013 Codethink Limited
+# Copyright (C) 2012-2014 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
@@ -103,6 +103,7 @@ class CachedRepo(object):
self.url = url
self.path = path
self.is_mirror = not url.startswith('file://')
+ self.already_updated = False
def ref_exists(self, ref):
'''Returns True if the given ref exists in the repo'''
@@ -221,6 +222,30 @@ class CachedRepo(object):
return self._ls_tree(sha1)
+ def requires_update_for_ref(self, ref):
+ '''Returns False if there's no need to update this cached repo.
+
+ If the ref points to a specific commit that's already available
+ locally, there's never any need to update. If it's a named ref and this
+ repo wasn't already updated in the lifetime of the current process,
+ it's necessary to update.
+
+ '''
+ if not self.is_mirror:
+ # Repos with file:/// URLs don't ever need updating.
+ return False
+
+ if self.already_updated:
+ return False
+
+ # Named refs that are valid SHA1s will confuse this code.
+ ref_can_change = not morphlib.git.is_valid_sha1(ref)
+
+ if ref_can_change or not self.ref_exists(ref):
+ return True
+ else:
+ return False
+
def update(self):
'''Updates the cached repository using its origin remote.
@@ -234,6 +259,7 @@ class CachedRepo(object):
try:
self._update()
+ self.already_updated = True
except cliapp.AppException, e:
raise UpdateError(self)
diff --git a/morphlib/cachedrepo_tests.py b/morphlib/cachedrepo_tests.py
index 8afe2063..74c16591 100644
--- a/morphlib/cachedrepo_tests.py
+++ b/morphlib/cachedrepo_tests.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2012-2013 Codethink Limited
+# Copyright (C) 2012-2014 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
@@ -31,6 +31,7 @@ class CachedRepoTests(unittest.TestCase):
"kind": "chunk"
}'''
+ known_commit = 'a4da32f5a81c8bc6d660404724cedc3bc0914a75'
bad_sha1_known_to_rev_parse = 'cafecafecafecafecafecafecafecafecafecafe'
def rev_parse(self, ref):
@@ -247,9 +248,24 @@ class CachedRepoTests(unittest.TestCase):
self.repo = morphlib.cachedrepo.CachedRepo(
object(), 'local:repo', 'file:///local/repo/', '/local/repo/')
self.repo._update = self.update_with_failure
+ self.assertFalse(self.repo.requires_update_for_ref(self.known_commit))
self.repo.update()
def test_clone_checkout(self):
self.repo.clone_checkout('master', '/.DOES_NOT_EXIST')
self.assertEqual(self.clone_target, '/.DOES_NOT_EXIST')
self.assertEqual(self.clone_ref, 'master')
+
+ def test_no_need_to_update_repo_for_existing_sha1(self):
+ # If the SHA1 is present locally already there's no need to update.
+ # If it's a named ref then it might have changed in the remote, so we
+ # must still update.
+ self.assertFalse(self.repo.requires_update_for_ref(self.known_commit))
+ self.assertTrue(self.repo.requires_update_for_ref('named_ref'))
+
+ def test_no_need_to_update_repo_if_already_updated(self):
+ self.repo._update = self.update_successfully
+
+ self.assertTrue(self.repo.requires_update_for_ref('named_ref'))
+ self.repo.update()
+ self.assertFalse(self.repo.requires_update_for_ref('named_ref'))