Skip to content
Browse files

A whole raft of str versus bytes issues.

  • Loading branch information...
1 parent 533a336 commit 5647daf6bccf5c4f91398765cdf51e39a4a093fb @chrisrossi chrisrossi committed Sep 13, 2012
Showing with 91 additions and 65 deletions.
  1. +73 −47 acidfs/__init__.py
  2. +18 −18 acidfs/tests.py
View
120 acidfs/__init__.py
@@ -4,6 +4,7 @@
import logging
import os
import shutil
+import sys
import subprocess
import tempfile
import traceback
@@ -496,9 +497,9 @@ def tpc_finish(self, tx):
# If not updating current head, just write the commit to the ref
# file directly.
reffile = os.path.join(self.db, 'refs', 'heads', self.head)
- with open(reffile, 'w') as f:
+ with open(reffile, 'wb') as f:
f.write(self.next_commit)
- f.write('\n')
+ f.write(b'\n')
self.close()
@@ -618,33 +619,33 @@ def expect(expectation, *msg):
while line:
if state is None: # default, scanning for start of a change
- if line[0].isalpha():
+ if isalpha(line[0]):
# If first column is a letter, then we have the first
# line of a change, which describes the change.
line = line.strip()
- if line in ('added in local', 'removed in local',
- 'removed in both'):
+ if line in (b'added in local', b'removed in local',
+ b'removed in both'):
# We don't care about changes to our current tree.
# We already know about those.
pass
- elif line == 'added in remote':
+ elif line == b'added in remote':
# The head got a new file, we should grab it
state = _MERGE_ADDED_IN_REMOTE
extra_state = []
- elif line == 'removed in remote':
+ elif line == b'removed in remote':
# File got deleted from head, remove it
state = _MERGE_REMOVED_IN_REMOTE
extra_state = []
- elif line == 'changed in both':
+ elif line == b'changed in both':
# File was edited in both branches, see if we can
# patch
state = _MERGE_CHANGED_IN_BOTH
extra_state = []
- elif line == 'added in both':
+ elif line == b'added in both':
state = _MERGE_ADDED_IN_BOTH
extra_state = []
@@ -653,39 +654,39 @@ def expect(expectation, *msg):
raise ConflictError()
elif state is _MERGE_ADDED_IN_REMOTE:
- if line[0].isalpha() or line[0] == '@':
+ if isalpha(line[0]) or line.startswith(b'@'):
# Done collecting tree lines, only expecting one
expect(len(extra_state) == 1, 'Wrong number of lines')
whose, mode, oid, path = extra_state[0].split()
- expect(whose == 'their', 'Unexpected whose: %s', whose)
- expect(mode == '100644', 'Unexpected mode: %s', mode)
- parsed = path.split('/')
+ expect(whose == b'their', 'Unexpected whose: %s', whose)
+ expect(mode == b'100644', 'Unexpected mode: %s', mode)
+ parsed = path.decode('ascii').split('/')
folder = self.find(parsed[:-1])
expect(isinstance(folder, _TreeNode),
'Not a folder: %s', path)
- folder.set(parsed[-1], ('blob', oid, None))
+ folder.set(parsed[-1], (b'blob', oid, None))
state = extra_state = None
continue
else:
extra_state.append(line)
elif state is _MERGE_REMOVED_IN_REMOTE:
- if line[0].isalpha() or line[0] == '@':
+ if isalpha(line[0]) or line.startswith(b'@'):
# Done collecting tree lines, expect two, one for base,
# one for our copy, whose sha1s should match
expect(len(extra_state) == 2, 'Wrong number of lines')
whose, mode, oid, path = extra_state[0].split()
- expect(whose in ('our', 'base'), 'Unexpected whose: %s',
- whose)
- expect(mode == '100644', 'Unexpected mode: %s', mode)
+ expect(whose in (b'our', b'base'),
+ 'Unexpected whose: %s', whose)
+ expect(mode == b'100644', 'Unexpected mode: %s', mode)
whose, mode, oid2, path2 = extra_state[1].split()
- expect(whose in ('our', 'base'), 'Unexpected whose: %s',
- whose)
- expect(mode == '100644', 'Unexpected mode: %s', mode)
+ expect(whose in (b'our', b'base'),
+ 'Unexpected whose: %s', whose)
+ expect(mode == b'100644', 'Unexpected mode: %s', mode)
expect(oid == oid2, "SHA1s don't match")
expect(path == path2, "Paths don't match")
- path = path.split('/')
+ path = path.decode('ascii').split('/')
folder = self.find(path[:-1])
expect(isinstance(folder, _TreeNode), "Not a folder")
folder.remove(path[-1])
@@ -696,22 +697,22 @@ def expect(expectation, *msg):
extra_state.append(line)
elif state is _MERGE_CHANGED_IN_BOTH:
- if line[0] == '@':
+ if line.startswith(b'@'):
# Done collecting tree lines, expect three, one for base
# and one for each copy
expect(len(extra_state) == 3, 'Wrong number of lines')
whose, mode, oid, path = extra_state[0].split()
- expect(whose in ('base', 'our', 'their'),
+ expect(whose in (b'base', b'our', b'their'),
'Unexpected whose: %s', whose)
- expect(mode == '100644', 'Unexpected mode: %s', mode)
+ expect(mode == b'100644', 'Unexpected mode: %s', mode)
for extra_line in extra_state[1:]:
whose, mode, oid2, path2 = extra_line.split()
- expect(whose in ('base', 'our', 'their'),
+ expect(whose in (b'base', b'our', b'their'),
'Unexpected whose: %s', whose)
- expect(mode == '100644', 'Unexpected mode: %s',
+ expect(mode == b'100644', 'Unexpected mode: %s',
mode)
expect(path == path2, "Paths don't match")
- parsed = path.split('/')
+ parsed = path.decode('ascii').split('/')
folder = self.find(parsed[:-1])
expect(isinstance(folder, _TreeNode), "Not a folder")
name = parsed[-1]
@@ -724,8 +725,8 @@ def expect(expectation, *msg):
stdout=subprocess.PIPE,
stderr=subprocess.PIPE) as p:
f = p.stdin
- while line and not line[0].isalpha():
- if line[1:9] == '<<<<<<< ':
+ while line and not isalpha(line[0]):
+ if line[1:9] == b'<<<<<<< ':
raise ConflictError()
f.write(line)
line = stream.readline()
@@ -739,18 +740,18 @@ def expect(expectation, *msg):
extra_state.append(line)
elif state is _MERGE_ADDED_IN_BOTH:
- if line[0].isalpha() or line[0] == '@':
+ if isalpha(line[0]) or line.startswith(b'@'):
# Done collecting tree lines, expect two, one for base,
# one for our copy, whose sha1s should match
expect(len(extra_state) == 2, 'Wrong number of lines')
whose, mode, oid, path = extra_state[0].split()
- expect(whose in ('our', 'their'), 'Unexpected whose: %s',
+ expect(whose in (b'our', b'their'), 'Unexpected whose: %s',
whose)
- expect(mode == '100644', 'Unexpected mode: %s', mode)
+ expect(mode == b'100644', 'Unexpected mode: %s', mode)
whose, mode, oid2, path2 = extra_state[1].split()
- expect(whose in ('our', 'their'), 'Unexpected whose: %s',
- whose)
- expect(mode == '100644', 'Unexpected mode: %s', mode)
+ expect(whose in (b'our', b'their'),
+ 'Unexpected whose: %s', whose)
+ expect(mode == b'100644', 'Unexpected mode: %s', mode)
expect(path == path2, "Paths don't match")
# Either it's the same file or a different file.
if oid != oid2:
@@ -779,6 +780,8 @@ def read(cls, db, oid):
stdout=subprocess.PIPE, cwd=db) as lstree:
for line in lstree.stdout.readlines():
mode, type, oid, name = line.split()
+ name = str(name, 'ascii')
+ oid = str(oid, 'ascii')
contents[name] = (type, oid, None)
return node
@@ -793,9 +796,9 @@ def get(self, name):
if not obj:
return None
type, oid, obj = obj
- assert type in ('tree', 'blob')
+ assert type in (b'tree', b'blob')
if not obj:
- if type == 'tree':
+ if type == b'tree':
obj = _TreeNode.read(self.db, oid)
else:
obj = _Blob(self.db, oid)
@@ -815,15 +818,15 @@ def new_blob(self, name, prev):
obj = _NewBlob(self.db, prev)
obj.parent = self
obj.name = name
- self.contents[name] = ('blob', None, weakref.proxy(obj))
+ self.contents[name] = (b'blob', None, weakref.proxy(obj))
self.set_dirty()
return obj
def new_tree(self, name):
node = _TreeNode(self.db)
node.parent = self
node.name = name
- self.contents[name] = ('tree', None, node)
+ self.contents[name] = (b'tree', None, node)
self.set_dirty()
return node
@@ -849,17 +852,22 @@ def save(self):
continue # Nothing to do
if isinstance(obj, _NewBlob):
raise ValueError("Cannot commit transaction with open files.")
- elif type == 'tree' and (obj.dirty or not oid):
+ elif type == b'tree' and (obj.dirty or not oid):
new_oid = obj.save()
- self.contents[name] = ('tree', new_oid, None)
+ self.contents[name] = (b'tree', new_oid, None)
# Save tree object out to database
with _popen(['git', 'mktree'], cwd=self.db,
stdin=subprocess.PIPE, stdout=subprocess.PIPE) as proc:
for name, (type, oid, obj) in self.contents.items():
- mode = '100644' if type == 'blob' else '040000'
- proc.stdin.write('%s %s %s\t%s' % (mode, type, oid, name))
- proc.stdin.write('\n')
+ proc.stdin.write(b'100644' if type == b'blob' else b'040000')
+ proc.stdin.write(b' ')
+ proc.stdin.write(type)
+ proc.stdin.write(b' ')
+ proc.stdin.write(b(oid))
+ proc.stdin.write(b'\t')
+ proc.stdin.write(b(name))
+ proc.stdin.write(b'\n')
proc.stdin.close()
oid = proc.stdout.read().strip()
return oid
@@ -909,7 +917,7 @@ def close(self):
if retcode != 0:
raise subprocess.CalledProcessError(
retcode, 'git hash-object -w --stdin')
- self.parent.contents[self.name] = ('blob', oid, None)
+ self.parent.contents[self.name] = (b'blob', oid, None)
def writable(self):
return True
@@ -965,7 +973,10 @@ def _popen(args, **kw):
yield proc
for stream in (proc.stdin, proc.stdout, proc.stderr):
if stream is not None:
- stream.close()
+ if not stream.closed:
+ if stream.readable():
+ stream.read()
+ stream.close()
retcode = proc.wait()
if retcode != 0:
raise subprocess.CalledProcessError(retcode, repr(args))
@@ -1024,3 +1035,18 @@ def _check_output(*popenargs, **kwargs):
cmd = popenargs[0]
raise subprocess.CalledProcessError(retcode, cmd, output=output)
return output
+
+
+if sys.version_info[0] == 2:
+ b = lambda s: s
+ isalpha = lambda s: s.isalpha()
+else:
+ def b(s):
+ if isinstance(s, str):
+ s = bytes(s, 'ascii')
+ return s
+
+ aa, zz, AA, ZZ = ord('a'), ord('z'), ord('A'), ord('Z')
+ def isalpha(b):
+ return aa <= b <= zz or AA <= b <= ZZ
+
View
36 acidfs/tests.py
@@ -102,7 +102,7 @@ def test_read_write_file(self):
self.assertTrue(f.readable())
self.assertEqual(f.read(), b'Hello\n')
with open(actual_file) as f:
- self.assertEqual(f.read(), b'Hello\n')
+ self.assertEqual(f.read(), 'Hello\n')
transaction.commit() # Nothing to commit
def test_read_write_file_in_subfolder(self):
@@ -122,7 +122,7 @@ def test_read_write_file_in_subfolder(self):
with fs.open('foo/bar') as f:
self.assertEqual(f.read(), b'Hello\n')
with open(actual_file) as f:
- self.assertEqual(f.read(), b'Hello\n')
+ self.assertEqual(f.read(), 'Hello\n')
def test_read_write_file_in_subfolder_bare_repo(self):
fs = self.make_one(bare=True)
@@ -199,8 +199,8 @@ def test_commit_metadata(self):
transaction.commit()
output = _check_output(['git', 'log'], cwd=self.tmp)
- self.assertIn('Author: Fred Flintstone <fred@bed.rock>', output)
- self.assertIn('A test commit.', output)
+ self.assertIn(b'Author: Fred Flintstone <fred@bed.rock>', output)
+ self.assertIn(b'A test commit.', output)
def test_commit_metadata_extended_info_for_user(self):
fs = self.make_one()
@@ -212,8 +212,8 @@ def test_commit_metadata_extended_info_for_user(self):
transaction.commit()
output = _check_output(['git', 'log'], cwd=self.tmp)
- self.assertIn('Author: Fred Flintstone <fred@bed.rock>', output)
- self.assertIn('A test commit.', output)
+ self.assertIn(b'Author: Fred Flintstone <fred@bed.rock>', output)
+ self.assertIn(b'A test commit.', output)
def test_modify_file(self):
fs = self.make_one()
@@ -226,10 +226,10 @@ def test_modify_file(self):
fprint(f, b"Hello!")
self.assertEqual(fs.open('foo').read(), b'Howdy!\n')
self.assertEqual(fs.open('foo').read(), b'Hello!\n')
- self.assertEqual(open(path).read(), b'Howdy!\n')
+ self.assertEqual(open(path).read(), 'Howdy!\n')
transaction.commit()
- self.assertEqual(open(path).read(), b'Hello!\n')
+ self.assertEqual(open(path).read(), 'Hello!\n')
def test_error_writing_blob(self):
fs = self.make_one()
@@ -255,13 +255,13 @@ def test_append(self):
with fs.open('foo', 'a') as f:
fprint(f, b'Daddy!')
self.assertEqual(fs.open('foo', 'rb').read(), b'Hello!\n')
- self.assertEqual(open(path).read(), b'Hello!\n')
+ self.assertEqual(open(path).read(), 'Hello!\n')
self.assertEqual(fs.open('foo', 'rb').read(), b'Hello!\nDaddy!\n')
- self.assertEqual(open(path).read(), b'Hello!\n')
+ self.assertEqual(open(path).read(), 'Hello!\n')
transaction.commit()
self.assertEqual(fs.open('foo', 'rb').read(), b'Hello!\nDaddy!\n')
- self.assertEqual(open(path).read(), b'Hello!\nDaddy!\n')
+ self.assertEqual(open(path).read(), 'Hello!\nDaddy!\n')
def test_rm(self):
fs = self.make_one()
@@ -373,13 +373,13 @@ def test_mv(self):
self.assertFalse(fs.exists('one/b/foo'))
self.assertEqual(fs.open('one/a/bar').read(), b'Howdy!')
self.assertTrue(pexists(j(self.tmp, 'one', 'b', 'foo')))
- self.assertEqual(open(j(self.tmp, 'one', 'a', 'bar')).read(), b'Hello!')
+ self.assertEqual(open(j(self.tmp, 'one', 'a', 'bar')).read(), 'Hello!')
transaction.commit()
self.assertFalse(fs.exists('one/b/foo'))
- self.assertEqual(fs.open('one/a/bar').read(), 'Howdy!')
+ self.assertEqual(fs.open('one/a/bar').read(), b'Howdy!')
self.assertFalse(pexists(j(self.tmp, 'one', 'b', 'foo')))
- self.assertEqual(open(j(self.tmp, 'one', 'a', 'bar')).read(), b'Howdy!')
+ self.assertEqual(open(j(self.tmp, 'one', 'a', 'bar')).read(), 'Howdy!')
fs.mv('one/a', 'one/b')
self.assertFalse(fs.exists('one/a'))
@@ -445,7 +445,7 @@ def test_conflict_error_on_first_commit(self):
from acidfs import ConflictError
fs = self.make_one()
fs.open('foo', 'w').write(b'Hello!')
- open(os.path.join(self.tmp, 'foo'), 'w').write(b'Howdy!')
+ open(os.path.join(self.tmp, 'foo'), 'w').write('Howdy!')
_check_output(['git', 'add', '.'], cwd=self.tmp)
_check_output(['git', 'commit', '-m', 'Haha! First!'], cwd=self.tmp)
with self.assertRaises(ConflictError):
@@ -457,7 +457,7 @@ def test_unable_to_merge_file(self):
fs.open('foo', 'w').write(b'Hello!')
transaction.commit()
fs.open('foo', 'w').write(b'Party!')
- open(os.path.join(self.tmp, 'foo'), 'w').write(b'Howdy!')
+ open(os.path.join(self.tmp, 'foo'), 'wb').write(b'Howdy!')
_check_output(['git', 'add', '.'], cwd=self.tmp)
_check_output(['git', 'commit', '-m', 'Haha! First!'], cwd=self.tmp)
with self.assertRaises(ConflictError):
@@ -469,7 +469,7 @@ def test_merge_add_file(self):
transaction.commit()
fs.open('bar', 'w').write(b'Howdy!\n')
- open(os.path.join(self.tmp, 'baz'), 'w').write(b'Ciao!\n')
+ open(os.path.join(self.tmp, 'baz'), 'w').write('Ciao!\n')
_check_output(['git', 'add', 'baz'], cwd=self.tmp)
_check_output(['git', 'commit', '-m', 'haha'], cwd=self.tmp)
transaction.commit()
@@ -656,4 +656,4 @@ def test_called_process_error(self, Popen):
def fprint(f, s):
f.write(s)
- f.write('\n')
+ f.write(b'\n')

0 comments on commit 5647daf

Please sign in to comment.
Something went wrong with that request. Please try again.