Merge lp:~jelmer/brz-git/remote-test-improvements into lp:brz-git

[Jelmer Vernooij]

Fix remote branch operations and add more tests.

Fix remote branch operations and add more tests.

[Jelmer Vernooij (jelmer)]

Rubberstamp! Proposer approves of own proposal.

review: Approve

=== modified file 'branch.py'
--- branch.py	2018-03-21 03:06:49 +0000
+++ branch.py	2018-03-22 01:32:19 +0000
@@ -1081,7 +1081,7 @@
             stop_revno = self.source.revision_id_to_revno(stop_revision)
         if type(stop_revision) is not str:
             raise TypeError(stop_revision)
-        main_ref = self.target.ref or "refs/heads/master"
+        main_ref = self.target.ref
         refs = { main_ref: (None, stop_revision) }
         if fetch_tags is None:
             c = self.source.get_config_stack()

=== modified file 'commands.py'
--- commands.py	2018-03-17 19:20:55 +0000
+++ commands.py	2018-03-22 01:32:19 +0000
@@ -124,16 +124,15 @@
         interrepo = InterRepository.get(source_repo, target_repo)
         mapping = source_repo.get_mapping()
         refs = interrepo.fetch()
-        refs_dict = refs.as_dict()
         pb = ui.ui_factory.nested_progress_bar()
         try:
-            for i, (name, sha) in enumerate(refs_dict.iteritems()):
+            for i, (name, sha) in enumerate(refs.iteritems()):
                 try:
                     branch_name = ref_to_branch_name(name)
                 except ValueError:
                     # Not a branch, ignore
                     continue
-                pb.update(gettext("creating branches"), i, len(refs_dict))
+                pb.update(gettext("creating branches"), i, len(refs))
                 if getattr(target_controldir._format, "colocated_branches", False) and colocated:
                     if name == "HEAD":
                         branch_name = None

