Use bytes internally everywhere for client paths.

dulwich/client.py

@@ -604,6 +604,10 @@ class TCPGitClient(TraditionalGitClient):
         TraditionalGitClient.__init__(self, *args, **kwargs)
 
     def _connect(self, cmd, path):
+        if type(cmd) is not bytes:
+            raise TypeError(path)
+        if type(path) is not bytes:
+            raise TypeError(path)
         sockaddrs = socket.getaddrinfo(
             self._host, self._port, socket.AF_UNSPEC, socket.SOCK_STREAM)
         s = None
@@ -695,6 +699,10 @@ class SubprocessGitClient(TraditionalGitClient):
     git_command = None
 
     def _connect(self, service, path):
+        if type(service) is not bytes:
+            raise TypeError(path)
+        if type(path) is not bytes:
+            raise TypeError(path)
         import subprocess
         if self.git_command is None:
             git_command = find_git_command()
@@ -980,17 +988,23 @@ class SSHGitClient(TraditionalGitClient):
 
     def _get_cmd_path(self, cmd):
         cmd = self.alternative_paths.get(cmd, b'git-' + cmd)
-        if sys.version_info[0:2] >= (2, 7):
-            return [b.decode('ascii') for b in shlex.split(cmd)]
+        assert isinstance(cmd, bytes)
+        if sys.version_info[:2] <= (2, 6):
+            return shlex.split(cmd)
         else:
-            return shlex.split(cmd.decode('ascii'))
+            # TODO(jelmer): Don't decode/encode here
+            return [x.encode('ascii') for x in shlex.split(cmd.decode('ascii'))]
 
     def _connect(self, cmd, path):
-        if path.startswith("/~"):
+        if type(cmd) is not bytes:
+            raise TypeError(path)
+        if type(path) is not bytes:
+            raise TypeError(path)
+        if path.startswith(b"/~"):
             path = path[1:]
+        argv = self._get_cmd_path(cmd) + [path]
         con = get_ssh_vendor().run_command(
-            self.host, self._get_cmd_path(cmd) + [path],
-            port=self.port, username=self.username)
+            self.host, argv, port=self.port, username=self.username)
         return (Protocol(con.read, con.write, con.close,
                          report_activity=self._report_activity),
                 con.can_read)

dulwich/porcelain.py

