diff options
-rw-r--r-- | morphlib/cachedrepo.py | 38 | ||||
-rw-r--r-- | morphlib/cachedrepo_tests.py | 38 | ||||
-rw-r--r-- | morphlib/git.py | 2 | ||||
-rw-r--r-- | morphlib/plugins/branch_and_merge_plugin.py | 15 | ||||
-rwxr-xr-x | tests.branching/ambiguous-refs.script | 6 | ||||
-rwxr-xr-x | tests/ambiguous-refs.script | 32 | ||||
-rwxr-xr-x | tests/update-gits-chunk.script | 4 |
7 files changed, 79 insertions, 56 deletions
diff --git a/morphlib/cachedrepo.py b/morphlib/cachedrepo.py index 0b5ce60f..0a085bb2 100644 --- a/morphlib/cachedrepo.py +++ b/morphlib/cachedrepo.py @@ -104,6 +104,15 @@ class CachedRepo(object): self.path = path self.is_mirror = not url.startswith('file://') + def ref_exists(self, ref): + '''Returns True if the given ref exists in the repo''' + + try: + self._rev_parse(ref) + except cliapp.AppException: + return False + return True + def resolve_ref(self, ref): '''Attempts to resolve a ref into its SHA1 and tree SHA1. @@ -112,18 +121,10 @@ class CachedRepo(object): ''' - if not morphlib.git.is_valid_sha1(ref): - try: - refs = self._show_ref(ref).split('\n') - refs = [x.split() for x in refs] - absref = refs[0][0] - except cliapp.AppException: - raise InvalidReferenceError(self, ref) - else: - try: - absref = self._rev_list(ref).strip() - except cliapp.AppException: - raise InvalidReferenceError(self, ref) + try: + absref = self._rev_parse(ref) + except cliapp.AppException: + raise InvalidReferenceError(self, ref) tree = self._show_tree_hash(absref) return absref, tree @@ -141,7 +142,7 @@ class CachedRepo(object): if not morphlib.git.is_valid_sha1(ref): raise UnresolvedNamedReferenceError(self, ref) try: - sha1 = self._rev_list(ref).strip() + sha1 = self._rev_parse(ref) except cliapp.AppException: raise InvalidReferenceError(self, ref) @@ -193,7 +194,7 @@ class CachedRepo(object): '''Loads a morphology from a given ref''' if not morphlib.git.is_valid_sha1(ref): - ref = self._rev_list(ref).strip() + ref = self._rev_parse(ref) text = self.cat(ref, '%s.morph' % name) morphology = morphlib.morph2.Morphology(text) return morphology @@ -210,7 +211,7 @@ class CachedRepo(object): if not morphlib.git.is_valid_sha1(ref): raise UnresolvedNamedReferenceError(self, ref) try: - sha1 = self._rev_list(ref).strip() + sha1 = self._rev_parse(ref) except cliapp.AppException: raise InvalidReferenceError(self, ref) @@ -237,16 +238,13 @@ class CachedRepo(object): kwargs['cwd'] = self.path return self.app.runcmd(*args, **kwargs) - def _show_ref(self, ref): # pragma: no cover - return self._runcmd(['git', 'show-ref', ref]) + def _rev_parse(self, ref): # pragma: no cover + return self._runcmd(['git', 'rev-parse', '--verify', ref])[0:40] def _show_tree_hash(self, absref): # pragma: no cover return self._runcmd( ['git', 'log', '-1', '--format=format:%T', absref]).strip() - def _rev_list(self, ref): # pragma: no cover - return self._runcmd(['git', 'rev-list', '--no-walk', ref]) - def _ls_tree(self, ref): # pragma: no cover result = self._runcmd(['git', 'ls-tree', '--name-only', ref]) return result.split('\n') diff --git a/morphlib/cachedrepo_tests.py b/morphlib/cachedrepo_tests.py index 81673880..9251a473 100644 --- a/morphlib/cachedrepo_tests.py +++ b/morphlib/cachedrepo_tests.py @@ -31,19 +31,19 @@ class CachedRepoTests(unittest.TestCase): "kind": "chunk" }''' - def show_ref(self, ref): + def rev_parse(self, ref): output = { - 'master': - 'e28a23812eadf2fce6583b8819b9c5dbd36b9fb9' - ' refs/heads/master', - 'baserock/morph': - '8b780e2e6f102fcf400ff973396566d36d730501' - ' refs/heads/baserock/morph', + 'a4da32f5a81c8bc6d660404724cedc3bc0914a75': + 'a4da32f5a81c8bc6d660404724cedc3bc0914a75', + 'e28a23812eadf2fce6583b8819b9c5dbd36b9fb9': + 'e28a23812eadf2fce6583b8819b9c5dbd36b9fb9', + 'master': 'e28a23812eadf2fce6583b8819b9c5dbd36b9fb9', + 'baserock/morph': '8b780e2e6f102fcf400ff973396566d36d730501' } try: return output[ref] except: - raise cliapp.AppException('git show-ref %s' % ref) + raise cliapp.AppException('git rev-parse --verify %s' % ref) def show_tree_hash(self, absref): output = { @@ -60,19 +60,6 @@ class CachedRepoTests(unittest.TestCase): raise cliapp.AppException('git log -1 --format=format:%%T %s' % absref) - def rev_list(self, ref): - output = { - 'master': 'e28a23812eadf2fce6583b8819b9c5dbd36b9fb9', - 'e28a23812eadf2fce6583b8819b9c5dbd36b9fb9': - 'e28a23812eadf2fce6583b8819b9c5dbd36b9fb9', - 'a4da32f5a81c8bc6d660404724cedc3bc0914a75': - 'a4da32f5a81c8bc6d660404724cedc3bc0914a75', - } - try: - return output[ref] - except: - raise cliapp.AppException('git rev-list %s' % ref) - def cat_file(self, ref, filename): output = { 'e28a23812eadf2fce6583b8819b9c5dbd36b9fb9:foo.morph': @@ -125,9 +112,8 @@ class CachedRepoTests(unittest.TestCase): self.repo_path = '/tmp/foo' self.repo = cachedrepo.CachedRepo( object(), self.repo_name, self.repo_url, self.repo_path) - self.repo._show_ref = self.show_ref + self.repo._rev_parse = self.rev_parse self.repo._show_tree_hash = self.show_tree_hash - self.repo._rev_list = self.rev_list self.repo._cat_file = self.cat_file self.repo._copy_repository = self.copy_repository self.repo._checkout_ref = self.checkout_ref @@ -143,6 +129,12 @@ class CachedRepoTests(unittest.TestCase): self.assertEqual(self.repo.url, self.repo_url) self.assertEqual(self.repo.path, self.repo_path) + def test_ref_exists(self): + self.assertEqual(self.repo.ref_exists('master'), True) + + def test_ref_does_not_exist(self): + self.assertEqual(self.repo.ref_exists('non-existant-ref'), False) + def test_resolve_named_ref_master(self): sha1, tree = self.repo.resolve_ref('master') self.assertEqual(sha1, 'e28a23812eadf2fce6583b8819b9c5dbd36b9fb9') diff --git a/morphlib/git.py b/morphlib/git.py index a37b6675..9ac9e292 100644 --- a/morphlib/git.py +++ b/morphlib/git.py @@ -269,4 +269,4 @@ def is_valid_sha1(ref): def rev_parse(runcmd, gitdir, ref): '''Find the sha1 for the given ref''' - return runcmd(['git', 'rev-parse', ref], cwd=gitdir)[0:40] + return runcmd(['git', 'rev-parse', '--verify', ref], cwd=gitdir)[0:40] diff --git a/morphlib/plugins/branch_and_merge_plugin.py b/morphlib/plugins/branch_and_merge_plugin.py index a65b28ed..0c6209da 100644 --- a/morphlib/plugins/branch_and_merge_plugin.py +++ b/morphlib/plugins/branch_and_merge_plugin.py @@ -195,8 +195,8 @@ class BranchAndMergePlugin(cliapp.Plugin): def resolve_ref(self, repodir, ref): try: - return self.app.runcmd(['git', 'show-ref', ref], - cwd=repodir).split()[0] + return self.app.runcmd(['git', 'rev-parse', '--verify', ref], + cwd=repodir)[0:40] except: return None @@ -517,11 +517,15 @@ class BranchAndMergePlugin(cliapp.Plugin): new_branch = args[1] commit = 'master' if len(args) == 2 else args[2] + self.lrc, self.rrc = morphlib.util.new_repo_caches(self.app) + if self.lrc.get_repo(repo).ref_exists(new_branch): + raise cliapp.AppException('branch %s already exists in ' + 'repository %s' % (new_branch, repo)) + # Create the system branch directory. workspace = self.deduce_workspace() branch_dir = os.path.join(workspace, new_branch) os.makedirs(branch_dir) - self.lrc, self.rrc = morphlib.util.new_repo_caches(self.app) try: # Create a .morph-system-branch directory to clearly identify @@ -541,11 +545,6 @@ class BranchAndMergePlugin(cliapp.Plugin): repo_dir = os.path.join(branch_dir, self.convert_uri_to_path(repo)) self.clone_to_directory(repo_dir, repo, commit) - # Check if branch already exists locally or in a remote - if self.resolve_ref(repo_dir, new_branch) is not None: - raise cliapp.AppException('branch %s already exists in ' - 'repository %s' % (new_branch, repo)) - # Create a new branch in the local morphs repository. self.app.runcmd(['git', 'checkout', '-b', new_branch, commit], cwd=repo_dir) diff --git a/tests.branching/ambiguous-refs.script b/tests.branching/ambiguous-refs.script index a93c4b66..0ea8a55f 100755 --- a/tests.branching/ambiguous-refs.script +++ b/tests.branching/ambiguous-refs.script @@ -16,8 +16,10 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -## 'git show-ref master' sorts its output alphabetically, so we can't rely on -## it to resolve 'master': we might get 'alpha/master' instead. +## Guard against a bug that occurs if 'git show-ref' is used to resolve refs +## instead of 'git rev-parse --verify': show-ref returns a list of partial +## matches sorted alphabetically, so any code using it may resolve refs +## incorrectly. set -eu diff --git a/tests/ambiguous-refs.script b/tests/ambiguous-refs.script new file mode 100755 index 00000000..58d09d26 --- /dev/null +++ b/tests/ambiguous-refs.script @@ -0,0 +1,32 @@ +#!/bin/sh +# +# Copyright (C) 2011, 2012 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 +# the Free Software Foundation; version 2 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + + +## Guard against a bug that occurs if 'git show-ref' is used to resolve refs +## instead of 'git rev-parse --verify': show-ref returns a list of partial +## matches sorted alphabetically, so any code using it may resolve refs + +set -eu + +# Create a ref that will show up in 'git show-ref' before the real master ref +cd "$DATADIR/morphs-repo" +git checkout -q -b alpha/master +git rm -q hello-stratum.morph +git commit -q -m "This ref will not build correctly" + +"$SRCDIR/scripts/test-morph" build-morphology \ + test:morphs-repo master hello-stratum diff --git a/tests/update-gits-chunk.script b/tests/update-gits-chunk.script index c2ad08d0..288438ec 100755 --- a/tests/update-gits-chunk.script +++ b/tests/update-gits-chunk.script @@ -31,7 +31,7 @@ REPO_ALIAS='test=git://localhost:9419/%s#git://localhost:9419/%s' cd "$DATADIR/chunk-repo" git checkout --quiet farrokh git commit --quiet --allow-empty --allow-empty-message -m "" -NEWREF="$(git show-ref --hash farrokh)" +NEWREF="$(git rev-parse --verify farrokh)" "$SRCDIR/scripts/test-morph" --repo-alias=$REPO_ALIAS update-gits \ test:chunk-repo farrokh hello @@ -40,4 +40,4 @@ kill $(cat "$DATADIR/git.pid") # Check the top commit of the cached repo's farrokh branch cd "$DATADIR/cache/gits/"*chunk?repo* -test "$(git show-ref --hash refs/heads/farrokh)" = "$NEWREF" +test "$(git rev-parse --verify refs/heads/farrokh)" = "$NEWREF" |