Convert client, protocol to python3.

docs/tutorial/remote.txt

@@ -6,8 +6,8 @@ Most of the tests in this file require a Dulwich server, so let's start one:
     >>> from dulwich.server import DictBackend, TCPGitServer
     >>> import threading
     >>> repo = Repo.init("remote", mkdir=True)
-    >>> cid = repo.do_commit("message", committer="Jelmer <jelmer@samba.org>")
-    >>> backend = DictBackend({'/': repo})
+    >>> cid = repo.do_commit(b"message", committer=b"Jelmer <jelmer@samba.org>")
+    >>> backend = DictBackend({b'/': repo})
     >>> dul_server = TCPGitServer(backend, 'localhost', 0)
     >>> threading.Thread(target=dul_server.serve).start()
     >>> server_address, server_port = dul_server.socket.getsockname()
@@ -59,7 +59,7 @@ which we will write to a ``BytesIO`` object::
 
    >>> from io import BytesIO
    >>> f = BytesIO()
-   >>> remote_refs = client.fetch_pack("/", determine_wants,
+   >>> remote_refs = client.fetch_pack(b"/", determine_wants,
    ...    DummyGraphWalker(), pack_data=f.write)
 
 ``f`` will now contain a full pack file::
@@ -76,7 +76,7 @@ importing the received pack file into the local repository::
 
    >>> from dulwich.repo import Repo
    >>> local = Repo.init("local", mkdir=True)
-   >>> remote_refs = client.fetch("/", local)
+   >>> remote_refs = client.fetch(b"/", local)
 
 Let's shut down the server now that all tests have been run::
 

dulwich/client.py

@@ -79,10 +79,10 @@ def _fileno_can_read(fileno):
     """Check if a file descriptor is readable."""
     return len(select.select([fileno], [], [], 0)[0]) > 0
 
-COMMON_CAPABILITIES = ['ofs-delta', 'side-band-64k']
-FETCH_CAPABILITIES = (['thin-pack', 'multi_ack', 'multi_ack_detailed'] +
+COMMON_CAPABILITIES = [b'ofs-delta', b'side-band-64k']
+FETCH_CAPABILITIES = ([b'thin-pack', b'multi_ack', b'multi_ack_detailed'] +
                       COMMON_CAPABILITIES)
-SEND_CAPABILITIES = ['report-status'] + COMMON_CAPABILITIES
+SEND_CAPABILITIES = [b'report-status'] + COMMON_CAPABILITIES
 
 
 class ReportStatusParser(object):
@@ -101,26 +101,26 @@ class ReportStatusParser(object):
         :raise SendPackError: Raised when the server could not unpack
         :raise UpdateRefsError: Raised when refs could not be updated
         """
-        if self._pack_status not in ('unpack ok', None):
+        if self._pack_status not in (b'unpack ok', None):
             raise SendPackError(self._pack_status)
         if not self._ref_status_ok:
             ref_status = {}
             ok = set()
             for status in self._ref_statuses:
-                if ' ' not in status:
+                if b' ' not in status:
                     # malformed response, move on to the next one
                     continue
-                status, ref = status.split(' ', 1)
+                status, ref = status.split(b' ', 1)
 
-                if status == 'ng':
-                    if ' ' in ref:
-                        ref, status = ref.split(' ', 1)
+                if status == b'ng':
+                    if b' ' in ref:
+                        ref, status = ref.split(b' ', 1)
                 else:
                     ok.add(ref)
                 ref_status[ref] = status
-            raise UpdateRefsError('%s failed to update' %
-                                  ', '.join([ref for ref in ref_status
-                                             if ref not in ok]),
+            raise UpdateRefsError(b', '.join([ref for ref in ref_status
+                                              if ref not in ok]) +
+                                              b' failed to update',
                                   ref_status=ref_status)
 
     def handle_packet(self, pkt):
@@ -139,7 +139,7 @@ class ReportStatusParser(object):
         else:
             ref_status = pkt.strip()
             self._ref_statuses.append(ref_status)
-            if not ref_status.startswith('ok '):
+            if not ref_status.startswith(b'ok '):
                 self._ref_status_ok = False
 
 
@@ -148,8 +148,8 @@ def read_pkt_refs(proto):
     refs = {}
     # Receive refs from server
     for pkt in proto.read_pkt_seq():
-        (sha, ref) = pkt.rstrip('\n').split(None, 1)
-        if sha == 'ERR':
+        (sha, ref) = pkt.rstrip(b'\n').split(None, 1)
+        if sha == b'ERR':
             raise GitProtocolError(ref)
         if server_capabilities is None:
             (ref, server_capabilities) = extract_capabilities(ref)
@@ -180,7 +180,7 @@ class GitClient(object):
         self._fetch_capabilities = set(FETCH_CAPABILITIES)
         self._send_capabilities = set(SEND_CAPABILITIES)
         if not thin_packs:
-            self._fetch_capabilities.remove('thin-pack')
+            self._fetch_capabilities.remove(b'thin-pack')
 
     def send_pack(self, path, determine_wants, generate_pack_contents,
                   progress=None, write_pack=write_pack_objects):
@@ -236,7 +236,7 @@ class GitClient(object):
 
     def _parse_status_report(self, proto):
         unpack = proto.read_pkt_line().strip()
-        if unpack != 'unpack ok':
+        if unpack != b'unpack ok':
             st = True
             # flush remaining error data
             while st is not None:
@@ -248,7 +248,7 @@ class GitClient(object):
         while ref_status:
             ref_status = ref_status.strip()
             statuses.append(ref_status)
-            if not ref_status.startswith('ok '):
+            if not ref_status.startswith(b'ok '):
                 errs = True
             ref_status = proto.read_pkt_line()
 
@@ -256,20 +256,20 @@ class GitClient(object):
             ref_status = {}
             ok = set()
             for status in statuses:
-                if ' ' not in status:
+                if b' ' not in status:
                     # malformed response, move on to the next one
                     continue
-                status, ref = status.split(' ', 1)
+                status, ref = status.split(b' ', 1)
 
-                if status == 'ng':
-                    if ' ' in ref:
-                        ref, status = ref.split(' ', 1)
+                if status == b'ng':
+                    if b' ' in ref:
+                        ref, status = ref.split(b' ', 1)
                 else:
                     ok.add(ref)
                 ref_status[ref] = status
-            raise UpdateRefsError('%s failed to update' %
-                                  ', '.join([ref for ref in ref_status
-                                             if ref not in ok]),
+            raise UpdateRefsError(b', '.join([ref for ref in ref_status
+                                             if ref not in ok]) +
+                                             b' failed to update',
                                   ref_status=ref_status)
 
     def _read_side_band64k_data(self, proto, channel_callbacks):
@@ -282,7 +282,7 @@ class GitClient(object):
             handlers to use. None for a callback discards channel data.
         """
         for pkt in proto.read_pkt_seq():
-            channel = ord(pkt[0])
+            channel = ord(pkt[:1])
             pkt = pkt[1:]
             try:
                 cb = channel_callbacks[channel]
@@ -306,18 +306,18 @@ class GitClient(object):
         have = [x for x in old_refs.values() if not x == ZERO_SHA]
         sent_capabilities = False
 
-        for refname in set(new_refs.keys() + old_refs.keys()):
+        all_refs = set(new_refs.keys()).union(set(old_refs.keys()))
+        for refname in all_refs:
             old_sha1 = old_refs.get(refname, ZERO_SHA)
             new_sha1 = new_refs.get(refname, ZERO_SHA)
 
             if old_sha1 != new_sha1:
                 if sent_capabilities:
-                    proto.write_pkt_line('%s %s %s' % (
-                        old_sha1, new_sha1, refname))
+                    proto.write_pkt_line(old_sha1 + b' ' + new_sha1 + b' ' + refname)
                 else:
                     proto.write_pkt_line(
-                        '%s %s %s\0%s' % (old_sha1, new_sha1, refname,
-                                          ' '.join(capabilities)))
+                        old_sha1 + b' ' + new_sha1 + b' ' + refname + b'\0' +
+                        b' '.join(capabilities))
                     sent_capabilities = True
             if new_sha1 not in have and new_sha1 != ZERO_SHA:
                 want.append(new_sha1)
@@ -331,16 +331,16 @@ class GitClient(object):
         :param capabilities: List of negotiated capabilities
         :param progress: Optional progress reporting function
         """
-        if "side-band-64k" in capabilities:
+        if b"side-band-64k" in capabilities:
             if progress is None:
                 progress = lambda x: None
             channel_callbacks = {2: progress}
-            if 'report-status' in capabilities:
+            if b'report-status' in capabilities:
                 channel_callbacks[1] = PktLineParser(
                     self._report_status_parser.handle_packet).parse
             self._read_side_band64k_data(proto, channel_callbacks)
         else:
-            if 'report-status' in capabilities:
+            if b'report-status' in capabilities:
                 for pkt in proto.read_pkt_seq():
                     self._report_status_parser.handle_packet(pkt)
         if self._report_status_parser is not None:
@@ -357,30 +357,29 @@ class GitClient(object):
         :param can_read: function that returns a boolean that indicates
             whether there is extra graph data to read on proto
         """
-        assert isinstance(wants, list) and isinstance(wants[0], str)
-        proto.write_pkt_line('want %s %s\n' % (
-            wants[0], ' '.join(capabilities)))
+        assert isinstance(wants, list) and isinstance(wants[0], bytes)
+        proto.write_pkt_line(b'want ' + wants[0] + b' ' + b' '.join(capabilities) + b'\n')
         for want in wants[1:]:
-            proto.write_pkt_line('want %s\n' % want)
+            proto.write_pkt_line(b'want ' + want + b'\n')
         proto.write_pkt_line(None)
         have = next(graph_walker)
         while have:
-            proto.write_pkt_line('have %s\n' % have)
+            proto.write_pkt_line(b'have ' + have + b'\n')
             if can_read():
                 pkt = proto.read_pkt_line()
-                parts = pkt.rstrip('\n').split(' ')
-                if parts[0] == 'ACK':
+                parts = pkt.rstrip(b'\n').split(b' ')
+                if parts[0] == b'ACK':
                     graph_walker.ack(parts[1])
-                    if parts[2] in ('continue', 'common'):
+                    if parts[2] in (b'continue', b'common'):
                         pass
-                    elif parts[2] == 'ready':
+                    elif parts[2] == b'ready':
                         break
                     else:
                         raise AssertionError(
                             "%s not in ('continue', 'ready', 'common)" %
                             parts[2])
             have = next(graph_walker)
-        proto.write_pkt_line('done\n')
+        proto.write_pkt_line(b'done\n')
 
     def _handle_upload_pack_tail(self, proto, capabilities, graph_walker,
                                  pack_data, progress=None, rbufsize=_RBUFSIZE):
@@ -395,14 +394,14 @@ class GitClient(object):
         """
         pkt = proto.read_pkt_line()
         while pkt:
-            parts = pkt.rstrip('\n').split(' ')
-            if parts[0] == 'ACK':
+            parts = pkt.rstrip(b'\n').split(b' ')
+            if parts[0] == b'ACK':
                 graph_walker.ack(parts[1])
             if len(parts) < 3 or parts[2] not in (
-                    'ready', 'continue', 'common'):
+                    b'ready', b'continue', b'common'):
                 break
             pkt = proto.read_pkt_line()
-        if "side-band-64k" in capabilities:
+        if b"side-band-64k" in capabilities:
             if progress is None:
                 # Just ignore progress data
                 progress = lambda x: None
@@ -410,7 +409,7 @@ class GitClient(object):
         else:
             while True:
                 data = proto.read(rbufsize)
-                if data == "":
+                if data == b"":
                     break
                 pack_data(data)
 
@@ -447,12 +446,12 @@ class TraditionalGitClient(GitClient):
         :raises UpdateRefsError: if the server supports report-status
                                  and rejects ref updates
         """
-        proto, unused_can_read = self._connect('receive-pack', path)
+        proto, unused_can_read = self._connect(b'receive-pack', path)
         with proto:
             old_refs, server_capabilities = read_pkt_refs(proto)
             negotiated_capabilities = self._send_capabilities & server_capabilities
 
-            if 'report-status' in negotiated_capabilities:
+            if b'report-status' in negotiated_capabilities:
                 self._report_status_parser = ReportStatusParser()
             report_status_parser = self._report_status_parser
 
@@ -462,15 +461,14 @@ class TraditionalGitClient(GitClient):
                 proto.write_pkt_line(None)
                 raise
 
-            if not 'delete-refs' in server_capabilities:
+            if not b'delete-refs' in server_capabilities:
                 # Server does not support deletions. Fail later.
                 new_refs = dict(orig_new_refs)
-                for ref, sha in orig_new_refs.iteritems():
+                for ref, sha in orig_new_refs.items():
                     if sha == ZERO_SHA:
-                        if 'report-status' in negotiated_capabilities:
+                        if b'report-status' in negotiated_capabilities:
                             report_status_parser._ref_statuses.append(
-                                'ng %s remote does not support deleting refs'
-                                % sha)
+                                b'ng ' + sha + b' remote does not support deleting refs')
                             report_status_parser._ref_status_ok = False
                         del new_refs[ref]
 
