Skip to content

Commit

Permalink
Use bytes internally everywhere for client paths.
Browse files Browse the repository at this point in the history
  • Loading branch information
@jelmer
jelmer committed Sep 13, 2015
1 parent 97bf01e commit 06b936b
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 28 deletions.
26 changes: 20 additions & 6 deletions dulwich/client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,10 @@ def __init__(self, host, port=None, *args, **kwargs):
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
Expand Down Expand Up @@ -695,6 +699,10 @@ def __init__(self, *args, **kwargs):
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()
Expand Down Expand Up @@ -980,17 +988,23 @@ def __init__(self, host, port=None, username=None, *args, **kwargs):

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)
Expand Down
2 changes: 2 additions & 0 deletions dulwich/porcelain.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
31 changes: 16 additions & 15 deletions dulwich/tests/compat/test_client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def _do_send_pack(self):
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):
Expand All @@ -126,7 +126,7 @@ def test_send_without_report_status(self):
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()

Expand Down Expand Up @@ -164,7 +164,7 @@ def test_send_pack_one_error(self):
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])
Expand All @@ -182,7 +182,7 @@ def test_send_pack_multiple_errors(self):
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(
Expand All @@ -196,15 +196,15 @@ def test_send_pack_multiple_errors(self):
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())

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()
Expand All @@ -216,7 +216,7 @@ def test_incremental_fetch_pack(self):
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()
Expand All @@ -225,7 +225,7 @@ def test_fetch_pack_no_side_band_64k(self):
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()
Expand All @@ -235,7 +235,7 @@ def test_fetch_pack_zero_sha(self):
# 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])
Expand All @@ -251,12 +251,12 @@ def test_send_remove_branch(self):
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:
Expand Down Expand Up @@ -312,7 +312,7 @@ def _client(self):
return client.TCPGitClient(b'localhost')

def _build_path(self, path):
return path.encode(sys.getfilesystemencoding())
return path

if sys.platform == 'win32':
@expectedFailure
Expand All @@ -321,10 +321,11 @@ def test_fetch_pack_no_side_band_64k(self):


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)
Expand All @@ -347,7 +348,7 @@ def _client(self):
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):
Expand All @@ -364,7 +365,7 @@ def _client(self):
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):
Expand Down
14 changes: 7 additions & 7 deletions dulwich/tests/test_client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -531,19 +531,19 @@ def tearDown(self):
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):
Expand All @@ -553,13 +553,13 @@ def test_connect(self):
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)


Expand Down

0 comments on commit 06b936b

Please sign in to comment.