=== modified file 'dir.py'
--- dir.py	2018-03-20 21:05:19 +0000
+++ dir.py	2018-03-22 01:32:19 +0000
@@ -261,6 +261,23 @@
         """
         return UseExistingRepository(self.find_repository())
 
+    def get_branches(self):
+        from .refs import ref_to_branch_name
+        ret = {}
+        for ref in self.get_refs_container().keys():
+            try:
+                branch_name = ref_to_branch_name(ref)
+            except ValueError:
+                continue
+            except UnicodeDecodeError:
+                trace.warning("Ignoring branch %r with unicode error ref", ref)
+                continue
+            ret[branch_name] = self.open_branch(ref=ref)
+        return ret
+
+    def list_branches(self):
+        return self.get_branches().values()
+
 
 class LocalGitControlDirFormat(GitControlDirFormat):
     """The .git directory control format."""
@@ -513,23 +530,6 @@
     def needs_format_conversion(self, format=None):
         return not isinstance(self._format, format.__class__)
 
-    def list_branches(self):
-        return self.get_branches().values()
-
-    def get_branches(self):
-        from .refs import ref_to_branch_name
-        ret = {}
-        for ref in self._git.refs.keys():
-            try:
-                branch_name = ref_to_branch_name(ref)
-            except ValueError:
-                continue
-            except UnicodeDecodeError:
-                trace.warning("Ignoring branch %r with unicode error ref", ref)
-                continue
-            ret[branch_name] = self.open_branch(ref=ref)
-        return ret
-
     def open_repository(self):
         """'open' a repository for this dir."""
         if self.control_transport.has('commondir'):

=== modified file 'fetch.py'
--- fetch.py	2018-03-21 03:06:49 +0000
+++ fetch.py	2018-03-22 01:32:19 +0000
@@ -602,7 +602,7 @@
 
     def determine_wants_all(self, refs):
         potential = set()
-        for k, v in refs.as_dict().iteritems():
+        for k, v in refs.iteritems():
             # For non-git target repositories, only worry about peeled
             if v == ZERO_SHA:
                 continue
@@ -614,7 +614,7 @@
         def determine_wants(refs):
             potential = set(wants)
             if include_tags:
-                for k, unpeeled in refs.as_dict().iteritems():
+                for k, unpeeled in refs.iteritems():
                     if not is_tag(k):
                         continue
                     if unpeeled == ZERO_SHA:
@@ -761,7 +761,7 @@
     def fetch_objects(self, determine_wants, mapping, limit=None, lossy=False):
         """See `InterGitNonGitRepository`."""
         self._warn_slow()
-        remote_refs = self.source.controldir.get_refs_container()
+        remote_refs = self.source.controldir.get_refs_container().as_dict()
         wants = determine_wants(remote_refs)
         create_pb = None
         pb = ui.ui_factory.nested_progress_bar()

=== modified file 'object_store.py'
--- object_store.py	2018-03-21 03:06:49 +0000
+++ object_store.py	2018-03-22 01:32:19 +0000
@@ -29,6 +29,9 @@
 from dulwich.object_store import (
     BaseObjectStore,
     )
+from dulwich.pack import (
+    pack_objects_to_data,
+    )
 
 from ... import (
     errors,
@@ -705,10 +708,11 @@
         else:
             raise KeyError(sha)
 
-    def generate_lossy_pack_contents(self, have, want, progress=None,
-            get_tagged=None):
-        return self.generate_pack_contents(have, want, progress, get_tagged,
-            lossy=True)
+    def generate_lossy_pack_data(self, have, want, progress=None,
+            get_tagged=None, ofs_delta=False):
+        return pack_objects_to_data(
+                self.generate_pack_contents(have, want, progress, get_tagged,
+            lossy=True))
 
     def generate_pack_contents(self, have, want, progress=None,
             ofs_delta=False, get_tagged=None, lossy=False):

=== modified file 'push.py'
--- push.py	2018-03-21 03:06:49 +0000
+++ push.py	2018-03-22 01:32:19 +0000
@@ -360,7 +360,7 @@
         self._warn_slow()
         with self.source_store.lock_read():
             new_refs = self.target.send_pack(determine_wants,
-                    self.source_store.generate_lossy_pack_contents)
+                    self.source_store.generate_lossy_pack_data)
         # FIXME: revidmap?
         return revidmap, self.old_refs, self.new_refs
 

=== modified file 'refs.py'
--- refs.py	2018-03-17 17:27:39 +0000
+++ refs.py	2018-03-22 01:32:19 +0000
@@ -25,6 +25,7 @@
 from ... import (
     errors,
     osutils,
+    revision as _mod_revision,
     )
 
 is_tag = lambda x: x.startswith("refs/tags/")
@@ -97,6 +98,9 @@
         self.dir = dir
         self.object_store = object_store
 
+    def get_packed_refs(self):
+        return {}
+
     def set_symbolic_ref(self, name, other):
         if name == "HEAD":
             pass # FIXME: Switch default branch
@@ -132,8 +136,11 @@
             revid = self._get_revid_by_tag_name(tag_name)
         else:
             revid = self._get_revid_by_branch_name(branch_name)
+        if revid == _mod_revision.NULL_REVISION:
+            return None
         # FIXME: Unpeel if necessary
-        return self.object_store._lookup_revision_sha1(revid)
+        with self.object_store.lock_read():
+            return self.object_store._lookup_revision_sha1(revid)
 
     def get_peeled(self, ref):
         return self.read_loose_ref(ref)

=== modified file 'remote.py'
--- remote.py	2018-03-21 03:46:30 +0000
+++ remote.py	2018-03-22 01:32:19 +0000
@@ -31,6 +31,7 @@
     InvalidRevisionId,
     NoSuchFile,
     NoSuchRevision,
+    NoSuchTag,
     NotBranchError,
     NotLocalUrl,
     UninitializableFormat,
@@ -68,6 +69,7 @@
     branch_name_to_ref,
     is_peeled,
     ref_to_tag_name,
+    tag_name_to_ref,
     )
 
 import dulwich
@@ -77,6 +79,7 @@
     )
 from dulwich.pack import (
     Pack,
+    pack_objects_to_data,
     )
 from dulwich.repo import DictRefsContainer
 import os
@@ -295,31 +298,28 @@
             if result.refs is None:
                 result.refs = {}
             self._refs = remote_refs_dict_to_container(result.refs)
-            return result.refs
+            return result
         except GitProtocolError, e:
             raise parse_git_error(self.transport.external_url(), e)
 
-    def send_pack(self, get_changed_refs, generate_pack_contents):
+    def send_pack(self, get_changed_refs, generate_pack_data):
         try:
             return self._client.send_pack(self._client_path, get_changed_refs,
-                generate_pack_contents)
+                generate_pack_data)
         except GitProtocolError, e:
             raise parse_git_error(self.transport.external_url(), e)
 
-    def _get_default_ref(self):
-        return "refs/heads/master"
-
     def destroy_branch(self, name=None):
         refname = self._get_selected_ref(name)
         def get_changed_refs(old_refs):
             ret = dict(old_refs)
             if not refname in ret:
                 raise NotBranchError(self.user_url)
-            ret[refname] = "00" * 20
+            ret[refname] = dulwich.client.ZERO_SHA
             return ret
-        def generate_pack_contents(have, want, ofs_delta=False):
-            return []
-        self.send_pack(get_changed_refs, generate_pack_contents)
+        def generate_pack_data(have, want, ofs_delta=False):
+            return pack_objects_to_data([])
+        self.send_pack(get_changed_refs, generate_pack_data)
 
     @property
     def user_url(self):
@@ -486,8 +486,8 @@
         return self.controldir.fetch_pack(determine_wants, graph_walker,
                                           pack_data, progress)
 
-    def send_pack(self, get_changed_refs, generate_pack_contents):
-        return self.controldir.send_pack(get_changed_refs, generate_pack_contents)
+    def send_pack(self, get_changed_refs, generate_pack_data):
+        return self.controldir.send_pack(get_changed_refs, generate_pack_data)
 
     def fetch_objects(self, determine_wants, graph_walker, resolve_ext_ref,
                       progress=None):
@@ -501,7 +501,7 @@
             return EmptyObjectStoreIterator()
         return TemporaryPackIterator(path[:-len(".pack")], resolve_ext_ref)
 
-    def lookup_bzr_revision_id(self, bzr_revid):
+    def lookup_bzr_revision_id(self, bzr_revid, mapping=None):
         # This won't work for any round-tripped bzr revisions, but it's a start..
         try:
             return mapping_registry.revision_id_bzr_to_foreign(bzr_revid)
@@ -530,8 +530,23 @@
 class RemoteGitTagDict(GitTags):
 
     def set_tag(self, name, revid):
-        # FIXME: Not supported yet, should do a push of a new ref
-        raise NotImplementedError(self.set_tag)
+        sha = self.branch.lookup_bzr_revision_id(revid)[0]
+        self._set_ref(name, sha)
+
+    def delete_tag(self, name):
+        self._set_ref(name, dulwich.client.ZERO_SHA)
+
+    def _set_ref(self, name, sha):
+        ref = tag_name_to_ref(name)
+        def get_changed_refs(old_refs):
+            ret = dict(old_refs)
+            if sha == dulwich.client.ZERO_SHA and ref not in ret:
+                raise NoSuchTag(name)
+            ret[ref] = sha
+            return ret
+        def generate_pack_data(have, want, ofs_delta=False):
+            return pack_objects_to_data([])
+        self.repository.send_pack(get_changed_refs, generate_pack_data)
 
 
 class RemoteGitBranch(GitBranch):

=== modified file 'tests/test_git_remote_helper.py'
--- tests/test_git_remote_helper.py	2018-03-17 17:34:12 +0000
+++ tests/test_git_remote_helper.py	2018-03-22 01:32:19 +0000
@@ -115,7 +115,7 @@
         f = StringIO()
         self.helper.cmd_list(f, [])
         self.assertEquals(
-            '0000000000000000000000000000000000000000 HEAD\n\n',
+            '\n',
             f.getvalue())
 
     def test_import(self):

=== modified file 'tests/test_refs.py'
--- tests/test_refs.py	2018-03-17 17:34:12 +0000
+++ tests/test_refs.py	2018-03-22 01:32:19 +0000
@@ -22,18 +22,66 @@
 
 from .... import tests
 
-from ...git import refs
+from ..object_store import BazaarObjectStore
+from ..refs import (
+    BazaarRefsContainer,
+    ref_to_branch_name,
+    branch_name_to_ref,
+    )
 
 
 class BranchNameRefConversionTests(tests.TestCase):
 
     def test_head(self):
-        self.assertEquals("", refs.ref_to_branch_name("HEAD"))
-        self.assertEquals("HEAD", refs.branch_name_to_ref(""))
+        self.assertEquals("", ref_to_branch_name("HEAD"))
+        self.assertEquals("HEAD", branch_name_to_ref(""))
 
     def test_tag(self):
-        self.assertRaises(ValueError, refs.ref_to_branch_name, "refs/tags/FOO")
+        self.assertRaises(ValueError, ref_to_branch_name, "refs/tags/FOO")
 
     def test_branch(self):
-        self.assertEquals("frost", refs.ref_to_branch_name("refs/heads/frost"))
-        self.assertEquals("refs/heads/frost", refs.branch_name_to_ref("frost"))
+        self.assertEquals("frost", ref_to_branch_name("refs/heads/frost"))
+        self.assertEquals("refs/heads/frost", branch_name_to_ref("frost"))
+
+
+class BazaarRefsContainerTests(tests.TestCaseWithTransport):
+
+    def test_empty(self):
+        tree = self.make_branch_and_tree('.')
+        store = BazaarObjectStore(tree.branch.repository)
+        refs = BazaarRefsContainer(tree.controldir, store)
+        self.assertEqual(refs.as_dict(), {})
+
+    def test_some_commit(self):
+        tree = self.make_branch_and_tree('.')
+        revid = tree.commit('somechange')
+        store = BazaarObjectStore(tree.branch.repository)
+        refs = BazaarRefsContainer(tree.controldir, store)
+        self.assertEqual(
+                refs.as_dict(),
+                {'HEAD': store._lookup_revision_sha1(revid)})
+
+    def test_some_tag(self):
+        tree = self.make_branch_and_tree('.')
+        revid = tree.commit('somechange')
+        tree.branch.tags.set_tag('sometag', revid)
+        store = BazaarObjectStore(tree.branch.repository)
+        refs = BazaarRefsContainer(tree.controldir, store)
+        self.assertEqual(
+                refs.as_dict(),
+                {'HEAD': store._lookup_revision_sha1(revid),
+                 'refs/tags/sometag': store._lookup_revision_sha1(revid),
+                 })
+
+    def test_some_branch(self):
+        tree = self.make_branch_and_tree('.')
+        revid = tree.commit('somechange')
+        otherbranch = tree.controldir.create_branch(name='otherbranch')
+        otherbranch.generate_revision_history(revid)
+        store = BazaarObjectStore(tree.branch.repository)
+        refs = BazaarRefsContainer(tree.controldir, store)
+        self.assertEqual(
+                refs.as_dict(),
+                {'HEAD': store._lookup_revision_sha1(revid),
+                 'refs/heads/otherbranch': store._lookup_revision_sha1(revid),
+                 })

=== modified file 'tests/test_remote.py'
--- tests/test_remote.py	2018-03-21 03:46:30 +0000
+++ tests/test_remote.py	2018-03-22 01:32:19 +0000
@@ -24,6 +24,7 @@
 from ....errors import (
     BzrError,
     NotBranchError,
+    NoSuchTag,
     )
 
 from ....tests import (
@@ -96,12 +97,14 @@
         self.assertTrue(self.format.supports_tags())
 
 
-class TestFetchFromRemote(TestCaseWithTransport):
+class FetchFromRemoteTestBase(object):
 
     _test_needs_features = [ExecutableFeature('git')]
 
+    _to_format = None
+
     def setUp(self):
-        super(TestFetchFromRemote, self).setUp()
+        TestCaseWithTransport.setUp(self)
         self.remote_real = GitRepo.init('remote', mkdir=True)
         self.remote_url = 'git://%s/' % os.path.abspath(self.remote_real.path)
         self.permit_url(self.remote_url)
@@ -112,8 +115,8 @@
                 committer='committer <[e-mail]>',
                 author='author <[e-mail]>')
 
-        self.make_branch_and_tree('.')
         remote = ControlDir.open(self.remote_url)
+        self.make_controldir('local', format=self._to_format)
         local = remote.sprout('local')
         self.assertEqual(
                 default_mapping.revision_id_foreign_to_bzr(self.remote_real.head()),
@@ -131,8 +134,8 @@
                 ref='refs/tags/another')
         self.remote_real.refs['refs/tags/blah'] = self.remote_real.head()
 
-        self.make_branch_and_tree('.')
         remote = ControlDir.open(self.remote_url)
+        self.make_controldir('local', format=self._to_format)
         local = remote.sprout('local')
         local_branch = local.open_branch()
         self.assertEqual(
@@ -144,12 +147,24 @@
                 local_branch.tags.get_tag_dict())
 
 
-class TestPushToRemote(TestCaseWithTransport):
+class FetchFromRemoteToBzrTests(FetchFromRemoteTestBase,TestCaseWithTransport):
+
+    _to_format = '2a'
+
+
+class FetchFromRemoteToGitTests(FetchFromRemoteTestBase,TestCaseWithTransport):
+
+    _to_format = 'git'
+
+
+class PushToRemoteBase(object):
 
     _test_needs_features = [ExecutableFeature('git')]
 
+    _from_format = None
+
     def setUp(self):
-        super(TestPushToRemote, self).setUp()
+        TestCaseWithTransport.setUp(self)
         self.remote_real = GitRepo.init('remote', mkdir=True)
         self.remote_url = 'git://%s/' % os.path.abspath(self.remote_real.path)
         self.permit_url(self.remote_url)
@@ -160,20 +175,138 @@
                 committer='committer <[e-mail]>',
                 author='author <[e-mail]>')
 
-        self.make_branch_and_tree('.')
         remote = ControlDir.open(self.remote_url)
+        self.make_controldir('local', format=self._from_format)
         local = remote.sprout('local')
         self.build_tree(['local/blah'])
         wt = local.open_workingtree()
         wt.add(['blah'])
         revid = wt.commit('blah')
         wt.branch.tags.set_tag('sometag', revid)
+        wt.branch.get_config_stack().set('branch.fetch_tags', True)
 
-        remote.push_branch(wt.branch)
+        if self._from_format == 'git':
+            wt.branch.push(remote.open_branch())
+        else:
+            wt.branch.push(remote.open_branch(), lossy=True)
 
         self.assertNotEqual(self.remote_real.head(), c1)
         self.assertEqual(
                 {'refs/heads/master': self.remote_real.head(),
                  'HEAD': self.remote_real.head(),
+                 'refs/tags/sometag': self.remote_real.head(),
                 },
                 self.remote_real.get_refs())
+
+
+class PushToRemoteFromBzrTests(PushToRemoteBase,TestCaseWithTransport):
+
+    _from_format = '2a'
+
+
+class PushToRemoteFromGitTests(PushToRemoteBase,TestCaseWithTransport):
+
+    _from_format = 'git'
+
+
+class RemoteControlDirTests(TestCaseWithTransport):
+
+    _test_needs_features = [ExecutableFeature('git')]
+
+    def setUp(self):
+        TestCaseWithTransport.setUp(self)
+        self.remote_real = GitRepo.init('remote', mkdir=True)
+        self.remote_url = 'git://%s/' % os.path.abspath(self.remote_real.path)
+        self.permit_url(self.remote_url)
+
+    def test_remove_branch(self):
+        c1 = self.remote_real.do_commit(
+                message='message',
+                committer='committer <[e-mail]>',
+                author='author <[e-mail]>')
+        c2 = self.remote_real.do_commit(
+                message='another commit',
+                committer='committer <[e-mail]>',
+                author='author <[e-mail]>',
+                ref='refs/heads/blah')
+
+        remote = ControlDir.open(self.remote_url)
+        remote.destroy_branch(name='blah')
+        self.assertEqual(
+                self.remote_real.get_refs(),
+                {'refs/heads/master': self.remote_real.head(),
+                 'HEAD': self.remote_real.head(),
+                })
+
+    def test_list_branches(self):
+        c1 = self.remote_real.do_commit(
+                message='message',
+                committer='committer <[e-mail]>',
+                author='author <[e-mail]>')
+        c2 = self.remote_real.do_commit(
+                message='another commit',
+                committer='committer <[e-mail]>',
+                author='author <[e-mail]>',
+                ref='refs/heads/blah')
+
+        remote = ControlDir.open(self.remote_url)
+        self.assertEqual(
+                ['', 'blah', 'master'],
+                [b.name for b in remote.list_branches()])
+
+    def test_get_branches(self):
+        c1 = self.remote_real.do_commit(
+                message='message',
+                committer='committer <[e-mail]>',
+                author='author <[e-mail]>')
+        c2 = self.remote_real.do_commit(
+                message='another commit',
+                committer='committer <[e-mail]>',
+                author='author <[e-mail]>',
+                ref='refs/heads/blah')
+
+        remote = ControlDir.open(self.remote_url)
+        self.assertEqual(
+                {'', 'blah', 'master'},
+                set(remote.get_branches().keys()))
+
+    def test_remove_tag(self):
+        c1 = self.remote_real.do_commit(
+                message='message',
+                committer='committer <[e-mail]>',
+                author='author <[e-mail]>')
+        c2 = self.remote_real.do_commit(
+                message='another commit',
+                committer='committer <[e-mail]>',
+                author='author <[e-mail]>',
+                ref='refs/tags/blah')
+
+        remote = ControlDir.open(self.remote_url)
+        remote_branch = remote.open_branch()
+        remote_branch.tags.delete_tag('blah')
+        self.assertRaises(NoSuchTag, remote_branch.tags.delete_tag, 'blah')
+        self.assertEqual(
+                self.remote_real.get_refs(),
+                {'refs/heads/master': self.remote_real.head(),
+                 'HEAD': self.remote_real.head(),
+                })
+
+    def test_set_tag(self):
+        c1 = self.remote_real.do_commit(
+                message='message',
+                committer='committer <[e-mail]>',
+                author='author <[e-mail]>')
+        c2 = self.remote_real.do_commit(
+                message='another commit',
+                committer='committer <[e-mail]>',
+                author='author <[e-mail]>')
+
+        remote = ControlDir.open(self.remote_url)
+        remote.open_branch().tags.set_tag(
+            'blah', default_mapping.revision_id_foreign_to_bzr(c1))
+        self.assertEqual(
+                self.remote_real.get_refs(),
+                {'refs/heads/master': self.remote_real.head(),
+                 'refs/tags/blah': c1,
+                 'HEAD': self.remote_real.head(),
+                })

=== modified file 'transportgit.py'
--- transportgit.py	2018-03-17 17:27:39 +0000
+++ transportgit.py	2018-03-22 01:32:19 +0000
@@ -94,16 +94,17 @@
                 pass
 
     def subkeys(self, base):
+        """Refs present in this container under a base.
+
+        :param base: The base to return refs under.
+        :return: A set of valid refs in this container under the base; the base
+            prefix is stripped from the ref names returned.
+        """
         keys = set()
-        try:
-            iter_files = self.transport.clone(base).iter_files_recursive()
-            keys.update(("%s/%s" % (base, urllib.unquote(refname))).strip("/") for
-                    refname in iter_files if check_ref_format("%s/%s" % (base, refname)))
-        except (TransportNotPossible, NoSuchFile):
-            pass
-        for key in self.get_packed_refs():
-            if key.startswith(base):
-                keys.add(key[len(base):].strip("/"))
+        base_len = len(base) + 1
+        for refname in self.allkeys():
+            if refname.startswith(base):
+                keys.add(refname[base_len:])
         return keys
 
     def allkeys(self):