@@ -493,7 +491,7 @@ class TraditionalGitClient(GitClient):
 
             dowrite = len(objects) > 0
             dowrite = dowrite or any(old_refs.get(ref) != sha
-                                     for (ref, sha) in new_refs.iteritems()
+                                     for (ref, sha) in new_refs.items()
                                      if sha != ZERO_SHA)
             if dowrite:
                 write_pack(proto.write_file(), objects)
@@ -511,7 +509,7 @@ class TraditionalGitClient(GitClient):
         :param pack_data: Callback called for each bit of data in the pack
         :param progress: Callback for progress reports (strings)
         """
-        proto, can_read = self._connect('upload-pack', path)
+        proto, can_read = self._connect(b'upload-pack', path)
         with proto:
             refs, server_capabilities = read_pkt_refs(proto)
             negotiated_capabilities = (
@@ -541,15 +539,15 @@ class TraditionalGitClient(GitClient):
                 write_error=None):
         proto, can_read = self._connect(b'upload-archive', path)
         with proto:
-            proto.write_pkt_line("argument %s" % committish)
+            proto.write_pkt_line(b"argument " + committish)
             proto.write_pkt_line(None)
             pkt = proto.read_pkt_line()
-            if pkt == "NACK\n":
+            if pkt == b"NACK\n":
                 return
-            elif pkt == "ACK\n":
+            elif pkt == b"ACK\n":
                 pass
-            elif pkt.startswith("ERR "):
-                raise GitProtocolError(pkt[4:].rstrip("\n"))
+            elif pkt.startswith(b"ERR "):
+                raise GitProtocolError(pkt[4:].rstrip(b"\n"))
             else:
                 raise AssertionError("invalid response %r" % pkt)
             ret = proto.read_pkt_line()
@@ -597,9 +595,9 @@ class TCPGitClient(TraditionalGitClient):
 
         proto = Protocol(rfile.read, wfile.write, close,
                          report_activity=self._report_activity)
-        if path.startswith("/~"):
+        if path.startswith(b"/~"):
             path = path[1:]
-        proto.send_cmd('git-%s' % cmd, path, 'host=%s' % self._host)
+        proto.send_cmd(b'git-' + cmd, path, b'host=' + self._host)
         return proto, lambda: _fileno_can_read(s)
 
 
@@ -688,7 +686,8 @@ class LocalGitClient(GitClient):
 
         have = [sha1 for sha1 in old_refs.values() if sha1 != ZERO_SHA]
         want = []
-        for refname in set(new_refs.keys() + old_refs.keys()):
+        all_refs = set(new_refs.keys()).union(set(old_refs.keys()))
+        for refname in all_refs:
             old_sha1 = old_refs.get(refname, ZERO_SHA)
             new_sha1 = new_refs.get(refname, ZERO_SHA)
             if new_sha1 not in have and new_sha1 != ZERO_SHA:
@@ -699,7 +698,7 @@ class LocalGitClient(GitClient):
 
         target.object_store.add_objects(generate_pack_contents(have, want))
 
-        for name, sha in new_refs.iteritems():
+        for name, sha in new_refs.items():
             target.refs[name] = sha
 
         return new_refs
@@ -909,16 +908,16 @@ class SSHGitClient(TraditionalGitClient):
         self.alternative_paths = {}
 
     def _get_cmd_path(self, cmd):
-        return self.alternative_paths.get(cmd, 'git-%s' % cmd)
+        return self.alternative_paths.get(cmd, b'git-' + cmd)
 
     def _connect(self, cmd, path):
-        if path.startswith("/~"):
+        if path.startswith(b"/~"):
             path = path[1:]
         con = get_ssh_vendor().run_command(
-            self.host, ["%s '%s'" % (self._get_cmd_path(cmd), path)],
+            self.host, [self._get_cmd_path(cmd) + b" '" + path + b"'"],
             port=self.port, username=self.username)
-        return (Protocol(con.read, con.write, con.close, 
-                         report_activity=self._report_activity), 
+        return (Protocol(con.read, con.write, con.close,
+                         report_activity=self._report_activity),
                 con.can_read)
 
 
@@ -1021,10 +1020,10 @@ class HttpGitClient(GitClient):
         """
         url = self._get_url(path)
         old_refs, server_capabilities = self._discover_references(
-            "git-receive-pack", url)
+            b"git-receive-pack", url)
         negotiated_capabilities = self._send_capabilities & server_capabilities
 