@@ -126,6 +126,8 @@ def archive(path, committish=None, outstream=sys.stdout,
     client = SubprocessGitClient()
     if committish is None:
         committish = "HEAD"
+    if not isinstance(path, bytes):
+        path = path.encode(sys.getfilesystemencoding())
     # TODO(jelmer): This invokes C git; this introduces a dependency.
     # Instead, dulwich should have its own archiver implementation.
     client.archive(path, committish, outstream.write, errstream.write,

dulwich/tests/compat/test_client.py

@@ -106,7 +106,7 @@ class DulwichClientTestBase(object):
         with closing(repo.Repo(srcpath)) as src:
             sendrefs = dict(src.get_refs())
             del sendrefs[b'HEAD']
-            c.send_pack(self._build_path('/dest'), lambda _: sendrefs,
+            c.send_pack(self._build_path(b'/dest'), lambda _: sendrefs,
                         src.object_store.generate_pack_contents)
 
     def test_send_pack(self):
@@ -126,7 +126,7 @@ class DulwichClientTestBase(object):
         with closing(repo.Repo(srcpath)) as src:
             sendrefs = dict(src.get_refs())
             del sendrefs[b'HEAD']
-            c.send_pack(self._build_path('/dest'), lambda _: sendrefs,
+            c.send_pack(self._build_path(b'/dest'), lambda _: sendrefs,
                         src.object_store.generate_pack_contents)
             self.assertDestEqualsSrc()
 
@@ -164,7 +164,7 @@ class DulwichClientTestBase(object):
             sendrefs, gen_pack = self.compute_send(src)
             c = self._client()
             try:
-                c.send_pack(self._build_path('/dest'), lambda _: sendrefs, gen_pack)
+                c.send_pack(self._build_path(b'/dest'), lambda _: sendrefs, gen_pack)
             except errors.UpdateRefsError as e:
                 self.assertEqual('refs/heads/master failed to update',
                                  e.args[0])
@@ -182,7 +182,7 @@ class DulwichClientTestBase(object):
             sendrefs, gen_pack = self.compute_send(src)
             c = self._client()
             try:
-                c.send_pack(self._build_path('/dest'), lambda _: sendrefs, gen_pack)
+                c.send_pack(self._build_path(b'/dest'), lambda _: sendrefs, gen_pack)
             except errors.UpdateRefsError as e:
                 self.assertIn(str(e),
                               ['{0}, {1} failed to update'.format(
@@ -196,7 +196,7 @@ class DulwichClientTestBase(object):
     def test_archive(self):
         c = self._client()
         f = BytesIO()
-        c.archive(self._build_path('/server_new.export'), b'HEAD', f.write)
+        c.archive(self._build_path(b'/server_new.export'), b'HEAD', f.write)
         f.seek(0)
         tf = tarfile.open(fileobj=f)
         self.assertEqual(['baz', 'foo'], tf.getnames())
@@ -204,7 +204,7 @@ class DulwichClientTestBase(object):
     def test_fetch_pack(self):
         c = self._client()
         with closing(repo.Repo(os.path.join(self.gitroot, 'dest'))) as dest:
-            refs = c.fetch(self._build_path('/server_new.export'), dest)
+            refs = c.fetch(self._build_path(b'/server_new.export'), dest)
             for r in refs.items():
                 dest.refs.set_if_equals(r[0], None, r[1])
             self.assertDestEqualsSrc()
@@ -216,7 +216,7 @@ class DulwichClientTestBase(object):
         c = self._client()
         repo_dir = os.path.join(self.gitroot, 'server_new.export')
         with closing(repo.Repo(repo_dir)) as dest:
-            refs = c.fetch(self._build_path('/dest'), dest)
+            refs = c.fetch(self._build_path(b'/dest'), dest)
             for r in refs.items():
                 dest.refs.set_if_equals(r[0], None, r[1])
             self.assertDestEqualsSrc()
@@ -225,7 +225,7 @@ class DulwichClientTestBase(object):
         c = self._client()
         c._fetch_capabilities.remove(b'side-band-64k')
         with closing(repo.Repo(os.path.join(self.gitroot, 'dest'))) as dest:
-            refs = c.fetch(self._build_path('/server_new.export'), dest)
+            refs = c.fetch(self._build_path(b'/server_new.export'), dest)
             for r in refs.items():
                 dest.refs.set_if_equals(r[0], None, r[1])
             self.assertDestEqualsSrc()
@@ -235,7 +235,7 @@ class DulwichClientTestBase(object):
         # be ignored
         c = self._client()
         with closing(repo.Repo(os.path.join(self.gitroot, 'dest'))) as dest:
-            refs = c.fetch(self._build_path('/server_new.export'), dest,
+            refs = c.fetch(self._build_path(b'/server_new.export'), dest,
                 lambda refs: [protocol.ZERO_SHA])
             for r in refs.items():
                 dest.refs.set_if_equals(r[0], None, r[1])
@@ -251,12 +251,12 @@ class DulwichClientTestBase(object):
             gen_pack = lambda have, want: []
             c = self._client()
             self.assertEqual(dest.refs[b"refs/heads/abranch"], dummy_commit)
-            c.send_pack(self._build_path('/dest'), lambda _: sendrefs, gen_pack)
+            c.send_pack(self._build_path(b'/dest'), lambda _: sendrefs, gen_pack)
             self.assertFalse(b"refs/heads/abranch" in dest.refs)
 
     def test_get_refs(self):
         c = self._client()
-        refs = c.get_refs(self._build_path('/server_new.export'))
+        refs = c.get_refs(self._build_path(b'/server_new.export'))
 
         repo_dir = os.path.join(self.gitroot, 'server_new.export')
         with closing(repo.Repo(repo_dir)) as dest:
@@ -312,7 +312,7 @@ class DulwichTCPClientTest(CompatTestCase, DulwichClientTestBase):
         return client.TCPGitClient(b'localhost')
 
     def _build_path(self, path):
-        return path.encode(sys.getfilesystemencoding())
+        return path
 
     if sys.platform == 'win32':
         @expectedFailure
@@ -321,10 +321,11 @@ class DulwichTCPClientTest(CompatTestCase, DulwichClientTestBase):
 
 
 class TestSSHVendor(object):
+
     @staticmethod
     def run_command(host, command, username=None, port=None):
         cmd, path = command
-        cmd = cmd.split('-', 1)
+        cmd = cmd.split(b'-', 1)
         p = subprocess.Popen(cmd + [path], bufsize=0, env=get_safe_env(), stdin=subprocess.PIPE,
                              stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         return client.SubprocessWrapper(p)
@@ -347,7 +348,7 @@ class DulwichMockSSHClientTest(CompatTestCase, DulwichClientTestBase):
         return client.SSHGitClient(b'localhost')
 
     def _build_path(self, path):
-        return self.gitroot + path
+        return self.gitroot.encode(sys.getfilesystemencoding()) + path
 
 
 class DulwichSubprocessClientTest(CompatTestCase, DulwichClientTestBase):
@@ -364,7 +365,7 @@ class DulwichSubprocessClientTest(CompatTestCase, DulwichClientTestBase):
         return client.SubprocessGitClient(stderr=subprocess.PIPE)
 
     def _build_path(self, path):
-        return self.gitroot + path
+        return self.gitroot.encode(sys.getfilesystemencoding()) + path
 
 
 class GitHTTPRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler):

dulwich/tests/test_client.py

@@ -531,19 +531,19 @@ class SSHGitClientTests(TestCase):
         client.get_ssh_vendor = self.real_vendor
 
     def test_default_command(self):
-        self.assertEqual(['git-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[b'upload-pack'] = (
             b'/usr/lib/git/git-upload-pack')
-        self.assertEqual(['/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_alternative_command_path_spaces(self):
         self.client.alternative_paths[b'upload-pack'] = (
             b'/usr/lib/git/git-upload-pack -ibla')
-        self.assertEqual(['/usr/lib/git/git-upload-pack', '-ibla'],
+        self.assertEqual([b'/usr/lib/git/git-upload-pack', b'-ibla'],
             self.client._get_cmd_path(b'upload-pack'))
 
     def test_connect(self):
@@ -553,13 +553,13 @@ class SSHGitClientTests(TestCase):
         client.username = b"username"
         client.port = 1337
 
-        client._connect(b"command", "/path/to/repo")
+        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", b"/path/to/repo"], server.command)
 
-        client._connect(b"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", b"~/path/to/repo"],
                           server.command)