-        if 'report-status' in negotiated_capabilities:
+        if b'report-status' in negotiated_capabilities:
             self._report_status_parser = ReportStatusParser()
 
         new_refs = determine_wants(dict(old_refs))
@@ -1041,7 +1040,7 @@ class HttpGitClient(GitClient):
         objects = generate_pack_contents(have, want)
         if len(objects) > 0:
             write_pack(req_proto.write_file(), objects)
-        resp = self._smart_request("git-receive-pack", url,
+        resp = self._smart_request(b"git-receive-pack", url,
                                    data=req_data.getvalue())
         try:
             resp_proto = Protocol(resp.read, None)
@@ -1064,7 +1063,7 @@ class HttpGitClient(GitClient):
         """
         url = self._get_url(path)
         refs, server_capabilities = self._discover_references(
-            "git-upload-pack", url)
+            b"git-upload-pack", url)
         negotiated_capabilities = self._fetch_capabilities & server_capabilities
         wants = determine_wants(refs)
         if wants is not None:
@@ -1079,7 +1078,7 @@ class HttpGitClient(GitClient):
             req_proto, negotiated_capabilities, graph_walker, wants,
             lambda: False)
         resp = self._smart_request(
-            "git-upload-pack", url, data=req_data.getvalue())
+            b"git-upload-pack", url, data=req_data.getvalue())
         try:
             resp_proto = Protocol(resp.read, None)
             self._handle_upload_pack_tail(resp_proto, negotiated_capabilities,

dulwich/tests/compat/test_client.py

@@ -158,9 +158,9 @@ class DulwichClientTestBase(object):
         try:
             c.send_pack(self._build_path('/dest'), lambda _: sendrefs, gen_pack)
         except errors.UpdateRefsError as e:
-            self.assertEqual('refs/heads/master failed to update', str(e))
-            self.assertEqual({'refs/heads/branch': 'ok',
-                              'refs/heads/master': 'non-fast-forward'},
+            self.assertEqual(b'refs/heads/master failed to update', str(e))
+            self.assertEqual({b'refs/heads/branch': b'ok',
+                              b'refs/heads/master': b'non-fast-forward'},
                              e.ref_status)
 
     def test_send_pack_multiple_errors(self):

dulwich/tests/test_client.py

@@ -59,7 +59,6 @@ from dulwich.repo import (
 from dulwich.tests import skipIf
 from dulwich.tests.utils import (
     open_repo,
-    skipIfPY3,
     )
 
 
@@ -76,7 +75,6 @@ class DummyClient(TraditionalGitClient):
 
 
 # TODO(durin42): add unit-level tests of GitClient
-@skipIfPY3
 class GitClientTests(TestCase):
 
     def setUp(self):
@@ -87,63 +85,63 @@ class GitClientTests(TestCase):
                                   self.rout.write)
 
     def test_caps(self):
-        self.assertEqual(set(['multi_ack', 'side-band-64k', 'ofs-delta',
-                               'thin-pack', 'multi_ack_detailed']),
+        self.assertEqual(set([b'multi_ack', b'side-band-64k', b'ofs-delta',
+                               b'thin-pack', b'multi_ack_detailed']),
                           set(self.client._fetch_capabilities))
-        self.assertEqual(set(['ofs-delta', 'report-status', 'side-band-64k']),
+        self.assertEqual(set([b'ofs-delta', b'report-status', b'side-band-64k']),
                           set(self.client._send_capabilities))
 
     def test_archive_ack(self):
         self.rin.write(
-            '0009NACK\n'
-            '0000')
+            b'0009NACK\n'
+            b'0000')
         self.rin.seek(0)
-        self.client.archive('bla', 'HEAD', None, None)
-        self.assertEqual(self.rout.getvalue(), '0011argument HEAD0000')
+        self.client.archive(b'bla', b'HEAD', None, None)
+        self.assertEqual(self.rout.getvalue(), b'0011argument HEAD0000')
 
     def test_fetch_empty(self):
-        self.rin.write('0000')
+        self.rin.write(b'0000')
         self.rin.seek(0)
-        self.client.fetch_pack('/', lambda heads: [], None, None)
+        self.client.fetch_pack(b'/', lambda heads: [], None, None)
 
     def test_fetch_pack_none(self):
         self.rin.write(
-            '008855dcc6bf963f922e1ed5c4bbaaefcfacef57b1d7 HEAD.multi_ack '
-            'thin-pack side-band side-band-64k ofs-delta shallow no-progress '
-            'include-tag\n'
-            '0000')
+            b'008855dcc6bf963f922e1ed5c4bbaaefcfacef57b1d7 HEAD.multi_ack '
+            b'thin-pack side-band side-band-64k ofs-delta shallow no-progress '
+            b'include-tag\n'
+            b'0000')
         self.rin.seek(0)
-        self.client.fetch_pack('bla', lambda heads: [], None, None, None)
-        self.assertEqual(self.rout.getvalue(), '0000')
+        self.client.fetch_pack(b'bla', lambda heads: [], None, None, None)
+        self.assertEqual(self.rout.getvalue(), b'0000')
 
     def test_send_pack_no_sideband64k_with_update_ref_error(self):
         # No side-bank-64k reported by server shouldn't try to parse
         # side band data
-        pkts = ['55dcc6bf963f922e1ed5c4bbaaefcfacef57b1d7 capabilities^{}'
-                '\x00 report-status delete-refs ofs-delta\n',
-                '',
-                "unpack ok",
-                "ng refs/foo/bar pre-receive hook declined",
-                '']
+        pkts = [b'55dcc6bf963f922e1ed5c4bbaaefcfacef57b1d7 capabilities^{}'
+                b'\x00 report-status delete-refs ofs-delta\n',
+                b'',
+                b"unpack ok",
+                b"ng refs/foo/bar pre-receive hook declined",
+                b'']
         for pkt in pkts:
-            if pkt == '':
-                self.rin.write("0000")
+            if pkt ==  b'':
+                self.rin.write(b"0000")
             else:
-                self.rin.write("%04x%s" % (len(pkt)+4, pkt))
+                self.rin.write(("%04x" % (len(pkt)+4)).encode('ascii') + pkt)
         self.rin.seek(0)
 
         tree = Tree()
         commit = Commit()
         commit.tree = tree
         commit.parents = []
-        commit.author = commit.committer = 'test user'
+        commit.author = commit.committer = b'test user'
         commit.commit_time = commit.author_time = 1174773719
         commit.commit_timezone = commit.author_timezone = 0
-        commit.encoding = 'UTF-8'
-        commit.message = 'test message'
+        commit.encoding = b'UTF-8'
+        commit.message = b'test message'
 
         def determine_wants(refs):
-            return {'refs/foo/bar': commit.id, }
+            return {b'refs/foo/bar': commit.id, }
 
         def generate_pack_contents(have, want):
             return [(commit, None), (tree, ''), ]
@@ -154,62 +152,62 @@ class GitClientTests(TestCase):
 
     def test_send_pack_none(self):
         self.rin.write(
-            '0078310ca9477129b8586fa2afc779c1f57cf64bba6c '
-            'refs/heads/master\x00 report-status delete-refs '
-            'side-band-64k quiet ofs-delta\n'
-            '0000')
+            b'0078310ca9477129b8586fa2afc779c1f57cf64bba6c '
+            b'refs/heads/master\x00 report-status delete-refs '
+            b'side-band-64k quiet ofs-delta\n'
+            b'0000')
         self.rin.seek(0)
 
         def determine_wants(refs):
             return {
-                'refs/heads/master': '310ca9477129b8586fa2afc779c1f57cf64bba6c'
+                b'refs/heads/master': b'310ca9477129b8586fa2afc779c1f57cf64bba6c'
             }
 
         def generate_pack_contents(have, want):
             return {}
 
-        self.client.send_pack('/', determine_wants, generate_pack_contents)
-        self.assertEqual(self.rout.getvalue(), '0000')
+        self.client.send_pack(b'/', determine_wants, generate_pack_contents)
+        self.assertEqual(self.rout.getvalue(), b'0000')
 
     def test_send_pack_delete_only(self):
         self.rin.write(
-            '0063310ca9477129b8586fa2afc779c1f57cf64bba6c '
-            'refs/heads/master\x00report-status delete-refs ofs-delta\n'
-            '0000000eunpack ok\n'
-            '0019ok refs/heads/master\n'
-            '0000')
+            b'0063310ca9477129b8586fa2afc779c1f57cf64bba6c '
+            b'refs/heads/master\x00report-status delete-refs ofs-delta\n'
+            b'0000000eunpack ok\n'
+            b'0019ok refs/heads/master\n'
+            b'0000')
         self.rin.seek(0)
 
         def determine_wants(refs):
-            return {'refs/heads/master': '0' * 40}
+            return {b'refs/heads/master': b'0' * 40}
 
         def generate_pack_contents(have, want):
             return {}
 
-        self.client.send_pack('/', determine_wants, generate_pack_contents)
+        self.client.send_pack(b'/', determine_wants, generate_pack_contents)
         self.assertIn(
             self.rout.getvalue(),
-            ['007f310ca9477129b8586fa2afc779c1f57cf64bba6c '
-             '0000000000000000000000000000000000000000 '
-             'refs/heads/master\x00report-status ofs-delta0000',
-             '007f310ca9477129b8586fa2afc779c1f57cf64bba6c '
-             '0000000000000000000000000000000000000000 '
-             'refs/heads/master\x00ofs-delta report-status0000'])
+            [b'007f310ca9477129b8586fa2afc779c1f57cf64bba6c '
+             b'0000000000000000000000000000000000000000 '
+             b'refs/heads/master\x00report-status ofs-delta0000',
+             b'007f310ca9477129b8586fa2afc779c1f57cf64bba6c '
+             b'0000000000000000000000000000000000000000 '
+             b'refs/heads/master\x00ofs-delta report-status0000'])
 
     def test_send_pack_new_ref_only(self):
         self.rin.write(
-            '0063310ca9477129b8586fa2afc779c1f57cf64bba6c '
-            'refs/heads/master\x00report-status delete-refs ofs-delta\n'
-            '0000000eunpack ok\n'
-            '0019ok refs/heads/blah12\n'
-            '0000')
+            b'0063310ca9477129b8586fa2afc779c1f57cf64bba6c '
+            b'refs/heads/master\x00report-status delete-refs ofs-delta\n'
+            b'0000000eunpack ok\n'
+            b'0019ok refs/heads/blah12\n'
+            b'0000')
         self.rin.seek(0)
 
         def determine_wants(refs):
             return {
-                'refs/heads/blah12':
-                '310ca9477129b8586fa2afc779c1f57cf64bba6c',
-                'refs/heads/master': '310ca9477129b8586fa2afc779c1f57cf64bba6c'
+                b'refs/heads/blah12':
+                b'310ca9477129b8586fa2afc779c1f57cf64bba6c',
+                b'refs/heads/master': b'310ca9477129b8586fa2afc779c1f57cf64bba6c'
             }
 
         def generate_pack_contents(have, want):
@@ -220,80 +218,77 @@ class GitClientTests(TestCase):
         self.client.send_pack('/', determine_wants, generate_pack_contents)
         self.assertIn(
             self.rout.getvalue(),
-            ['007f0000000000000000000000000000000000000000 '
-             '310ca9477129b8586fa2afc779c1f57cf64bba6c '
-             'refs/heads/blah12\x00report-status ofs-delta0000%s'
-             % f.getvalue(),
-             '007f0000000000000000000000000000000000000000 '
-             '310ca9477129b8586fa2afc779c1f57cf64bba6c '
-             'refs/heads/blah12\x00ofs-delta report-status0000%s'
-             % f.getvalue()])
+            [b'007f0000000000000000000000000000000000000000 '
+             b'310ca9477129b8586fa2afc779c1f57cf64bba6c '
+             b'refs/heads/blah12\x00report-status ofs-delta0000' +
+             f.getvalue(),
+             b'007f0000000000000000000000000000000000000000 '
+             b'310ca9477129b8586fa2afc779c1f57cf64bba6c '
+             b'refs/heads/blah12\x00ofs-delta report-status0000' +
+             f.getvalue()])
 
     def test_send_pack_new_ref(self):
         self.rin.write(
-            '0064310ca9477129b8586fa2afc779c1f57cf64bba6c '
-            'refs/heads/master\x00 report-status delete-refs ofs-delta\n'
-            '0000000eunpack ok\n'
-            '0019ok refs/heads/blah12\n'
-            '0000')
+            b'0064310ca9477129b8586fa2afc779c1f57cf64bba6c '
+            b'refs/heads/master\x00 report-status delete-refs ofs-delta\n'
+            b'0000000eunpack ok\n'
+            b'0019ok refs/heads/blah12\n'
+            b'0000')
         self.rin.seek(0)
 
         tree = Tree()
         commit = Commit()
         commit.tree = tree
         commit.parents = []
-        commit.author = commit.committer = 'test user'
+        commit.author = commit.committer = b'test user'
         commit.commit_time = commit.author_time = 1174773719
         commit.commit_timezone = commit.author_timezone = 0
-        commit.encoding = 'UTF-8'
-        commit.message = 'test message'
+        commit.encoding = b'UTF-8'
+        commit.message = b'test message'
 
         def determine_wants(refs):
             return {
-                'refs/heads/blah12': commit.id,
-                'refs/heads/master': '310ca9477129b8586fa2afc779c1f57cf64bba6c'
+                b'refs/heads/blah12': commit.id,
+                b'refs/heads/master': b'310ca9477129b8586fa2afc779c1f57cf64bba6c'
             }
 
         def generate_pack_contents(have, want):
-            return [(commit, None), (tree, ''), ]
+            return [(commit, None), (tree, b''), ]
 
         f = BytesIO()
         write_pack_objects(f, generate_pack_contents(None, None))
-        self.client.send_pack('/', determine_wants, generate_pack_contents)
+        self.client.send_pack(b'/', determine_wants, generate_pack_contents)
         self.assertIn(
             self.rout.getvalue(),
-            ['007f0000000000000000000000000000000000000000 %s '
-             'refs/heads/blah12\x00report-status ofs-delta0000%s'
-             % (commit.id, f.getvalue()),
-             '007f0000000000000000000000000000000000000000 %s '
-             'refs/heads/blah12\x00ofs-delta report-status0000%s'
-             % (commit.id, f.getvalue())])
+            [b'007f0000000000000000000000000000000000000000 ' + commit.id +
+             b' refs/heads/blah12\x00report-status ofs-delta0000' + f.getvalue(),
+             b'007f0000000000000000000000000000000000000000 ' + commit.id +
+             b' refs/heads/blah12\x00ofs-delta report-status0000' + f.getvalue()])
 
     def test_send_pack_no_deleteref_delete_only(self):
-        pkts = ['310ca9477129b8586fa2afc779c1f57cf64bba6c refs/heads/master'
-                '\x00 report-status ofs-delta\n',
-                '',
-                '']
+        pkts = [b'310ca9477129b8586fa2afc779c1f57cf64bba6c refs/heads/master'
+                b'\x00 report-status ofs-delta\n',
+                b'',
+                b'']
         for pkt in pkts:
-            if pkt == '':
-                self.rin.write("0000")
+            if pkt == b'':
+                self.rin.write(b"0000")
             else:
-                self.rin.write("%04x%s" % (len(pkt)+4, pkt))
+                self.rin.write(("%04x" % (len(pkt)+4)).encode('ascii') + pkt)
         self.rin.seek(0)
 
         def determine_wants(refs):
-            return {'refs/heads/master': '0' * 40}
+            return {b'refs/heads/master': b'0' * 40}
 
         def generate_pack_contents(have, want):
             return {}
 
         self.assertRaises(UpdateRefsError,
-                          self.client.send_pack, "/",
+                          self.client.send_pack, b"/",
                           determine_wants, generate_pack_contents)
-        self.assertEqual(self.rout.getvalue(), '0000')
+        self.assertEqual(self.rout.getvalue(), b'0000')
 
 
-@skipIfPY3
 class TestGetTransportAndPath(TestCase):
 
     def test_tcp(self):
@@ -414,7 +409,6 @@ class TestGetTransportAndPath(TestCase):
         self.assertEqual('/jelmer/dulwich', path)
 
 
-@skipIfPY3
 class TestGetTransportAndPathFromUrl(TestCase):
 
     def test_tcp(self):
@@ -493,7 +487,6 @@ class TestGetTransportAndPathFromUrl(TestCase):
         self.assertEqual('/home/jelmer/foo', path)
 
 
-@skipIfPY3
 class TestSSHVendor(object):
 
     def __init__(self):
@@ -516,7 +509,6 @@ class TestSSHVendor(object):
         return Subprocess()
 
 
-@skipIfPY3
 class SSHGitClientTests(TestCase):
 
     def setUp(self):
@@ -533,58 +525,56 @@ class SSHGitClientTests(TestCase):
         client.get_ssh_vendor = self.real_vendor
 
     def test_default_command(self):
-        self.assertEqual('git-upload-pack',
-                self.client._get_cmd_path('upload-pack'))
+        self.assertEqual(b'git-upload-pack',
+                self.client._get_cmd_path(b'upload-pack'))
 
     def test_alternative_command_path(self):
-        self.client.alternative_paths['upload-pack'] = (
-            '/usr/lib/git/git-upload-pack')
-        self.assertEqual('/usr/lib/git/git-upload-pack',
-            self.client._get_cmd_path('upload-pack'))
+        self.client.alternative_paths[b'upload-pack'] = (
+            b'/usr/lib/git/git-upload-pack')
+        self.assertEqual(b'/usr/lib/git/git-upload-pack',
+            self.client._get_cmd_path(b'upload-pack'))
 
     def test_connect(self):
         server = self.server
         client = self.client
 
-        client.username = "username"
+        client.username = b"username"
         client.port = 1337
 
-        client._connect("command", "/path/to/repo")
-        self.assertEqual("username", server.username)
+        client._connect(b"command", b"/path/to/repo")
+        self.assertEqual(b"username", server.username)
         self.assertEqual(1337, server.port)
-        self.assertEqual(["git-command '/path/to/repo'"], server.command)
+        self.assertEqual([b"git-command '/path/to/repo'"], server.command)
 
-        client._connect("relative-command", "/~/path/to/repo")
-        self.assertEqual(["git-relative-command '~/path/to/repo'"],
+        client._connect(b"relative-command", b"/~/path/to/repo")
+        self.assertEqual([b"git-relative-command '~/path/to/repo'"],
                           server.command)
 
 
-@skipIfPY3
 class ReportStatusParserTests(TestCase):
 
     def test_invalid_pack(self):
         parser = ReportStatusParser()
-        parser.handle_packet("unpack error - foo bar")
-        parser.handle_packet("ok refs/foo/bar")
+        parser.handle_packet(b"unpack error - foo bar")
+        parser.handle_packet(b"ok refs/foo/bar")
         parser.handle_packet(None)
         self.assertRaises(SendPackError, parser.check)
 
     def test_update_refs_error(self):
         parser = ReportStatusParser()
-        parser.handle_packet("unpack ok")
-        parser.handle_packet("ng refs/foo/bar need to pull")
+        parser.handle_packet(b"unpack ok")
+        parser.handle_packet(b"ng refs/foo/bar need to pull")
         parser.handle_packet(None)
         self.assertRaises(UpdateRefsError, parser.check)
 
     def test_ok(self):
         parser = ReportStatusParser()
-        parser.handle_packet("unpack ok")
-        parser.handle_packet("ok refs/foo/bar")
+        parser.handle_packet(b"unpack ok")
+        parser.handle_packet(b"ok refs/foo/bar")
         parser.handle_packet(None)
         parser.check()
 
 
-@skipIfPY3
 class LocalGitClientTests(TestCase):
 
     def test_fetch_into_empty(self):
@@ -600,8 +590,8 @@ class LocalGitClientTests(TestCase):
         walker = {}
         c.fetch_pack(s.path, lambda heads: [], graph_walker=walker,
             pack_data=out.write)
-        self.assertEqual("PACK\x00\x00\x00\x02\x00\x00\x00\x00\x02\x9d\x08"
-            "\x82;\xd8\xa8\xea\xb5\x10\xadj\xc7\\\x82<\xfd>\xd3\x1e", out.getvalue())
+        self.assertEqual(b"PACK\x00\x00\x00\x02\x00\x00\x00\x00\x02\x9d\x08"
+            b"\x82;\xd8\xa8\xea\xb5\x10\xadj\xc7\\\x82<\xfd>\xd3\x1e", out.getvalue())
 
     def test_fetch_pack_none(self):
         c = LocalGitClient()
@@ -609,33 +599,33 @@ class LocalGitClientTests(TestCase):
         out = BytesIO()
         walker = MemoryRepo().get_graph_walker()
         c.fetch_pack(s.path,
-            lambda heads: ["a90fa2d900a17e99b433217e988c4eb4a2e9a097"],
+            lambda heads: [b"a90fa2d900a17e99b433217e988c4eb4a2e9a097"],
             graph_walker=walker, pack_data=out.write)
         # Hardcoding is not ideal, but we'll fix that some other day..
-        self.assertTrue(out.getvalue().startswith('PACK\x00\x00\x00\x02\x00\x00\x00\x07'))
+        self.assertTrue(out.getvalue().startswith(b'PACK\x00\x00\x00\x02\x00\x00\x00\x07'))
 
     def test_send_pack_without_changes(self):
         local = open_repo('a.git')
         target = open_repo('a.git')
-        self.send_and_verify("master", local, target)
+        self.send_and_verify(b"master", local, target)
 
     def test_send_pack_with_changes(self):
         local = open_repo('a.git')
         target_path = tempfile.mkdtemp()
         self.addCleanup(shutil.rmtree, target_path)
         target = Repo.init_bare(target_path)
-        self.send_and_verify("master", local, target)
+        self.send_and_verify(b"master", local, target)
 
     def send_and_verify(self, branch, local, target):
         client = LocalGitClient()
-        ref_name = "refs/heads/" + branch
+        ref_name = b"refs/heads/" + branch
         new_refs = client.send_pack(target.path,
                                     lambda _: { ref_name: local.refs[ref_name] },
                                     local.object_store.generate_pack_contents)
 
         self.assertEqual(local.refs[ref_name], new_refs[ref_name])
 
-        for name, sha in new_refs.iteritems():
+        for name, sha in new_refs.items():
             self.assertEqual(new_refs[name], target.refs[name])
 
         obj_local = local.get_object(new_refs[ref_name])