Merge branch 'master' into experimental

HACKING → CONTRIBUTING

@@ -5,7 +5,7 @@ kernel/git coding style.
 
 Where possible include updates to NEWS along with your improvements.
 
-New functionality and bug fixes should be accompanied with matching unit tests.
+New functionality and bug fixes should be accompanied by matching unit tests.
 
 Coding style
 ------------
@@ -29,8 +29,8 @@ unittest2 (which you will need to have installed) on older versions of Python.
 String Types
 ------------
 Like Linux, Git treats filenames as arbitrary bytestrings. There is no prescribed
-encoding for these strings, and although it is fairly common to use UTF-8, anything
-can and is used as encoding with Git.
+encoding for these strings, and although it is fairly common to use UTF-8, any
+raw byte strings are supported.
 
 For this reason, Dulwich internally treats git-based filenames as bytestrings. It is up
 to the Dulwich API user to encode and decode them if necessary.
@@ -39,3 +39,13 @@ to the Dulwich API user to encode and decode them if necessary.
 * object sha1 digests (20 bytes long): bytes
 * object sha1 hexdigests (40 bytes long): str (bytestrings on python2, strings on python3)
 
+Merge requests
+--------------
+Please either send pull requests to the author (jelmer@jelmer.uk) or create new pull
+requests on GitHub.
+
+Licensing
+---------
+Dulwich is currently licensed under the GNU General Public License, version 2
+or later. We would like to change the license to Apachev2 or later eventually;
+new contributions should be made under dual GPLv2+/Apachev2+.

NEWS

@@ -4,6 +4,19 @@
 
   * Extended Python3 support to most of the codebase.
     (Gary van der Merwe, Jelmer Vernooij)
+  * The `Repo` object has a new `close` method that can be called to close any
+    open resources. (Gary van der Merwe)
+  * Support 'git.bat' in SubprocessGitClient on Windows.
+    (Stefan Zimmermann)
+  * Advertise 'ofs-delta' capability in receive-pack server side
+    capabilities. (Jelmer Vernooij)
+  * Switched `default_local_git_client_cls` to `LocalGitClient`.
+    (Gary van der Merwe)
+
+ BUG FIXES
+
+  * Fix handling of 'done' in graph walker and implement the
+    'no-done' capability. (Tommy Yu, #88)
 
 0.10.1  2015-03-25
 

README.md

@@ -1,7 +1,7 @@
-This is the Dulwich project.
-
 [![Build Status](https://travis-ci.org/jelmer/dulwich.png?branch=master)](https://travis-ci.org/jelmer/dulwich)
 
+This is the Dulwich project.
+
 It aims to provide an interface to git repos (both local and remote) that
 doesn't call out to git directly but instead uses pure Python.
 
@@ -43,9 +43,8 @@ Help
 There is a #dulwich IRC channel on Freenode, and a dulwich mailing list at
 https://launchpad.net/~dulwich-users.
 
-Python3
--------
+Supported versions of Python
+----------------------------
 
-The process of porting to Python3 is ongoing. Please note that although the
-test suite pass in python3, this is due to the tests of features that are not
-yet ported being skipped, and *not* an indication that the port is complete.
+At the moment, Dulwich supports (and is tested on) CPython 2.6, 2.7, 3.4, 3.5 and Pypy.
+The ``dulwich.web`` module is currently broken on Python 3 (issue #295).

appveyor.yml

@@ -2,9 +2,15 @@ environment:
   matrix:
     - PYTHON: "C:\\Python27"
       PYTHON_ARCH: "32"
+      PYWIN32_URL: "https://downloads.sourceforge.net/project/pywin32/pywin32/Build%20219/pywin32-219.win32-py2.7.exe"
 
     - PYTHON: "C:\\Python34"
       PYTHON_ARCH: "32"
+      PYWIN32_URL: "https://downloads.sourceforge.net/project/pywin32/pywin32/Build%20219/pywin32-219.win32-py3.4.exe"
+
+install:
+  - ps: (new-object net.webclient).DownloadFile($env:PYWIN32_URL, 'c:\\pywin32.exe')
+  - "%PYTHON%/Scripts/easy_install.exe c:\\pywin32.exe"
 
 build: off
 

dulwich/client.py

@@ -38,6 +38,7 @@ Known capabilities that are not supported:
 
 __docformat__ = 'restructuredText'
 
+from contextlib import closing
 from io import BytesIO, BufferedReader
 import dulwich
 import select
@@ -649,6 +650,21 @@ class SubprocessWrapper(object):
         self.proc.wait()
 
 
+def find_git_command():
+    """Find command to run for system Git (usually C Git).
+    """
+    if sys.platform == 'win32': # support .exe, .bat and .cmd
+        try: # to avoid overhead
+            import win32api
+        except ImportError: # run through cmd.exe with some overhead
+            return ['cmd', '/c', 'git']
+        else:
+            status, git = win32api.FindExecutable('git')
+            return [git]
+    else:
+        return ['git']
+
+
 class SubprocessGitClient(TraditionalGitClient):
     """Git client that talks to a server using a subprocess."""
 
@@ -660,9 +676,14 @@ class SubprocessGitClient(TraditionalGitClient):
             del kwargs['stderr']
         TraditionalGitClient.__init__(self, *args, **kwargs)
 
+    git_command = None
+
     def _connect(self, service, path):
         import subprocess
-        argv = ['git', service, path]
+        if self.git_command is None:
+            git_command = find_git_command()
+        argv = git_command + [service, path]
+        argv = ['git', service.decode('ascii'), path]
         p = SubprocessWrapper(
             subprocess.Popen(argv, bufsize=0, stdin=subprocess.PIPE,
                              stdout=subprocess.PIPE,
@@ -702,26 +723,26 @@ class LocalGitClient(GitClient):
         """
         from dulwich.repo import Repo
 
-        target = Repo(path)
-        old_refs = target.get_refs()
-        new_refs = determine_wants(old_refs)
+        with closing(Repo(path)) as target:
+            old_refs = target.get_refs()
+            new_refs = determine_wants(dict(old_refs))
 
-        have = [sha1 for sha1 in old_refs.values() if sha1 != ZERO_SHA]
-        want = []
-        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:
-                want.append(new_sha1)
+            have = [sha1 for sha1 in old_refs.values() if sha1 != ZERO_SHA]
+            want = []
+            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:
+                    want.append(new_sha1)
 
-        if not want and old_refs == new_refs:
-            return new_refs
+            if not want and old_refs == new_refs:
+                return new_refs
 
-        target.object_store.add_objects(generate_pack_contents(have, want))
+            target.object_store.add_objects(generate_pack_contents(have, want))
 
-        for name, sha in new_refs.items():
-            target.refs[name] = sha
+            for name, sha in new_refs.items():
+                target.refs[name] = sha
 
         return new_refs
 
@@ -736,9 +757,9 @@ class LocalGitClient(GitClient):
         :return: remote refs as dictionary
         """
         from dulwich.repo import Repo
-        r = Repo(path)
-        return r.fetch(target, determine_wants=determine_wants,
-                       progress=progress)
+        with closing(Repo(path)) as r:
+            return r.fetch(target, determine_wants=determine_wants,
+                           progress=progress)
 
     def fetch_pack(self, path, determine_wants, graph_walker, pack_data,
                    progress=None):
@@ -750,18 +771,18 @@ class LocalGitClient(GitClient):
         :param progress: Callback for progress reports (strings)
         """
         from dulwich.repo import Repo
-        r = Repo(path)
-        objects_iter = r.fetch_objects(determine_wants, graph_walker, progress)
+        with closing(Repo(path)) as r:
+            objects_iter = r.fetch_objects(determine_wants, graph_walker, progress)
 
-        # Did the process short-circuit (e.g. in a stateless RPC call)? Note
-        # that the client still expects a 0-object pack in most cases.
-        if objects_iter is None:
-            return
-        write_pack_objects(ProtocolFile(None, pack_data), objects_iter)
+            # Did the process short-circuit (e.g. in a stateless RPC call)? Note
+            # that the client still expects a 0-object pack in most cases.
+            if objects_iter is None:
+                return
+            write_pack_objects(ProtocolFile(None, pack_data), objects_iter)
 
 
 # What Git client to use for local access
-default_local_git_client_cls = SubprocessGitClient
+default_local_git_client_cls = LocalGitClient
 
 
 class SSHVendor(object):
@@ -930,13 +951,14 @@ class SSHGitClient(TraditionalGitClient):
         self.alternative_paths = {}
 
     def _get_cmd_path(self, cmd):
-        return self.alternative_paths.get(cmd, b'git-' + cmd)
+        cmd = cmd.decode('ascii')
+        return self.alternative_paths.get(cmd, 'git-' + cmd)
 
     def _connect(self, cmd, path):
-        if path.startswith(b"/~"):
+        if path.startswith("/~"):
             path = path[1:]
         con = get_ssh_vendor().run_command(
-            self.host, [self._get_cmd_path(cmd) + b" '" + path + b"'"],
+            self.host, [self._get_cmd_path(cmd), path],
             port=self.port, username=self.username)
         return (Protocol(con.read, con.write, con.close,
                          report_activity=self._report_activity),
@@ -978,6 +1000,9 @@ class HttpGitClient(GitClient):
             self.opener = opener
         GitClient.__init__(self, *args, **kwargs)
 
+    def __repr__(self):
+        return "%s(%r, dumb=%r)" % (type(self).__name__, self.base_url, self.dumb)
+
     def _get_url(self, path):
         return urlparse.urljoin(self.base_url, path).rstrip("/") + "/"
 

dulwich/file.py

@@ -104,7 +104,7 @@ class _GitFile(object):
                      'truncate', 'write', 'writelines')
     def __init__(self, filename, mode, bufsize):
         self._filename = filename
-        self._lockfilename = self._filename + b'.lock'
+        self._lockfilename = '%s.lock' % self._filename
         fd = os.open(self._lockfilename,
             os.O_RDWR | os.O_CREAT | os.O_EXCL | getattr(os, "O_BINARY", 0))
         self._file = os.fdopen(fd, mode, bufsize)

dulwich/hooks.py

@@ -20,6 +20,7 @@
 
 import os
 import subprocess
+import sys
 import tempfile
 
 from dulwich.errors import (
@@ -71,6 +72,11 @@ class ShellHook(Hook):
         self.pre_exec_callback = pre_exec_callback
         self.post_exec_callback = post_exec_callback
 
+        if sys.version_info[0] == 2 and sys.platform == 'win32':
+            # Python 2 on windows does not support unicode file paths
+            # http://bugs.python.org/issue1759845
+            self.filepath = self.filepath.encode(sys.getfilesystemencoding())
+
     def execute(self, *args):
         """Execute the hook with given args"""
 
@@ -100,7 +106,7 @@ class PreCommitShellHook(ShellHook):
     """pre-commit shell hook"""
 
     def __init__(self, controldir):
-        filepath = os.path.join(controldir, b'hooks', b'pre-commit')
+        filepath = os.path.join(controldir, 'hooks', 'pre-commit')
 
         ShellHook.__init__(self, 'pre-commit', filepath, 0)
 
@@ -109,7 +115,7 @@ class PostCommitShellHook(ShellHook):
     """post-commit shell hook"""
 
     def __init__(self, controldir):
-        filepath = os.path.join(controldir, b'hooks', b'post-commit')
+        filepath = os.path.join(controldir, 'hooks', 'post-commit')
 
         ShellHook.__init__(self, 'post-commit', filepath, 0)
 
@@ -122,7 +128,7 @@ class CommitMsgShellHook(ShellHook):
     """
 
     def __init__(self, controldir):
-        filepath = os.path.join(controldir, b'hooks', b'commit-msg')
+        filepath = os.path.join(controldir, 'hooks', 'commit-msg')
 
         def prepare_msg(*args):
             (fd, path) = tempfile.mkstemp()

dulwich/index.py

@@ -462,13 +462,13 @@ def validate_path(path, element_validator=validate_path_element_default):
         return True
 
 
-def build_index_from_tree(prefix, index_path, object_store, tree_id,
+def build_index_from_tree(root_path, index_path, object_store, tree_id,
                           honor_filemode=True,
                           validate_path_element=validate_path_element_default):
     """Generate and materialize index from a tree
 
     :param tree_id: Tree to materialize
-    :param prefix: Target dir for materialized index files
+    :param root_path: Target dir for materialized index files
     :param index_path: Target path for generated index
     :param object_store: Non-empty object store holding tree contents
     :param honor_filemode: An optional flag to honor core.filemode setting in
@@ -477,18 +477,17 @@ def build_index_from_tree(prefix, index_path, object_store, tree_id,
         default just refuses .git and .. directories.
 
     :note:: existing index is wiped and contents are not merged
-        in a working dir. Suiteable only for fresh clones.
+        in a working dir. Suitable only for fresh clones.
     """
 
-    if not isinstance(prefix, bytes):
-        prefix = prefix.encode(sys.getfilesystemencoding())
-
     index = Index(index_path)
+    if not isinstance(root_path, bytes):
+        root_path = root_path.encode(sys.getfilesystemencoding())
 
     for entry in object_store.iter_tree_contents(tree_id):
-        if not validate_path(entry.path):
+        if not validate_path(entry.path, validate_path_element):
             continue
-        full_path = os.path.join(prefix, entry.path)
+        full_path = _tree_to_fs_path(root_path, entry.path)
 
         if not os.path.exists(os.path.dirname(full_path)):
             os.makedirs(os.path.dirname(full_path))
@@ -504,39 +503,73 @@ def build_index_from_tree(prefix, index_path, object_store, tree_id,
     index.write()
 
 
-def blob_from_path_and_stat(path, st):
+def blob_from_path_and_stat(fs_path, st):
     """Create a blob from a path and a stat object.
 
-    :param path: Full path to file
+    :param fs_path: Full file system path to file
     :param st: A stat object
     :return: A `Blob` object
     """
+    assert isinstance(fs_path, bytes)
     blob = Blob()
     if not stat.S_ISLNK(st.st_mode):
-        with open(path, 'rb') as f:
+        with open(fs_path, 'rb') as f:
             blob.data = f.read()
     else:
-        if not isinstance(path, bytes):
-            blob.data = os.readlink(path.encode(sys.getfilesystemencoding()))
-        else:
-            blob.data = os.readlink(path)
-
+        blob.data = os.readlink(fs_path)
     return blob
 
 
-def get_unstaged_changes(index, path):
+def get_unstaged_changes(index, root_path):
     """Walk through an index and check for differences against working tree.
 
     :param index: index to check
-    :param path: path in which to find files
+    :param root_path: path in which to find files
     :return: iterator over paths with unstaged changes
     """
     # For each entry in the index check the sha1 & ensure not staged
-    if not isinstance(path, bytes):
-        path = path.encode(sys.getfilesystemencoding())
+    if not isinstance(root_path, bytes):
+        root_path = root_path.encode(sys.getfilesystemencoding())
 
-    for name, entry in index.iteritems():
-        fp = os.path.join(path, name)
-        blob = blob_from_path_and_stat(fp, os.lstat(fp))
+    for tree_path, entry in index.iteritems():
+        full_path = _tree_to_fs_path(root_path, tree_path)
+        blob = blob_from_path_and_stat(full_path, os.lstat(full_path))
         if blob.id != entry.sha:
-            yield name
+            yield tree_path
+
+
+os_sep_bytes = os.sep.encode('ascii')
+
+
+def _tree_to_fs_path(root_path, tree_path):
+    """Convert a git tree path to a file system path.
+
+    :param root_path: Root filesystem path
+    :param tree_path: Git tree path as bytes
+
+    :return: File system path.
+    """
+    assert isinstance(tree_path, bytes)
+    if os_sep_bytes != b'/':
+        sep_corrected_path = tree_path.replace(b'/', os_sep_bytes)
+    else:
+        sep_corrected_path = tree_path
+    return os.path.join(root_path, sep_corrected_path)
+
+
+def _fs_to_tree_path(fs_path):
+    """Convert a file system path to a git tree path.
+
+    :param fs_path: File system path.
+
+    :return:  Git tree path as bytes
+    """
+    if not isinstance(fs_path, bytes):
+        fs_path_bytes = fs_path.encode(sys.getfilesystemencoding())
+    else:
+        fs_path_bytes = fs_path
+    if os_sep_bytes != b'/':
+        tree_path = fs_path_bytes.replace(os_sep_bytes, b'/')
+    else:
+        tree_path = fs_path_bytes
+    return tree_path

dulwich/object_store.py

@@ -63,8 +63,8 @@ from dulwich.pack import (
     PackStreamCopier,
     )
 
-INFODIR = b'info'
-PACKDIR = b'pack'
+INFODIR = 'info'
+PACKDIR = 'pack'
 
 
 class BaseObjectStore(object):
@@ -425,7 +425,7 @@ class DiskObjectStore(PackBasedObjectStore):
 
     def _read_alternate_paths(self):
         try:
-            f = GitFile(os.path.join(self.path, INFODIR, b'alternates'),
+            f = GitFile(os.path.join(self.path, INFODIR, "alternates"),
                     'rb')
         except (OSError, IOError) as e:
             if e.errno == errno.ENOENT:
@@ -437,9 +437,9 @@ class DiskObjectStore(PackBasedObjectStore):
                 if l[0] == b"#":
                     continue
                 if os.path.isabs(l):
-                    yield l
+                    yield l.decode(sys.getfilesystemencoding())
                 else:
-                    yield os.path.join(self.path, l)
+                    yield os.path.join(self.path, l).decode(sys.getfilesystemencoding())
 
     def add_alternate_path(self, path):
         """Add an alternate path to this object store.
@@ -449,7 +449,7 @@ class DiskObjectStore(PackBasedObjectStore):
         except OSError as e:
             if e.errno != errno.EEXIST:
                 raise
-        alternates_path = os.path.join(self.path, INFODIR, b'alternates')
+        alternates_path = os.path.join(self.path, INFODIR, "alternates")
         with GitFile(alternates_path, 'wb') as f:
             try:
                 orig_f = open(alternates_path, 'rb')
@@ -459,7 +459,7 @@ class DiskObjectStore(PackBasedObjectStore):
             else:
                 with orig_f:
                     f.write(orig_f.read())
-            f.write(path + b"\n")
+            f.write(path.encode(sys.getfilesystemencoding()) + b"\n")
 
         if not os.path.isabs(path):
             path = os.path.join(self.path, path)
@@ -477,10 +477,10 @@ class DiskObjectStore(PackBasedObjectStore):
         self._pack_cache_time = os.stat(self.pack_dir).st_mtime
         pack_files = set()
         for name in pack_dir_contents:
-            assert type(name) is bytes
+            assert isinstance(name, basestring if sys.version_info[0] == 2 else str)
             # TODO: verify that idx exists first
-            if name.startswith(b'pack-') and name.endswith(b'.pack'):
-                pack_files.add(name[:-len(b'.pack')])
+            if name.startswith("pack-") and name.endswith(".pack"):
+                pack_files.add(name[:-len(".pack")])
 
         # Open newly appeared pack files
         for f in pack_files:
@@ -507,7 +507,7 @@ class DiskObjectStore(PackBasedObjectStore):
             if len(base) != 2:
                 continue
             for rest in os.listdir(os.path.join(self.path, base)):
-                yield (base+rest)
+                yield (base+rest).encode(sys.getfilesystemencoding())
 
     def _get_loose_object(self, sha):
         path = self._get_shafile_path(sha)
@@ -524,7 +524,8 @@ class DiskObjectStore(PackBasedObjectStore):
     def _get_pack_basepath(self, entries):
         suffix = iter_sha1(entry[0] for entry in entries)
         # TODO: Handle self.pack_dir being bytes
-        return os.path.join(self.pack_dir, b"pack-" + suffix)
+        suffix = suffix.decode('ascii')
+        return os.path.join(self.pack_dir, "pack-" + suffix)
 
     def _complete_thin_pack(self, f, path, copier, indexer):
         """Move a specific file containing a pack into the pack directory.
@@ -566,10 +567,10 @@ class DiskObjectStore(PackBasedObjectStore):
         # Move the pack in.
         entries.sort()
         pack_base_name = self._get_pack_basepath(entries)
-        os.rename(path, pack_base_name + b'.pack')
+        os.rename(path, pack_base_name + '.pack')
 
         # Write the index.
-        index_file = GitFile(pack_base_name + b'.idx', 'wb')
+        index_file = GitFile(pack_base_name + '.idx', 'wb')
         try:
             write_pack_index_v2(index_file, entries, pack_sha)
             index_file.close()
@@ -596,10 +597,7 @@ class DiskObjectStore(PackBasedObjectStore):
         :return: A Pack object pointing at the now-completed thin pack in the
             objects/pack directory.
         """
-        fd, path = tempfile.mkstemp(
-            dir=self.path.decode(sys.getfilesystemencoding()),
-            prefix='tmp_pack_')
-        path = path.encode(sys.getfilesystemencoding())
+        fd, path = tempfile.mkstemp(dir=self.path, prefix='tmp_pack_')
         with os.fdopen(fd, 'w+b') as f:
             indexer = PackIndexer(f, resolve_ext_ref=self.get_raw)
             copier = PackStreamCopier(read_all, read_some, f,
@@ -618,9 +616,9 @@ class DiskObjectStore(PackBasedObjectStore):
         with PackData(path) as p:
             entries = p.sorted_entries()
             basename = self._get_pack_basepath(entries)
-            with GitFile(basename+b'.idx', "wb") as f:
+            with GitFile(basename+".idx", "wb") as f:
                 write_pack_index_v2(f, entries, p.get_stored_checksum())
-        os.rename(path, basename + b'.pack')
+        os.rename(path, basename + ".pack")
         final_pack = Pack(basename)
         self._add_known_pack(basename, final_pack)
         return final_pack
@@ -632,8 +630,7 @@ class DiskObjectStore(PackBasedObjectStore):
             call when the pack is finished and an abort
             function.
         """
-        pack_dir_str = self.pack_dir.decode(sys.getfilesystemencoding())
-        fd, path = tempfile.mkstemp(dir=pack_dir_str, suffix='.pack')
+        fd, path = tempfile.mkstemp(dir=self.pack_dir, suffix=".pack")
         f = os.fdopen(fd, 'wb')
         def commit():
             os.fsync(fd)
@@ -672,7 +669,7 @@ class DiskObjectStore(PackBasedObjectStore):
         except OSError as e:
             if e.errno != errno.EEXIST:
                 raise
-        os.mkdir(os.path.join(path, INFODIR))
+        os.mkdir(os.path.join(path, "info"))
         os.mkdir(os.path.join(path, PACKDIR))
         return cls(path)
 
@@ -1044,7 +1041,7 @@ class MissingObjectFinder(object):
         if sha in self._tagged:
             self.add_todo([(self._tagged[sha], None, True)])
         self.sha_done.add(sha)
-        self.progress("counting objects: %d\r" % len(self.sha_done))
+        self.progress(("counting objects: %d\r" % len(self.sha_done)).encode('ascii'))
         return (sha, name)
 
     __next__ = next

dulwich/objects.py

@@ -112,10 +112,12 @@ def hex_to_filename(path, hex):
     # os.path.join accepts bytes or unicode, but all args must be of the same
     # type. Make sure that hex which is expected to be bytes, is the same type
     # as path.
-    directory = hex[:2]
+    if getattr(path, 'encode', None) is not None:
+        hex = hex.decode('ascii')
+    dir = hex[:2]
     file = hex[2:]
     # Check from object dir
-    return os.path.join(path, directory, file)
+    return os.path.join(path, dir, file)
 
 
 def filename_to_hex(filename):

dulwich/pack.py

@@ -1474,12 +1474,12 @@ def write_pack(filename, objects, deltify=None, delta_window_size=None):
     :param deltify: Whether to deltify pack objects
     :return: Tuple with checksum of pack file and index file
     """
-    with GitFile(filename + b'.pack', 'wb') as f:
+    with GitFile(filename + '.pack', 'wb') as f:
         entries, data_sum = write_pack_objects(f, objects,
             delta_window_size=delta_window_size, deltify=deltify)
     entries = [(k, v[0], v[1]) for (k, v) in entries.items()]
     entries.sort()
-    with GitFile(filename + b'.idx', 'wb') as f:
+    with GitFile(filename + '.idx', 'wb') as f:
         return data_sum, write_pack_index_v2(f, entries, data_sum)
 
 
@@ -1785,8 +1785,8 @@ class Pack(object):
         self._basename = basename
         self._data = None
         self._idx = None
-        self._idx_path = self._basename + b'.idx'
-        self._data_path = self._basename + b'.pack'
+        self._idx_path = self._basename + '.idx'
+        self._data_path = self._basename + '.pack'
         self._data_load = lambda: PackData(self._data_path)
         self._idx_load = lambda: load_pack_index(self._idx_path)
         self.resolve_ext_ref = resolve_ext_ref
@@ -1795,7 +1795,7 @@ class Pack(object):
     def from_lazy_objects(self, data_fn, idx_fn):
         """Create a new pack object from callables to load pack data and
         index objects."""
-        ret = Pack(b'')
+        ret = Pack('')
         ret._data_load = data_fn
         ret._idx_load = idx_fn
         return ret
@@ -1803,7 +1803,7 @@ class Pack(object):
     @classmethod
     def from_objects(self, data, idx):
         """Create a new pack object from pack data and index objects."""
-        ret = Pack(b'')
+        ret = Pack('')
         ret._data_load = lambda: data
         ret._idx_load = lambda: idx
         return ret
@@ -1930,7 +1930,7 @@ class Pack(object):
                     determine whether or not a .keep file is obsolete.
         :return: The path of the .keep file, as a string.
         """
-        keepfile_name = self._basename + b'.keep'
+        keepfile_name = '%s.keep' % self._basename
         with GitFile(keepfile_name, 'wb') as keepfile:
             if msg:
                 keepfile.write(msg)

dulwich/patch.py

@@ -22,7 +22,6 @@ These patches are basically unified diffs with some extra metadata tacked
 on.
 """
 
-from io import BytesIO
 from difflib import SequenceMatcher
 import email.parser
 import time
@@ -35,20 +34,23 @@ from dulwich.objects import (
 FIRST_FEW_BYTES = 8000
 
 
-def write_commit_patch(f, commit, contents, progress, version=None):
+def write_commit_patch(f, commit, contents, progress, version=None, encoding=None):
     """Write a individual file patch.
 
     :param commit: Commit object
     :param progress: Tuple with current patch number and total.
     :return: tuple with filename and contents
     """
+    encoding = encoding or getattr(f, "encoding", "ascii")
+    if type(contents) is str:
+        contents = contents.encode(encoding)
     (num, total) = progress
-    f.write("From %s %s\n" % (commit.id, time.ctime(commit.commit_time)))
-    f.write("From: %s\n" % commit.author)
-    f.write("Date: %s\n" % time.strftime("%a, %d %b %Y %H:%M:%S %Z"))
-    f.write("Subject: [PATCH %d/%d] %s\n" % (num, total, commit.message))
-    f.write("\n")
-    f.write("---\n")
+    f.write(b"From " + commit.id + b" " + time.ctime(commit.commit_time).encode(encoding) + b"\n")
+    f.write(b"From: " + commit.author + b"\n")
+    f.write(b"Date: " + time.strftime("%a, %d %b %Y %H:%M:%S %Z").encode(encoding) + b"\n")
+    f.write(("Subject: [PATCH %d/%d] " % (num, total)).encode(encoding) + commit.message + b"\n")
+    f.write(b"\n")
+    f.write(b"---\n")
     try:
         import subprocess
         p = subprocess.Popen(["diffstat"], stdout=subprocess.PIPE,
@@ -58,14 +60,14 @@ def write_commit_patch(f, commit, contents, progress, version=None):
     else:
         (diffstat, _) = p.communicate(contents)
         f.write(diffstat)
-        f.write("\n")
+        f.write(b"\n")
     f.write(contents)
-    f.write("-- \n")
+    f.write(b"-- \n")
     if version is None:
         from dulwich import __version__ as dulwich_version
-        f.write("Dulwich %d.%d.%d\n" % dulwich_version)
+        f.write(b"Dulwich %d.%d.%d\n" % dulwich_version)
     else:
-        f.write("%s\n" % version)
+        f.write(version.encode(encoding) + b"\n")
 
 
 def get_summary(commit):
@@ -77,7 +79,7 @@ def get_summary(commit):
     return commit.message.splitlines()[0].replace(" ", "-")
 
 
-def unified_diff(a, b, fromfile='', tofile='', n=3):
+def unified_diff(a, b, fromfile, tofile, n=3):
     """difflib.unified_diff that doesn't write any dates or trailing spaces.
 
     Based on the same function in Python2.6.5-rc2's difflib.py
@@ -85,26 +87,27 @@ def unified_diff(a, b, fromfile='', tofile='', n=3):
     started = False
     for group in SequenceMatcher(None, a, b).get_grouped_opcodes(n):
         if not started:
-            yield '--- %s\n' % fromfile
-            yield '+++ %s\n' % tofile
+            yield b'--- ' + fromfile + b'\n'
+            yield b'+++ ' + tofile + b'\n'
             started = True
         i1, i2, j1, j2 = group[0][1], group[-1][2], group[0][3], group[-1][4]
-        yield "@@ -%d,%d +%d,%d @@\n" % (i1+1, i2-i1, j1+1, j2-j1)
+        sizes = "@@ -%d,%d +%d,%d @@\n" % (i1+1, i2-i1, j1+1, j2-j1)
+        yield sizes.encode('ascii')
         for tag, i1, i2, j1, j2 in group:
             if tag == 'equal':
                 for line in a[i1:i2]:
-                    yield ' ' + line
+                    yield b' ' + line
                 continue
             if tag == 'replace' or tag == 'delete':
                 for line in a[i1:i2]:
-                    if not line[-1] == '\n':
-                        line += '\n\\ No newline at end of file\n'
-                    yield '-' + line
+                    if not line[-1:] == b'\n':
+                        line += b'\n\\ No newline at end of file\n'
+                    yield b'-' + line
             if tag == 'replace' or tag == 'insert':
                 for line in b[j1:j2]:
-                    if not line[-1] == '\n':
-                        line += '\n\\ No newline at end of file\n'
-                    yield '+' + line
+                    if not line[-1:] == b'\n':
+                        line += b'\n\\ No newline at end of file\n'
+                    yield b'+' + line
 
 
 def is_binary(content):
@@ -112,21 +115,21 @@ def is_binary(content):
 
     :param content: Bytestring to check for binary content
     """
-    return '\0' in content[:FIRST_FEW_BYTES]
+    return b'\0' in content[:FIRST_FEW_BYTES]
 
 
 def shortid(hexsha):
     if hexsha is None:
-        return "0" * 7
+        return b"0" * 7
     else:
         return hexsha[:7]
 
 
 def patch_filename(p, root):
     if p is None:
-        return "/dev/null"
+        return b"/dev/null"
     else:
-        return root + "/" + p
+        return root + b"/" + p
 
 
 def write_object_diff(f, store, old_file, new_file, diff_binary=False):
@@ -143,13 +146,13 @@ def write_object_diff(f, store, old_file, new_file, diff_binary=False):
     """
     (old_path, old_mode, old_id) = old_file
     (new_path, new_mode, new_id) = new_file
-    old_path = patch_filename(old_path, "a")
-    new_path = patch_filename(new_path, "b")
+    old_path = patch_filename(old_path, b"a")
+    new_path = patch_filename(new_path, b"b")
     def content(mode, hexsha):
         if hexsha is None:
-            return ''
+            return b''
         elif S_ISGITLINK(mode):
-            return "Submodule commit " + hexsha + "\n"
+            return b"Submodule commit " + hexsha + b"\n"
         else:
             return store[hexsha].data
 
@@ -163,12 +166,13 @@ def write_object_diff(f, store, old_file, new_file, diff_binary=False):
     old_content = content(old_mode, old_id)
     new_content = content(new_mode, new_id)
     if not diff_binary and (is_binary(old_content) or is_binary(new_content)):
-        f.write("Binary files %s and %s differ\n" % (old_path, new_path))
+        f.write(b"Binary files " + old_path + b" and " + new_path + b" differ\n")
     else:
         f.writelines(unified_diff(lines(old_content), lines(new_content),
             old_path, new_path))
 
 
+# TODO(jelmer): Support writing unicode, rather than bytes.
 def gen_diff_header(paths, modes, shas):
     """Write a blob diff header.
 
@@ -179,20 +183,21 @@ def gen_diff_header(paths, modes, shas):
     (old_path, new_path) = paths
     (old_mode, new_mode) = modes
     (old_sha, new_sha) = shas
-    yield "diff --git %s %s\n" % (old_path, new_path)
+    yield b"diff --git " + old_path + b" " + new_path + b"\n"
     if old_mode != new_mode:
         if new_mode is not None:
             if old_mode is not None:
-                yield "old mode %o\n" % old_mode
-            yield "new mode %o\n" % new_mode
+                yield ("old mode %o\n" % old_mode).encode('ascii')
+            yield ("new mode %o\n" % new_mode).encode('ascii')
         else:
-            yield "deleted mode %o\n" % old_mode
-    yield "index " + shortid(old_sha) + ".." + shortid(new_sha)
+            yield ("deleted mode %o\n" % old_mode).encode('ascii')
+    yield b"index " + shortid(old_sha) + b".." + shortid(new_sha)
     if new_mode is not None:
-        yield " %o" % new_mode
-    yield "\n"
+        yield (" %o" % new_mode).encode('ascii')
+    yield b"\n"
 
 
+# TODO(jelmer): Support writing unicode, rather than bytes.
 def write_blob_diff(f, old_file, new_file):
     """Write blob diff.
 
@@ -204,8 +209,8 @@ def write_blob_diff(f, old_file, new_file):
     """
     (old_path, old_mode, old_blob) = old_file
     (new_path, new_mode, new_blob) = new_file
-    old_path = patch_filename(old_path, "a")
-    new_path = patch_filename(new_path, "b")
+    old_path = patch_filename(old_path, b"a")
+    new_path = patch_filename(new_path, b"b")
     def lines(blob):
         if blob is not None:
             return blob.data.splitlines(True)
@@ -220,6 +225,7 @@ def write_blob_diff(f, old_file, new_file):
         old_path, new_path))
 
 
+# TODO(jelmer): Support writing unicode, rather than bytes.
 def write_tree_diff(f, store, old_tree, new_tree, diff_binary=False):
     """Write tree diff.
 
@@ -236,17 +242,34 @@ def write_tree_diff(f, store, old_tree, new_tree, diff_binary=False):
                                     diff_binary=diff_binary)
 
 
-def git_am_patch_split(f):
+def git_am_patch_split(f, encoding=None):
     """Parse a git-am-style patch and split it up into bits.
 
     :param f: File-like object to parse
+    :param encoding: Encoding to use when creating Git objects
+    :return: Tuple with commit object, diff contents and git version
+    """
+    encoding = encoding or getattr(f, "encoding", "ascii")
+    contents = f.read()
+    if type(contents) is bytes and getattr(email.parser, "BytesParser", None):
+        parser = email.parser.BytesParser()
+        msg = parser.parsebytes(contents)
+    else:
+        parser = email.parser.Parser()
+        msg = parser.parsestr(contents)
+    return parse_patch_message(msg, encoding)
+
+
+def parse_patch_message(msg, encoding=None):
+    """Extract a Commit object and patch from an e-mail message.
+
+    :param msg: An email message (email.message.Message)
+    :param encoding: Encoding to use to encode Git commits
     :return: Tuple with commit object, diff contents and git version
     """
-    parser = email.parser.Parser()
-    msg = parser.parse(f)
     c = Commit()
-    c.author = msg["from"]
-    c.committer = msg["from"]
+    c.author = msg["from"].encode(encoding)
+    c.committer = msg["from"].encode(encoding)
     try:
         patch_tag_start = msg["subject"].index("[PATCH")
     except ValueError:
@@ -254,29 +277,31 @@ def git_am_patch_split(f):
     else:
         close = msg["subject"].index("] ", patch_tag_start)
         subject = msg["subject"][close+2:]
-    c.message = subject.replace("\n", "") + "\n"
+    c.message = (subject.replace("\n", "") + "\n").encode(encoding)
     first = True
 
-    body = BytesIO(msg.get_payload())
+    body = msg.get_payload(decode=True)
+    lines = body.splitlines(True)
+    line_iter = iter(lines)
 
-    for l in body:
-        if l == "---\n":
+    for l in line_iter:
+        if l == b"---\n":
             break
         if first:
-            if l.startswith("From: "):
-                c.author = l[len("From: "):].rstrip()
+            if l.startswith(b"From: "):
+                c.author = l[len(b"From: "):].rstrip()
             else:
-                c.message += "\n" + l
+                c.message += b"\n" + l
             first = False
         else:
             c.message += l
-    diff = ""
-    for l in body:
-        if l == "-- \n":
+    diff = b""
+    for l in line_iter:
+        if l == b"-- \n":
             break
         diff += l
     try:
-        version = next(body).rstrip("\n")
+        version = next(line_iter).rstrip(b"\n")
     except StopIteration:
         version = None
     return c, diff, version

dulwich/porcelain.py

@@ -48,11 +48,18 @@ Differences in behaviour are considered bugs.
 __docformat__ = 'restructuredText'
 
 from collections import namedtuple
+from contextlib import (
+    closing,
+    contextmanager,
+)
 import os
 import sys
 import time
 
-from dulwich.client import get_transport_and_path
+from dulwich.client import (
+    get_transport_and_path,
+    SubprocessGitClient,
+    )
 from dulwich.errors import (
     SendPackError,
     UpdateRefsError,
@@ -86,17 +93,33 @@ def open_repo(path_or_repo):
     return Repo(path_or_repo)
 
 
-def archive(location, committish=None, outstream=sys.stdout,
+@contextmanager
+def _noop_context_manager(obj):
+    """Context manager that has the same api as closing but does nothing."""
+    yield obj
+
+
+def open_repo_closing(path_or_repo):
+    """Open an argument that can be a repository or a path for a repository.
+    returns a context manager that will close the repo on exit if the argument
+    is a path, else does nothing if the argument is a repo.
+    """
+    if isinstance(path_or_repo, BaseRepo):
+        return _noop_context_manager(path_or_repo)
+    return closing(Repo(path_or_repo))
+
+
+def archive(path, committish=None, outstream=sys.stdout,
             errstream=sys.stderr):
     """Create an archive.
 
-    :param location: Location of repository for which to generate an archive.
+    :param path: Path of repository for which to generate an archive.
     :param committish: Commit SHA1 or ref to use
     :param outstream: Output stream (defaults to stdout)
     :param errstream: Error stream (defaults to stderr)
     """
 
-    client, path = get_transport_and_path(location)
+    client = SubprocessGitClient()
     if committish is None:
         committish = "HEAD"
     # TODO(jelmer): This invokes C git; this introduces a dependency.
@@ -110,8 +133,8 @@ def update_server_info(repo="."):
 
     :param repo: path to the repository
     """
-    r = open_repo(repo)
-    server_update_server_info(r)
+    with open_repo_closing(repo) as r:
+        server_update_server_info(r)
 
 
 def symbolic_ref(repo, ref_name, force=False):
@@ -121,11 +144,11 @@ def symbolic_ref(repo, ref_name, force=False):
     :param ref_name: short name of the new ref
     :param force: force settings without checking if it exists in refs/heads
     """
-    repo_obj = open_repo(repo)
-    ref_path = b'refs/heads/' + ref_name
-    if not force and ref_path not in repo_obj.refs.keys():
-        raise ValueError('fatal: ref `%s` is not a ref' % ref_name)
-    repo_obj.refs.set_symbolic_ref(b'HEAD', ref_path)
+    with open_repo_closing(repo) as repo_obj:
+        ref_path = b'refs/heads/' + ref_name
+        if not force and ref_path not in repo_obj.refs.keys():
+            raise ValueError('fatal: ref `%s` is not a ref' % ref_name)
+        repo_obj.refs.set_symbolic_ref(b'HEAD', ref_path)
 
 
 def commit(repo=".", message=None, author=None, committer=None):
@@ -139,9 +162,9 @@ def commit(repo=".", message=None, author=None, committer=None):
     """
     # FIXME: Support --all argument
     # FIXME: Support --signoff argument
-    r = open_repo(repo)
-    return r.do_commit(message=message, author=author,
-        committer=committer)
+    with open_repo_closing(repo) as r:
+        return r.do_commit(message=message, author=author,
+            committer=committer)
 
 
 def commit_tree(repo, tree, message=None, author=None, committer=None):
@@ -152,9 +175,9 @@ def commit_tree(repo, tree, message=None, author=None, committer=None):
     :param author: Optional author name and email
     :param committer: Optional committer name and email
     """
-    r = open_repo(repo)
-    return r.do_commit(message=message, tree=tree, committer=committer,
-            author=author)
+    with open_repo_closing(repo) as r:
+        return r.do_commit(message=message, tree=tree, committer=committer,
+                author=author)
 
 
 def init(path=".", bare=False):
@@ -200,17 +223,22 @@ def clone(source, target=None, bare=False, checkout=None, errstream=sys.stdout,
 
     if not os.path.exists(target):
         os.mkdir(target)
+
     if bare:
         r = Repo.init_bare(target)
     else:
         r = Repo.init(target)
-    remote_refs = client.fetch(host_path, r,
-        determine_wants=r.object_store.determine_wants_all,
-        progress=errstream.write)
-    r[b"HEAD"] = remote_refs[b"HEAD"]
-    if checkout:
-        errstream.write(b'Checking out HEAD')
-        r.reset_index()
+    try:
+        remote_refs = client.fetch(host_path, r,
+            determine_wants=r.object_store.determine_wants_all,
+            progress=errstream.write)
+        r[b"HEAD"] = remote_refs[b"HEAD"]
+        if checkout:
+            errstream.write(b'Checking out HEAD')
+            r.reset_index()
+    except:
+        r.close()
+        raise
 
     return r
 
@@ -222,17 +250,17 @@ def add(repo=".", paths=None):
     :param paths: Paths to add.  No value passed stages all modified files.
     """
     # FIXME: Support patterns, directories.
-    r = open_repo(repo)
-    if not paths:
-        # If nothing is specified, add all non-ignored files.
-        paths = []
-        for dirpath, dirnames, filenames in os.walk(r.path):
-            # Skip .git and below.
-            if '.git' in dirnames:
-                dirnames.remove('.git')
-            for filename in filenames:
-                paths.append(os.path.join(dirpath[len(r.path)+1:], filename))
-    r.stage(paths)
+    with open_repo_closing(repo) as r:
+        if not paths:
+            # If nothing is specified, add all non-ignored files.
+            paths = []
+            for dirpath, dirnames, filenames in os.walk(r.path):
+                # Skip .git and below.
+                if '.git' in dirnames:
+                    dirnames.remove('.git')
+                for filename in filenames:
+                    paths.append(os.path.join(dirpath[len(r.path)+1:], filename))
+        r.stage(paths)
 
 
 def rm(repo=".", paths=None):
@@ -241,11 +269,11 @@ def rm(repo=".", paths=None):
     :param repo: Repository for the files
     :param paths: Paths to remove
     """
-    r = open_repo(repo)
-    index = r.open_index()
-    for p in paths:
-        del index[p.encode(sys.getfilesystemencoding())]
-    index.write()
+    with open_repo_closing(repo) as r:
+        index = r.open_index()
+        for p in paths:
+            del index[p.encode(sys.getfilesystemencoding())]
+        index.write()
 
 
 def commit_decode(commit, contents):
@@ -344,10 +372,10 @@ def log(repo=".", outstream=sys.stdout, max_entries=None):
     :param outstream: Stream to write log output to
     :param max_entries: Optional maximum number of entries to display
     """
-    r = open_repo(repo)
-    walker = r.get_walker(max_entries=max_entries)
-    for entry in walker:
-        print_commit(entry.commit, outstream)
+    with open_repo_closing(repo) as r:
+        walker = r.get_walker(max_entries=max_entries)
+        for entry in walker:
+            print_commit(entry.commit, outstream)
 
 
 def show(repo=".", objects=None, outstream=sys.stdout):
@@ -361,9 +389,9 @@ def show(repo=".", objects=None, outstream=sys.stdout):
         objects = ["HEAD"]
     if not isinstance(objects, list):
         objects = [objects]
-    r = open_repo(repo)
-    for objectish in objects:
-        show_object(r, parse_object(r, objectish), outstream)
+    with open_repo_closing(repo) as r:
+        for objectish in objects:
+            show_object(r, parse_object(r, objectish), outstream)
 
 
 def diff_tree(repo, old_tree, new_tree, outstream=sys.stdout):
@@ -374,8 +402,8 @@ def diff_tree(repo, old_tree, new_tree, outstream=sys.stdout):
     :param new_tree: Id of new tree
     :param outstream: Stream to write to
     """
-    r = open_repo(repo)
-    write_tree_diff(outstream, r.object_store, old_tree, new_tree)
+    with open_repo_closing(repo) as r:
+        write_tree_diff(outstream, r.object_store, old_tree, new_tree)
 
 
 def rev_list(repo, commits, outstream=sys.stdout):
@@ -385,9 +413,9 @@ def rev_list(repo, commits, outstream=sys.stdout):
     :param commits: Commits over which to iterate
     :param outstream: Stream to write to
     """
-    r = open_repo(repo)
-    for entry in r.get_walker(include=[r[c].id for c in commits]):
-        outstream.write(entry.commit.id + b"\n")
+    with open_repo_closing(repo) as r:
+        for entry in r.get_walker(include=[r[c].id for c in commits]):
+            outstream.write(entry.commit.id + b"\n")
 
 
 def tag(*args, **kwargs):
@@ -410,34 +438,34 @@ def tag_create(repo, tag, author=None, message=None, annotated=False,
     :param tag_timezone: Optional timezone for annotated tag
     """
 
-    r = open_repo(repo)
-    object = parse_object(r, objectish)
-
-    if annotated:
-        # Create the tag object
-        tag_obj = Tag()
-        if author is None:
-            # TODO(jelmer): Don't use repo private method.
-            author = r._get_user_identity()
-        tag_obj.tagger = author
-        tag_obj.message = message
-        tag_obj.name = tag
-        tag_obj.object = (type(object), object.id)
-        tag_obj.tag_time = tag_time
-        if tag_time is None:
-            tag_time = int(time.time())
-        if tag_timezone is None:
-            # TODO(jelmer) Use current user timezone rather than UTC
-            tag_timezone = 0
-        elif isinstance(tag_timezone, str):
-            tag_timezone = parse_timezone(tag_timezone)
-        tag_obj.tag_timezone = tag_timezone
-        r.object_store.add_object(tag_obj)
-        tag_id = tag_obj.id
-    else:
-        tag_id = object.id
+    with open_repo_closing(repo) as r:
+        object = parse_object(r, objectish)
+
+        if annotated:
+            # Create the tag object
+            tag_obj = Tag()
+            if author is None:
+                # TODO(jelmer): Don't use repo private method.
+                author = r._get_user_identity()
+            tag_obj.tagger = author
+            tag_obj.message = message
+            tag_obj.name = tag
+            tag_obj.object = (type(object), object.id)
+            tag_obj.tag_time = tag_time
+            if tag_time is None:
+                tag_time = int(time.time())
+            if tag_timezone is None:
+                # TODO(jelmer) Use current user timezone rather than UTC
+                tag_timezone = 0
+            elif isinstance(tag_timezone, str):
+                tag_timezone = parse_timezone(tag_timezone)
+            tag_obj.tag_timezone = tag_timezone
+            r.object_store.add_object(tag_obj)
+            tag_id = tag_obj.id
+        else:
+            tag_id = object.id
 
-    r.refs[b'refs/tags/' + tag] = tag_id
+        r.refs[b'refs/tags/' + tag] = tag_id
 
 
 def list_tags(*args, **kwargs):
@@ -452,10 +480,10 @@ def tag_list(repo, outstream=sys.stdout):
     :param repo: Path to repository
     :param outstream: Stream to write tags to
     """
-    r = open_repo(repo)
-    tags = list(r.refs.as_dict(b"refs/tags"))
-    tags.sort()
-    return tags
+    with open_repo_closing(repo) as r:
+        tags = list(r.refs.as_dict(b"refs/tags"))
+        tags.sort()
+        return tags
 
 
 def tag_delete(repo, name):
@@ -464,15 +492,15 @@ def tag_delete(repo, name):
     :param repo: Path to repository
     :param name: Name of tag to remove
     """
-    r = open_repo(repo)
-    if isinstance(name, bytes):
-        names = [name]
-    elif isinstance(name, list):
-        names = name
-    else:
-        raise TypeError("Unexpected tag name type %r" % name)
-    for name in names:
-        del r.refs[b"refs/tags/" + name]
+    with open_repo_closing(repo) as r:
+        if isinstance(name, bytes):
+            names = [name]
+        elif isinstance(name, list):
+            names = name
+        else:
+            raise TypeError("Unexpected tag name type %r" % name)
+        for name in names:
+            del r.refs[b"refs/tags/" + name]
 
 
 def reset(repo, mode, committish="HEAD"):
@@ -485,10 +513,9 @@ def reset(repo, mode, committish="HEAD"):
     if mode != "hard":
         raise ValueError("hard is the only mode currently supported")
 
-    r = open_repo(repo)
-
-    tree = r[committish].tree
-    r.reset_index()
+    with open_repo_closing(repo) as r:
+        tree = r[committish].tree
+        r.reset_index()
 
 
 def push(repo, remote_location, refs_path,
@@ -503,28 +530,25 @@ def push(repo, remote_location, refs_path,
     """
 
     # Open the repo
-    r = open_repo(repo)
+    with open_repo_closing(repo) as r:
 
-    # Get the client and path
-    client, path = get_transport_and_path(remote_location)
+        # Get the client and path
+        client, path = get_transport_and_path(remote_location)
 
-    def update_refs(refs):
-        new_refs = r.get_refs()
-        refs[refs_path] = new_refs[b'HEAD']
-        del new_refs[b'HEAD']
-        return refs
+        def update_refs(refs):
+            new_refs = r.get_refs()
+            refs[refs_path] = new_refs[b'HEAD']
+            del new_refs[b'HEAD']
+            return refs
 
-    err_encoding = getattr(errstream, 'encoding', 'utf-8')
-    if not isinstance(remote_location, bytes):
+        err_encoding = getattr(errstream, 'encoding', 'utf-8')
         remote_location_bytes = remote_location.encode(err_encoding)
-    else:
-        remote_location_bytes = remote_location
-    try:
-        client.send_pack(path, update_refs,
-            r.object_store.generate_pack_contents, progress=errstream.write)
-        errstream.write(b"Push to " + remote_location_bytes + b" successful.\n")
-    except (UpdateRefsError, SendPackError) as e:
-        errstream.write(b"Push to " + remote_location_bytes + b" failed -> " + e.message.encode(err_encoding) + b"\n")
+        try:
+            client.send_pack(path, update_refs,
+                r.object_store.generate_pack_contents, progress=errstream.write)
+            errstream.write(b"Push to " + remote_location_bytes + b" successful.\n")
+        except (UpdateRefsError, SendPackError) as e:
+            errstream.write(b"Push to " + remote_location_bytes + b" failed -> " + e.message.encode(err_encoding) + b"\n")
 
 
 def pull(repo, remote_location, refs_path,
@@ -539,15 +563,14 @@ def pull(repo, remote_location, refs_path,
     """
 
     # Open the repo
-    r = open_repo(repo)
+    with open_repo_closing(repo) as r:
+        client, path = get_transport_and_path(remote_location)
+        remote_refs = client.fetch(path, r, progress=errstream.write)
+        r[b'HEAD'] = remote_refs[refs_path]
 
-    client, path = get_transport_and_path(remote_location)
-    remote_refs = client.fetch(path, r, progress=errstream.write)
-    r[b'HEAD'] = remote_refs[refs_path]
-
-    # Perform 'git checkout .' - syncs staged changes
-    tree = r[b"HEAD"].tree
-    r.reset_index()
+        # Perform 'git checkout .' - syncs staged changes
+        tree = r[b"HEAD"].tree
+        r.reset_index()
 
 
 def status(repo="."):
@@ -559,15 +582,14 @@ def status(repo="."):
         unstaged -  list of unstaged paths (diff index/working-tree)
         untracked - list of untracked, un-ignored & non-.git paths
     """
-    r = open_repo(repo)
-
-    # 1. Get status of staged
-    tracked_changes = get_tree_changes(r)
-    # 2. Get status of unstaged
-    unstaged_changes = list(get_unstaged_changes(r.open_index(), r.path))
-    # TODO - Status of untracked - add untracked changes, need gitignore.
-    untracked_changes = []
-    return GitStatus(tracked_changes, unstaged_changes, untracked_changes)
+    with open_repo_closing(repo) as r:
+        # 1. Get status of staged
+        tracked_changes = get_tree_changes(r)
+        # 2. Get status of unstaged
+        unstaged_changes = list(get_unstaged_changes(r.open_index(), r.path))
+        # TODO - Status of untracked - add untracked changes, need gitignore.
+        untracked_changes = []
+        return GitStatus(tracked_changes, unstaged_changes, untracked_changes)
 
 
 def get_tree_changes(repo):
@@ -576,27 +598,27 @@ def get_tree_changes(repo):
     :param repo: repo path or object
     :return: dict with lists for each type of change
     """
-    r = open_repo(repo)
-    index = r.open_index()
-
-    # Compares the Index to the HEAD & determines changes
-    # Iterate through the changes and report add/delete/modify
-    # TODO: call out to dulwich.diff_tree somehow.
-    tracked_changes = {
-        'add': [],
-        'delete': [],
-        'modify': [],
-    }
-    for change in index.changes_from_tree(r.object_store, r[b'HEAD'].tree):
-        if not change[0][0]:
-            tracked_changes['add'].append(change[0][1])
-        elif not change[0][1]:
-            tracked_changes['delete'].append(change[0][0])
-        elif change[0][0] == change[0][1]:
-            tracked_changes['modify'].append(change[0][0])
-        else:
-            raise AssertionError('git mv ops not yet supported')
-    return tracked_changes
+    with open_repo_closing(repo) as r:
+        index = r.open_index()
+
+        # Compares the Index to the HEAD & determines changes
+        # Iterate through the changes and report add/delete/modify
+        # TODO: call out to dulwich.diff_tree somehow.
+        tracked_changes = {
+            'add': [],
+            'delete': [],
+            'modify': [],
+        }
+        for change in index.changes_from_tree(r.object_store, r[b'HEAD'].tree):
+            if not change[0][0]:
+                tracked_changes['add'].append(change[0][1])
+            elif not change[0][1]:
+                tracked_changes['delete'].append(change[0][0])
+            elif change[0][0] == change[0][1]:
+                tracked_changes['modify'].append(change[0][0])
+            else:
+                raise AssertionError('git mv ops not yet supported')
+        return tracked_changes
 
 
 def daemon(path=".", address=None, port=None):
@@ -644,7 +666,7 @@ def upload_pack(path=".", inf=None, outf=None):
         outf = getattr(sys.stdout, 'buffer', sys.stdout)
     if inf is None:
         inf = getattr(sys.stdin, 'buffer', sys.stdin)
-    backend = FileSystemBackend()
+    backend = FileSystemBackend(path)
     def send_fn(data):
         outf.write(data)
         outf.flush()
@@ -666,7 +688,7 @@ def receive_pack(path=".", inf=None, outf=None):
         outf = getattr(sys.stdout, 'buffer', sys.stdout)
     if inf is None:
         inf = getattr(sys.stdin, 'buffer', sys.stdin)
-    backend = FileSystemBackend()
+    backend = FileSystemBackend(path)
     def send_fn(data):
         outf.write(data)
         outf.flush()
@@ -683,15 +705,15 @@ def branch_delete(repo, name):
     :param repo: Path to the repository
     :param name: Name of the branch
     """
-    r = open_repo(repo)
-    if isinstance(name, bytes):
-        names = [name]
-    elif isinstance(name, list):
-        names = name
-    else:
-        raise TypeError("Unexpected branch name type %r" % name)
-    for name in names:
-        del r.refs[b"refs/heads/" + name]
+    with open_repo_closing(repo) as r:
+        if isinstance(name, bytes):
+            names = [name]
+        elif isinstance(name, list):
+            names = name
+        else:
+            raise TypeError("Unexpected branch name type %r" % name)
+        for name in names:
+            del r.refs[b"refs/heads/" + name]
 
 
 def branch_create(repo, name, objectish=None, force=False):
@@ -702,20 +724,20 @@ def branch_create(repo, name, objectish=None, force=False):
     :param objectish: Target object to point new branch at (defaults to HEAD)
     :param force: Force creation of branch, even if it already exists
     """
-    r = open_repo(repo)
-    if isinstance(name, bytes):
-        names = [name]
-    elif isinstance(name, list):
-        names = name
-    else:
-        raise TypeError("Unexpected branch name type %r" % name)
-    if objectish is None:
-        objectish = "HEAD"
-    object = parse_object(r, objectish)
-    refname = b"refs/heads/" + name
-    if refname in r.refs and not force:
-        raise KeyError("Branch with name %s already exists." % name)
-    r.refs[refname] = object.id
+    with open_repo_closing(repo) as r:
+        if isinstance(name, bytes):
+            names = [name]
+        elif isinstance(name, list):
+            names = name
+        else:
+            raise TypeError("Unexpected branch name type %r" % name)
+        if objectish is None:
+            objectish = "HEAD"
+        object = parse_object(r, objectish)
+        refname = b"refs/heads/" + name
+        if refname in r.refs and not force:
+            raise KeyError("Branch with name %s already exists." % name)
+        r.refs[refname] = object.id
 
 
 def branch_list(repo):
@@ -723,8 +745,8 @@ def branch_list(repo):
 
     :param repo: Path to the repository
     """
-    r = open_repo(repo)
-    return r.refs.keys(base=b"refs/heads/")
+    with open_repo_closing(repo) as r:
+        return r.refs.keys(base=b"refs/heads/")
 
 
 def fetch(repo, remote_location, outstream=sys.stdout, errstream=sys.stderr):
@@ -736,7 +758,7 @@ def fetch(repo, remote_location, outstream=sys.stdout, errstream=sys.stderr):
     :param errstream: Error stream (defaults to stderr)
     :return: Dictionary with refs on the remote
     """
-    r = open_repo(repo)
-    client, path = get_transport_and_path(remote_location)
-    remote_refs = client.fetch(path, r, progress=errstream.write)
+    with open_repo_closing(repo) as r:
+        client, path = get_transport_and_path(remote_location)
+        remote_refs = client.fetch(path, r, progress=errstream.write)
     return remote_refs

dulwich/protocol.py

@@ -49,8 +49,10 @@ CAPABILITY_DELETE_REFS = b'delete-refs'
 CAPABILITY_INCLUDE_TAG = b'include-tag'
 CAPABILITY_MULTI_ACK = b'multi_ack'
 CAPABILITY_MULTI_ACK_DETAILED = b'multi_ack_detailed'
+CAPABILITY_NO_DONE = b'no-done'
 CAPABILITY_NO_PROGRESS = b'no-progress'
 CAPABILITY_OFS_DELTA = b'ofs-delta'
+CAPABILITY_QUIET = b'quiet'
 CAPABILITY_REPORT_STATUS = b'report-status'
 CAPABILITY_SHALLOW = b'shallow'
 CAPABILITY_SIDE_BAND_64K = b'side-band-64k'

dulwich/refs.py

@@ -23,7 +23,6 @@
 """
 import errno
 import os
-import sys
 
 from dulwich.errors import (
     PackedRefsException,
@@ -44,8 +43,6 @@ SYMREF = b'ref: '
 LOCAL_BRANCH_PREFIX = b'refs/heads/'
 BAD_REF_CHARS = set(b'\177 ~^:?*[')
 
-path_sep_bytes = os.path.sep.encode(sys.getfilesystemencoding())
-
 
 def check_ref_format(refname):
     """Check if a refname is correctly formatted.
@@ -398,9 +395,10 @@ class DiskRefsContainer(RefsContainer):
         subkeys = set()
         path = self.refpath(base)
         for root, dirs, files in os.walk(path):
-            dir = root[len(path):].strip(path_sep_bytes).replace(path_sep_bytes, b'/')
+            dir = root[len(path):].strip(os.path.sep).replace(os.path.sep, "/")
             for filename in files:
-                refname = (dir + b'/' + filename).strip(b'/')
+                refname = (("%s/%s" % (dir, filename))
+                           .strip("/").encode('ascii'))
                 # check_ref_format requires at least one /, so we prepend the
                 # base before calling it.
                 if check_ref_format(base + b'/' + refname):
@@ -416,9 +414,9 @@ class DiskRefsContainer(RefsContainer):
             allkeys.add(b'HEAD')
         path = self.refpath(b'')
         for root, dirs, files in os.walk(self.refpath(b'refs')):
-            dir = root[len(path):].strip(path_sep_bytes).replace(path_sep_bytes, b'/')
+            dir = root[len(path):].strip(os.path.sep).replace(os.path.sep, "/")
             for filename in files:
-                refname = (dir + b'/' + filename).strip(b'/')
+                refname = ("%s/%s" % (dir, filename)).strip("/").encode('ascii')
                 if check_ref_format(refname):
                     allkeys.add(refname)
         allkeys.update(self.get_packed_refs())
@@ -428,8 +426,9 @@ class DiskRefsContainer(RefsContainer):
         """Return the disk path of a ref.
 
         """
-        if path_sep_bytes != b'/':
-            name = name.replace(b'/', path_sep_bytes)
+        name = name.decode('ascii')
+        if os.path.sep != "/":
+            name = name.replace("/", os.path.sep)
         return os.path.join(self.path, name)
 
     def get_packed_refs(self):
@@ -446,7 +445,7 @@ class DiskRefsContainer(RefsContainer):
             # None if and only if _packed_refs is also None.
             self._packed_refs = {}
             self._peeled_refs = {}
-            path = os.path.join(self.path, b'packed-refs')
+            path = os.path.join(self.path, 'packed-refs')
             try:
                 f = GitFile(path, 'rb')
             except IOError as e:
@@ -514,7 +513,7 @@ class DiskRefsContainer(RefsContainer):
     def _remove_packed_ref(self, name):
         if self._packed_refs is None:
             return
-        filename = os.path.join(self.path, b'packed-refs')
+        filename = os.path.join(self.path, 'packed-refs')
         # reread cached refs from disk, while holding the lock
         f = GitFile(filename, 'wb')
         try:

dulwich/repo.py

@@ -82,19 +82,19 @@ from dulwich.refs import (
 import warnings
 
 
-OBJECTDIR = b'objects'
-REFSDIR = b'refs'
-REFSDIR_TAGS = b'tags'
-REFSDIR_HEADS = b'heads'
-INDEX_FILENAME = b'index'
+OBJECTDIR = 'objects'
+REFSDIR = 'refs'
+REFSDIR_TAGS = 'tags'
+REFSDIR_HEADS = 'heads'
+INDEX_FILENAME = "index"
 
 BASE_DIRECTORIES = [
-    [b'branches'],
+    ["branches"],
     [REFSDIR],
     [REFSDIR, REFSDIR_TAGS],
     [REFSDIR, REFSDIR_HEADS],
-    [b'hooks'],
-    [b'info']
+    ["hooks"],
+    ["info"]
     ]
 
 
@@ -176,7 +176,7 @@ class BaseRepo(object):
     def _init_files(self, bare):
         """Initialize a default set of named files."""
         from dulwich.config import ConfigFile
-        self._put_named_file(b'description', b"Unnamed repository")
+        self._put_named_file('description', b"Unnamed repository")
         f = BytesIO()
         cf = ConfigFile()
         cf.set(b"core", b"repositoryformatversion", b"0")
@@ -184,8 +184,8 @@ class BaseRepo(object):
         cf.set(b"core", b"bare", bare)
         cf.set(b"core", b"logallrefupdates", True)
         cf.write_to_file(f)
-        self._put_named_file(b'config', f.getvalue())
-        self._put_named_file(os.path.join(b'info', b'exclude'), b'')
+        self._put_named_file('config', f.getvalue())
+        self._put_named_file(os.path.join('info', 'exclude'), b'')
 
     def get_named_file(self, path):
         """Get a file from the control dir with a specific name.
@@ -263,6 +263,9 @@ class BaseRepo(object):
 
             return []
 
+        # If the graph walker is set up with an implementation that can
+        # ACK/NAK to the wire, it will write data to the client through
+        # this call as a side-effect.
         haves = self.object_store.find_common_revisions(graph_walker)
 
         # Deal with shallow requests separately because the haves do
@@ -627,7 +630,6 @@ class BaseRepo(object):
 
         return c.id
 
-path_sep_bytes = os.path.sep.encode(sys.getfilesystemencoding())
 
 class Repo(BaseRepo):
     """A git repository backed by local disk.
@@ -638,40 +640,36 @@ class Repo(BaseRepo):
     To create a new repository, use the Repo.init class method.
     """
 
-    def __init__(self, path):
-        self.path = path
-        if not isinstance(path, bytes):
-            self._path_bytes = path.encode(sys.getfilesystemencoding())
-        else:
-            self._path_bytes = path
-        if os.path.isdir(os.path.join(self._path_bytes, b'.git', OBJECTDIR)):
+    def __init__(self, root):
+        if os.path.isdir(os.path.join(root, ".git", OBJECTDIR)):
             self.bare = False
-            self._controldir = os.path.join(self._path_bytes, b'.git')
-        elif (os.path.isdir(os.path.join(self._path_bytes, OBJECTDIR)) and
-              os.path.isdir(os.path.join(self._path_bytes, REFSDIR))):
+            self._controldir = os.path.join(root, ".git")
+        elif (os.path.isdir(os.path.join(root, OBJECTDIR)) and
+              os.path.isdir(os.path.join(root, REFSDIR))):
             self.bare = True
-            self._controldir = self._path_bytes
-        elif (os.path.isfile(os.path.join(self._path_bytes, b'.git'))):
+            self._controldir = root
+        elif (os.path.isfile(os.path.join(root, ".git"))):
             import re
-            with open(os.path.join(self._path_bytes, b'.git'), 'rb') as f:
-                _, gitdir = re.match(b'(gitdir: )(.+$)', f.read()).groups()
+            with open(os.path.join(root, ".git"), 'r') as f:
+                _, path = re.match('(gitdir: )(.+$)', f.read()).groups()
             self.bare = False
-            self._controldir = os.path.join(self._path_bytes, gitdir)
+            self._controldir = os.path.join(root, path)
         else:
             raise NotGitRepository(
-                "No git repository was found at %(path)s" % dict(path=path)
+                "No git repository was found at %(path)s" % dict(path=root)
             )
+        self.path = root
         object_store = DiskObjectStore(os.path.join(self.controldir(),
                                                     OBJECTDIR))
         refs = DiskRefsContainer(self.controldir())
         BaseRepo.__init__(self, object_store, refs)
 
         self._graftpoints = {}
-        graft_file = self.get_named_file(os.path.join(b'info', b'grafts'))
+        graft_file = self.get_named_file(os.path.join("info", "grafts"))
         if graft_file:
             with graft_file:
                 self._graftpoints.update(parse_graftpoints(graft_file))
-        graft_file = self.get_named_file(b'shallow')
+        graft_file = self.get_named_file("shallow")
         if graft_file:
             with graft_file:
                 self._graftpoints.update(parse_graftpoints(graft_file))
@@ -690,7 +688,7 @@ class Repo(BaseRepo):
         :param path: The path to the file, relative to the control dir.
         :param contents: A string to write to the file.
         """
-        path = path.lstrip(path_sep_bytes)
+        path = path.lstrip(os.path.sep)
         with GitFile(os.path.join(self.controldir(), path), 'wb') as f:
             f.write(contents)
 
@@ -706,7 +704,7 @@ class Repo(BaseRepo):
         """
         # TODO(dborowitz): sanitize filenames, since this is used directly by
         # the dumb web serving code.
-        path = path.lstrip(path_sep_bytes)
+        path = path.lstrip(os.path.sep)
         try:
             return open(os.path.join(self.controldir(), path), 'rb')
         except (IOError, OSError) as e:
@@ -735,37 +733,39 @@ class Repo(BaseRepo):
         # missing index file, which is treated as empty.
         return not self.bare
 
-    def stage(self, paths, fsencoding=sys.getfilesystemencoding()):
+    def stage(self, fs_paths):
         """Stage a set of paths.
 
-        :param paths: List of paths, relative to the repository path
+        :param fs_paths: List of paths, relative to the repository path
         """
-        if not isinstance(paths, list):
-            paths = [paths]
+
+        root_path_bytes = self.path.encode(sys.getfilesystemencoding())
+
+        if not isinstance(fs_paths, list):
+            fs_paths = [fs_paths]
         from dulwich.index import (
             blob_from_path_and_stat,
             index_entry_from_stat,
+            _fs_to_tree_path,
             )
         index = self.open_index()
-        for path in paths:
-            if not isinstance(path, bytes):
-                disk_path_bytes = path.encode(sys.getfilesystemencoding())
-                repo_path_bytes = path.encode(fsencoding)
-            else:
-                disk_path_bytes, repo_path_bytes = path, path
-            full_path = os.path.join(self._path_bytes, disk_path_bytes)
+        for fs_path in fs_paths:
+            if not isinstance(fs_path, bytes):
+                fs_path = fs_path.encode(sys.getfilesystemencoding())
+            tree_path = _fs_to_tree_path(fs_path)
+            full_path = os.path.join(root_path_bytes, fs_path)
             try:
                 st = os.lstat(full_path)
             except OSError:
                 # File no longer exists
                 try:
-                    del index[repo_path_bytes]
+                    del index[tree_path]
                 except KeyError:
                     pass  # already removed
             else:
                 blob = blob_from_path_and_stat(full_path, st)
                 self.object_store.add_object(blob)
-                index[repo_path_bytes] = index_entry_from_stat(st, blob.id, 0)
+                index[tree_path] = index_entry_from_stat(st, blob.id, 0)
         index.write()
 
     def clone(self, target_path, mkdir=True, bare=False,
@@ -825,7 +825,7 @@ class Repo(BaseRepo):
             validate_path_element = validate_path_element_ntfs
         else:
             validate_path_element = validate_path_element_default
-        return build_index_from_tree(self._path_bytes, self.index_path(),
+        return build_index_from_tree(self.path, self.index_path(),
                 self.object_store, tree, honor_filemode=honor_filemode,
                 validate_path_element=validate_path_element)
 
@@ -835,7 +835,7 @@ class Repo(BaseRepo):
         :return: `ConfigFile` object for the ``.git/config`` file.
         """
         from dulwich.config import ConfigFile
-        path = os.path.join(self._controldir, b'config')
+        path = os.path.join(self._controldir, 'config')
         try:
             return ConfigFile.from_path(path)
         except (IOError, OSError) as e:
@@ -850,7 +850,7 @@ class Repo(BaseRepo):
 
         :return: A string describing the repository or None.
         """
-        path = os.path.join(self._controldir, b'description')
+        path = os.path.join(self._controldir, 'description')
         try:
             with GitFile(path, 'rb') as f:
                 return f.read()
@@ -868,17 +868,13 @@ class Repo(BaseRepo):
         :param description: Text to set as description for this repository.
         """
 
-        self._put_named_file(b'description', description)
+        self._put_named_file('description', description)
 
     @classmethod
     def _init_maybe_bare(cls, path, bare):
-        if not isinstance(path, bytes):
-            path_bytes = path.encode(sys.getfilesystemencoding())
-        else:
-            path_bytes = path
         for d in BASE_DIRECTORIES:
-            os.mkdir(os.path.join(path_bytes, *d))
-        DiskObjectStore.init(os.path.join(path_bytes, OBJECTDIR))
+            os.mkdir(os.path.join(path, *d))
+        DiskObjectStore.init(os.path.join(path, OBJECTDIR))
         ret = cls(path)
         ret.refs.set_symbolic_ref(b'HEAD', b"refs/heads/master")
         ret._init_files(bare)
@@ -892,13 +888,9 @@ class Repo(BaseRepo):
         :param mkdir: Whether to create the directory
         :return: `Repo` instance
         """
-        if not isinstance(path, bytes):
-            path_bytes = path.encode(sys.getfilesystemencoding())
-        else:
-            path_bytes = path
         if mkdir:
-            os.mkdir(path_bytes)
-        controldir = os.path.join(path_bytes, b'.git')
+            os.mkdir(path)
+        controldir = os.path.join(path, ".git")
         os.mkdir(controldir)
         cls._init_maybe_bare(controldir, False)
         return cls(path)
@@ -916,6 +908,10 @@ class Repo(BaseRepo):
 
     create = init_bare
 
+    def close(self):
+        """Close any files opened by this repository."""
+        self.object_store.close()
+
 
 class MemoryRepo(BaseRepo):
     """Repo that stores refs, objects, and named files in memory.

dulwich/server.py

@@ -72,6 +72,7 @@ from dulwich.protocol import (
     CAPABILITY_INCLUDE_TAG,
     CAPABILITY_MULTI_ACK_DETAILED,
     CAPABILITY_MULTI_ACK,
+    CAPABILITY_NO_DONE,
     CAPABILITY_NO_PROGRESS,
     CAPABILITY_OFS_DELTA,
     CAPABILITY_REPORT_STATUS,
@@ -183,15 +184,17 @@ class DictBackend(Backend):
 class FileSystemBackend(Backend):
     """Simple backend that looks up Git repositories in the local file system."""
 
-    def __init__(self, root="/"):
+    def __init__(self, root=os.sep):
         super(FileSystemBackend, self).__init__()
-        self.root = (os.path.abspath(root) + "/").replace("//", "/")
+        self.root = (os.path.abspath(root) + os.sep).replace(os.sep * 2, os.sep)
 
     def open_repository(self, path):
         logger.debug('opening repository at %s', path)
-        abspath = os.path.abspath(os.path.join(self.root, path)) + "/"
-        if not abspath.startswith(self.root):
-            raise NotGitRepository("Invalid path %r" % path)
+        abspath = os.path.abspath(os.path.join(self.root, path)) + os.sep
+        normcase_abspath = os.path.normcase(abspath)
+        normcase_root = os.path.normcase(self.root)
+        if not normcase_abspath.startswith(normcase_root):
+            raise NotGitRepository("Path %r not inside root %r" % (path, self.root))
         return Repo(abspath)
 
 
@@ -203,10 +206,12 @@ class Handler(object):
         self.proto = proto
         self.http_req = http_req
         self._client_capabilities = None
+        # Flags needed for the no-done capability
+        self._done_received = False
 
     @classmethod
     def capability_line(cls):
-        return b" ".join(cls.capabilities())
+        return b"".join([b" " + c for c in cls.capabilities()])
 
     @classmethod
     def capabilities(cls):
@@ -242,6 +247,9 @@ class Handler(object):
                                    'before asking client' % cap)
         return cap in self._client_capabilities
 
+    def notify_done(self):
+        self._done_received = True
+
 
 class UploadPackHandler(Handler):
     """Protocol handler for uploading a pack to the server."""
@@ -262,7 +270,7 @@ class UploadPackHandler(Handler):
         return (CAPABILITY_MULTI_ACK_DETAILED, CAPABILITY_MULTI_ACK,
                 CAPABILITY_SIDE_BAND_64K, CAPABILITY_THIN_PACK,
                 CAPABILITY_OFS_DELTA, CAPABILITY_NO_PROGRESS,
-                CAPABILITY_INCLUDE_TAG, CAPABILITY_SHALLOW)
+                CAPABILITY_INCLUDE_TAG, CAPABILITY_SHALLOW, CAPABILITY_NO_DONE)
 
     @classmethod
     def required_capabilities(cls):
@@ -329,6 +337,10 @@ class UploadPackHandler(Handler):
         # band data now.
         self._processing_have_lines = False
 
+        if not graph_walker.handle_done(
+                not self.has_capability(CAPABILITY_NO_DONE), self._done_received):
+            return
+
         self.progress(b"dul-daemon says what\n")
         self.progress(("counting objects: %d, done.\n" % len(objects_iter)).encode('ascii'))
         write_pack_objects(ProtocolFile(None, write), objects_iter)
@@ -605,6 +617,10 @@ class ProtocolGraphWalker(object):
 
         self.proto.write_pkt_line(None)
 
+    def notify_done(self):
+        # relay the message down to the handler.
+        self.handler.notify_done()
+
     def send_ack(self, sha, ack_type=b''):
         if ack_type:
             ack_type = b' ' + ack_type
@@ -613,6 +629,10 @@ class ProtocolGraphWalker(object):
     def send_nak(self):
         self.proto.write_pkt_line(b'NAK\n')
 
+    def handle_done(self, done_required, done_received):
+        # Delegate this to the implementation.
+        return self._impl.handle_done(done_required, done_received)
+
     def set_wants(self, wants):
         self._wants = wants
 
@@ -642,24 +662,44 @@ class SingleAckGraphWalkerImpl(object):
 
     def __init__(self, walker):
         self.walker = walker
-        self._sent_ack = False
+        self._common = []
 
     def ack(self, have_ref):
-        if not self._sent_ack:
+        if not self._common:
             self.walker.send_ack(have_ref)
-            self._sent_ack = True
+            self._common.append(have_ref)
 
     def next(self):
         command, sha = self.walker.read_proto_line(_GRAPH_WALKER_COMMANDS)
         if command in (None, COMMAND_DONE):
-            if not self._sent_ack:
-                self.walker.send_nak()
+            # defer the handling of done
+            self.walker.notify_done()
             return None
         elif command == COMMAND_HAVE:
             return sha
 
     __next__ = next
 
+    def handle_done(self, done_required, done_received):
+        if not self._common:
+            self.walker.send_nak()
+
+        if done_required and not done_received:
+            # we are not done, especially when done is required; skip
+            # the pack for this request and especially do not handle
+            # the done.
+            return False
+
+        if not done_received and not self._common:
+            # Okay we are not actually done then since the walker picked
+            # up no haves.  This is usually triggered when client attempts
+            # to pull from a source that has no common base_commit.
+            # See: test_server.MultiAckDetailedGraphWalkerImplTestCase.\
+            #          test_multi_ack_stateless_nodone
+            return False
+
+        return True
+
 
 class MultiAckGraphWalkerImpl(object):
     """Graph walker implementation that speaks the multi-ack protocol."""
@@ -686,12 +726,7 @@ class MultiAckGraphWalkerImpl(object):
                 # flush but more have lines are still coming
                 continue
             elif command == COMMAND_DONE:
-                # don't nak unless no common commits were found, even if not
-                # everything is satisfied
-                if self._common:
-                    self.walker.send_ack(self._common[-1])
-                else:
-                    self.walker.send_nak()
+                self.walker.notify_done()
                 return None
             elif command == COMMAND_HAVE:
                 if self._found_base:
@@ -701,49 +736,94 @@ class MultiAckGraphWalkerImpl(object):
 
     __next__ = next
 
+    def handle_done(self, done_required, done_received):
+        if done_required and not done_received:
+            # we are not done, especially when done is required; skip
+            # the pack for this request and especially do not handle
+            # the done.
+            return False
+
+        if not done_received and not self._common:
+            # Okay we are not actually done then since the walker picked
+            # up no haves.  This is usually triggered when client attempts
+            # to pull from a source that has no common base_commit.
+            # See: test_server.MultiAckDetailedGraphWalkerImplTestCase.\
+            #          test_multi_ack_stateless_nodone
+            return False
+
+        # don't nak unless no common commits were found, even if not
+        # everything is satisfied
+        if self._common:
+            self.walker.send_ack(self._common[-1])
+        else:
+            self.walker.send_nak()
+        return True
+
 
 class MultiAckDetailedGraphWalkerImpl(object):
     """Graph walker implementation speaking the multi-ack-detailed protocol."""
 
     def __init__(self, walker):
         self.walker = walker
-        self._found_base = False
         self._common = []
 
     def ack(self, have_ref):
+        # Should only be called iff have_ref is common
         self._common.append(have_ref)
-        if not self._found_base:
-            self.walker.send_ack(have_ref, b'common')
-            if self.walker.all_wants_satisfied(self._common):
-                self._found_base = True
-                self.walker.send_ack(have_ref, b'ready')
-        # else we blind ack within next
+        self.walker.send_ack(have_ref, b'common')
 
     def next(self):
         while True:
             command, sha = self.walker.read_proto_line(_GRAPH_WALKER_COMMANDS)
             if command is None:
+                if self.walker.all_wants_satisfied(self._common):
+                    self.walker.send_ack(self._common[-1], b'ready')
                 self.walker.send_nak()
                 if self.walker.http_req:
+                    # The HTTP version of this request a flush-pkt always
+                    # signifies an end of request, so we also return
+                    # nothing here as if we are done (but not really, as
+                    # it depends on whether no-done capability was
+                    # specified and that's handled in handle_done which
+                    # may or may not call post_nodone_check depending on
+                    # that).
                     return None
-                continue
             elif command == COMMAND_DONE:
-                # don't nak unless no common commits were found, even if not
-                # everything is satisfied
-                if self._common:
-                    self.walker.send_ack(self._common[-1])
-                else:
-                    self.walker.send_nak()
-                return None
+                # Let the walker know that we got a done.
+                self.walker.notify_done()
+                break
             elif command == COMMAND_HAVE:
-                if self._found_base:
-                    # blind ack; can happen if the client has more requests
-                    # inflight
-                    self.walker.send_ack(sha, b'ready')
+                # return the sha and let the caller ACK it with the
+                # above ack method.
                 return sha
+        # don't nak unless no common commits were found, even if not
+        # everything is satisfied
 
     __next__ = next
 
+    def handle_done(self, done_required, done_received):
+        if done_required and not done_received:
+            # we are not done, especially when done is required; skip
+            # the pack for this request and especially do not handle
+            # the done.
+            return False
+
+        if not done_received and not self._common:
+            # Okay we are not actually done then since the walker picked
+            # up no haves.  This is usually triggered when client attempts
+            # to pull from a source that has no common base_commit.
+            # See: test_server.MultiAckDetailedGraphWalkerImplTestCase.\
+            #          test_multi_ack_stateless_nodone
+            return False
+
+        # don't nak unless no common commits were found, even if not
+        # everything is satisfied
+        if self._common:
+            self.walker.send_ack(self._common[-1])
+        else:
+            self.walker.send_nak()
+        return True
+
 
 class ReceivePackHandler(Handler):
     """Protocol handler for downloading a pack from the client."""
@@ -756,7 +836,8 @@ class ReceivePackHandler(Handler):
 
     @classmethod
     def capabilities(cls):
-        return (CAPABILITY_REPORT_STATUS, CAPABILITY_DELETE_REFS, CAPABILITY_SIDE_BAND_64K)
+        return (CAPABILITY_REPORT_STATUS, CAPABILITY_DELETE_REFS,
+                CAPABILITY_OFS_DELTA, CAPABILITY_SIDE_BAND_64K, CAPABILITY_NO_DONE)
 
     def _apply_pack(self, refs):
         all_exceptions = (IOError, OSError, ChecksumMismatch, ApplyDeltaError,
@@ -989,10 +1070,10 @@ def update_server_info(repo):
     This generates info/refs and objects/info/packs,
     similar to "git update-server-info".
     """
-    repo._put_named_file(os.path.join(b'info', b'refs'),
+    repo._put_named_file(os.path.join('info', 'refs'),
         b"".join(generate_info_refs(repo)))
 
-    repo._put_named_file(os.path.join(b'objects', b'info', b'packs'),
+    repo._put_named_file(os.path.join('objects', 'info', 'packs'),
         b"".join(generate_objects_info_packs(repo)))
 
 

dulwich/tests/__init__.py

@@ -30,9 +30,9 @@ import tempfile
 # If Python itself provides an exception, use that
 import unittest
 if sys.version_info < (2, 7):
-    from unittest2 import SkipTest, TestCase as _TestCase, skipIf
+    from unittest2 import SkipTest, TestCase as _TestCase, skipIf, expectedFailure
 else:
-    from unittest import SkipTest, TestCase as _TestCase, skipIf
+    from unittest import SkipTest, TestCase as _TestCase, skipIf, expectedFailure
 
 
 def get_safe_env(env=None):
@@ -186,7 +186,7 @@ def compat_test_suite():
 def test_suite():
     result = unittest.TestSuite()
     result.addTests(self_test_suite())
-    if sys.version_info[0] == 2:
+    if sys.version_info[0] == 2 and sys.platform != 'win32':
         result.addTests(tutorial_test_suite())
     from dulwich.tests.compat import test_suite as compat_test_suite
     result.addTests(compat_test_suite())

dulwich/tests/compat/server_utils.py

@@ -23,7 +23,6 @@ import errno
 import os
 import shutil
 import socket
-import sys
 import tempfile
 
 from dulwich.repo import Repo
@@ -33,10 +32,8 @@ from dulwich.server import (
     )
 from dulwich.tests.utils import (
     tear_down_repo,
-    skipIfPY3,
     )
 from dulwich.tests.compat.utils import (
-    import_repo,
     run_git_or_fail,
     )
 from dulwich.tests.compat.utils import require_git_version
@@ -48,13 +45,11 @@ class _StubRepo(object):
     def __init__(self, name):
         temp_dir = tempfile.mkdtemp()
         self.path = os.path.join(temp_dir, name)
-        if not isinstance(self.path, bytes):
-            self._path_bytes = self.path.encode(sys.getfilesystemencoding())
-        else:
-            self._path_bytes = self.path
-
         os.mkdir(self.path)
 
+    def close(self):
+        pass
+
 
 def _get_shallow(repo):
     shallow_file = repo.get_named_file('shallow')
@@ -71,7 +66,6 @@ def _get_shallow(repo):
     return shallows
 
 
-@skipIfPY3
 class ServerTests(object):
     """Base tests for testing servers.
 
@@ -81,10 +75,8 @@ class ServerTests(object):
     min_single_branch_version = (1, 7, 10,)
 
     def import_repos(self):
-        self._old_repo = import_repo('server_old.export')
-        self.addCleanup(tear_down_repo, self._old_repo)
-        self._new_repo = import_repo('server_new.export')
-        self.addCleanup(tear_down_repo, self._new_repo)
+        self._old_repo = self.import_repo('server_old.export')
+        self._new_repo = self.import_repo('server_new.export')
 
     def url(self, port):
         return '%s://localhost:%s/' % (self.protocol, port)
@@ -104,10 +96,8 @@ class ServerTests(object):
         self.assertReposEqual(self._old_repo, self._new_repo)
 
     def test_push_to_dulwich_no_op(self):
-        self._old_repo = import_repo('server_old.export')
-        self.addCleanup(tear_down_repo, self._old_repo)
-        self._new_repo = import_repo('server_old.export')
-        self.addCleanup(tear_down_repo, self._new_repo)
+        self._old_repo = self.import_repo('server_old.export')
+        self._new_repo = self.import_repo('server_old.export')
         self.assertReposEqual(self._old_repo, self._new_repo)
         port = self._start_server(self._old_repo)
 
@@ -116,10 +106,8 @@ class ServerTests(object):
         self.assertReposEqual(self._old_repo, self._new_repo)
 
     def test_push_to_dulwich_remove_branch(self):
-        self._old_repo = import_repo('server_old.export')
-        self.addCleanup(tear_down_repo, self._old_repo)
-        self._new_repo = import_repo('server_old.export')
-        self.addCleanup(tear_down_repo, self._new_repo)
+        self._old_repo = self.import_repo('server_old.export')
+        self._new_repo = self.import_repo('server_old.export')
         self.assertReposEqual(self._old_repo, self._new_repo)
         port = self._start_server(self._old_repo)
 
@@ -127,7 +115,7 @@ class ServerTests(object):
                         cwd=self._new_repo.path)
 
         self.assertEqual(
-            self._old_repo.get_refs().keys(), ["refs/heads/branch"])
+            list(self._old_repo.get_refs().keys()), [b"refs/heads/branch"])
 
     def test_fetch_from_dulwich(self):
         self.import_repos()
@@ -141,10 +129,8 @@ class ServerTests(object):
         self.assertReposEqual(self._old_repo, self._new_repo)
 
     def test_fetch_from_dulwich_no_op(self):
-        self._old_repo = import_repo('server_old.export')
-        self.addCleanup(tear_down_repo, self._old_repo)
-        self._new_repo = import_repo('server_old.export')
-        self.addCleanup(tear_down_repo, self._new_repo)
+        self._old_repo = self.import_repo('server_old.export')
+        self._new_repo = self.import_repo('server_old.export')
         self.assertReposEqual(self._old_repo, self._new_repo)
         port = self._start_server(self._new_repo)
 
@@ -155,34 +141,28 @@ class ServerTests(object):
         self.assertReposEqual(self._old_repo, self._new_repo)
 
     def test_clone_from_dulwich_empty(self):
-        old_repo_dir = os.path.join(tempfile.mkdtemp(), 'empty_old')
-        run_git_or_fail(['init', '--quiet', '--bare', old_repo_dir])
-        self._old_repo = Repo(old_repo_dir)
-        self.addCleanup(tear_down_repo, self._old_repo)
+        old_repo_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, old_repo_dir)
+        self._old_repo = Repo.init_bare(old_repo_dir)
         port = self._start_server(self._old_repo)
 
         new_repo_base_dir = tempfile.mkdtemp()
-        try:
-            new_repo_dir = os.path.join(new_repo_base_dir, 'empty_new')
-            run_git_or_fail(['clone', self.url(port), new_repo_dir],
-                            cwd=new_repo_base_dir)
-            new_repo = Repo(new_repo_dir)
-            self.assertReposEqual(self._old_repo, new_repo)
-        finally:
-            # We don't create a Repo from new_repo_dir until after some errors
-            # may have occurred, so don't depend on tearDown to clean it up.
-            shutil.rmtree(new_repo_base_dir)
+        self.addCleanup(shutil.rmtree, new_repo_base_dir)
+        new_repo_dir = os.path.join(new_repo_base_dir, 'empty_new')
+        run_git_or_fail(['clone', self.url(port), new_repo_dir],
+                        cwd=new_repo_base_dir)
+        new_repo = Repo(new_repo_dir)
+        self.assertReposEqual(self._old_repo, new_repo)
 
     def test_lsremote_from_dulwich(self):
-        self._repo = import_repo('server_old.export')
+        self._repo = self.import_repo('server_old.export')
         port = self._start_server(self._repo)
         o = run_git_or_fail(['ls-remote', self.url(port)])
-        self.assertEqual(len(o.split('\n')), 4)
+        self.assertEqual(len(o.split(b'\n')), 4)
 
     def test_new_shallow_clone_from_dulwich(self):
         require_git_version(self.min_single_branch_version)
-        self._source_repo = import_repo('server_new.export')
-        self.addCleanup(tear_down_repo, self._source_repo)
+        self._source_repo = self.import_repo('server_new.export')
         self._stub_repo = _StubRepo('shallow')
         self.addCleanup(tear_down_repo, self._stub_repo)
         port = self._start_server(self._source_repo)
@@ -191,15 +171,14 @@ class ServerTests(object):
         run_git_or_fail(['clone', '--mirror', '--depth=1', '--no-single-branch',
                         self.url(port), self._stub_repo.path])
         clone = self._stub_repo = Repo(self._stub_repo.path)
-        expected_shallow = ['94de09a530df27ac3bb613aaecdd539e0a0655e1',
-                            'da5cd81e1883c62a25bb37c4d1f8ad965b29bf8d']
+        expected_shallow = [b'94de09a530df27ac3bb613aaecdd539e0a0655e1',
+                            b'da5cd81e1883c62a25bb37c4d1f8ad965b29bf8d']
         self.assertEqual(expected_shallow, _get_shallow(clone))
         self.assertReposNotEqual(clone, self._source_repo)
 
     def test_fetch_same_depth_into_shallow_clone_from_dulwich(self):
         require_git_version(self.min_single_branch_version)
-        self._source_repo = import_repo('server_new.export')
-        self.addCleanup(tear_down_repo, self._source_repo)
+        self._source_repo = self.import_repo('server_new.export')
         self._stub_repo = _StubRepo('shallow')
         self.addCleanup(tear_down_repo, self._stub_repo)
         port = self._start_server(self._source_repo)
@@ -213,15 +192,14 @@ class ServerTests(object):
         run_git_or_fail(
           ['fetch', '--depth=1', self.url(port)] + self.branch_args(),
           cwd=self._stub_repo.path)
-        expected_shallow = ['94de09a530df27ac3bb613aaecdd539e0a0655e1',
-                            'da5cd81e1883c62a25bb37c4d1f8ad965b29bf8d']
+        expected_shallow = [b'94de09a530df27ac3bb613aaecdd539e0a0655e1',
+                            b'da5cd81e1883c62a25bb37c4d1f8ad965b29bf8d']
         self.assertEqual(expected_shallow, _get_shallow(clone))
         self.assertReposNotEqual(clone, self._source_repo)
 
     def test_fetch_full_depth_into_shallow_clone_from_dulwich(self):
         require_git_version(self.min_single_branch_version)
-        self._source_repo = import_repo('server_new.export')
-        self.addCleanup(tear_down_repo, self._source_repo)
+        self._source_repo = self.import_repo('server_new.export')
         self._stub_repo = _StubRepo('shallow')
         self.addCleanup(tear_down_repo, self._stub_repo)
         port = self._start_server(self._source_repo)
@@ -243,6 +221,43 @@ class ServerTests(object):
         self.assertEqual([], _get_shallow(clone))
         self.assertReposEqual(clone, self._source_repo)
 
+    def test_fetch_from_dulwich_issue_88_standard(self):
+        # Basically an integration test to see that the ACK/NAK
+        # generation works on repos with common head.
+        self._source_repo = self.import_repo('issue88_expect_ack_nak_server.export')
+        self._client_repo = self.import_repo('issue88_expect_ack_nak_client.export')
+        port = self._start_server(self._source_repo)
+
+        run_git_or_fail(['fetch', self.url(port), 'master',],
+                        cwd=self._client_repo.path)
+        self.assertObjectStoreEqual(
+            self._source_repo.object_store,
+            self._client_repo.object_store)
+
+    def test_fetch_from_dulwich_issue_88_alternative(self):
+        # likewise, but the case where the two repos have no common parent
+        self._source_repo = self.import_repo('issue88_expect_ack_nak_other.export')
+        self._client_repo = self.import_repo('issue88_expect_ack_nak_client.export')
+        port = self._start_server(self._source_repo)
+
+        self.assertRaises(KeyError, self._client_repo.get_object,
+            b'02a14da1fc1fc13389bbf32f0af7d8899f2b2323')
+        run_git_or_fail(['fetch', self.url(port), 'master',],
+                        cwd=self._client_repo.path)
+        self.assertEqual(b'commit', self._client_repo.get_object(
+            b'02a14da1fc1fc13389bbf32f0af7d8899f2b2323').type_name)
+
+    def test_push_to_dulwich_issue_88_standard(self):
+        # Same thing, but we reverse the role of the server/client
+        # and do a push instead.
+        self._source_repo = self.import_repo('issue88_expect_ack_nak_client.export')
+        self._client_repo = self.import_repo('issue88_expect_ack_nak_server.export')
+        port = self._start_server(self._source_repo)
+
+        run_git_or_fail(['push', self.url(port), 'master',],
+                        cwd=self._client_repo.path)
+        self.assertReposEqual(self._source_repo, self._client_repo)
+
 
 # TODO(dborowitz): Come up with a better way of testing various permutations of
 # capabilities. The only reason it is the way it is now is that side-band-64k
@@ -253,7 +268,7 @@ class NoSideBand64kReceivePackHandler(ReceivePackHandler):
     @classmethod
     def capabilities(cls):
         return tuple(c for c in ReceivePackHandler.capabilities()
-                     if c != 'side-band-64k')
+                     if c != b'side-band-64k')
 
 
 def ignore_error(error):

dulwich/tests/compat/test_client.py

@@ -19,11 +19,11 @@
 
 """Compatibilty tests between the Dulwich client and the cgit server."""
 
-from io import BytesIO
+from contextlib import closing
 import copy
+from io import BytesIO
 import os
 import select
-import shutil
 import signal
 import subprocess
 import sys
@@ -70,6 +70,7 @@ from dulwich.tests.compat.utils import (
     import_repo_to_dir,
     run_git_or_fail,
     _DEFAULT_GIT,
+    rmtree_ro,
     )
 
 
@@ -77,18 +78,20 @@ class DulwichClientTestBase(object):
     """Tests for client/server compatibility."""
 
     def setUp(self):
-        self.gitroot = os.path.dirname(import_repo_to_dir('server_new.export'))
+        self.gitroot = os.path.dirname(import_repo_to_dir('server_new.export').rstrip(os.sep))
         self.dest = os.path.join(self.gitroot, 'dest')
         file.ensure_dir_exists(self.dest)
         run_git_or_fail(['init', '--quiet', '--bare'], cwd=self.dest)
 
     def tearDown(self):
-        shutil.rmtree(self.gitroot)
+        rmtree_ro(self.gitroot)
 
     def assertDestEqualsSrc(self):
-        src = repo.Repo(os.path.join(self.gitroot, 'server_new.export'))
-        dest = repo.Repo(os.path.join(self.gitroot, 'dest'))
-        self.assertReposEqual(src, dest)
+        repo_dir = os.path.join(self.gitroot, 'server_new.export')
+        dest_repo_dir = os.path.join(self.gitroot, 'dest')
+        with closing(repo.Repo(repo_dir)) as src:
+            with closing(repo.Repo(dest_repo_dir)) as dest:
+                self.assertReposEqual(src, dest)
 
     def _client(self):
         raise NotImplementedError()
@@ -99,11 +102,11 @@ class DulwichClientTestBase(object):
     def _do_send_pack(self):
         c = self._client()
         srcpath = os.path.join(self.gitroot, 'server_new.export')
-        src = repo.Repo(srcpath)
-        sendrefs = dict(src.get_refs())
-        del sendrefs[b'HEAD']
-        c.send_pack(self._build_path('/dest'), lambda _: sendrefs,
-                    src.object_store.generate_pack_contents)
+        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,
+                        src.object_store.generate_pack_contents)
 
     def test_send_pack(self):
         self._do_send_pack()
@@ -119,12 +122,12 @@ class DulwichClientTestBase(object):
         c = self._client()
         c._send_capabilities.remove(b'report-status')
         srcpath = os.path.join(self.gitroot, 'server_new.export')
-        src = repo.Repo(srcpath)
-        sendrefs = dict(src.get_refs())
-        del sendrefs[b'HEAD']
-        c.send_pack(self._build_path('/dest'), lambda _: sendrefs,
-                    src.object_store.generate_pack_contents)
-        self.assertDestEqualsSrc()
+        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,
+                        src.object_store.generate_pack_contents)
+            self.assertDestEqualsSrc()
 
     def make_dummy_commit(self, dest):
         b = objects.Blob.from_string(b'hi')
@@ -147,9 +150,7 @@ class DulwichClientTestBase(object):
         commit_id = self.make_dummy_commit(dest)
         return dest, commit_id
 
-    def compute_send(self):
-        srcpath = os.path.join(self.gitroot, 'server_new.export')
-        src = repo.Repo(srcpath)
+    def compute_send(self, src):
         sendrefs = dict(src.get_refs())
         del sendrefs[b'HEAD']
         return sendrefs, src.object_store.generate_pack_contents
@@ -157,35 +158,39 @@ class DulwichClientTestBase(object):
     def test_send_pack_one_error(self):
         dest, dummy_commit = self.disable_ff_and_make_dummy_commit()
         dest.refs[b'refs/heads/master'] = dummy_commit
-        sendrefs, gen_pack = self.compute_send()
-        c = self._client()
-        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',
-                             e.args[0])
-            self.assertEqual({b'refs/heads/branch': b'ok',
-                              b'refs/heads/master': b'non-fast-forward'},
-                             e.ref_status)
+        repo_dir = os.path.join(self.gitroot, 'server_new.export')
+        with closing(repo.Repo(repo_dir)) as src:
+            sendrefs, gen_pack = self.compute_send(src)
+            c = self._client()
+            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',
+                                 e.args[0])
+                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):
         dest, dummy = self.disable_ff_and_make_dummy_commit()
         # set up for two non-ff errors
         branch, master = b'refs/heads/branch', b'refs/heads/master'
         dest.refs[branch] = dest.refs[master] = dummy
-        sendrefs, gen_pack = self.compute_send()
-        c = self._client()
-        try:
-            c.send_pack(self._build_path('/dest'), lambda _: sendrefs, gen_pack)
-        except errors.UpdateRefsError as e:
-            self.assertIn(str(e),
-                          ['{0}, {1} failed to update'.format(
-                              branch.decode('ascii'), master.decode('ascii')),
-                           '{1}, {0} failed to update'.format(
-                               branch.decode('ascii'), master.decode('ascii'))])
-            self.assertEqual({branch: b'non-fast-forward',
-                              master: b'non-fast-forward'},
-                             e.ref_status)
+        repo_dir = os.path.join(self.gitroot, 'server_new.export')
+        with closing(repo.Repo(repo_dir)) as src:
+            sendrefs, gen_pack = self.compute_send(src)
+            c = self._client()
+            try:
+                c.send_pack(self._build_path('/dest'), lambda _: sendrefs, gen_pack)
+            except errors.UpdateRefsError as e:
+                self.assertIn(str(e),
+                              ['{0}, {1} failed to update'.format(
+                                  branch.decode('ascii'), master.decode('ascii')),
+                               '{1}, {0} failed to update'.format(
+                                   branch.decode('ascii'), master.decode('ascii'))])
+                self.assertEqual({branch: b'non-fast-forward',
+                                  master: b'non-fast-forward'},
+                                 e.ref_status)
 
     def test_archive(self):
         c = self._client()
@@ -197,55 +202,56 @@ class DulwichClientTestBase(object):
 
     def test_fetch_pack(self):
         c = self._client()
-        dest = repo.Repo(os.path.join(self.gitroot, 'dest'))
-        refs = c.fetch(self._build_path('/server_new.export'), dest)
-        for r in refs.items():
-            dest.refs.set_if_equals(r[0], None, r[1])
-        self.assertDestEqualsSrc()
+        with closing(repo.Repo(os.path.join(self.gitroot, 'dest'))) as dest:
+            refs = c.fetch(self._build_path('/server_new.export'), dest)
+            for r in refs.items():
+                dest.refs.set_if_equals(r[0], None, r[1])
+            self.assertDestEqualsSrc()
 
     def test_incremental_fetch_pack(self):
         self.test_fetch_pack()
         dest, dummy = self.disable_ff_and_make_dummy_commit()
         dest.refs[b'refs/heads/master'] = dummy
         c = self._client()
-        dest = repo.Repo(os.path.join(self.gitroot, 'server_new.export'))
-        refs = c.fetch(self._build_path('/dest'), dest)
-        for r in refs.items():
-            dest.refs.set_if_equals(r[0], None, r[1])
-        self.assertDestEqualsSrc()
+        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)
+            for r in refs.items():
+                dest.refs.set_if_equals(r[0], None, r[1])
+            self.assertDestEqualsSrc()
 
     def test_fetch_pack_no_side_band_64k(self):
         c = self._client()
         c._fetch_capabilities.remove(b'side-band-64k')
-        dest = repo.Repo(os.path.join(self.gitroot, 'dest'))
-        refs = c.fetch(self._build_path('/server_new.export'), dest)
-        for r in refs.items():
-            dest.refs.set_if_equals(r[0], None, r[1])
-        self.assertDestEqualsSrc()
+        with closing(repo.Repo(os.path.join(self.gitroot, 'dest'))) as dest:
+            refs = c.fetch(self._build_path('/server_new.export'), dest)
+            for r in refs.items():
+                dest.refs.set_if_equals(r[0], None, r[1])
+            self.assertDestEqualsSrc()
 
     def test_fetch_pack_zero_sha(self):
         # zero sha1s are already present on the client, and should
         # be ignored
         c = self._client()
-        dest = repo.Repo(os.path.join(self.gitroot, 'dest'))
-        refs = c.fetch(self._build_path('/server_new.export'), dest,
-            lambda refs: [protocol.ZERO_SHA])
-        for r in refs.items():
-            dest.refs.set_if_equals(r[0], None, r[1])
+        with closing(repo.Repo(os.path.join(self.gitroot, 'dest'))) as dest:
+            refs = c.fetch(self._build_path('/server_new.export'), dest,
+                lambda refs: [protocol.ZERO_SHA])
+            for r in refs.items():
+                dest.refs.set_if_equals(r[0], None, r[1])
 
     def test_send_remove_branch(self):
-        dest = repo.Repo(os.path.join(self.gitroot, 'dest'))
-        dummy_commit = self.make_dummy_commit(dest)
-        dest.refs[b'refs/heads/master'] = dummy_commit
-        dest.refs[b'refs/heads/abranch'] = dummy_commit
-        sendrefs = dict(dest.refs)
-        sendrefs[b'refs/heads/abranch'] = b"00" * 20
-        del sendrefs[b'HEAD']
-        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)
-        self.assertFalse(b"refs/heads/abranch" in dest.refs)
+        with closing(repo.Repo(os.path.join(self.gitroot, 'dest'))) as dest:
+            dummy_commit = self.make_dummy_commit(dest)
+            dest.refs[b'refs/heads/master'] = dummy_commit
+            dest.refs[b'refs/heads/abranch'] = dummy_commit
+            sendrefs = dict(dest.refs)
+            sendrefs[b'refs/heads/abranch'] = b"00" * 20
+            del sendrefs[b'HEAD']
+            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)
+            self.assertFalse(b"refs/heads/abranch" in dest.refs)
 
 
 class DulwichTCPClientTest(CompatTestCase, DulwichClientTestBase):
@@ -287,6 +293,9 @@ class DulwichTCPClientTest(CompatTestCase, DulwichClientTestBase):
                 os.unlink(self.pidfile)
             except (OSError, IOError):
                 pass
+        self.process.wait()
+        self.process.stdout.close()
+        self.process.stderr.close()
         DulwichClientTestBase.tearDown(self)
         CompatTestCase.tearDown(self)
 
@@ -300,14 +309,13 @@ class DulwichTCPClientTest(CompatTestCase, DulwichClientTestBase):
 class TestSSHVendor(object):
     @staticmethod
     def run_command(host, command, username=None, port=None):
-        cmd, path = command[0].replace("'", '').split(' ')
+        cmd, path = command
         cmd = cmd.split('-', 1)
-        p = subprocess.Popen(cmd + [path], env=get_safe_env(), stdin=subprocess.PIPE,
+        p = subprocess.Popen(cmd + [path], bufsize=0, env=get_safe_env(), stdin=subprocess.PIPE,
                              stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         return client.SubprocessWrapper(p)
 
 
-@skipIfPY3
 class DulwichMockSSHClientTest(CompatTestCase, DulwichClientTestBase):
 
     def setUp(self):
@@ -322,7 +330,7 @@ class DulwichMockSSHClientTest(CompatTestCase, DulwichClientTestBase):
         client.get_ssh_vendor = self.real_vendor
 
     def _client(self):
-        return client.SSHGitClient('localhost')
+        return client.SSHGitClient(b'localhost')
 
     def _build_path(self, path):
         return self.gitroot + path

dulwich/tests/compat/test_pack.py

@@ -24,7 +24,6 @@ import binascii
 import os
 import re
 import shutil
-import sys
 import tempfile
 
 from dulwich.pack import (
@@ -67,26 +66,24 @@ class TestPack(PackTests):
         require_git_version((1, 5, 0))
         super(TestPack, self).setUp()
         self._tempdir = tempfile.mkdtemp()
-        if not isinstance(self._tempdir, bytes):
-            self._tempdir = self._tempdir.encode(sys.getfilesystemencoding())
         self.addCleanup(shutil.rmtree, self._tempdir)
 
     def test_copy(self):
         with self.get_pack(pack1_sha) as origpack:
             self.assertSucceeds(origpack.index.check)
-            pack_path = os.path.join(self._tempdir, b'Elch')
+            pack_path = os.path.join(self._tempdir, "Elch")
             write_pack(pack_path, origpack.pack_tuples())
             output = run_git_or_fail(['verify-pack', '-v', pack_path])
             orig_shas = set(o.id for o in origpack.iterobjects())
             self.assertEqual(orig_shas, _git_verify_pack_object_list(output))
 
     def test_deltas_work(self):
-        orig_pack = self.get_pack(pack1_sha)
-        orig_blob = orig_pack[a_sha]
-        new_blob = Blob()
-        new_blob.data = orig_blob.data + b'x'
-        all_to_pack = list(orig_pack.pack_tuples()) + [(new_blob, None)]
-        pack_path = os.path.join(self._tempdir, b'pack_with_deltas')
+        with self.get_pack(pack1_sha) as orig_pack:
+            orig_blob = orig_pack[a_sha]
+            new_blob = Blob()
+            new_blob.data = orig_blob.data + b'x'
+            all_to_pack = list(orig_pack.pack_tuples()) + [(new_blob, None)]
+        pack_path = os.path.join(self._tempdir, 'pack_with_deltas')
         write_pack(pack_path, all_to_pack, deltify=True)
         output = run_git_or_fail(['verify-pack', '-v', pack_path])
         self.assertEqual(set(x[0].id for x in all_to_pack),
@@ -102,15 +99,15 @@ class TestPack(PackTests):
     def test_delta_medium_object(self):
         # This tests an object set that will have a copy operation
         # 2**20 in size.
-        orig_pack = self.get_pack(pack1_sha)
-        orig_blob = orig_pack[a_sha]
-        new_blob = Blob()
-        new_blob.data = orig_blob.data + (b'x' * 2 ** 20)
-        new_blob_2 = Blob()
-        new_blob_2.data = new_blob.data + b'y'
-        all_to_pack = list(orig_pack.pack_tuples()) + [(new_blob, None),
-                                                       (new_blob_2, None)]
-        pack_path = os.path.join(self._tempdir, b'pack_with_deltas')
+        with self.get_pack(pack1_sha) as orig_pack:
+            orig_blob = orig_pack[a_sha]
+            new_blob = Blob()
+            new_blob.data = orig_blob.data + (b'x' * 2 ** 20)
+            new_blob_2 = Blob()
+            new_blob_2.data = new_blob.data + b'y'
+            all_to_pack = list(orig_pack.pack_tuples()) + [(new_blob, None),
+                                                           (new_blob_2, None)]
+        pack_path = os.path.join(self._tempdir, 'pack_with_deltas')
         write_pack(pack_path, all_to_pack, deltify=True)
         output = run_git_or_fail(['verify-pack', '-v', pack_path])
         self.assertEqual(set(x[0].id for x in all_to_pack),
@@ -136,14 +133,14 @@ class TestPack(PackTests):
         # 2**25 in size. This is a copy large enough that it requires
         # two copy operations in git's binary delta format.
         raise SkipTest('skipping slow, large test')
-        orig_pack = self.get_pack(pack1_sha)
-        orig_blob = orig_pack[a_sha]
-        new_blob = Blob()
-        new_blob.data = 'big blob' + ('x' * 2 ** 25)
-        new_blob_2 = Blob()
-        new_blob_2.data = new_blob.data + 'y'
-        all_to_pack = list(orig_pack.pack_tuples()) + [(new_blob, None),
-                                                       (new_blob_2, None)]
+        with self.get_pack(pack1_sha) as orig_pack:
+            orig_blob = orig_pack[a_sha]
+            new_blob = Blob()
+            new_blob.data = 'big blob' + ('x' * 2 ** 25)
+            new_blob_2 = Blob()
+            new_blob_2.data = new_blob.data + 'y'
+            all_to_pack = list(orig_pack.pack_tuples()) + [(new_blob, None),
+                                                           (new_blob_2, None)]
         pack_path = os.path.join(self._tempdir, "pack_with_deltas")
         write_pack(pack_path, all_to_pack, deltify=True)
         output = run_git_or_fail(['verify-pack', '-v', pack_path])

dulwich/tests/compat/test_repository.py

@@ -30,13 +30,9 @@ from dulwich.objects import (
 from dulwich.repo import (
     check_ref_format,
     )
-from dulwich.tests.utils import (
-    tear_down_repo,
-    )
 
 from dulwich.tests.compat.utils import (
     run_git_or_fail,
-    import_repo,
     CompatTestCase,
     )
 
@@ -46,8 +42,7 @@ class ObjectStoreTestCase(CompatTestCase):
 
     def setUp(self):
         super(ObjectStoreTestCase, self).setUp()
-        self._repo = import_repo('server_new.export')
-        self.addCleanup(tear_down_repo, self._repo)
+        self._repo = self.import_repo('server_new.export')
 
     def _run_git(self, args):
         return run_git_or_fail(args, cwd=self._repo.path)

dulwich/tests/compat/test_server.py

@@ -52,16 +52,16 @@ class GitServerTestCase(ServerTests, CompatTestCase):
     protocol = 'git'
 
     def _handlers(self):
-        return {'git-receive-pack': NoSideBand64kReceivePackHandler}
+        return {b'git-receive-pack': NoSideBand64kReceivePackHandler}
 
     def _check_server(self, dul_server):
-        receive_pack_handler_cls = dul_server.handlers['git-receive-pack']
+        receive_pack_handler_cls = dul_server.handlers[b'git-receive-pack']
         caps = receive_pack_handler_cls.capabilities()
-        self.assertFalse('side-band-64k' in caps)
+        self.assertFalse(b'side-band-64k' in caps)
 
     def _start_server(self, repo):
-        backend = DictBackend({'/': repo})
-        dul_server = TCPGitServer(backend, 'localhost', 0,
+        backend = DictBackend({b'/': repo})
+        dul_server = TCPGitServer(backend, b'localhost', 0,
                                   handlers=self._handlers())
         self._check_server(dul_server)
         self.addCleanup(dul_server.shutdown)
@@ -92,6 +92,6 @@ class GitServerSideBand64kTestCase(GitServerTestCase):
         return None  # default handlers include side-band-64k
 
     def _check_server(self, server):
-        receive_pack_handler_cls = server.handlers['git-receive-pack']
+        receive_pack_handler_cls = server.handlers[b'git-receive-pack']
         caps = receive_pack_handler_cls.capabilities()
-        self.assertTrue('side-band-64k' in caps)
+        self.assertTrue(b'side-band-64k' in caps)

dulwich/tests/compat/test_web.py

@@ -30,11 +30,16 @@ import sys
 
 from dulwich.server import (
     DictBackend,
+    UploadPackHandler,
+    ReceivePackHandler,
     )
 from dulwich.tests import (
     SkipTest,
     skipIf,
     )
+from dulwich.tests.utils import (
+    skipIfPY3,
+    )
 from dulwich.web import (
     make_wsgi_chain,
     HTTPGitApplication,
@@ -76,6 +81,7 @@ class WebTests(ServerTests):
 
 
 @skipIf(sys.platform == 'win32', 'Broken on windows, with very long fail time.')
+@skipIfPY3
 class SmartWebTestCase(WebTests, CompatTestCase):
     """Test cases for smart HTTP server.
 
@@ -88,9 +94,9 @@ class SmartWebTestCase(WebTests, CompatTestCase):
         return {'git-receive-pack': NoSideBand64kReceivePackHandler}
 
     def _check_app(self, app):
-        receive_pack_handler_cls = app.handlers['git-receive-pack']
+        receive_pack_handler_cls = app.handlers[b'git-receive-pack']
         caps = receive_pack_handler_cls.capabilities()
-        self.assertFalse('side-band-64k' in caps)
+        self.assertFalse(b'side-band-64k' in caps)
 
     def _make_app(self, backend):
         app = make_wsgi_chain(backend, handlers=self._handlers())
@@ -102,23 +108,66 @@ class SmartWebTestCase(WebTests, CompatTestCase):
         return app
 
 
+def patch_capabilities(handler, caps_removed):
+    # Patch a handler's capabilities by specifying a list of them to be
+    # removed, and return the original classmethod for restoration.
+    original_capabilities = handler.capabilities
+    filtered_capabilities = tuple(
+        i for i in original_capabilities() if i not in caps_removed)
+    def capabilities(cls):
+        return filtered_capabilities
+    handler.capabilities = classmethod(capabilities)
+    return original_capabilities
+
+
 @skipIf(sys.platform == 'win32', 'Broken on windows, with very long fail time.')
+@skipIfPY3
 class SmartWebSideBand64kTestCase(SmartWebTestCase):
     """Test cases for smart HTTP server with side-band-64k support."""
 
     # side-band-64k in git-receive-pack was introduced in git 1.7.0.2
     min_git_version = (1, 7, 0, 2)
 
+    def setUp(self):
+        self.o_uph_cap = patch_capabilities(UploadPackHandler, ("no-done",))
+        self.o_rph_cap = patch_capabilities(ReceivePackHandler, ("no-done",))
+        super(SmartWebSideBand64kTestCase, self).setUp()
+
+    def tearDown(self):
+        super(SmartWebSideBand64kTestCase, self).tearDown()
+        UploadPackHandler.capabilities = self.o_uph_cap
+        ReceivePackHandler.capabilities = self.o_rph_cap
+
+    def _handlers(self):
+        return None  # default handlers include side-band-64k
+
+    def _check_app(self, app):
+        receive_pack_handler_cls = app.handlers[b'git-receive-pack']
+        caps = receive_pack_handler_cls.capabilities()
+        self.assertTrue(b'side-band-64k' in caps)
+        self.assertFalse(b'no-done' in caps)
+
+
+class SmartWebSideBand64kNoDoneTestCase(SmartWebTestCase):
+    """Test cases for smart HTTP server with side-band-64k and no-done
+    support.
+    """
+
+    # no-done was introduced in git 1.7.4
+    min_git_version = (1, 7, 4)
+
     def _handlers(self):
         return None  # default handlers include side-band-64k
 
     def _check_app(self, app):
-        receive_pack_handler_cls = app.handlers['git-receive-pack']
+        receive_pack_handler_cls = app.handlers[b'git-receive-pack']
         caps = receive_pack_handler_cls.capabilities()
-        self.assertTrue('side-band-64k' in caps)
+        self.assertTrue(b'side-band-64k' in caps)
+        self.assertTrue(b'no-done' in caps)
 
 
 @skipIf(sys.platform == 'win32', 'Broken on windows, with very long fail time.')
+@skipIfPY3
 class DumbWebTestCase(WebTests, CompatTestCase):
     """Test cases for dumb HTTP server."""
 
@@ -147,3 +196,7 @@ class DumbWebTestCase(WebTests, CompatTestCase):
         # Note: remove this if C git and dulwich implement dumb web shallow
         # clones.
         raise SkipTest('Dumb web shallow cloning not supported.')
+
+    def test_push_to_dulwich_issue_88_standard(self):
+        raise SkipTest('Dumb web pushing not supported.')
+

dulwich/tests/compat/utils.py

@@ -20,9 +20,13 @@
 """Utilities for interacting with cgit."""
 
 import errno
+import functools
 import os
+import shutil
 import socket
+import stat
 import subprocess
+import sys
 import tempfile
 import time
 
@@ -148,7 +152,7 @@ def import_repo_to_dir(name):
     """Import a repo from a fast-export file in a temporary directory.
 
     These are used rather than binary repos for compat tests because they are
-    more compact an human-editable, and we already depend on git.
+    more compact and human-editable, and we already depend on git.
 
     :param name: The name of the repository export file, relative to
         dulwich/tests/data/repos.
@@ -165,16 +169,6 @@ def import_repo_to_dir(name):
     return temp_repo_dir
 
 
-def import_repo(name):
-    """Import a repo from a fast-export file in a temporary directory.
-
-    :param name: The name of the repository export file, relative to
-        dulwich/tests/data/repos.
-    :returns: An initialized Repo object that lives in a temporary directory.
-    """
-    return Repo(import_repo_to_dir(name))
-
-
 def check_for_daemon(limit=10, delay=0.1, timeout=0.1, port=TCP_GIT_PORT):
     """Check for a running TCP daemon.
 
@@ -219,10 +213,12 @@ class CompatTestCase(TestCase):
         super(CompatTestCase, self).setUp()
         require_git_version(self.min_git_version)
 
+    def assertObjectStoreEqual(self, store1, store2):
+        self.assertEqual(sorted(set(store1)), sorted(set(store2)))
+
     def assertReposEqual(self, repo1, repo2):
         self.assertEqual(repo1.get_refs(), repo2.get_refs())
-        self.assertEqual(sorted(set(repo1.object_store)),
-                         sorted(set(repo2.object_store)))
+        self.assertObjectStoreEqual(repo1.object_store, repo2.object_store)
 
     def assertReposNotEqual(self, repo1, repo2):
         refs1 = repo1.get_refs()
@@ -230,3 +226,28 @@ class CompatTestCase(TestCase):
         refs2 = repo2.get_refs()
         objs2 = set(repo2.object_store)
         self.assertFalse(refs1 == refs2 and objs1 == objs2)
+
+    def import_repo(self, name):
+        """Import a repo from a fast-export file in a temporary directory.
+
+        :param name: The name of the repository export file, relative to
+            dulwich/tests/data/repos.
+        :returns: An initialized Repo object that lives in a temporary directory.
+        """
+        path = import_repo_to_dir(name)
+        repo = Repo(path)
+        def cleanup():
+            repo.close()
+            rmtree_ro(os.path.dirname(path.rstrip(os.sep)))
+        self.addCleanup(cleanup)
+        return repo
+
+
+if sys.platform == 'win32':
+    def remove_ro(action, name, exc):
+        os.chmod(name, stat.S_IWRITE)
+        os.remove(name)
+
+    rmtree_ro = functools.partial(shutil.rmtree, onerror=remove_ro)
+else:
+    rmtree_ro = shutil.rmtree

dulwich/tests/data/repos/issue88_expect_ack_nak_client.export

@@ -0,0 +1,260 @@
+reset refs/heads/master
+commit refs/heads/master
+mark :1
+author User <user@localhost> 1427183369 +1300
+committer User <user@localhost> 1427183369 +1300
+data 6
+empty
+
+blob
+mark :2
+data 35
+We will reproduce a problem here.
+
+commit refs/heads/master
+mark :3
+author User <user@localhost> 1427183376 +1300
+committer User <user@localhost> 1427183376 +1300
+data 11
+demo file.
+from :1
+M 100644 :2 demo.txt
+
+blob
+mark :4
+data 62
+We will reproduce a problem here.
+
+This will take some time.
+
+commit refs/heads/master
+mark :5
+author User <user@localhost> 1427185135 +1300
+committer User <user@localhost> 1427185135 +1300
+data 13
+added a line
+from :3
+M 100644 :4 demo.txt
+
+blob
+mark :6
+data 57
+We will reproduce a problem here.
+
+We will change these.
+
+commit refs/heads/master
+mark :7
+author User <user@localhost> 1427185245 +1300
+committer User <user@localhost> 1427185245 +1300
+data 14
+replace a line
+from :5
+M 100644 :6 demo.txt
+
+blob
+mark :8
+data 52
+We will change these.
+
+Then issues will be proven.
+
+commit refs/heads/master
+mark :9
+author User <user@localhost> 1427185343 +1300
+committer User <user@localhost> 1427185343 +1300
+data 13
+Yes we will.
+from :7
+M 100644 :8 demo.txt
+
+blob
+mark :10
+data 69
+We will change these. 
+
+Then issues will be proven once and for all.
+
+commit refs/heads/master
+mark :11
+author User <user@localhost> 1427185440 +1300
+committer User <user@localhost> 1427185440 +1300
+data 6
+sure.
+from :9
+M 100644 :10 demo.txt
+
+blob
+mark :12
+data 0
+
+commit refs/heads/master
+mark :13
+author User <user@localhost> 1427185512 +1300
+committer User <user@localhost> 1427185516 +1300
+data 26
+not an actual readme, yet
+from :11
+M 100644 :12 readme.txt
+
+blob
+mark :14
+data 61
+This will for sure we will prove a problem exist somewhere.
+
+blob
+mark :15
+data 49
+okay fine add something here this is only a test
+
+commit refs/heads/master
+mark :16
+author User <user@localhost> 1427185569 +1300
+committer User <user@localhost> 1427185569 +1300
+data 12
+more things
+from :13
+M 100644 :14 demo.txt
+M 100644 :15 readme.txt
+
+blob
+mark :17
+data 100
+This will for sure we will prove a problem exist somewhere. 
+
+Just that we need a few more commits.
+
+commit refs/heads/master
+mark :18
+author User <user@localhost> 1427185659 +1300
+committer User <user@localhost> 1427185659 +1300
+data 13
+one more try
+from :16
+M 100644 :17 demo.txt
+
+blob
+mark :19
+data 54
+It might have something to do with number of commits?
+
+commit refs/heads/master
+mark :20
+author User <user@localhost> 1427185905 +1300
+committer User <user@localhost> 1427185905 +1300
+data 18
+is this number 9?
+from :18
+M 100644 :19 commitcount
+
+blob
+mark :21
+data 123
+This will for sure we will prove a problem exist somewhere. 
+
+Just that we need a few more commits.
+
+Hey look we need more
+
+commit refs/heads/master
+mark :22
+author User <user@localhost> 1427185922 +1300
+committer User <user@localhost> 1427185922 +1300
+data 5
+cool
+from :20
+M 100644 :21 demo.txt
+
+blob
+mark :23
+data 50
+Okay fine add something here this is only a test.
+
+commit refs/heads/master
+mark :24
+author User <user@localhost> 1427185936 +1300
+committer User <user@localhost> 1427185936 +1300
+data 7
+readme
+from :22
+M 100644 :23 readme.txt
+
+blob
+mark :25
+data 74
+Okay come on this is getting boring.
+
+Yes I went and edit all the things.
+
+commit refs/heads/master
+mark :26
+author User <user@localhost> 1427185954 +1300
+committer User <user@localhost> 1427185954 +1300
+data 14
+remove a line
+from :24
+M 100644 :25 demo.txt
+
+blob
+mark :27
+data 186
+Okay come on this is getting boring. 
+
+Yes I went and edit all the things. 
+
+Of course, making test data can be somewhat tedious, especially a
+minimum set that can be easily reproduced.
+
+commit refs/heads/master
+mark :28
+author User <user@localhost> 1427185996 +1300
+committer User <user@localhost> 1427185996 +1300
+data 25
+Getting serious mode on.
+from :26
+M 100644 :27 demo.txt
+
+blob
+mark :29
+data 48
+This is taking a bit longer than I remembered.
+
+commit refs/heads/master
+mark :30
+author User <user@localhost> 1427186065 +1300
+committer User <user@localhost> 1427186065 +1300
+data 40
+At least we will have things minimized.
+from :28
+M 100644 :29 demo.txt
+
+blob
+mark :31
+data 11
+there yet?
+
+commit refs/heads/master
+mark :32
+author User <user@localhost> 1427186080 +1300
+committer User <user@localhost> 1427186080 +1300
+data 7
+are we
+from :30
+M 100644 :31 demo.txt
+
+blob
+mark :33
+data 237
+This should be the head commit for the client repo for testing out
+the failure case reported in issue 88.  Just do a git pull from the
+repo that includes the following commit that is hosted with dulwich.
+The issue should be reproduced.
+
+commit refs/heads/master
+mark :34
+author User <user@localhost> 1427186109 +1300
+committer User <user@localhost> 1427186109 +1300
+data 6
+okay?
+from :32
+M 100644 :33 readme.txt

dulwich/tests/data/repos/issue88_expect_ack_nak_other.export

@@ -0,0 +1,293 @@
+blob
+mark :1
+data 33
+We will sneak in a blob like so.
+
+reset refs/heads/master
+commit refs/heads/master
+mark :2
+author User <user@localhost> 1427183369 +1300
+committer User <user@localhost> 1427183369 +1300
+data 7
+sneaky
+M 100644 :1 problem.questionmark
+
+blob
+mark :3
+data 35
+We will introduce a problem here.
+
+
+commit refs/heads/master
+mark :4
+author User <user@localhost> 1427183376 +1300
+committer User <user@localhost> 1427183376 +1300
+data 11
+demo file.
+from :2
+M 100644 :3 demo.rst
+
+blob
+mark :5
+data 62
+We will introduce a problem here.
+
+This will take some time.
+
+
+commit refs/heads/master
+mark :6
+author User <user@localhost> 1427185135 +1300
+committer User <user@localhost> 1427185135 +1300
+data 13
+added a line
+from :4
+M 100644 :5 demo.rst
+
+blob
+mark :7
+data 57
+We will introduce a problem here.
+
+We will change these.
+
+commit refs/heads/master
+mark :8
+author User <user@localhost> 1427185245 +1300
+committer User <user@localhost> 1427185245 +1300
+data 14
+replace a linefrom :6
+M 100644 :7 demo.rst
+
+blob
+mark :9
+data 52
+We will change these.
+
+Then issues will be proven.
+
+
+commit refs/heads/master
+mark :10
+author User <user@localhost> 1427185343 +1300
+committer User <user@localhost> 1427185343 +1300
+data 13
+Yes we will.
+from :8
+M 100644 :9 demo.rst
+
+blob
+mark :11
+data 72
+We will change these. 
+
+Then issues will be construed once and for all.
+
+commit refs/heads/master
+mark :12
+author User <user@localhost> 1427185440 +1300
+committer User <user@localhost> 1427185440 +1300
+data 6
+sure.
+from :10
+M 100644 :11 demo.rst
+
+blob
+mark :13
+data 0
+
+commit refs/heads/master
+mark :14
+author User <user@localhost> 1427185512 +1300
+committer User <user@localhost> 1427185516 +1300
+data 26
+not an actual readme, yet
+from :12
+M 100644 :13 emdaer.txt
+
+blob
+mark :15
+data 58
+This will for sure we will prove issues exist somewhere.
+
+
+blob
+mark :16
+data 49
+okay fine add something here this is only a test
+
+commit refs/heads/master
+mark :17
+author User <user@localhost> 1427185569 +1300
+committer User <user@localhost> 1427185569 +1300
+data 12
+more things
+from :14
+M 100644 :15 demo.rst
+M 100644 :16 emdaer.txt
+
+blob
+mark :18
+data 97
+This will for sure prove issue exist somewhere.
+
+Just that we need a few more commits as usual.
+
+
+commit refs/heads/master
+mark :19
+author User <user@localhost> 1427185659 +1300
+committer User <user@localhost> 1427185659 +1300
+data 13
+one more try
+from :17
+M 100644 :18 demo.rst
+
+blob
+mark :20
+data 54
+It might have something to do with number of commits?
+
+commit refs/heads/master
+mark :21
+author User <user@localhost> 1427185905 +1300
+committer User <user@localhost> 1427185905 +1300
+data 18
+is this number 9?
+from :19
+M 100644 :20 count
+
+blob
+mark :22
+data 119
+This will for sure we will prove issues exist somewhere.
+
+Just that we need a few more commits.
+
+Hey look we need more
+
+commit refs/heads/master
+mark :23
+author User <user@localhost> 1427185922 +1300
+committer User <user@localhost> 1427185922 +1300
+data 5
+cool
+from :21
+M 100644 :22 demo.rst
+
+blob
+mark :24
+data 50
+Okay fine add something here this is only a test.
+
+commit refs/heads/master
+mark :25
+author User <user@localhost> 1427185936 +1300
+committer User <user@localhost> 1427185936 +1300
+data 7
+readme
+from :23
+M 100644 :24 emdaer.txt
+
+blob
+mark :26
+data 74
+Okay come on this is getting boring.
+
+Yes I went and edit all the things.
+
+commit refs/heads/master
+mark :27
+author User <user@localhost> 1427185954 +1300
+committer User <user@localhost> 1427185954 +1300
+data 14
+remove a line
+from :25
+M 100644 :26 demo.rst
+
+blob
+mark :28
+data 186
+Okay come on this is getting boring. 
+
+Yes I went and edit all the things. 
+
+Of course, making test data can be somewhat tedious, especially a
+minimum set that can be easily reproduced.
+
+commit refs/heads/master
+mark :29
+author User <user@localhost> 1427185996 +1300
+committer User <user@localhost> 1427185996 +1300
+data 25
+Getting serious mode on.
+from :27
+M 100644 :28 demo.rst
+
+blob
+mark :30
+data 48
+This is taking a bit longer than I remembered.
+
+
+commit refs/heads/master
+mark :31
+author User <user@localhost> 1427186065 +1300
+committer User <user@localhost> 1427186065 +1300
+data 40
+At least we will have things minimized.
+from :29
+M 100644 :30 demo.rst
+
+blob
+mark :32
+data 11
+there yet?
+
+commit refs/heads/master
+mark :33
+author User <user@localhost> 1427186080 +1300
+committer User <user@localhost> 1427186080 +1300
+data 7
+are we
+from :31
+M 100644 :32 demo.rst
+
+blob
+mark :34
+data 237
+This should be the head commit for the client repo for testing out
+the failure case reported in issue 88.  Just do a git pull from the
+repo that includes the following commit that is hosted with dulwich.
+The issue should be reproduced.
+
+
+commit refs/heads/master
+mark :35
+author User <user@localhost> 1427186109 +1300
+committer User <user@localhost> 1427186109 +1300
+data 6
+okay?
+from :33
+M 100644 :34 emdaer.txt
+
+blob
+mark :36
+data 394
+This should be the commit that will trigger the bug noted in issue 88
+(https://github.com/jelmer/dulwich/issues/88).  To reproduce, run git
+fast-import using this fast-export and host this using dulwich, and
+then make a copy of this, strip out this blob and the following commit
+block, import to another git repo and then git clone from the previous.
+
+Naturally, this is part of the test case.
+
+commit refs/heads/master
+mark :37
+author User <user@localhost> 1427244891 +1300
+committer User <user@localhost> 1427248186 +1300
+data 49
+Added instructions on how to use this to readme.
+from :35
+M 100644 :36 emdaer.txt
+

dulwich/tests/data/repos/issue88_expect_ack_nak_server.export

@@ -0,0 +1,281 @@
+reset refs/heads/master
+commit refs/heads/master
+mark :1
+author User <user@localhost> 1427183369 +1300
+committer User <user@localhost> 1427183369 +1300
+data 6
+empty
+
+blob
+mark :2
+data 35
+We will reproduce a problem here.
+
+commit refs/heads/master
+mark :3
+author User <user@localhost> 1427183376 +1300
+committer User <user@localhost> 1427183376 +1300
+data 11
+demo file.
+from :1
+M 100644 :2 demo.txt
+
+blob
+mark :4
+data 62
+We will reproduce a problem here.
+
+This will take some time.
+
+commit refs/heads/master
+mark :5
+author User <user@localhost> 1427185135 +1300
+committer User <user@localhost> 1427185135 +1300
+data 13
+added a line
+from :3
+M 100644 :4 demo.txt
+
+blob
+mark :6
+data 57
+We will reproduce a problem here.
+
+We will change these.
+
+commit refs/heads/master
+mark :7
+author User <user@localhost> 1427185245 +1300
+committer User <user@localhost> 1427185245 +1300
+data 14
+replace a line
+from :5
+M 100644 :6 demo.txt
+
+blob
+mark :8
+data 52
+We will change these.
+
+Then issues will be proven.
+
+commit refs/heads/master
+mark :9
+author User <user@localhost> 1427185343 +1300
+committer User <user@localhost> 1427185343 +1300
+data 13
+Yes we will.
+from :7
+M 100644 :8 demo.txt
+
+blob
+mark :10
+data 69
+We will change these. 
+
+Then issues will be proven once and for all.
+
+commit refs/heads/master
+mark :11
+author User <user@localhost> 1427185440 +1300
+committer User <user@localhost> 1427185440 +1300
+data 6
+sure.
+from :9
+M 100644 :10 demo.txt
+
+blob
+mark :12
+data 0
+
+commit refs/heads/master
+mark :13
+author User <user@localhost> 1427185512 +1300
+committer User <user@localhost> 1427185516 +1300
+data 26
+not an actual readme, yet
+from :11
+M 100644 :12 readme.txt
+
+blob
+mark :14
+data 61
+This will for sure we will prove a problem exist somewhere.
+
+blob
+mark :15
+data 49
+okay fine add something here this is only a test
+
+commit refs/heads/master
+mark :16
+author User <user@localhost> 1427185569 +1300
+committer User <user@localhost> 1427185569 +1300
+data 12
+more things
+from :13
+M 100644 :14 demo.txt
+M 100644 :15 readme.txt
+
+blob
+mark :17
+data 100
+This will for sure we will prove a problem exist somewhere. 
+
+Just that we need a few more commits.
+
+commit refs/heads/master
+mark :18
+author User <user@localhost> 1427185659 +1300
+committer User <user@localhost> 1427185659 +1300
+data 13
+one more try
+from :16
+M 100644 :17 demo.txt
+
+blob
+mark :19
+data 54
+It might have something to do with number of commits?
+
+commit refs/heads/master
+mark :20
+author User <user@localhost> 1427185905 +1300
+committer User <user@localhost> 1427185905 +1300
+data 18
+is this number 9?
+from :18
+M 100644 :19 commitcount
+
+blob
+mark :21
+data 123
+This will for sure we will prove a problem exist somewhere. 
+
+Just that we need a few more commits.
+
+Hey look we need more
+
+commit refs/heads/master
+mark :22
+author User <user@localhost> 1427185922 +1300
+committer User <user@localhost> 1427185922 +1300
+data 5
+cool
+from :20
+M 100644 :21 demo.txt
+
+blob
+mark :23
+data 50
+Okay fine add something here this is only a test.
+
+commit refs/heads/master
+mark :24
+author User <user@localhost> 1427185936 +1300
+committer User <user@localhost> 1427185936 +1300
+data 7
+readme
+from :22
+M 100644 :23 readme.txt
+
+blob
+mark :25
+data 74
+Okay come on this is getting boring.
+
+Yes I went and edit all the things.
+
+commit refs/heads/master
+mark :26
+author User <user@localhost> 1427185954 +1300
+committer User <user@localhost> 1427185954 +1300
+data 14
+remove a line
+from :24
+M 100644 :25 demo.txt
+
+blob
+mark :27
+data 186
+Okay come on this is getting boring. 
+
+Yes I went and edit all the things. 
+
+Of course, making test data can be somewhat tedious, especially a
+minimum set that can be easily reproduced.
+
+commit refs/heads/master
+mark :28
+author User <user@localhost> 1427185996 +1300
+committer User <user@localhost> 1427185996 +1300
+data 25
+Getting serious mode on.
+from :26
+M 100644 :27 demo.txt
+
+blob
+mark :29
+data 48
+This is taking a bit longer than I remembered.
+
+commit refs/heads/master
+mark :30
+author User <user@localhost> 1427186065 +1300
+committer User <user@localhost> 1427186065 +1300
+data 40
+At least we will have things minimized.
+from :28
+M 100644 :29 demo.txt
+
+blob
+mark :31
+data 11
+there yet?
+
+commit refs/heads/master
+mark :32
+author User <user@localhost> 1427186080 +1300
+committer User <user@localhost> 1427186080 +1300
+data 7
+are we
+from :30
+M 100644 :31 demo.txt
+
+blob
+mark :33
+data 237
+This should be the head commit for the client repo for testing out
+the failure case reported in issue 88.  Just do a git pull from the
+repo that includes the following commit that is hosted with dulwich.
+The issue should be reproduced.
+
+commit refs/heads/master
+mark :34
+author User <user@localhost> 1427186109 +1300
+committer User <user@localhost> 1427186109 +1300
+data 6
+okay?
+from :32
+M 100644 :33 readme.txt
+
+blob
+mark :35
+data 394
+This should be the commit that will trigger the bug noted in issue 88
+(https://github.com/jelmer/dulwich/issues/88).  To reproduce, run git
+fast-import using this fast-export and host this using dulwich, and
+then make a copy of this, strip out this blob and the following commit
+block, import to another git repo and then git clone from the previous.
+
+Naturally, this is part of the test case.
+
+commit refs/heads/master
+mark :36
+author User <user@localhost> 1427244891 +1300
+committer User <user@localhost> 1427248186 +1300
+data 49
+Added instructions on how to use this to readme.
+from :34
+M 100644 :35 readme.txt
+

dulwich/tests/test_blackbox.py

@@ -19,6 +19,7 @@
 """Blackbox tests for Dulwich commands."""
 
 import tempfile
+import shutil
 
 from dulwich.repo import (
     Repo,
@@ -34,6 +35,7 @@ class GitReceivePackTests(BlackboxTestCase):
     def setUp(self):
         super(GitReceivePackTests, self).setUp()
         self.path = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.path)
         self.repo = Repo.init(self.path)
 
     def test_basic(self):
@@ -57,6 +59,7 @@ class GitUploadPackTests(BlackboxTestCase):
     def setUp(self):
         super(GitUploadPackTests, self).setUp()
         self.path = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.path)
         self.repo = Repo.init(self.path)
 
     def test_missing_arg(self):

dulwich/tests/test_client.py

@@ -16,6 +16,7 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
 # MA  02110-1301, USA.
 
+from contextlib import closing
 from io import BytesIO
 import sys
 import shutil
@@ -59,6 +60,7 @@ from dulwich.repo import (
 from dulwich.tests import skipIf
 from dulwich.tests.utils import (
     open_repo,
+    tear_down_repo,
     )
 
 
@@ -385,15 +387,15 @@ class TestGetTransportAndPath(TestCase):
         self.assertEqual('user', c.username)
         self.assertEqual('bar/baz', path)
 
-    def test_subprocess(self):
+    def test_local(self):
         c, path = get_transport_and_path('foo.bar/baz')
-        self.assertTrue(isinstance(c, SubprocessGitClient))
+        self.assertTrue(isinstance(c, LocalGitClient))
         self.assertEqual('foo.bar/baz', path)
 
     @skipIf(sys.platform != 'win32', 'Behaviour only happens on windows.')
     def test_local_abs_windows_path(self):
         c, path = get_transport_and_path('C:\\foo.bar\\baz')
-        self.assertTrue(isinstance(c, SubprocessGitClient))
+        self.assertTrue(isinstance(c, LocalGitClient))
         self.assertEqual('C:\\foo.bar\\baz', path)
 
     def test_error(self):
@@ -483,7 +485,7 @@ class TestGetTransportAndPathFromUrl(TestCase):
 
     def test_file(self):
         c, path = get_transport_and_path_from_url('file:///home/jelmer/foo')
-        self.assertTrue(isinstance(c, SubprocessGitClient))
+        self.assertTrue(isinstance(c, LocalGitClient))
         self.assertEqual('/home/jelmer/foo', path)
 
 
@@ -525,13 +527,13 @@ class SSHGitClientTests(TestCase):
         client.get_ssh_vendor = self.real_vendor
 
     def test_default_command(self):
-        self.assertEqual(b'git-upload-pack',
+        self.assertEqual('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(b'/usr/lib/git/git-upload-pack',
+        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(b'upload-pack'))
 
     def test_connect(self):
@@ -541,13 +543,13 @@ class SSHGitClientTests(TestCase):
         client.username = b"username"
         client.port = 1337
 
-        client._connect(b"command", b"/path/to/repo")
+        client._connect(b"command", "/path/to/repo")
         self.assertEqual(b"username", server.username)
         self.assertEqual(1337, server.port)
-        self.assertEqual([b"git-command '/path/to/repo'"], server.command)
+        self.assertEqual(["git-command", "/path/to/repo"], server.command)
 
-        client._connect(b"relative-command", b"/~/path/to/repo")
-        self.assertEqual([b"git-relative-command '~/path/to/repo'"],
+        client._connect(b"relative-command", "/~/path/to/repo")
+        self.assertEqual(["git-relative-command",  "~/path/to/repo"],
                           server.command)
 
 
@@ -581,11 +583,13 @@ class LocalGitClientTests(TestCase):
         c = LocalGitClient()
         t = MemoryRepo()
         s = open_repo('a.git')
+        self.addCleanup(tear_down_repo, s)
         self.assertEqual(s.get_refs(), c.fetch(s.path, t))
 
     def test_fetch_empty(self):
         c = LocalGitClient()
         s = open_repo('a.git')
+        self.addCleanup(tear_down_repo, s)
         out = BytesIO()
         walker = {}
         c.fetch_pack(s.path, lambda heads: [], graph_walker=walker,
@@ -596,6 +600,7 @@ class LocalGitClientTests(TestCase):
     def test_fetch_pack_none(self):
         c = LocalGitClient()
         s = open_repo('a.git')
+        self.addCleanup(tear_down_repo, s)
         out = BytesIO()
         walker = MemoryRepo().get_graph_walker()
         c.fetch_pack(s.path,
@@ -606,15 +611,21 @@ class LocalGitClientTests(TestCase):
 
     def test_send_pack_without_changes(self):
         local = open_repo('a.git')
+        self.addCleanup(tear_down_repo, local)
+
         target = open_repo('a.git')
+        self.addCleanup(tear_down_repo, target)
+
         self.send_and_verify(b"master", local, target)
 
     def test_send_pack_with_changes(self):
         local = open_repo('a.git')
+        self.addCleanup(tear_down_repo, local)
+
         target_path = tempfile.mkdtemp()
         self.addCleanup(shutil.rmtree, target_path)
-        target = Repo.init_bare(target_path)
-        self.send_and_verify(b"master", local, target)
+        with closing(Repo.init_bare(target_path)) as target:
+            self.send_and_verify(b"master", local, target)
 
     def send_and_verify(self, branch, local, target):
         client = LocalGitClient()

dulwich/tests/test_file.py

@@ -92,9 +92,7 @@ class GitFileTests(TestCase):
     def setUp(self):
         super(GitFileTests, self).setUp()
         self._tempdir = tempfile.mkdtemp()
-        if not isinstance(self._tempdir, bytes):
-            self._tempdir = self._tempdir.encode(sys.getfilesystemencoding())
-        f = open(self.path(b'foo'), 'wb')
+        f = open(self.path('foo'), 'wb')
         f.write(b'foo contents')
         f.close()
 
@@ -106,7 +104,7 @@ class GitFileTests(TestCase):
         return os.path.join(self._tempdir, filename)
 
     def test_invalid(self):
-        foo = self.path(b'foo')
+        foo = self.path('foo')
         self.assertRaises(IOError, GitFile, foo, mode='r')
         self.assertRaises(IOError, GitFile, foo, mode='ab')
         self.assertRaises(IOError, GitFile, foo, mode='r+b')
@@ -114,7 +112,7 @@ class GitFileTests(TestCase):
         self.assertRaises(IOError, GitFile, foo, mode='a+bU')
 
     def test_readonly(self):
-        f = GitFile(self.path(b'foo'), 'rb')
+        f = GitFile(self.path('foo'), 'rb')
         self.assertTrue(isinstance(f, io.IOBase))
         self.assertEqual(b'foo contents', f.read())
         self.assertEqual(b'', f.read())
@@ -123,13 +121,13 @@ class GitFileTests(TestCase):
         f.close()
 
     def test_default_mode(self):
-        f = GitFile(self.path(b'foo'))
+        f = GitFile(self.path('foo'))
         self.assertEqual(b'foo contents', f.read())
         f.close()
 
     def test_write(self):
-        foo = self.path(b'foo')
-        foo_lock = foo + b'.lock'
+        foo = self.path('foo')
+        foo_lock = '%s.lock' % foo
 
         orig_f = open(foo, 'rb')
         self.assertEqual(orig_f.read(), b'foo contents')
@@ -152,7 +150,7 @@ class GitFileTests(TestCase):
         new_f.close()
 
     def test_open_twice(self):
-        foo = self.path(b'foo')
+        foo = self.path('foo')
         f1 = GitFile(foo, 'wb')
         f1.write(b'new')
         try:
@@ -171,8 +169,8 @@ class GitFileTests(TestCase):
         f.close()
 
     def test_abort(self):
-        foo = self.path(b'foo')
-        foo_lock = foo + b'.lock'
+        foo = self.path('foo')
+        foo_lock = '%s.lock' % foo
 
         orig_f = open(foo, 'rb')
         self.assertEqual(orig_f.read(), b'foo contents')
@@ -189,7 +187,7 @@ class GitFileTests(TestCase):
         new_orig_f.close()
 
     def test_abort_close(self):
-        foo = self.path(b'foo')
+        foo = self.path('foo')
         f = GitFile(foo, 'wb')
         f.abort()
         try:
@@ -205,11 +203,11 @@ class GitFileTests(TestCase):
             self.fail()
 
     def test_abort_close_removed(self):
-        foo = self.path(b'foo')
+        foo = self.path('foo')
         f = GitFile(foo, 'wb')
 
         f._file.close()
-        os.remove(foo + b'.lock')
+        os.remove(foo+".lock")
 
         f.abort()
         self.assertTrue(f._closed)

dulwich/tests/test_grafts.py

@@ -163,7 +163,7 @@ class GraftsInRepoTests(GraftsInRepositoryBase, TestCase):
 
     def test_init_with_empty_info_grafts(self):
         r = self._repo
-        r._put_named_file(os.path.join(b'info', b'grafts'), b'')
+        r._put_named_file(os.path.join('info', 'grafts'), b'')
 
         r = Repo(self._repo_dir)
         self.assertEqual({}, r._graftpoints)
@@ -171,7 +171,7 @@ class GraftsInRepoTests(GraftsInRepositoryBase, TestCase):
     def test_init_with_info_grafts(self):
         r = self._repo
         r._put_named_file(
-            os.path.join(b'info', b'grafts'),
+            os.path.join('info', 'grafts'),
             self._shas[-1] + b' ' + self._shas[0])
 
         r = Repo(self._repo_dir)

dulwich/tests/test_hooks.py

@@ -37,6 +37,7 @@ from dulwich.tests import TestCase
 class ShellHookTests(TestCase):
 
     def setUp(self):
+        super(ShellHookTests, self).setUp()
         if os.name != 'posix':
             self.skipTest('shell hook tests requires POSIX shell')
 
@@ -49,14 +50,11 @@ exit 1
 exit 0
 """
 
-        repo_dir = tempfile.mkdtemp()
-        if not isinstance(repo_dir, bytes):
-            repo_dir = repo_dir.encode(sys.getfilesystemencoding())
-
-        os.mkdir(os.path.join(repo_dir, b'hooks'))
+        repo_dir = os.path.join(tempfile.mkdtemp())
+        os.mkdir(os.path.join(repo_dir, 'hooks'))
         self.addCleanup(shutil.rmtree, repo_dir)
 
-        pre_commit = os.path.join(repo_dir, b'hooks', b'pre-commit')
+        pre_commit = os.path.join(repo_dir, 'hooks', 'pre-commit')
         hook = PreCommitShellHook(repo_dir)
 
         with open(pre_commit, 'w') as f:
@@ -82,13 +80,10 @@ exit 0
 """
 
         repo_dir = os.path.join(tempfile.mkdtemp())
-        if not isinstance(repo_dir, bytes):
-            repo_dir = repo_dir.encode(sys.getfilesystemencoding())
-
-        os.mkdir(os.path.join(repo_dir, b'hooks'))
+        os.mkdir(os.path.join(repo_dir, 'hooks'))
         self.addCleanup(shutil.rmtree, repo_dir)
 
-        commit_msg = os.path.join(repo_dir, b'hooks', b'commit-msg')
+        commit_msg = os.path.join(repo_dir, 'hooks', 'commit-msg')
         hook = CommitMsgShellHook(repo_dir)
 
         with open(commit_msg, 'w') as f:
@@ -106,6 +101,7 @@ exit 0
     def test_hook_post_commit(self):
 
         (fd, path) = tempfile.mkstemp()
+        os.close(fd)
 
         post_commit_msg = """#!/bin/sh
 rm """ + path + "\n"
@@ -115,13 +111,10 @@ exit 1
 """
 
         repo_dir = os.path.join(tempfile.mkdtemp())
-        if not isinstance(repo_dir, bytes):
-            repo_dir = repo_dir.encode(sys.getfilesystemencoding())
-
-        os.mkdir(os.path.join(repo_dir, b'hooks'))
+        os.mkdir(os.path.join(repo_dir, 'hooks'))
         self.addCleanup(shutil.rmtree, repo_dir)
 
-        post_commit = os.path.join(repo_dir, b'hooks', b'post-commit')
+        post_commit = os.path.join(repo_dir, 'hooks', 'post-commit')
         hook = PostCommitShellHook(repo_dir)
 
         with open(post_commit, 'w') as f:

dulwich/tests/test_index.py

@@ -1,4 +1,6 @@
+# -*- coding: utf-8 -*-
 # test_index.py -- Tests for the git index
+# encoding: utf-8
 # Copyright (C) 2008-2009 Jelmer Vernooij <jelmer@samba.org>
 #
 # This program is free software; you can redistribute it and/or
@@ -19,11 +21,13 @@
 """Tests for the index."""
 
 
+from contextlib import closing
 from io import BytesIO
 import os
 import shutil
 import stat
 import struct
+import sys
 import tempfile
 
 from dulwich.index import (
@@ -40,6 +44,8 @@ from dulwich.index import (
     write_cache_time,
     write_index,
     write_index_dict,
+    _tree_to_fs_path,
+    _fs_to_tree_path,
     )
 from dulwich.object_store import (
     MemoryObjectStore,
@@ -49,7 +55,10 @@ from dulwich.objects import (
     Tree,
     )
 from dulwich.repo import Repo
-from dulwich.tests import TestCase
+from dulwich.tests import (
+    TestCase,
+    skipIf,
+    )
 
 class IndexTestCase(TestCase):
 
@@ -255,122 +264,169 @@ class BuildIndexTests(TestCase):
 
     def test_empty(self):
         repo_dir = tempfile.mkdtemp()
-        repo = Repo.init(repo_dir)
         self.addCleanup(shutil.rmtree, repo_dir)
+        with closing(Repo.init(repo_dir)) as repo:
+            tree = Tree()
+            repo.object_store.add_object(tree)
 
-        tree = Tree()
-        repo.object_store.add_object(tree)
-
-        build_index_from_tree(repo.path, repo.index_path(),
-                repo.object_store, tree.id)
+            build_index_from_tree(repo.path, repo.index_path(),
+                    repo.object_store, tree.id)
 
-        # Verify index entries
-        index = repo.open_index()
-        self.assertEqual(len(index), 0)
+            # Verify index entries
+            index = repo.open_index()
+            self.assertEqual(len(index), 0)
 
-        # Verify no files
-        self.assertEqual(['.git'], os.listdir(repo.path))
+            # Verify no files
+            self.assertEqual(['.git'], os.listdir(repo.path))
 
     def test_git_dir(self):
-        if os.name != 'posix':
-            self.skipTest("test depends on POSIX shell")
-
         repo_dir = tempfile.mkdtemp()
-        repo = Repo.init(repo_dir)
         self.addCleanup(shutil.rmtree, repo_dir)
+        with closing(Repo.init(repo_dir)) as repo:
 
-        # Populate repo
-        filea = Blob.from_string(b'file a')
-        filee = Blob.from_string(b'd')
+            # Populate repo
+            filea = Blob.from_string(b'file a')
+            filee = Blob.from_string(b'd')
 
-        tree = Tree()
-        tree[b'.git/a'] = (stat.S_IFREG | 0o644, filea.id)
-        tree[b'c/e'] = (stat.S_IFREG | 0o644, filee.id)
+            tree = Tree()
+            tree[b'.git/a'] = (stat.S_IFREG | 0o644, filea.id)
+            tree[b'c/e'] = (stat.S_IFREG | 0o644, filee.id)
 
-        repo.object_store.add_objects([(o, None)
-            for o in [filea, filee, tree]])
+            repo.object_store.add_objects([(o, None)
+                for o in [filea, filee, tree]])
 
-        build_index_from_tree(repo.path, repo.index_path(),
-                repo.object_store, tree.id)
+            build_index_from_tree(repo.path, repo.index_path(),
+                    repo.object_store, tree.id)
 
-        # Verify index entries
-        index = repo.open_index()
-        self.assertEqual(len(index), 1)
+            # Verify index entries
+            index = repo.open_index()
+            self.assertEqual(len(index), 1)
 
-        # filea
-        apath = os.path.join(repo.path, '.git', 'a')
-        self.assertFalse(os.path.exists(apath))
+            # filea
+            apath = os.path.join(repo.path, '.git', 'a')
+            self.assertFalse(os.path.exists(apath))
 
-        # filee
-        epath = os.path.join(repo.path, 'c', 'e')
-        self.assertTrue(os.path.exists(epath))
-        self.assertReasonableIndexEntry(index[b'c/e'],
-            stat.S_IFREG | 0o644, 1, filee.id)
-        self.assertFileContents(epath, b'd')
+            # filee
+            epath = os.path.join(repo.path, 'c', 'e')
+            self.assertTrue(os.path.exists(epath))
+            self.assertReasonableIndexEntry(index[b'c/e'],
+                stat.S_IFREG | 0o644, 1, filee.id)
+            self.assertFileContents(epath, b'd')
 
     def test_nonempty(self):
-        if os.name != 'posix':
-            self.skipTest("test depends on POSIX shell")
+        repo_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, repo_dir)
+        with closing(Repo.init(repo_dir)) as repo:
+
+            # Populate repo
+            filea = Blob.from_string(b'file a')
+            fileb = Blob.from_string(b'file b')
+            filed = Blob.from_string(b'file d')
+
+            tree = Tree()
+            tree[b'a'] = (stat.S_IFREG | 0o644, filea.id)
+            tree[b'b'] = (stat.S_IFREG | 0o644, fileb.id)
+            tree[b'c/d'] = (stat.S_IFREG | 0o644, filed.id)
+
+            repo.object_store.add_objects([(o, None)
+                for o in [filea, fileb, filed, tree]])
+
+            build_index_from_tree(repo.path, repo.index_path(),
+                    repo.object_store, tree.id)
+
+            # Verify index entries
+            index = repo.open_index()
+            self.assertEqual(len(index), 3)
+
+            # filea
+            apath = os.path.join(repo.path, 'a')
+            self.assertTrue(os.path.exists(apath))
+            self.assertReasonableIndexEntry(index[b'a'],
+                stat.S_IFREG | 0o644, 6, filea.id)
+            self.assertFileContents(apath, b'file a')
+
+            # fileb
+            bpath = os.path.join(repo.path, 'b')
+            self.assertTrue(os.path.exists(bpath))
+            self.assertReasonableIndexEntry(index[b'b'],
+                stat.S_IFREG | 0o644, 6, fileb.id)
+            self.assertFileContents(bpath, b'file b')
+
+            # filed
+            dpath = os.path.join(repo.path, 'c', 'd')
+            self.assertTrue(os.path.exists(dpath))
+            self.assertReasonableIndexEntry(index[b'c/d'],
+                stat.S_IFREG | 0o644, 6, filed.id)
+            self.assertFileContents(dpath, b'file d')
+
+            # Verify no extra files
+            self.assertEqual(['.git', 'a', 'b', 'c'],
+                sorted(os.listdir(repo.path)))
+            self.assertEqual(['d'],
+                sorted(os.listdir(os.path.join(repo.path, 'c'))))
+
+    @skipIf(not getattr(os, 'symlink', None), 'Requires symlink support')
+    def test_symlink(self):
+        repo_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, repo_dir)
+        with closing(Repo.init(repo_dir)) as repo:
+
+            # Populate repo
+            filed = Blob.from_string(b'file d')
+            filee = Blob.from_string(b'd')
+
+            tree = Tree()
+            tree[b'c/d'] = (stat.S_IFREG | 0o644, filed.id)
+            tree[b'c/e'] = (stat.S_IFLNK, filee.id)  # symlink
 
+            repo.object_store.add_objects([(o, None)
+                for o in [filed, filee, tree]])
+
+            build_index_from_tree(repo.path, repo.index_path(),
+                    repo.object_store, tree.id)
+
+            # Verify index entries
+            index = repo.open_index()
+
+            # symlink to d
+            epath = os.path.join(repo.path, 'c', 'e')
+            self.assertTrue(os.path.exists(epath))
+            self.assertReasonableIndexEntry(
+                index[b'c/e'], stat.S_IFLNK,
+                0 if sys.platform == 'win32' else 1,
+                filee.id)
+            self.assertFileContents(epath, 'd', symlink=True)
+
+    def test_no_decode_encode(self):
         repo_dir = tempfile.mkdtemp()
-        repo = Repo.init(repo_dir)
+        repo_dir_bytes = repo_dir.encode(sys.getfilesystemencoding())
         self.addCleanup(shutil.rmtree, repo_dir)
+        with closing(Repo.init(repo_dir)) as repo:
 
-        # Populate repo
-        filea = Blob.from_string(b'file a')
-        fileb = Blob.from_string(b'file b')
-        filed = Blob.from_string(b'file d')
-        filee = Blob.from_string(b'd')
+            # Populate repo
+            file = Blob.from_string(b'foo')
 
-        tree = Tree()
-        tree[b'a'] = (stat.S_IFREG | 0o644, filea.id)
-        tree[b'b'] = (stat.S_IFREG | 0o644, fileb.id)
-        tree[b'c/d'] = (stat.S_IFREG | 0o644, filed.id)
-        tree[b'c/e'] = (stat.S_IFLNK, filee.id)  # symlink
+            tree = Tree()
+            latin1_name = u'À'.encode('latin1')
+            utf8_name = u'À'.encode('utf8')
+            tree[latin1_name] = (stat.S_IFREG | 0o644, file.id)
+            tree[utf8_name] = (stat.S_IFREG | 0o644, file.id)
 
-        repo.object_store.add_objects([(o, None)
-            for o in [filea, fileb, filed, filee, tree]])
+            repo.object_store.add_objects(
+                [(o, None) for o in [file, tree]])
 
-        build_index_from_tree(repo.path, repo.index_path(),
+            build_index_from_tree(
+                repo.path, repo.index_path(),
                 repo.object_store, tree.id)
 
-        # Verify index entries
-        index = repo.open_index()
-        self.assertEqual(len(index), 4)
-
-        # filea
-        apath = os.path.join(repo.path, 'a')
-        self.assertTrue(os.path.exists(apath))
-        self.assertReasonableIndexEntry(index[b'a'],
-            stat.S_IFREG | 0o644, 6, filea.id)
-        self.assertFileContents(apath, b'file a')
-
-        # fileb
-        bpath = os.path.join(repo.path, 'b')
-        self.assertTrue(os.path.exists(bpath))
-        self.assertReasonableIndexEntry(index[b'b'],
-            stat.S_IFREG | 0o644, 6, fileb.id)
-        self.assertFileContents(bpath, b'file b')
-
-        # filed
-        dpath = os.path.join(repo.path, 'c', 'd')
-        self.assertTrue(os.path.exists(dpath))
-        self.assertReasonableIndexEntry(index[b'c/d'],
-            stat.S_IFREG | 0o644, 6, filed.id)
-        self.assertFileContents(dpath, b'file d')
-
-        # symlink to d
-        epath = os.path.join(repo.path, 'c', 'e')
-        self.assertTrue(os.path.exists(epath))
-        self.assertReasonableIndexEntry(index[b'c/e'],
-            stat.S_IFLNK, 1, filee.id)
-        self.assertFileContents(epath, 'd', symlink=True)
-
-        # Verify no extra files
-        self.assertEqual(['.git', 'a', 'b', 'c'],
-            sorted(os.listdir(repo.path)))
-        self.assertEqual(['d', 'e'],
-            sorted(os.listdir(os.path.join(repo.path, 'c'))))
+            # Verify index entries
+            index = repo.open_index()
+
+            latin1_path = os.path.join(repo_dir_bytes, latin1_name)
+            self.assertTrue(os.path.exists(latin1_path))
+
+            utf8_path = os.path.join(repo_dir_bytes, utf8_name)
+            self.assertTrue(os.path.exists(utf8_path))
 
 
 class GetUnstagedChangesTests(TestCase):
@@ -379,30 +435,30 @@ class GetUnstagedChangesTests(TestCase):
         """Unit test for get_unstaged_changes."""
 
         repo_dir = tempfile.mkdtemp()
-        repo = Repo.init(repo_dir)
         self.addCleanup(shutil.rmtree, repo_dir)
+        with closing(Repo.init(repo_dir)) as repo:
 
-        # Commit a dummy file then modify it
-        foo1_fullpath = os.path.join(repo_dir, 'foo1')
-        with open(foo1_fullpath, 'wb') as f:
-            f.write(b'origstuff')
+            # Commit a dummy file then modify it
+            foo1_fullpath = os.path.join(repo_dir, 'foo1')
+            with open(foo1_fullpath, 'wb') as f:
+                f.write(b'origstuff')
 
-        foo2_fullpath = os.path.join(repo_dir, 'foo2')
-        with open(foo2_fullpath, 'wb') as f:
-            f.write(b'origstuff')
+            foo2_fullpath = os.path.join(repo_dir, 'foo2')
+            with open(foo2_fullpath, 'wb') as f:
+                f.write(b'origstuff')
 
-        repo.stage(['foo1', 'foo2'])
-        repo.do_commit(b'test status', author=b'', committer=b'')
+            repo.stage(['foo1', 'foo2'])
+            repo.do_commit(b'test status', author=b'', committer=b'')
 
-        with open(foo1_fullpath, 'wb') as f:
-            f.write(b'newstuff')
+            with open(foo1_fullpath, 'wb') as f:
+                f.write(b'newstuff')
 
-        # modify access and modify time of path
-        os.utime(foo1_fullpath, (0, 0))
+            # modify access and modify time of path
+            os.utime(foo1_fullpath, (0, 0))
 
-        changes = get_unstaged_changes(repo.open_index(), repo_dir)
+            changes = get_unstaged_changes(repo.open_index(), repo_dir)
 
-        self.assertEqual(list(changes), [b'foo1'])
+            self.assertEqual(list(changes), [b'foo1'])
 
 
 class TestValidatePathElement(TestCase):
@@ -422,3 +478,23 @@ class TestValidatePathElement(TestCase):
         self.assertFalse(validate_path_element_ntfs(b".giT"))
         self.assertFalse(validate_path_element_ntfs(b".."))
         self.assertFalse(validate_path_element_ntfs(b"git~1"))
+
+
+class TestTreeFSPathConversion(TestCase):
+
+    def test_tree_to_fs_path(self):
+        tree_path = u'délwíçh/foo'.encode('utf8')
+        fs_path = _tree_to_fs_path(b'/prefix/path', tree_path)
+        self.assertEqual(
+            fs_path,
+            os.path.join(u'/prefix/path', u'délwíçh', u'foo').encode('utf8'))
+
+    def test_fs_to_tree_path_str(self):
+        fs_path = os.path.join(os.path.join(u'délwíçh', u'foo'))
+        tree_path = _fs_to_tree_path(fs_path)
+        self.assertEqual(tree_path, u'délwíçh/foo'.encode(sys.getfilesystemencoding()))
+
+    def test_fs_to_tree_path_bytes(self):
+        fs_path = os.path.join(os.path.join(u'délwíçh', u'foo').encode('utf8'))
+        tree_path = _fs_to_tree_path(fs_path)
+        self.assertEqual(tree_path, u'délwíçh/foo'.encode('utf8'))

dulwich/tests/test_object_store.py

@@ -19,10 +19,10 @@
 """Tests for the object store interface."""
 
 
+from contextlib import closing
 from io import BytesIO
 import os
 import shutil
-import sys
 import tempfile
 
 from dulwich.index import (
@@ -263,8 +263,6 @@ class DiskObjectStoreTests(PackBasedObjectStoreTests, TestCase):
     def setUp(self):
         TestCase.setUp(self)
         self.store_dir = tempfile.mkdtemp()
-        if not isinstance(self.store_dir, bytes):
-            self.store_dir = self.store_dir.encode(sys.getfilesystemencoding())
         self.addCleanup(shutil.rmtree, self.store_dir)
         self.store = DiskObjectStore.init(self.store_dir)
 
@@ -274,8 +272,6 @@ class DiskObjectStoreTests(PackBasedObjectStoreTests, TestCase):
 
     def test_alternates(self):
         alternate_dir = tempfile.mkdtemp()
-        if not isinstance(alternate_dir, bytes):
-            alternate_dir = alternate_dir.encode(sys.getfilesystemencoding())
         self.addCleanup(shutil.rmtree, alternate_dir)
         alternate_store = DiskObjectStore(alternate_dir)
         b2 = make_object(Blob, data=b"yummy data")
@@ -289,17 +285,15 @@ class DiskObjectStoreTests(PackBasedObjectStoreTests, TestCase):
     def test_add_alternate_path(self):
         store = DiskObjectStore(self.store_dir)
         self.assertEqual([], list(store._read_alternate_paths()))
-        store.add_alternate_path(b'/foo/path')
-        self.assertEqual([b'/foo/path'], list(store._read_alternate_paths()))
-        store.add_alternate_path(b'/bar/path')
+        store.add_alternate_path("/foo/path")
+        self.assertEqual(["/foo/path"], list(store._read_alternate_paths()))
+        store.add_alternate_path("/bar/path")
         self.assertEqual(
-            [b'/foo/path', b'/bar/path'],
+            ["/foo/path", "/bar/path"],
             list(store._read_alternate_paths()))
 
     def test_rel_alternative_path(self):
         alternate_dir = tempfile.mkdtemp()
-        if not isinstance(alternate_dir, bytes):
-            alternate_dir = alternate_dir.encode(sys.getfilesystemencoding())
         self.addCleanup(shutil.rmtree, alternate_dir)
         alternate_store = DiskObjectStore(alternate_dir)
         b2 = make_object(Blob, data=b"yummy data")
@@ -313,7 +307,7 @@ class DiskObjectStoreTests(PackBasedObjectStoreTests, TestCase):
 
     def test_pack_dir(self):
         o = DiskObjectStore(self.store_dir)
-        self.assertEqual(os.path.join(self.store_dir, b'pack'), o.pack_dir)
+        self.assertEqual(os.path.join(self.store_dir, "pack"), o.pack_dir)
 
     def test_add_pack(self):
         o = DiskObjectStore(self.store_dir)
@@ -350,12 +344,11 @@ class DiskObjectStoreTests(PackBasedObjectStoreTests, TestCase):
             o.close()
 
     def test_add_thin_pack_empty(self):
-        o = DiskObjectStore(self.store_dir)
-
-        f = BytesIO()
-        entries = build_pack(f, [], store=o)
-        self.assertEqual([], entries)
-        o.add_thin_pack(f.read, None)
+        with closing(DiskObjectStore(self.store_dir)) as o:
+            f = BytesIO()
+            entries = build_pack(f, [], store=o)
+            self.assertEqual([], entries)
+            o.add_thin_pack(f.read, None)
 
 
 class TreeLookupPathTests(TestCase):

dulwich/tests/test_objects.py

@@ -29,7 +29,6 @@ from itertools import (
     )
 import os
 import stat
-import sys
 import warnings
 from contextlib import contextmanager
 
@@ -86,23 +85,21 @@ class BlobReadTests(TestCase):
     """Test decompression of blobs"""
 
     def get_sha_file(self, cls, base, sha):
-        dir = os.path.join(
-            os.path.dirname(__file__.encode(sys.getfilesystemencoding())),
-            b'data', base)
+        dir = os.path.join(os.path.dirname(__file__), 'data', base)
         return cls.from_path(hex_to_filename(dir, sha))
 
     def get_blob(self, sha):
         """Return the blob named sha from the test data dir"""
-        return self.get_sha_file(Blob, b'blobs', sha)
+        return self.get_sha_file(Blob, 'blobs', sha)
 
     def get_tree(self, sha):
-        return self.get_sha_file(Tree, b'trees', sha)
+        return self.get_sha_file(Tree, 'trees', sha)
 
     def get_tag(self, sha):
-        return self.get_sha_file(Tag, b'tags', sha)
+        return self.get_sha_file(Tag, 'tags', sha)
 
     def commit(self, sha):
-        return self.get_sha_file(Commit, b'commits', sha)
+        return self.get_sha_file(Commit, 'commits', sha)
 
     def test_decompress_simple_blob(self):
         b = self.get_blob(a_sha)
@@ -705,9 +702,7 @@ class TreeTests(ShaFileCheckTests):
         self.assertEqual(_SORTED_TREE_ITEMS, x.items())
 
     def _do_test_parse_tree(self, parse_tree):
-        dir = os.path.join(
-            os.path.dirname(__file__.encode(sys.getfilesystemencoding())),
-            b'data', b'trees')
+        dir = os.path.join(os.path.dirname(__file__), 'data', 'trees')
         o = Tree.from_path(hex_to_filename(dir, tree_sha))
         self.assertEqual([(b'a', 0o100644, a_sha), (b'b', 0o100644, b_sha)],
                          list(parse_tree(o.as_raw_string())))

dulwich/tests/test_pack.py

@@ -24,7 +24,6 @@ from io import BytesIO
 from hashlib import sha1
 import os
 import shutil
-import sys
 import tempfile
 import zlib
 
@@ -90,24 +89,21 @@ class PackTests(TestCase):
     def setUp(self):
         super(PackTests, self).setUp()
         self.tempdir = tempfile.mkdtemp()
-        if not isinstance(self.tempdir, bytes):
-            self.tempdir = self.tempdir.encode(sys.getfilesystemencoding())
         self.addCleanup(shutil.rmtree, self.tempdir)
 
-    datadir = os.path.abspath(
-        os.path.join(os.path.dirname(__file__.encode(sys.getfilesystemencoding())),
-        b'data/packs'))
+    datadir = os.path.abspath(os.path.join(os.path.dirname(__file__),
+        'data/packs'))
 
     def get_pack_index(self, sha):
         """Returns a PackIndex from the datadir with the given sha"""
-        return load_pack_index(os.path.join(self.datadir, b'pack-' + sha + b'.idx'))
+        return load_pack_index(os.path.join(self.datadir, 'pack-%s.idx' % sha.decode('ascii')))
 
     def get_pack_data(self, sha):
         """Returns a PackData object from the datadir with the given sha"""
-        return PackData(os.path.join(self.datadir, b'pack-' + sha + b'.pack'))
+        return PackData(os.path.join(self.datadir, 'pack-%s.pack' % sha.decode('ascii')))
 
     def get_pack(self, sha):
-        return Pack(os.path.join(self.datadir, b'pack-' + sha))
+        return Pack(os.path.join(self.datadir, 'pack-%s' % sha.decode('ascii')))
 
     def assertSucceeds(self, func, *args, **kwargs):
         try:
@@ -208,7 +204,7 @@ class TestPackData(PackTests):
         self.get_pack_data(pack1_sha).close()
 
     def test_from_file(self):
-        path = os.path.join(self.datadir, b'pack-' + pack1_sha + b'.pack')
+        path = os.path.join(self.datadir, 'pack-%s.pack' % pack1_sha.decode('ascii'))
         with open(path, 'rb') as f:
             PackData.from_file(f, os.path.getsize(path))
 
@@ -251,7 +247,7 @@ class TestPackData(PackTests):
 
     def test_create_index_v1(self):
         with self.get_pack_data(pack1_sha) as p:
-            filename = os.path.join(self.tempdir, b'v1test.idx')
+            filename = os.path.join(self.tempdir, 'v1test.idx')
             p.create_index_v1(filename)
             idx1 = load_pack_index(filename)
             idx2 = self.get_pack_index(pack1_sha)
@@ -259,7 +255,7 @@ class TestPackData(PackTests):
 
     def test_create_index_v2(self):
         with self.get_pack_data(pack1_sha) as p:
-            filename = os.path.join(self.tempdir, b'v2test.idx')
+            filename = os.path.join(self.tempdir, 'v2test.idx')
             p.create_index_v2(filename)
             idx1 = load_pack_index(filename)
             idx2 = self.get_pack_index(pack1_sha)
@@ -334,7 +330,7 @@ class TestPack(PackTests):
     def test_copy(self):
         with self.get_pack(pack1_sha) as origpack:
             self.assertSucceeds(origpack.index.check)
-            basename = os.path.join(self.tempdir, b'Elch')
+            basename = os.path.join(self.tempdir, 'Elch')
             write_pack(basename, origpack.pack_tuples())
 
             with Pack(basename) as newpack:
@@ -357,7 +353,7 @@ class TestPack(PackTests):
             self.assertEqual([], commit.parents)
 
     def _copy_pack(self, origpack):
-        basename = os.path.join(self.tempdir, b'somepack')
+        basename = os.path.join(self.tempdir, 'somepack')
         write_pack(basename, origpack.pack_tuples())
         return Pack(basename)
 
@@ -405,7 +401,7 @@ class TestPack(PackTests):
             write_pack_header(bad_file, 9999)
             bad_file.write(data._file.read())
             bad_file = BytesIO(bad_file.getvalue())
-            bad_data = PackData(b'', file=bad_file)
+            bad_data = PackData('', file=bad_file)
             bad_pack = Pack.from_lazy_objects(lambda: bad_data, lambda: index)
             self.assertRaises(AssertionError, lambda: bad_pack.data)
             self.assertRaises(AssertionError,
@@ -418,7 +414,7 @@ class TestPack(PackTests):
 
             data._file.seek(0)
             bad_file = BytesIO(data._file.read()[:-20] + (b'\xff' * 20))
-            bad_data = PackData(b'', file=bad_file)
+            bad_data = PackData('', file=bad_file)
             bad_pack = Pack.from_lazy_objects(lambda: bad_data, lambda: index)
             self.assertRaises(ChecksumMismatch, lambda: bad_pack.data)
             self.assertRaises(ChecksumMismatch, lambda:
@@ -448,12 +444,10 @@ class TestThinPack(PackTests):
         # Build a thin pack. 'foo' is as an external reference, 'bar' an
         # internal reference.
         self.pack_dir = tempfile.mkdtemp()
-        if not isinstance(self.pack_dir, bytes):
-            self.pack_dir = self.pack_dir.encode(sys.getfilesystemencoding())
         self.addCleanup(shutil.rmtree, self.pack_dir)
-        self.pack_prefix = os.path.join(self.pack_dir, b'pack')
+        self.pack_prefix = os.path.join(self.pack_dir, 'pack')
 
-        with open(self.pack_prefix + b'.pack', 'wb') as f:
+        with open(self.pack_prefix + '.pack', 'wb') as f:
             build_pack(f, [
                 (REF_DELTA, (self.blobs[b'foo'].id, b'foo1234')),
                 (Blob.type_num, b'bar'),
@@ -464,7 +458,7 @@ class TestThinPack(PackTests):
         with self.make_pack(True) as pack:
             with PackData(pack._data_path) as data:
                 data.pack = pack
-                data.create_index(self.pack_prefix + b'.idx')
+                data.create_index(self.pack_prefix + '.idx')
 
         del self.store[self.blobs[b'bar'].id]
 
@@ -543,7 +537,7 @@ class BaseTestPackIndexWriting(object):
         raise NotImplementedError(self.index)
 
     def test_empty(self):
-        idx = self.index(b'empty.idx', [], pack_checksum)
+        idx = self.index('empty.idx', [], pack_checksum)
         self.assertEqual(idx.get_pack_checksum(), pack_checksum)
         self.assertEqual(0, len(idx))
 
@@ -556,7 +550,7 @@ class BaseTestPackIndexWriting(object):
             self.assertRaises(TypeError, self.index, 'single.idx',
                 entries, pack_checksum)
             return
-        idx = self.index(b'single.idx', entries, pack_checksum)
+        idx = self.index('single.idx', entries, pack_checksum)
         self.assertEqual(idx.get_pack_checksum(), pack_checksum)
         self.assertEqual(2, len(idx))
         actual_entries = list(idx.iterentries())
@@ -574,7 +568,7 @@ class BaseTestPackIndexWriting(object):
     def test_single(self):
         entry_sha = hex_to_sha('6f670c0fb53f9463760b7295fbb814e965fb20c8')
         my_entries = [(entry_sha, 178, 42)]
-        idx = self.index(b'single.idx', my_entries, pack_checksum)
+        idx = self.index('single.idx', my_entries, pack_checksum)
         self.assertEqual(idx.get_pack_checksum(), pack_checksum)
         self.assertEqual(1, len(idx))
         actual_entries = list(idx.iterentries())
@@ -594,8 +588,6 @@ class BaseTestFilePackIndexWriting(BaseTestPackIndexWriting):
 
     def setUp(self):
         self.tempdir = tempfile.mkdtemp()
-        if not isinstance(self.tempdir, bytes):
-            self.tempdir = self.tempdir.encode(sys.getfilesystemencoding())
 
     def tearDown(self):
         shutil.rmtree(self.tempdir)

dulwich/tests/test_patch.py

@@ -18,7 +18,8 @@
 
 """Tests for patch.py."""
 
-from io import BytesIO
+from io import BytesIO, StringIO
+import sys
 
 from dulwich.objects import (
     Blob,
@@ -40,47 +41,71 @@ from dulwich.tests import (
     SkipTest,
     TestCase,
     )
-from dulwich.tests.utils import (
-    skipIfPY3,
-    )
 
 
-@skipIfPY3
 class WriteCommitPatchTests(TestCase):
 
-    def test_simple(self):
+    def test_simple_bytesio(self):
         f = BytesIO()
         c = Commit()
-        c.committer = c.author = "Jelmer <jelmer@samba.org>"
+        c.committer = c.author = b"Jelmer <jelmer@samba.org>"
         c.commit_time = c.author_time = 1271350201
         c.commit_timezone = c.author_timezone = 0
-        c.message = "This is the first line\nAnd this is the second line.\n"
+        c.message = b"This is the first line\nAnd this is the second line.\n"
         c.tree = Tree().id
-        write_commit_patch(f, c, "CONTENTS", (1, 1), version="custom")
+        write_commit_patch(f, c, b"CONTENTS", (1, 1), version="custom")
         f.seek(0)
         lines = f.readlines()
-        self.assertTrue(lines[0].startswith("From 0b0d34d1b5b596c928adc9a727a4b9e03d025298"))
-        self.assertEqual(lines[1], "From: Jelmer <jelmer@samba.org>\n")
-        self.assertTrue(lines[2].startswith("Date: "))
+        self.assertTrue(lines[0].startswith(b"From 0b0d34d1b5b596c928adc9a727a4b9e03d025298"))
+        self.assertEqual(lines[1], b"From: Jelmer <jelmer@samba.org>\n")
+        self.assertTrue(lines[2].startswith(b"Date: "))
         self.assertEqual([
-            "Subject: [PATCH 1/1] This is the first line\n",
-            "And this is the second line.\n",
-            "\n",
-            "\n",
-            "---\n"], lines[3:8])
+            b"Subject: [PATCH 1/1] This is the first line\n",
+            b"And this is the second line.\n",
+            b"\n",
+            b"\n",
+            b"---\n"], lines[3:8])
         self.assertEqual([
-            "CONTENTS-- \n",
-            "custom\n"], lines[-2:])
+            b"CONTENTS-- \n",
+            b"custom\n"], lines[-2:])
         if len(lines) >= 12:
             # diffstat may not be present
-            self.assertEqual(lines[8], " 0 files changed\n")
+            self.assertEqual(lines[8], b" 0 files changed\n")
 
 
-@skipIfPY3
 class ReadGitAmPatch(TestCase):
 
-    def test_extract(self):
-        text = """From ff643aae102d8870cac88e8f007e70f58f3a7363 Mon Sep 17 00:00:00 2001
+    def test_extract_string(self):
+        if sys.version_info[:2] <= (2, 6):
+            raise SkipTest("email.parser.Parser.parsestr() inserts extra lines")
+
+        text = b"""From ff643aae102d8870cac88e8f007e70f58f3a7363 Mon Sep 17 00:00:00 2001
+From: Jelmer Vernooij <jelmer@samba.org>
+Date: Thu, 15 Apr 2010 15:40:28 +0200
+Subject: [PATCH 1/2] Remove executable bit from prey.ico (triggers a lintian warning).
+
+---
+ pixmaps/prey.ico |  Bin 9662 -> 9662 bytes
+ 1 files changed, 0 insertions(+), 0 deletions(-)
+ mode change 100755 => 100644 pixmaps/prey.ico
+
+-- 
+1.7.0.4
+"""
+        c, diff, version = git_am_patch_split(StringIO(text.decode("utf-8")), "utf-8")
+        self.assertEqual(b"Jelmer Vernooij <jelmer@samba.org>", c.committer)
+        self.assertEqual(b"Jelmer Vernooij <jelmer@samba.org>", c.author)
+        self.assertEqual(b"Remove executable bit from prey.ico "
+            b"(triggers a lintian warning).\n", c.message)
+        self.assertEqual(b""" pixmaps/prey.ico |  Bin 9662 -> 9662 bytes
+ 1 files changed, 0 insertions(+), 0 deletions(-)
+ mode change 100755 => 100644 pixmaps/prey.ico
+
+""", diff)
+        self.assertEqual(b"1.7.0.4", version)
+
+    def test_extract_bytes(self):
+        text = b"""From ff643aae102d8870cac88e8f007e70f58f3a7363 Mon Sep 17 00:00:00 2001
 From: Jelmer Vernooij <jelmer@samba.org>
 Date: Thu, 15 Apr 2010 15:40:28 +0200
 Subject: [PATCH 1/2] Remove executable bit from prey.ico (triggers a lintian warning).
@@ -94,19 +119,19 @@ Subject: [PATCH 1/2] Remove executable bit from prey.ico (triggers a lintian war
 1.7.0.4
 """
         c, diff, version = git_am_patch_split(BytesIO(text))
-        self.assertEqual("Jelmer Vernooij <jelmer@samba.org>", c.committer)
-        self.assertEqual("Jelmer Vernooij <jelmer@samba.org>", c.author)
-        self.assertEqual("Remove executable bit from prey.ico "
-            "(triggers a lintian warning).\n", c.message)
-        self.assertEqual(""" pixmaps/prey.ico |  Bin 9662 -> 9662 bytes
+        self.assertEqual(b"Jelmer Vernooij <jelmer@samba.org>", c.committer)
+        self.assertEqual(b"Jelmer Vernooij <jelmer@samba.org>", c.author)
+        self.assertEqual(b"Remove executable bit from prey.ico "
+            b"(triggers a lintian warning).\n", c.message)
+        self.assertEqual(b""" pixmaps/prey.ico |  Bin 9662 -> 9662 bytes
  1 files changed, 0 insertions(+), 0 deletions(-)
  mode change 100755 => 100644 pixmaps/prey.ico
 
 """, diff)
-        self.assertEqual("1.7.0.4", version)
+        self.assertEqual(b"1.7.0.4", version)
 
     def test_extract_spaces(self):
-        text = """From ff643aae102d8870cac88e8f007e70f58f3a7363 Mon Sep 17 00:00:00 2001
+        text = b"""From ff643aae102d8870cac88e8f007e70f58f3a7363 Mon Sep 17 00:00:00 2001
 From: Jelmer Vernooij <jelmer@samba.org>
 Date: Thu, 15 Apr 2010 15:40:28 +0200
 Subject:  [Dulwich-users] [PATCH] Added unit tests for
@@ -123,11 +148,11 @@ Subject:  [Dulwich-users] [PATCH] Added unit tests for
 -- 
 1.7.0.4
 """
-        c, diff, version = git_am_patch_split(BytesIO(text))
-        self.assertEqual('Added unit tests for dulwich.object_store.tree_lookup_path.\n\n* dulwich/tests/test_object_store.py\n  (TreeLookupPathTests): This test case contains a few tests that ensure the\n   tree_lookup_path function works as expected.\n', c.message)
+        c, diff, version = git_am_patch_split(BytesIO(text), "utf-8")
+        self.assertEqual(b'Added unit tests for dulwich.object_store.tree_lookup_path.\n\n* dulwich/tests/test_object_store.py\n  (TreeLookupPathTests): This test case contains a few tests that ensure the\n   tree_lookup_path function works as expected.\n', c.message)
 
     def test_extract_pseudo_from_header(self):
-        text = """From ff643aae102d8870cac88e8f007e70f58f3a7363 Mon Sep 17 00:00:00 2001
+        text = b"""From ff643aae102d8870cac88e8f007e70f58f3a7363 Mon Sep 17 00:00:00 2001
 From: Jelmer Vernooij <jelmer@samba.org>
 Date: Thu, 15 Apr 2010 15:40:28 +0200
 Subject:  [Dulwich-users] [PATCH] Added unit tests for
@@ -146,12 +171,12 @@ From: Jelmer Vernooy <jelmer@debian.org>
 -- 
 1.7.0.4
 """
-        c, diff, version = git_am_patch_split(BytesIO(text))
-        self.assertEqual("Jelmer Vernooy <jelmer@debian.org>", c.author)
-        self.assertEqual('Added unit tests for dulwich.object_store.tree_lookup_path.\n\n* dulwich/tests/test_object_store.py\n  (TreeLookupPathTests): This test case contains a few tests that ensure the\n   tree_lookup_path function works as expected.\n', c.message)
+        c, diff, version = git_am_patch_split(BytesIO(text), "utf-8")
+        self.assertEqual(b"Jelmer Vernooy <jelmer@debian.org>", c.author)
+        self.assertEqual(b'Added unit tests for dulwich.object_store.tree_lookup_path.\n\n* dulwich/tests/test_object_store.py\n  (TreeLookupPathTests): This test case contains a few tests that ensure the\n   tree_lookup_path function works as expected.\n', c.message)
 
     def test_extract_no_version_tail(self):
-        text = """From ff643aae102d8870cac88e8f007e70f58f3a7363 Mon Sep 17 00:00:00 2001
+        text = b"""From ff643aae102d8870cac88e8f007e70f58f3a7363 Mon Sep 17 00:00:00 2001
 From: Jelmer Vernooij <jelmer@samba.org>
 Date: Thu, 15 Apr 2010 15:40:28 +0200
 Subject:  [Dulwich-users] [PATCH] Added unit tests for
@@ -165,7 +190,7 @@ From: Jelmer Vernooy <jelmer@debian.org>
  mode change 100755 => 100644 pixmaps/prey.ico
 
 """
-        c, diff, version = git_am_patch_split(BytesIO(text))
+        c, diff, version = git_am_patch_split(BytesIO(text), "utf-8")
         self.assertEqual(None, version)
 
     def test_extract_mercurial(self):
@@ -206,270 +231,269 @@ More help   : https://help.launchpad.net/ListHelp
         self.assertEqual(None, version)
 
 
-@skipIfPY3
 class DiffTests(TestCase):
     """Tests for write_blob_diff and write_tree_diff."""
 
     def test_blob_diff(self):
         f = BytesIO()
-        write_blob_diff(f, ("foo.txt", 0o644, Blob.from_string("old\nsame\n")),
-                           ("bar.txt", 0o644, Blob.from_string("new\nsame\n")))
+        write_blob_diff(f, (b"foo.txt", 0o644, Blob.from_string(b"old\nsame\n")),
+                           (b"bar.txt", 0o644, Blob.from_string(b"new\nsame\n")))
         self.assertEqual([
-            "diff --git a/foo.txt b/bar.txt",
-            "index 3b0f961..a116b51 644",
-            "--- a/foo.txt",
-            "+++ b/bar.txt",
-            "@@ -1,2 +1,2 @@",
-            "-old",
-            "+new",
-            " same"
+            b"diff --git a/foo.txt b/bar.txt",
+            b"index 3b0f961..a116b51 644",
+            b"--- a/foo.txt",
+            b"+++ b/bar.txt",
+            b"@@ -1,2 +1,2 @@",
+            b"-old",
+            b"+new",
+            b" same"
             ], f.getvalue().splitlines())
 
     def test_blob_add(self):
         f = BytesIO()
         write_blob_diff(f, (None, None, None),
-                           ("bar.txt", 0o644, Blob.from_string("new\nsame\n")))
+                           (b"bar.txt", 0o644, Blob.from_string(b"new\nsame\n")))
         self.assertEqual([
-            'diff --git /dev/null b/bar.txt',
-             'new mode 644',
-             'index 0000000..a116b51 644',
-             '--- /dev/null',
-             '+++ b/bar.txt',
-             '@@ -1,0 +1,2 @@',
-             '+new',
-             '+same'
+             b'diff --git /dev/null b/bar.txt',
+             b'new mode 644',
+             b'index 0000000..a116b51 644',
+             b'--- /dev/null',
+             b'+++ b/bar.txt',
+             b'@@ -1,0 +1,2 @@',
+             b'+new',
+             b'+same'
             ], f.getvalue().splitlines())
 
     def test_blob_remove(self):
         f = BytesIO()
-        write_blob_diff(f, ("bar.txt", 0o644, Blob.from_string("new\nsame\n")),
+        write_blob_diff(f, (b"bar.txt", 0o644, Blob.from_string(b"new\nsame\n")),
                            (None, None, None))
         self.assertEqual([
-            'diff --git a/bar.txt /dev/null',
-            'deleted mode 644',
-            'index a116b51..0000000',
-            '--- a/bar.txt',
-            '+++ /dev/null',
-            '@@ -1,2 +1,0 @@',
-            '-new',
-            '-same'
+            b'diff --git a/bar.txt /dev/null',
+            b'deleted mode 644',
+            b'index a116b51..0000000',
+            b'--- a/bar.txt',
+            b'+++ /dev/null',
+            b'@@ -1,2 +1,0 @@',
+            b'-new',
+            b'-same'
             ], f.getvalue().splitlines())
 
     def test_tree_diff(self):
         f = BytesIO()
         store = MemoryObjectStore()
-        added = Blob.from_string("add\n")
-        removed = Blob.from_string("removed\n")
-        changed1 = Blob.from_string("unchanged\nremoved\n")
-        changed2 = Blob.from_string("unchanged\nadded\n")
-        unchanged = Blob.from_string("unchanged\n")
+        added = Blob.from_string(b"add\n")
+        removed = Blob.from_string(b"removed\n")
+        changed1 = Blob.from_string(b"unchanged\nremoved\n")
+        changed2 = Blob.from_string(b"unchanged\nadded\n")
+        unchanged = Blob.from_string(b"unchanged\n")
         tree1 = Tree()
-        tree1.add("removed.txt", 0o644, removed.id)
-        tree1.add("changed.txt", 0o644, changed1.id)
-        tree1.add("unchanged.txt", 0o644, changed1.id)
+        tree1.add(b"removed.txt", 0o644, removed.id)
+        tree1.add(b"changed.txt", 0o644, changed1.id)
+        tree1.add(b"unchanged.txt", 0o644, changed1.id)
         tree2 = Tree()
-        tree2.add("added.txt", 0o644, added.id)
-        tree2.add("changed.txt", 0o644, changed2.id)
-        tree2.add("unchanged.txt", 0o644, changed1.id)
+        tree2.add(b"added.txt", 0o644, added.id)
+        tree2.add(b"changed.txt", 0o644, changed2.id)
+        tree2.add(b"unchanged.txt", 0o644, changed1.id)
         store.add_objects([(o, None) for o in [
             tree1, tree2, added, removed, changed1, changed2, unchanged]])
         write_tree_diff(f, store, tree1.id, tree2.id)
         self.assertEqual([
-            'diff --git /dev/null b/added.txt',
-            'new mode 644',
-            'index 0000000..76d4bb8 644',
-            '--- /dev/null',
-            '+++ b/added.txt',
-            '@@ -1,0 +1,1 @@',
-            '+add',
-            'diff --git a/changed.txt b/changed.txt',
-            'index bf84e48..1be2436 644',
-            '--- a/changed.txt',
-            '+++ b/changed.txt',
-            '@@ -1,2 +1,2 @@',
-            ' unchanged',
-            '-removed',
-            '+added',
-            'diff --git a/removed.txt /dev/null',
-            'deleted mode 644',
-            'index 2c3f0b3..0000000',
-            '--- a/removed.txt',
-            '+++ /dev/null',
-            '@@ -1,1 +1,0 @@',
-            '-removed',
+            b'diff --git /dev/null b/added.txt',
+            b'new mode 644',
+            b'index 0000000..76d4bb8 644',
+            b'--- /dev/null',
+            b'+++ b/added.txt',
+            b'@@ -1,0 +1,1 @@',
+            b'+add',
+            b'diff --git a/changed.txt b/changed.txt',
+            b'index bf84e48..1be2436 644',
+            b'--- a/changed.txt',
+            b'+++ b/changed.txt',
+            b'@@ -1,2 +1,2 @@',
+            b' unchanged',
+            b'-removed',
+            b'+added',
+            b'diff --git a/removed.txt /dev/null',
+            b'deleted mode 644',
+            b'index 2c3f0b3..0000000',
+            b'--- a/removed.txt',
+            b'+++ /dev/null',
+            b'@@ -1,1 +1,0 @@',
+            b'-removed',
             ], f.getvalue().splitlines())
 
     def test_tree_diff_submodule(self):
         f = BytesIO()
         store = MemoryObjectStore()
         tree1 = Tree()
-        tree1.add("asubmodule", S_IFGITLINK,
-            "06d0bdd9e2e20377b3180e4986b14c8549b393e4")
+        tree1.add(b"asubmodule", S_IFGITLINK,
+            b"06d0bdd9e2e20377b3180e4986b14c8549b393e4")
         tree2 = Tree()
-        tree2.add("asubmodule", S_IFGITLINK,
-            "cc975646af69f279396d4d5e1379ac6af80ee637")
+        tree2.add(b"asubmodule", S_IFGITLINK,
+            b"cc975646af69f279396d4d5e1379ac6af80ee637")
         store.add_objects([(o, None) for o in [tree1, tree2]])
         write_tree_diff(f, store, tree1.id, tree2.id)
         self.assertEqual([
-            'diff --git a/asubmodule b/asubmodule',
-            'index 06d0bdd..cc97564 160000',
-            '--- a/asubmodule',
-            '+++ b/asubmodule',
-            '@@ -1,1 +1,1 @@',
-            '-Submodule commit 06d0bdd9e2e20377b3180e4986b14c8549b393e4',
-            '+Submodule commit cc975646af69f279396d4d5e1379ac6af80ee637',
+            b'diff --git a/asubmodule b/asubmodule',
+            b'index 06d0bdd..cc97564 160000',
+            b'--- a/asubmodule',
+            b'+++ b/asubmodule',
+            b'@@ -1,1 +1,1 @@',
+            b'-Submodule commit 06d0bdd9e2e20377b3180e4986b14c8549b393e4',
+            b'+Submodule commit cc975646af69f279396d4d5e1379ac6af80ee637',
             ], f.getvalue().splitlines())
 
     def test_object_diff_blob(self):
         f = BytesIO()
-        b1 = Blob.from_string("old\nsame\n")
-        b2 = Blob.from_string("new\nsame\n")
+        b1 = Blob.from_string(b"old\nsame\n")
+        b2 = Blob.from_string(b"new\nsame\n")
         store = MemoryObjectStore()
         store.add_objects([(b1, None), (b2, None)])
-        write_object_diff(f, store, ("foo.txt", 0o644, b1.id),
-                                    ("bar.txt", 0o644, b2.id))
+        write_object_diff(f, store, (b"foo.txt", 0o644, b1.id),
+                                    (b"bar.txt", 0o644, b2.id))
         self.assertEqual([
-            "diff --git a/foo.txt b/bar.txt",
-            "index 3b0f961..a116b51 644",
-            "--- a/foo.txt",
-            "+++ b/bar.txt",
-            "@@ -1,2 +1,2 @@",
-            "-old",
-            "+new",
-            " same"
+            b"diff --git a/foo.txt b/bar.txt",
+            b"index 3b0f961..a116b51 644",
+            b"--- a/foo.txt",
+            b"+++ b/bar.txt",
+            b"@@ -1,2 +1,2 @@",
+            b"-old",
+            b"+new",
+            b" same"
             ], f.getvalue().splitlines())
 
     def test_object_diff_add_blob(self):
         f = BytesIO()
         store = MemoryObjectStore()
-        b2 = Blob.from_string("new\nsame\n")
+        b2 = Blob.from_string(b"new\nsame\n")
         store.add_object(b2)
         write_object_diff(f, store, (None, None, None),
-                                    ("bar.txt", 0o644, b2.id))
+                                    (b"bar.txt", 0o644, b2.id))
         self.assertEqual([
-            'diff --git /dev/null b/bar.txt',
-             'new mode 644',
-             'index 0000000..a116b51 644',
-             '--- /dev/null',
-             '+++ b/bar.txt',
-             '@@ -1,0 +1,2 @@',
-             '+new',
-             '+same'
+             b'diff --git /dev/null b/bar.txt',
+             b'new mode 644',
+             b'index 0000000..a116b51 644',
+             b'--- /dev/null',
+             b'+++ b/bar.txt',
+             b'@@ -1,0 +1,2 @@',
+             b'+new',
+             b'+same'
             ], f.getvalue().splitlines())
 
     def test_object_diff_remove_blob(self):
         f = BytesIO()
-        b1 = Blob.from_string("new\nsame\n")
+        b1 = Blob.from_string(b"new\nsame\n")
         store = MemoryObjectStore()
         store.add_object(b1)
-        write_object_diff(f, store, ("bar.txt", 0o644, b1.id),
+        write_object_diff(f, store, (b"bar.txt", 0o644, b1.id),
                                     (None, None, None))
         self.assertEqual([
-            'diff --git a/bar.txt /dev/null',
-            'deleted mode 644',
-            'index a116b51..0000000',
-            '--- a/bar.txt',
-            '+++ /dev/null',
-            '@@ -1,2 +1,0 @@',
-            '-new',
-            '-same'
+            b'diff --git a/bar.txt /dev/null',
+            b'deleted mode 644',
+            b'index a116b51..0000000',
+            b'--- a/bar.txt',
+            b'+++ /dev/null',
+            b'@@ -1,2 +1,0 @@',
+            b'-new',
+            b'-same'
             ], f.getvalue().splitlines())
 
     def test_object_diff_bin_blob_force(self):
         f = BytesIO()
         # Prepare two slightly different PNG headers
         b1 = Blob.from_string(
-            "\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52"
-            "\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x04\x00\x00\x00\x05\x04\x8b")
+            b"\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52"
+            b"\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x04\x00\x00\x00\x05\x04\x8b")
         b2 = Blob.from_string(
-            "\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52"
-            "\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x03\x00\x00\x00\x98\xd3\xb3")
+            b"\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52"
+            b"\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x03\x00\x00\x00\x98\xd3\xb3")
         store = MemoryObjectStore()
         store.add_objects([(b1, None), (b2, None)])
-        write_object_diff(f, store, ('foo.png', 0o644, b1.id),
-                                    ('bar.png', 0o644, b2.id), diff_binary=True)
+        write_object_diff(f, store, (b'foo.png', 0o644, b1.id),
+                                    (b'bar.png', 0o644, b2.id), diff_binary=True)
         self.assertEqual([
-            'diff --git a/foo.png b/bar.png',
-            'index f73e47d..06364b7 644',
-            '--- a/foo.png',
-            '+++ b/bar.png',
-            '@@ -1,4 +1,4 @@',
-            ' \x89PNG',
-            ' \x1a',
-            ' \x00\x00\x00',
-            '-IHDR\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x04\x00\x00\x00\x05\x04\x8b',
-            '\\ No newline at end of file',
-            '+IHDR\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x03\x00\x00\x00\x98\xd3\xb3',
-            '\\ No newline at end of file'
+            b'diff --git a/foo.png b/bar.png',
+            b'index f73e47d..06364b7 644',
+            b'--- a/foo.png',
+            b'+++ b/bar.png',
+            b'@@ -1,4 +1,4 @@',
+            b' \x89PNG',
+            b' \x1a',
+            b' \x00\x00\x00',
+            b'-IHDR\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x04\x00\x00\x00\x05\x04\x8b',
+            b'\\ No newline at end of file',
+            b'+IHDR\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x03\x00\x00\x00\x98\xd3\xb3',
+            b'\\ No newline at end of file'
             ], f.getvalue().splitlines())
 
     def test_object_diff_bin_blob(self):
         f = BytesIO()
         # Prepare two slightly different PNG headers
         b1 = Blob.from_string(
-            "\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52"
-            "\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x04\x00\x00\x00\x05\x04\x8b")
+            b"\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52"
+            b"\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x04\x00\x00\x00\x05\x04\x8b")
         b2 = Blob.from_string(
-            "\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52"
-            "\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x03\x00\x00\x00\x98\xd3\xb3")
+            b"\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52"
+            b"\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x03\x00\x00\x00\x98\xd3\xb3")
         store = MemoryObjectStore()
         store.add_objects([(b1, None), (b2, None)])
-        write_object_diff(f, store, ('foo.png', 0o644, b1.id),
-                                    ('bar.png', 0o644, b2.id))
+        write_object_diff(f, store, (b'foo.png', 0o644, b1.id),
+                                    (b'bar.png', 0o644, b2.id))
         self.assertEqual([
-            'diff --git a/foo.png b/bar.png',
-            'index f73e47d..06364b7 644',
-            'Binary files a/foo.png and b/bar.png differ'
+            b'diff --git a/foo.png b/bar.png',
+            b'index f73e47d..06364b7 644',
+            b'Binary files a/foo.png and b/bar.png differ'
             ], f.getvalue().splitlines())
 
     def test_object_diff_add_bin_blob(self):
         f = BytesIO()
         b2 = Blob.from_string(
-            '\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52'
-            '\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x03\x00\x00\x00\x98\xd3\xb3')
+            b'\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52'
+            b'\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x03\x00\x00\x00\x98\xd3\xb3')
         store = MemoryObjectStore()
         store.add_object(b2)
         write_object_diff(f, store, (None, None, None),
-                                    ('bar.png', 0o644, b2.id))
+                                    (b'bar.png', 0o644, b2.id))
         self.assertEqual([
-            'diff --git /dev/null b/bar.png',
-            'new mode 644',
-            'index 0000000..06364b7 644',
-            'Binary files /dev/null and b/bar.png differ'
+            b'diff --git /dev/null b/bar.png',
+            b'new mode 644',
+            b'index 0000000..06364b7 644',
+            b'Binary files /dev/null and b/bar.png differ'
             ], f.getvalue().splitlines())
 
     def test_object_diff_remove_bin_blob(self):
         f = BytesIO()
         b1 = Blob.from_string(
-            '\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52'
-            '\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x04\x00\x00\x00\x05\x04\x8b')
+            b'\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52'
+            b'\x00\x00\x01\xd5\x00\x00\x00\x9f\x08\x04\x00\x00\x00\x05\x04\x8b')
         store = MemoryObjectStore()
         store.add_object(b1)
-        write_object_diff(f, store, ('foo.png', 0o644, b1.id),
+        write_object_diff(f, store, (b'foo.png', 0o644, b1.id),
                                     (None, None, None))
         self.assertEqual([
-            'diff --git a/foo.png /dev/null',
-            'deleted mode 644',
-            'index f73e47d..0000000',
-            'Binary files a/foo.png and /dev/null differ'
+            b'diff --git a/foo.png /dev/null',
+            b'deleted mode 644',
+            b'index f73e47d..0000000',
+            b'Binary files a/foo.png and /dev/null differ'
             ], f.getvalue().splitlines())
 
     def test_object_diff_kind_change(self):
         f = BytesIO()
-        b1 = Blob.from_string("new\nsame\n")
+        b1 = Blob.from_string(b"new\nsame\n")
         store = MemoryObjectStore()
         store.add_object(b1)
-        write_object_diff(f, store, ("bar.txt", 0o644, b1.id),
-            ("bar.txt", 0o160000, "06d0bdd9e2e20377b3180e4986b14c8549b393e4"))
+        write_object_diff(f, store, (b"bar.txt", 0o644, b1.id),
+            (b"bar.txt", 0o160000, b"06d0bdd9e2e20377b3180e4986b14c8549b393e4"))
         self.assertEqual([
-            'diff --git a/bar.txt b/bar.txt',
-            'old mode 644',
-            'new mode 160000',
-            'index a116b51..06d0bdd 160000',
-            '--- a/bar.txt',
-            '+++ b/bar.txt',
-            '@@ -1,2 +1,1 @@',
-            '-new',
-            '-same',
-            '+Submodule commit 06d0bdd9e2e20377b3180e4986b14c8549b393e4',
+            b'diff --git a/bar.txt b/bar.txt',
+            b'old mode 644',
+            b'new mode 160000',
+            b'index a116b51..06d0bdd 160000',
+            b'--- a/bar.txt',
+            b'+++ b/bar.txt',
+            b'@@ -1,2 +1,1 @@',
+            b'-new',
+            b'-same',
+            b'+Submodule commit 06d0bdd9e2e20377b3180e4986b14c8549b393e4',
             ], f.getvalue().splitlines())

dulwich/tests/test_porcelain.py

@@ -18,6 +18,7 @@
 
 """Tests for dulwich.porcelain."""
 
+from contextlib import closing
 from io import BytesIO
 import os
 import shutil
@@ -45,11 +46,15 @@ from dulwich.tests.utils import (
 class PorcelainTestCase(TestCase):
 
     def setUp(self):
-        super(TestCase, self).setUp()
+        super(PorcelainTestCase, self).setUp()
         repo_dir = tempfile.mkdtemp()
         self.addCleanup(shutil.rmtree, repo_dir)
         self.repo = Repo.init(repo_dir)
 
+    def tearDown(self):
+        super(PorcelainTestCase, self).tearDown()
+        self.repo.close()
+
 
 class ArchiveTests(PorcelainTestCase):
     """Tests for the archive command."""
@@ -77,7 +82,7 @@ class UpdateServerInfoTests(PorcelainTestCase):
         self.repo.refs[b"refs/heads/foo"] = c3.id
         porcelain.update_server_info(self.repo.path)
         self.assertTrue(os.path.exists(os.path.join(self.repo.controldir(),
-            b'info', b'refs')))
+            'info', 'refs')))
 
 
 class CommitTests(PorcelainTestCase):
@@ -127,10 +132,12 @@ class CloneTests(PorcelainTestCase):
         target_path = tempfile.mkdtemp()
         errstream = BytesIO()
         self.addCleanup(shutil.rmtree, target_path)
-        r = porcelain.clone(self.repo.path, target_path,
-                            checkout=True, errstream=errstream)
-        self.assertEqual(r.path, target_path)
-        self.assertEqual(Repo(target_path).head(), c3.id)
+        with closing(porcelain.clone(self.repo.path, target_path,
+                                     checkout=True,
+                                     errstream=errstream)) as r:
+            self.assertEqual(r.path, target_path)
+        with closing(Repo(target_path)) as r:
+            self.assertEqual(r.head(), c3.id)
         self.assertTrue('f1' in os.listdir(target_path))
         self.assertTrue('f2' in os.listdir(target_path))
 
@@ -281,7 +288,7 @@ class SymbolicRefTests(PorcelainTestCase):
         porcelain.symbolic_ref(self.repo.path, b'force_foobar', force=True)
 
         #test if we actually changed the file
-        with self.repo.get_named_file(b'HEAD') as f:
+        with self.repo.get_named_file('HEAD') as f:
             new_ref = f.read()
         self.assertEqual(new_ref, b'ref: refs/heads/force_foobar\n')
 
@@ -301,7 +308,7 @@ class SymbolicRefTests(PorcelainTestCase):
         porcelain.symbolic_ref(self.repo.path, b'develop')
 
         #test if we actually changed the file
-        with self.repo.get_named_file(b'HEAD') as f:
+        with self.repo.get_named_file('HEAD') as f:
             new_ref = f.read()
         self.assertEqual(new_ref, b'ref: refs/heads/develop\n')
 
@@ -445,10 +452,13 @@ class PushTests(PorcelainTestCase):
 
         # Setup target repo cloned from temp test repo
         clone_path = tempfile.mkdtemp()
-        porcelain.clone(self.repo.path, target=clone_path, errstream=errstream)
+        self.addCleanup(shutil.rmtree, clone_path)
+        target_repo = porcelain.clone(self.repo.path, target=clone_path, errstream=errstream)
+        target_repo.close()
 
         # create a second file to be pushed back to origin
         handle, fullpath = tempfile.mkstemp(dir=clone_path)
+        os.close(handle)
         porcelain.add(repo=clone_path, paths=[os.path.basename(fullpath)])
         porcelain.commit(repo=clone_path, message=b'push',
             author=b'', committer=b'')
@@ -462,15 +472,14 @@ class PushTests(PorcelainTestCase):
                 errstream=errstream)
 
         # Check that the target and source
-        r_clone = Repo(clone_path)
+        with closing(Repo(clone_path)) as r_clone:
+            self.assertEqual(r_clone[b'HEAD'].id, self.repo[refs_path].id)
 
-        # Get the change in the target repo corresponding to the add
-        # this will be in the foo branch.
-        change = list(tree_changes(self.repo, self.repo[b'HEAD'].tree,
-                                   self.repo[b'refs/heads/foo'].tree))[0]
-
-        self.assertEqual(r_clone[b'HEAD'].id, self.repo[refs_path].id)
-        self.assertEqual(os.path.basename(fullpath), change.new.path.decode('ascii'))
+            # Get the change in the target repo corresponding to the add
+            # this will be in the foo branch.
+            change = list(tree_changes(self.repo, self.repo[b'HEAD'].tree,
+                                       self.repo[b'refs/heads/foo'].tree))[0]
+            self.assertEqual(os.path.basename(fullpath), change.new.path.decode('ascii'))
 
 
 class PullTests(PorcelainTestCase):
@@ -481,6 +490,7 @@ class PullTests(PorcelainTestCase):
 
         # create a file for initial commit
         handle, fullpath = tempfile.mkstemp(dir=self.repo.path)
+        os.close(handle)
         filename = os.path.basename(fullpath)
         porcelain.add(repo=self.repo.path, paths=filename)
         porcelain.commit(repo=self.repo.path, message=b'test',
@@ -488,10 +498,14 @@ class PullTests(PorcelainTestCase):
 
         # Setup target repo
         target_path = tempfile.mkdtemp()
-        porcelain.clone(self.repo.path, target=target_path, errstream=errstream)
+        self.addCleanup(shutil.rmtree, target_path)
+        target_repo = porcelain.clone(self.repo.path, target=target_path,
+                                      errstream=errstream)
+        target_repo.close()
 
         # create a second file to be pushed
         handle, fullpath = tempfile.mkstemp(dir=self.repo.path)
+        os.close(handle)
         filename = os.path.basename(fullpath)
         porcelain.add(repo=self.repo.path, paths=filename)
         porcelain.commit(repo=self.repo.path, message=b'test2',
@@ -502,8 +516,8 @@ class PullTests(PorcelainTestCase):
             outstream=outstream, errstream=errstream)
 
         # Check the target repo for pushed changes
-        r = Repo(target_path)
-        self.assertEqual(r[b'HEAD'].id, self.repo[b'HEAD'].id)
+        with closing(Repo(target_path)) as r:
+            self.assertEqual(r[b'HEAD'].id, self.repo[b'HEAD'].id)
 
 
 class StatusTests(PorcelainTestCase):
@@ -629,8 +643,8 @@ class ReceivePackTests(PorcelainTestCase):
         exitcode = porcelain.receive_pack(self.repo.path, BytesIO(b"0000"), outf)
         outlines = outf.getvalue().splitlines()
         self.assertEqual([
-            b'005a9e65bdcf4a22cdd4f3700604a275cd2aaf146b23 HEAD\x00report-status '
-            b'delete-refs side-band-64k',
+            b'006d9e65bdcf4a22cdd4f3700604a275cd2aaf146b23 HEAD\x00 report-status '
+            b'delete-refs ofs-delta side-band-64k no-done',
             b'003f9e65bdcf4a22cdd4f3700604a275cd2aaf146b23 refs/heads/master',
             b'0000'], outlines)
         self.assertEqual(0, exitcode)
@@ -687,6 +701,7 @@ class FetchTests(PorcelainTestCase):
 
         # create a file for initial commit
         handle, fullpath = tempfile.mkstemp(dir=self.repo.path)
+        os.close(handle)
         filename = os.path.basename(fullpath)
         porcelain.add(repo=self.repo.path, paths=filename)
         porcelain.commit(repo=self.repo.path, message=b'test',
@@ -694,22 +709,25 @@ class FetchTests(PorcelainTestCase):
 
         # Setup target repo
         target_path = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, target_path)
         target_repo = porcelain.clone(self.repo.path, target=target_path,
             errstream=errstream)
 
         # create a second file to be pushed
         handle, fullpath = tempfile.mkstemp(dir=self.repo.path)
+        os.close(handle)
         filename = os.path.basename(fullpath)
         porcelain.add(repo=self.repo.path, paths=filename)
         porcelain.commit(repo=self.repo.path, message=b'test2',
             author=b'test2', committer=b'test2')
 
         self.assertFalse(self.repo[b'HEAD'].id in target_repo)
+        target_repo.close()
 
         # Fetch changes into the cloned repo
         porcelain.fetch(target_path, self.repo.path, outstream=outstream,
             errstream=errstream)
 
         # Check the target repo for pushed changes
-        r = Repo(target_path)
-        self.assertTrue(self.repo[b'HEAD'].id in r)
+        with closing(Repo(target_path)) as r:
+            self.assertTrue(self.repo[b'HEAD'].id in r)

dulwich/tests/test_refs.py

@@ -280,12 +280,9 @@ class DiskRefsContainerTests(RefsContainerTests, TestCase):
     def setUp(self):
         TestCase.setUp(self)
         self._repo = open_repo('refs.git')
+        self.addCleanup(tear_down_repo, self._repo)
         self._refs = self._repo.refs
 
-    def tearDown(self):
-        tear_down_repo(self._repo)
-        TestCase.tearDown(self)
-
     def test_get_packed_refs(self):
         self.assertEqual({
             b'refs/heads/packed': b'42d06bd4b77fed026b154d16493e5deab78f02ec',
@@ -308,7 +305,7 @@ class DiskRefsContainerTests(RefsContainerTests, TestCase):
 
     def test_setitem(self):
         RefsContainerTests.test_setitem(self)
-        f = open(os.path.join(self._refs.path, b'refs', b'some', b'ref'), 'rb')
+        f = open(os.path.join(self._refs.path, 'refs', 'some', 'ref'), 'rb')
         self.assertEqual(b'42d06bd4b77fed026b154d16493e5deab78f02ec',
                          f.read()[:40])
         f.close()
@@ -319,12 +316,12 @@ class DiskRefsContainerTests(RefsContainerTests, TestCase):
         self.assertEqual(ones, self._refs[b'HEAD'])
 
         # ensure HEAD was not modified
-        f = open(os.path.join(self._refs.path, b'HEAD'), 'rb')
+        f = open(os.path.join(self._refs.path, 'HEAD'), 'rb')
         self.assertEqual(b'ref: refs/heads/master', next(iter(f)).rstrip(b'\n'))
         f.close()
 
         # ensure the symbolic link was written through
-        f = open(os.path.join(self._refs.path, b'refs', b'heads', b'master'), 'rb')
+        f = open(os.path.join(self._refs.path, 'refs', 'heads', 'master'), 'rb')
         self.assertEqual(ones, f.read()[:40])
         f.close()
 
@@ -336,9 +333,9 @@ class DiskRefsContainerTests(RefsContainerTests, TestCase):
 
         # ensure lockfile was deleted
         self.assertFalse(os.path.exists(
-            os.path.join(self._refs.path, b'refs', b'heads', b'master.lock')))
+            os.path.join(self._refs.path, 'refs', 'heads', 'master.lock')))
         self.assertFalse(os.path.exists(
-            os.path.join(self._refs.path, b'HEAD.lock')))
+            os.path.join(self._refs.path, 'HEAD.lock')))
 
     def test_add_if_new_packed(self):
         # don't overwrite packed ref
@@ -349,11 +346,11 @@ class DiskRefsContainerTests(RefsContainerTests, TestCase):
 
     def test_add_if_new_symbolic(self):
         # Use an empty repo instead of the default.
-        tear_down_repo(self._repo)
         repo_dir = os.path.join(tempfile.mkdtemp(), 'test')
         os.makedirs(repo_dir)
-        self._repo = Repo.init(repo_dir)
-        refs = self._repo.refs
+        repo = Repo.init(repo_dir)
+        self.addCleanup(tear_down_repo, repo)
+        refs = repo.refs
 
         nines = b'9' * 40
         self.assertEqual(b'ref: refs/heads/master', refs.read_ref(b'HEAD'))
@@ -377,7 +374,7 @@ class DiskRefsContainerTests(RefsContainerTests, TestCase):
 
     def test_delitem(self):
         RefsContainerTests.test_delitem(self)
-        ref_file = os.path.join(self._refs.path, b'refs', b'heads', b'master')
+        ref_file = os.path.join(self._refs.path, 'refs', 'heads', 'master')
         self.assertFalse(os.path.exists(ref_file))
         self.assertFalse(b'refs/heads/master' in self._refs.get_packed_refs())
 
@@ -388,7 +385,7 @@ class DiskRefsContainerTests(RefsContainerTests, TestCase):
         self.assertRaises(KeyError, lambda: self._refs[b'HEAD'])
         self.assertEqual(b'42d06bd4b77fed026b154d16493e5deab78f02ec',
                          self._refs[b'refs/heads/master'])
-        self.assertFalse(os.path.exists(os.path.join(self._refs.path, b'HEAD')))
+        self.assertFalse(os.path.exists(os.path.join(self._refs.path, 'HEAD')))
 
     def test_remove_if_equals_symref(self):
         # HEAD is a symref, so shouldn't equal its dereferenced value
@@ -404,12 +401,12 @@ class DiskRefsContainerTests(RefsContainerTests, TestCase):
                          self._refs.read_loose_ref(b'HEAD'))
 
         self.assertFalse(os.path.exists(
-            os.path.join(self._refs.path, b'refs', b'heads', b'master.lock')))
+            os.path.join(self._refs.path, 'refs', 'heads', 'master.lock')))
         self.assertFalse(os.path.exists(
-            os.path.join(self._refs.path, b'HEAD.lock')))
+            os.path.join(self._refs.path, 'HEAD.lock')))
 
     def test_remove_packed_without_peeled(self):
-        refs_file = os.path.join(self._repo._controldir, b'packed-refs')
+        refs_file = os.path.join(self._repo.path, 'packed-refs')
         f = GitFile(refs_file)
         refs_data = f.read()
         f.close()

dulwich/tests/test_repository.py

@@ -20,13 +20,14 @@
 
 """Tests for the repository."""
 
+from contextlib import closing
+import locale
 import os
 import stat
 import shutil
 import sys
 import tempfile
 import warnings
-import sys
 
 from dulwich import errors
 from dulwich.object_store import (
@@ -40,6 +41,7 @@ from dulwich.repo import (
     )
 from dulwich.tests import (
     TestCase,
+    skipIf,
     )
 from dulwich.tests.utils import (
     open_repo,
@@ -50,21 +52,9 @@ from dulwich.tests.utils import (
 missing_sha = b'b91fa4d900e17e99b433218e988c4eb4a3e9a097'
 
 
-def mkdtemp_bytes():
-    tmp_dir = tempfile.mkdtemp()
-    if sys.version_info[0] > 2:
-        tmp_dir = tmp_dir.encode(sys.getfilesystemencoding())
-    return tmp_dir
-
 def mkdtemp_unicode():
-    suffix = u'déłwíçh'
-    if sys.version_info[0] == 2:
-        suffix = suffix.encode(sys.getfilesystemencoding())
-    tmp_dir = tempfile.mkdtemp(suffix=suffix)
-    if sys.version_info[0] == 2:
-        tmp_dir = tmp_dir.decode(sys.getfilesystemencoding())
-    return tmp_dir
-
+    suffix = u'délwíçh'
+    return tempfile.mkdtemp(suffix=suffix)
 
 
 class CreateRepositoryTests(TestCase):
@@ -79,81 +69,56 @@ class CreateRepositoryTests(TestCase):
 
     def _check_repo_contents(self, repo, expect_bare):
         self.assertEqual(expect_bare, repo.bare)
-        self.assertFileContentsEqual(b'Unnamed repository', repo, b'description')
-        self.assertFileContentsEqual(b'', repo, os.path.join(b'info', b'exclude'))
-        self.assertFileContentsEqual(None, repo, b'nonexistent file')
+        self.assertFileContentsEqual(b'Unnamed repository', repo, 'description')
+        self.assertFileContentsEqual(b'', repo, os.path.join('info', 'exclude'))
+        self.assertFileContentsEqual(None, repo, 'nonexistent file')
         barestr = b'bare = ' + str(expect_bare).lower().encode('ascii')
-        with repo.get_named_file(b'config') as f:
+        with repo.get_named_file('config') as f:
             config_text = f.read()
             self.assertTrue(barestr in config_text, "%r" % config_text)
 
-
-class CreateMemoryRepositoryTests(CreateRepositoryTests):
-
     def test_create_memory(self):
         repo = MemoryRepo.init_bare([], {})
         self._check_repo_contents(repo, True)
 
-
-class CreateRepositoryBytesRootTests(CreateRepositoryTests):
-
-    def mkdtemp(self):
-        tmp_dir = mkdtemp_bytes()
-        return tmp_dir, tmp_dir
-
     def test_create_disk_bare(self):
-        tmp_dir, tmp_dir_bytes = self.mkdtemp()
+        tmp_dir = mkdtemp_unicode()
         self.addCleanup(shutil.rmtree, tmp_dir)
         repo = Repo.init_bare(tmp_dir)
-        self.assertEqual(tmp_dir_bytes, repo._controldir)
+        self.assertEqual(tmp_dir, repo._controldir)
         self._check_repo_contents(repo, True)
 
     def test_create_disk_non_bare(self):
-        tmp_dir, tmp_dir_bytes = self.mkdtemp()
+        tmp_dir = mkdtemp_unicode()
         self.addCleanup(shutil.rmtree, tmp_dir)
         repo = Repo.init(tmp_dir)
-        self.assertEqual(os.path.join(tmp_dir_bytes, b'.git'), repo._controldir)
+        self.assertEqual(os.path.join(tmp_dir, '.git'), repo._controldir)
         self._check_repo_contents(repo, False)
 
 
-class CreateRepositoryUnicodeRootTests(CreateRepositoryBytesRootTests):
-
-    def mktemp(self):
-        tmp_dir = mkdtemp_unicode()
-        tmp_dir_bytes = tmp_dir.encode(sys.getfilesystemencoding())
-        return tmp_dir, tmp_dir_bytes
-
-
-class RepositoryBytesRootTests(TestCase):
-
-    def setUp(self):
-        super(RepositoryBytesRootTests, self).setUp()
-        self._repo = None
-
-    def tearDown(self):
-        if self._repo is not None:
-            tear_down_repo(self._repo)
-        super(RepositoryBytesRootTests, self).tearDown()
+class RepositoryRootTests(TestCase):
 
     def mkdtemp(self):
-        return mkdtemp_bytes()
+        return mkdtemp_unicode()
 
     def open_repo(self, name):
         temp_dir = self.mkdtemp()
-        return open_repo(name, temp_dir)
+        repo = open_repo(name, temp_dir)
+        self.addCleanup(tear_down_repo, repo)
+        return repo
 
     def test_simple_props(self):
-        r = self._repo = self.open_repo('a.git')
-        self.assertEqual(r.controldir(), r._path_bytes)
+        r = self.open_repo('a.git')
+        self.assertEqual(r.controldir(), r.path)
 
     def test_setitem(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         r[b"refs/tags/foo"] = b'a90fa2d900a17e99b433217e988c4eb4a2e9a097'
         self.assertEqual(b'a90fa2d900a17e99b433217e988c4eb4a2e9a097',
                           r[b"refs/tags/foo"].id)
 
     def test_getitem_unicode(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
 
         test_keys = [
             (b'refs/heads/master', True),
@@ -171,7 +136,7 @@ class RepositoryBytesRootTests(TestCase):
             )
 
     def test_delitem(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
 
         del r[b'refs/heads/master']
         self.assertRaises(KeyError, lambda: r[b'refs/heads/master'])
@@ -182,7 +147,7 @@ class RepositoryBytesRootTests(TestCase):
         self.assertRaises(ValueError, r.__delitem__, b'notrefs/foo')
 
     def test_get_refs(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         self.assertEqual({
             b'HEAD': b'a90fa2d900a17e99b433217e988c4eb4a2e9a097',
             b'refs/heads/master': b'a90fa2d900a17e99b433217e988c4eb4a2e9a097',
@@ -191,49 +156,49 @@ class RepositoryBytesRootTests(TestCase):
             }, r.get_refs())
 
     def test_head(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         self.assertEqual(r.head(), b'a90fa2d900a17e99b433217e988c4eb4a2e9a097')
 
     def test_get_object(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         obj = r.get_object(r.head())
         self.assertEqual(obj.type_name, b'commit')
 
     def test_get_object_non_existant(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         self.assertRaises(KeyError, r.get_object, missing_sha)
 
     def test_contains_object(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         self.assertTrue(r.head() in r)
 
     def test_contains_ref(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         self.assertTrue(b"HEAD" in r)
 
     def test_get_no_description(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         self.assertIs(None, r.get_description())
 
     def test_get_description(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         with open(os.path.join(r.path, 'description'), 'wb') as f:
             f.write(b"Some description")
         self.assertEqual(b"Some description", r.get_description())
 
     def test_set_description(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         description = b"Some description"
         r.set_description(description)
         self.assertEqual(description, r.get_description())
 
     def test_contains_missing(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         self.assertFalse(b"bar" in r)
 
     def test_get_peeled(self):
         # unpacked ref
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         tag_sha = b'28237f4dc30d0d462658d6b937b08a0f0b6ef55a'
         self.assertNotEqual(r[tag_sha].sha().hexdigest(), r.head())
         self.assertEqual(r.get_peeled(b'refs/tags/mytag'), r.head())
@@ -247,11 +212,11 @@ class RepositoryBytesRootTests(TestCase):
         # TODO: add more corner cases to test repo
 
     def test_get_peeled_not_tag(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         self.assertEqual(r.get_peeled(b'HEAD'), r.head())
 
     def test_get_walker(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         # include defaults to [r.head()]
         self.assertEqual([e.commit.id for e in r.get_walker()],
                          [r.head(), b'2a72d929692c41d8554c07f6301757ba18a65d91'])
@@ -263,38 +228,34 @@ class RepositoryBytesRootTests(TestCase):
             [b'2a72d929692c41d8554c07f6301757ba18a65d91'])
 
     def test_clone(self):
-        r = self._repo = self.open_repo('a.git')
+        r = self.open_repo('a.git')
         tmp_dir = self.mkdtemp()
         self.addCleanup(shutil.rmtree, tmp_dir)
-        t = r.clone(tmp_dir, mkdir=False)
-        self.assertEqual({
-            b'HEAD': b'a90fa2d900a17e99b433217e988c4eb4a2e9a097',
-            b'refs/remotes/origin/master':
-                b'a90fa2d900a17e99b433217e988c4eb4a2e9a097',
-            b'refs/heads/master': b'a90fa2d900a17e99b433217e988c4eb4a2e9a097',
-            b'refs/tags/mytag': b'28237f4dc30d0d462658d6b937b08a0f0b6ef55a',
-            b'refs/tags/mytag-packed':
-                b'b0931cadc54336e78a1d980420e3268903b57a50',
-            }, t.refs.as_dict())
-        shas = [e.commit.id for e in r.get_walker()]
-        self.assertEqual(shas, [t.head(),
-                         b'2a72d929692c41d8554c07f6301757ba18a65d91'])
+        with closing(r.clone(tmp_dir, mkdir=False)) as t:
+            self.assertEqual({
+                b'HEAD': b'a90fa2d900a17e99b433217e988c4eb4a2e9a097',
+                b'refs/remotes/origin/master':
+                    b'a90fa2d900a17e99b433217e988c4eb4a2e9a097',
+                b'refs/heads/master': b'a90fa2d900a17e99b433217e988c4eb4a2e9a097',
+                b'refs/tags/mytag': b'28237f4dc30d0d462658d6b937b08a0f0b6ef55a',
+                b'refs/tags/mytag-packed':
+                    b'b0931cadc54336e78a1d980420e3268903b57a50',
+                }, t.refs.as_dict())
+            shas = [e.commit.id for e in r.get_walker()]
+            self.assertEqual(shas, [t.head(),
+                             b'2a72d929692c41d8554c07f6301757ba18a65d91'])
 
     def test_clone_no_head(self):
         temp_dir = self.mkdtemp()
-        if isinstance(temp_dir, bytes):
-            temp_dir_str = temp_dir.decode(sys.getfilesystemencoding())
-        else:
-            temp_dir_str = temp_dir
         self.addCleanup(shutil.rmtree, temp_dir)
         repo_dir = os.path.join(os.path.dirname(__file__), 'data', 'repos')
-        dest_dir = os.path.join(temp_dir_str, 'a.git')
+        dest_dir = os.path.join(temp_dir, 'a.git')
         shutil.copytree(os.path.join(repo_dir, 'a.git'),
                         dest_dir, symlinks=True)
         r = Repo(dest_dir)
         del r.refs[b"refs/heads/master"]
         del r.refs[b"HEAD"]
-        t = r.clone(os.path.join(temp_dir_str, 'b.git'), mkdir=True)
+        t = r.clone(os.path.join(temp_dir, 'b.git'), mkdir=True)
         self.assertEqual({
             b'refs/tags/mytag': b'28237f4dc30d0d462658d6b937b08a0f0b6ef55a',
             b'refs/tags/mytag-packed':
@@ -309,13 +270,13 @@ class RepositoryBytesRootTests(TestCase):
         to the server.
         Non-bare repo HEAD always points to an existing ref.
         """
-        r = self._repo = self.open_repo('empty.git')
+        r = self.open_repo('empty.git')
         tmp_dir = self.mkdtemp()
         self.addCleanup(shutil.rmtree, tmp_dir)
         r.clone(tmp_dir, mkdir=False, bare=True)
 
     def test_merge_history(self):
-        r = self._repo = self.open_repo('simple_merge.git')
+        r = self.open_repo('simple_merge.git')
         shas = [e.commit.id for e in r.get_walker()]
         self.assertEqual(shas, [b'5dac377bdded4c9aeb8dff595f0faeebcc8498cc',
                                 b'ab64bbdcc51b170d21588e5c5d391ee5c0c96dfd',
@@ -325,7 +286,7 @@ class RepositoryBytesRootTests(TestCase):
 
     def test_out_of_order_merge(self):
         """Test that revision history is ordered by date, not parent order."""
-        r = self._repo = self.open_repo('ooo_merge.git')
+        r = self.open_repo('ooo_merge.git')
         shas = [e.commit.id for e in r.get_walker()]
         self.assertEqual(shas, [b'7601d7f6231db6a57f7bbb79ee52e4d462fd44d1',
                                 b'f507291b64138b875c28e03469025b1ea20bc614',
@@ -333,30 +294,28 @@ class RepositoryBytesRootTests(TestCase):
                                 b'f9e39b120c68182a4ba35349f832d0e4e61f485c'])
 
     def test_get_tags_empty(self):
-        r = self._repo = self.open_repo('ooo_merge.git')
+        r = self.open_repo('ooo_merge.git')
         self.assertEqual({}, r.refs.as_dict(b'refs/tags'))
 
     def test_get_config(self):
-        r = self._repo = self.open_repo('ooo_merge.git')
+        r = self.open_repo('ooo_merge.git')
         self.assertIsInstance(r.get_config(), Config)
 
     def test_get_config_stack(self):
-        r = self._repo = self.open_repo('ooo_merge.git')
+        r = self.open_repo('ooo_merge.git')
         self.assertIsInstance(r.get_config_stack(), Config)
 
+    @skipIf(not getattr(os, 'symlink', None), 'Requires symlink support')
     def test_submodule(self):
         temp_dir = self.mkdtemp()
+        self.addCleanup(shutil.rmtree, temp_dir)
         repo_dir = os.path.join(os.path.dirname(__file__), 'data', 'repos')
-        if isinstance(temp_dir, bytes):
-            temp_dir_str = temp_dir.decode(sys.getfilesystemencoding())
-        else:
-            temp_dir_str = temp_dir
         shutil.copytree(os.path.join(repo_dir, 'a.git'),
-                        os.path.join(temp_dir_str, 'a.git'), symlinks=True)
-        rel = os.path.relpath(os.path.join(repo_dir, 'submodule'), temp_dir_str)
-        os.symlink(os.path.join(rel, 'dotgit'), os.path.join(temp_dir_str, '.git'))
-        r = Repo(temp_dir)
-        self.assertEqual(r.head(), b'a90fa2d900a17e99b433217e988c4eb4a2e9a097')
+                        os.path.join(temp_dir, 'a.git'), symlinks=True)
+        rel = os.path.relpath(os.path.join(repo_dir, 'submodule'), temp_dir)
+        os.symlink(os.path.join(rel, 'dotgit'), os.path.join(temp_dir, '.git'))
+        with closing(Repo(temp_dir)) as r:
+            self.assertEqual(r.head(), b'a90fa2d900a17e99b433217e988c4eb4a2e9a097')
 
     def test_common_revisions(self):
         """
@@ -377,45 +336,43 @@ class RepositoryBytesRootTests(TestCase):
         # corrupted, but we're only checking for commits for the purpose of this
         # test, so it's immaterial.
         r1_dir = self.mkdtemp()
+        self.addCleanup(shutil.rmtree, r1_dir)
         r1_commits = [b'ab64bbdcc51b170d21588e5c5d391ee5c0c96dfd', # HEAD
                       b'60dacdc733de308bb77bb76ce0fb0f9b44c9769e',
                       b'0d89f20333fbb1d2f3a94da77f4981373d8f4310']
 
         r2_dir = self.mkdtemp()
+        self.addCleanup(shutil.rmtree, r2_dir)
         r2_commits = [b'4cffe90e0a41ad3f5190079d7c8f036bde29cbe6', # HEAD
                       b'60dacdc733de308bb77bb76ce0fb0f9b44c9769e',
                       b'0d89f20333fbb1d2f3a94da77f4981373d8f4310']
 
-        try:
-            r1 = Repo.init_bare(r1_dir)
-            for c in r1_commits:
-                r1.object_store.add_object(r_base.get_object(c))
-            r1.refs[b'HEAD'] = r1_commits[0]
+        r1 = Repo.init_bare(r1_dir)
+        for c in r1_commits:
+            r1.object_store.add_object(r_base.get_object(c))
+        r1.refs[b'HEAD'] = r1_commits[0]
 
-            r2 = Repo.init_bare(r2_dir)
-            for c in r2_commits:
-                r2.object_store.add_object(r_base.get_object(c))
-            r2.refs[b'HEAD'] = r2_commits[0]
+        r2 = Repo.init_bare(r2_dir)
+        for c in r2_commits:
+            r2.object_store.add_object(r_base.get_object(c))
+        r2.refs[b'HEAD'] = r2_commits[0]
 
-            # Finally, the 'real' testing!
-            shas = r2.object_store.find_common_revisions(r1.get_graph_walker())
-            self.assertEqual(set(shas), expected_shas)
+        # Finally, the 'real' testing!
+        shas = r2.object_store.find_common_revisions(r1.get_graph_walker())
+        self.assertEqual(set(shas), expected_shas)
 
-            shas = r1.object_store.find_common_revisions(r2.get_graph_walker())
-            self.assertEqual(set(shas), expected_shas)
-        finally:
-            shutil.rmtree(r1_dir)
-            shutil.rmtree(r2_dir)
+        shas = r1.object_store.find_common_revisions(r2.get_graph_walker())
+        self.assertEqual(set(shas), expected_shas)
 
     def test_shell_hook_pre_commit(self):
         if os.name != 'posix':
             self.skipTest('shell hook tests requires POSIX shell')
 
-        pre_commit_fail = b"""#!/bin/sh
+        pre_commit_fail = """#!/bin/sh
 exit 1
 """
 
-        pre_commit_success = b"""#!/bin/sh
+        pre_commit_success = """#!/bin/sh
 exit 0
 """
 
@@ -423,9 +380,9 @@ exit 0
         r = Repo.init(repo_dir)
         self.addCleanup(shutil.rmtree, repo_dir)
 
-        pre_commit = os.path.join(r.controldir(), b'hooks', b'pre-commit')
+        pre_commit = os.path.join(r.controldir(), 'hooks', 'pre-commit')
 
-        with open(pre_commit, 'wb') as f:
+        with open(pre_commit, 'w') as f:
             f.write(pre_commit_fail)
         os.chmod(pre_commit, stat.S_IREAD | stat.S_IWRITE | stat.S_IEXEC)
 
@@ -435,7 +392,7 @@ exit 0
                           commit_timestamp=12345, commit_timezone=0,
                           author_timestamp=12345, author_timezone=0)
 
-        with open(pre_commit, 'wb') as f:
+        with open(pre_commit, 'w') as f:
             f.write(pre_commit_success)
         os.chmod(pre_commit, stat.S_IREAD | stat.S_IWRITE | stat.S_IEXEC)
 
@@ -451,21 +408,21 @@ exit 0
         if os.name != 'posix':
             self.skipTest('shell hook tests requires POSIX shell')
 
-        commit_msg_fail = b"""#!/bin/sh
+        commit_msg_fail = """#!/bin/sh
 exit 1
 """
 
-        commit_msg_success = b"""#!/bin/sh
+        commit_msg_success = """#!/bin/sh
 exit 0
 """
 
-        repo_dir = os.path.join(self.mkdtemp())
+        repo_dir = self.mkdtemp()
         r = Repo.init(repo_dir)
         self.addCleanup(shutil.rmtree, repo_dir)
 
-        commit_msg = os.path.join(r.controldir(), b'hooks', b'commit-msg')
+        commit_msg = os.path.join(r.controldir(), 'hooks', 'commit-msg')
 
-        with open(commit_msg, 'wb') as f:
+        with open(commit_msg, 'w') as f:
             f.write(commit_msg_fail)
         os.chmod(commit_msg, stat.S_IREAD | stat.S_IWRITE | stat.S_IEXEC)
 
@@ -475,7 +432,7 @@ exit 0
                           commit_timestamp=12345, commit_timezone=0,
                           author_timestamp=12345, author_timezone=0)
 
-        with open(commit_msg, 'wb') as f:
+        with open(commit_msg, 'w') as f:
             f.write(commit_msg_success)
         os.chmod(commit_msg, stat.S_IREAD | stat.S_IWRITE | stat.S_IEXEC)
 
@@ -492,15 +449,12 @@ exit 0
             self.skipTest('shell hook tests requires POSIX shell')
 
         repo_dir = self.mkdtemp()
-        if isinstance(repo_dir, bytes):
-            repo_dir_str = repo_dir.decode(sys.getfilesystemencoding())
-        else:
-            repo_dir_str = repo_dir
 
         r = Repo.init(repo_dir)
         self.addCleanup(shutil.rmtree, repo_dir)
 
-        (fd, path) = tempfile.mkstemp(dir=repo_dir_str)
+        (fd, path) = tempfile.mkstemp(dir=repo_dir)
+        os.close(fd)
         post_commit_msg = """#!/bin/sh
 rm """ + path + """
 """
@@ -513,10 +467,10 @@ rm """ + path + """
             author_timestamp=12345, author_timezone=0)
         self.assertEqual([], r[root_sha].parents)
 
-        post_commit = os.path.join(r.controldir(), b'hooks', b'post-commit')
+        post_commit = os.path.join(r.controldir(), 'hooks', 'post-commit')
 
-        with open(post_commit, 'w') as f:
-            f.write(post_commit_msg)
+        with open(post_commit, 'wb') as f:
+            f.write(post_commit_msg.encode(locale.getpreferredencoding()))
         os.chmod(post_commit, stat.S_IREAD | stat.S_IWRITE | stat.S_IEXEC)
 
         commit_sha = r.do_commit(
@@ -553,13 +507,7 @@ exit 1
         self.assertEqual([commit_sha], r[commit_sha2].parents)
 
 
-class RepositoryUnicodeRootTests(RepositoryBytesRootTests):
-
-    def mktemp(self):
-        return mkdtemp_unicode()
-
-
-class BuildRepoBytesRootTests(TestCase):
+class BuildRepoRootTests(TestCase):
     """Tests that build on-disk repos from scratch.
 
     Repos live in a temp dir and are torn down after each test. They start with
@@ -567,21 +515,19 @@ class BuildRepoBytesRootTests(TestCase):
     """
 
     def get_repo_dir(self):
-        return os.path.join(mkdtemp_bytes(), b'test')
-
-    def get_a_filename(self):
-        return b'a'
+        return os.path.join(mkdtemp_unicode(), 'test')
 
     def setUp(self):
-        super(BuildRepoBytesRootTests, self).setUp()
+        super(BuildRepoRootTests, self).setUp()
         self._repo_dir = self.get_repo_dir()
         os.makedirs(self._repo_dir)
         r = self._repo = Repo.init(self._repo_dir)
+        self.addCleanup(tear_down_repo, r)
         self.assertFalse(r.bare)
         self.assertEqual(b'ref: refs/heads/master', r.refs.read_ref(b'HEAD'))
         self.assertRaises(KeyError, lambda: r.refs[b'refs/heads/master'])
 
-        with open(os.path.join(r._path_bytes, b'a'), 'wb') as f:
+        with open(os.path.join(r.path, 'a'), 'wb') as f:
             f.write(b'file contents')
         r.stage(['a'])
         commit_sha = r.do_commit(b'msg',
@@ -592,10 +538,6 @@ class BuildRepoBytesRootTests(TestCase):
         self.assertEqual([], r[commit_sha].parents)
         self._root_commit = commit_sha
 
-    def tearDown(self):
-        tear_down_repo(self._repo)
-        super(BuildRepoBytesRootTests, self).tearDown()
-
     def test_build_repo(self):
         r = self._repo
         self.assertEqual(b'ref: refs/heads/master', r.refs.read_ref(b'HEAD'))
@@ -607,10 +549,9 @@ class BuildRepoBytesRootTests(TestCase):
 
     def test_commit_modified(self):
         r = self._repo
-        with open(os.path.join(r._path_bytes, b'a'), 'wb') as f:
+        with open(os.path.join(r.path, 'a'), 'wb') as f:
             f.write(b'new contents')
-        os.symlink('a', os.path.join(r._path_bytes, b'b'))
-        r.stage(['a', 'b'])
+        r.stage(['a'])
         commit_sha = r.do_commit(b'modified a',
                                  committer=b'Test Committer <test@nodomain.com>',
                                  author=b'Test Author <test@nodomain.com>',
@@ -620,13 +561,25 @@ class BuildRepoBytesRootTests(TestCase):
         a_mode, a_id = tree_lookup_path(r.get_object, r[commit_sha].tree, b'a')
         self.assertEqual(stat.S_IFREG | 0o644, a_mode)
         self.assertEqual(b'new contents', r[a_id].data)
+
+    @skipIf(not getattr(os, 'symlink', None), 'Requires symlink support')
+    def test_commit_symlink(self):
+        r = self._repo
+        os.symlink('a', os.path.join(r.path, 'b'))
+        r.stage(['a', 'b'])
+        commit_sha = r.do_commit(b'Symlink b',
+                                 committer=b'Test Committer <test@nodomain.com>',
+                                 author=b'Test Author <test@nodomain.com>',
+                                 commit_timestamp=12395, commit_timezone=0,
+                                 author_timestamp=12395, author_timezone=0)
+        self.assertEqual([self._root_commit], r[commit_sha].parents)
         b_mode, b_id = tree_lookup_path(r.get_object, r[commit_sha].tree, b'b')
         self.assertTrue(stat.S_ISLNK(b_mode))
         self.assertEqual(b'a', r[b_id].data)
 
     def test_commit_deleted(self):
         r = self._repo
-        os.remove(os.path.join(r._path_bytes, b'a'))
+        os.remove(os.path.join(r.path, 'a'))
         r.stage(['a'])
         commit_sha = r.do_commit(b'deleted a',
                                  committer=b'Test Committer <test@nodomain.com>',
@@ -788,17 +741,32 @@ class BuildRepoBytesRootTests(TestCase):
 
     def test_stage_deleted(self):
         r = self._repo
-        os.remove(os.path.join(r._path_bytes, b'a'))
+        os.remove(os.path.join(r.path, 'a'))
         r.stage(['a'])
         r.stage(['a'])  # double-stage a deleted path
 
+    def test_commit_no_encode_decode(self):
+        r = self._repo
+        repo_path_bytes = r.path.encode(sys.getfilesystemencoding())
+        encodings = ('utf8', 'latin1')
+        names = [u'À'.encode(encoding) for encoding in encodings]
+        for name, encoding in zip(names, encodings):
+            full_path = os.path.join(repo_path_bytes, name)
+            with open(full_path, 'wb') as f:
+                f.write(encoding.encode('ascii'))
+            # These files are break tear_down_repo, so cleanup these files
+            # ourselves.
+            self.addCleanup(os.remove, full_path)
+
+        r.stage(names)
+        commit_sha = r.do_commit(b'Files with different encodings',
+             committer=b'Test Committer <test@nodomain.com>',
+             author=b'Test Author <test@nodomain.com>',
+             commit_timestamp=12395, commit_timezone=0,
+             author_timestamp=12395, author_timezone=0,
+             ref=None, merge_heads=[self._root_commit])
 
-class BuildRepoUnicodeRootTests(TestCase):
-    """Tests that build on-disk repos from scratch.
-
-    Repos live in a temp dir and are torn down after each test. They start with
-    a single commit in master having single file named 'a'.
-    """
-
-    def get_repo_dir(self):
-        return os.path.join(mkdtemp_unicode(), 'test')
+        for name, encoding in zip(names, encodings):
+            mode, id = tree_lookup_path(r.get_object, r[commit_sha].tree, name)
+            self.assertEqual(stat.S_IFREG | 0o644, mode)
+            self.assertEqual(encoding.encode('ascii'), r[id].data)

dulwich/tests/test_server.py

@@ -20,6 +20,7 @@
 
 from io import BytesIO
 import os
+import shutil
 import tempfile
 
 from dulwich.errors import (
@@ -126,7 +127,7 @@ class HandlerTestCase(TestCase):
             self.fail(e)
 
     def test_capability_line(self):
-        self.assertEqual(b'cap1 cap2 cap3', self._handler.capability_line())
+        self.assertEqual(b' cap1 cap2 cap3', self._handler.capability_line())
 
     def test_set_client_capabilities(self):
         set_caps = self._handler.set_client_capabilities
@@ -211,6 +212,7 @@ class UploadPackHandlerTestCase(TestCase):
 class FindShallowTests(TestCase):
 
     def setUp(self):
+        super(FindShallowTests, self).setUp()
         self._store = MemoryObjectStore()
 
     def make_commit(self, **attrs):
@@ -513,9 +515,14 @@ class TestProtocolGraphWalker(object):
     def __init__(self):
         self.acks = []
         self.lines = []
-        self.done = False
+        self.wants_satisified = False
         self.http_req = None
         self.advertise_refs = False
+        self._impl = None
+        self.done_required = True
+        self.done_received = False
+        self._empty = False
+        self.pack_sent = False
 
     def read_proto_line(self, allowed):
         command, sha = self.lines.pop(0)
@@ -530,13 +537,26 @@ class TestProtocolGraphWalker(object):
         self.acks.append((None, b'nak'))
 
     def all_wants_satisfied(self, haves):
-        return self.done
+        if haves:
+            return self.wants_satisified
 
     def pop_ack(self):
         if not self.acks:
             return None
         return self.acks.pop(0)
 
+    def handle_done(self):
+        if not self._impl:
+            return
+        # Whether or not PACK is sent after is determined by this, so
+        # record this value.
+        self.pack_sent = self._impl.handle_done(self.done_required,
+            self.done_received)
+        return self.pack_sent
+
+    def notify_done(self):
+        self.done_received = True
+
 
 class AckGraphWalkerImplTestCase(TestCase):
     """Base setup and asserts for AckGraphWalker tests."""
@@ -551,6 +571,7 @@ class AckGraphWalkerImplTestCase(TestCase):
           (b'done', None),
           ]
         self._impl = self.impl_cls(self._walker)
+        self._walker._impl = self._impl
 
     def assertNoAck(self):
         self.assertEqual(None, self._walker.pop_ack())
@@ -569,6 +590,15 @@ class AckGraphWalkerImplTestCase(TestCase):
     def assertNextEquals(self, sha):
         self.assertEqual(sha, next(self._impl))
 
+    def assertNextEmpty(self):
+        # This is necessary because of no-done - the assumption that it
+        # it safe to immediately send out the final ACK is no longer
+        # true but the test is still needed for it.  TestProtocolWalker
+        # does implement the handle_done which will determine whether
+        # the final confirmation can be sent.
+        self.assertRaises(IndexError, next, self._impl)
+        self._walker.handle_done()
+
 
 class SingleAckGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
 
@@ -579,7 +609,6 @@ class SingleAckGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertNoAck()
 
         self.assertNextEquals(ONE)
-        self._walker.done = True
         self._impl.ack(ONE)
         self.assertAck(ONE)
 
@@ -598,7 +627,6 @@ class SingleAckGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertNoAck()
 
         self.assertNextEquals(ONE)
-        self._walker.done = True
         self._impl.ack(ONE)
         self.assertAck(ONE)
 
@@ -619,6 +647,7 @@ class SingleAckGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertNoAck()
 
         self.assertNextEquals(None)
+        self.assertNextEmpty()
         self.assertNak()
 
     def test_single_ack_nak_flush(self):
@@ -635,6 +664,7 @@ class SingleAckGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertNoAck()
 
         self.assertNextEquals(None)
+        self.assertNextEmpty()
         self.assertNak()
 
 
@@ -647,7 +677,6 @@ class MultiAckGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertNoAck()
 
         self.assertNextEquals(ONE)
-        self._walker.done = True
         self._impl.ack(ONE)
         self.assertAck(ONE, b'continue')
 
@@ -656,6 +685,7 @@ class MultiAckGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertAck(THREE, b'continue')
 
         self.assertNextEquals(None)
+        self.assertNextEmpty()
         self.assertAck(THREE)
 
     def test_multi_ack_partial(self):
@@ -670,7 +700,7 @@ class MultiAckGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertNoAck()
 
         self.assertNextEquals(None)
-        # done, re-send ack of last common
+        self.assertNextEmpty()
         self.assertAck(ONE)
 
     def test_multi_ack_flush(self):
@@ -687,7 +717,6 @@ class MultiAckGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertNextEquals(ONE)
         self.assertNak()  # nak the flush-pkt
 
-        self._walker.done = True
         self._impl.ack(ONE)
         self.assertAck(ONE, b'continue')
 
@@ -696,6 +725,7 @@ class MultiAckGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertAck(THREE, b'continue')
 
         self.assertNextEquals(None)
+        self.assertNextEmpty()
         self.assertAck(THREE)
 
     def test_multi_ack_nak(self):
@@ -709,6 +739,7 @@ class MultiAckGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertNoAck()
 
         self.assertNextEquals(None)
+        self.assertNextEmpty()
         self.assertNak()
 
 
@@ -721,16 +752,88 @@ class MultiAckDetailedGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertNoAck()
 
         self.assertNextEquals(ONE)
-        self._walker.done = True
         self._impl.ack(ONE)
-        self.assertAcks([(ONE, b'common'), (ONE, b'ready')])
+        self.assertAck(ONE, b'common')
 
         self.assertNextEquals(THREE)
         self._impl.ack(THREE)
-        self.assertAck(THREE, b'ready')
+        self.assertAck(THREE, b'common')
 
+        # done is read.
+        self._walker.wants_satisified = True
         self.assertNextEquals(None)
-        self.assertAck(THREE)
+        self._walker.lines.append((None, None))
+        self.assertNextEmpty()
+        self.assertAcks([(THREE, b'ready'), (None, b'nak'), (THREE, b'')])
+        # PACK is sent
+        self.assertTrue(self._walker.pack_sent)
+
+    def test_multi_ack_nodone(self):
+        self._walker.done_required = False
+        self.assertNextEquals(TWO)
+        self.assertNoAck()
+
+        self.assertNextEquals(ONE)
+        self._impl.ack(ONE)
+        self.assertAck(ONE, b'common')
+
+        self.assertNextEquals(THREE)
+        self._impl.ack(THREE)
+        self.assertAck(THREE, b'common')
+
+        # done is read.
+        self._walker.wants_satisified = True
+        self.assertNextEquals(None)
+        self._walker.lines.append((None, None))
+        self.assertNextEmpty()
+        self.assertAcks([(THREE, b'ready'), (None, b'nak'), (THREE, b'')])
+        # PACK is sent
+        self.assertTrue(self._walker.pack_sent)
+
+    def test_multi_ack_flush_end(self):
+        # transmission ends with a flush-pkt without a done but no-done is
+        # assumed.
+        self._walker.lines[-1] = (None, None)
+        self.assertNextEquals(TWO)
+        self.assertNoAck()
+
+        self.assertNextEquals(ONE)
+        self._impl.ack(ONE)
+        self.assertAck(ONE, b'common')
+
+        self.assertNextEquals(THREE)
+        self._impl.ack(THREE)
+        self.assertAck(THREE, b'common')
+
+        # no done is read
+        self._walker.wants_satisified = True
+        self.assertNextEmpty()
+        self.assertAcks([(THREE, b'ready'), (None, b'nak')])
+        # PACK is NOT sent
+        self.assertFalse(self._walker.pack_sent)
+
+    def test_multi_ack_flush_end_nodone(self):
+        # transmission ends with a flush-pkt without a done but no-done is
+        # assumed.
+        self._walker.lines[-1] = (None, None)
+        self._walker.done_required = False
+        self.assertNextEquals(TWO)
+        self.assertNoAck()
+
+        self.assertNextEquals(ONE)
+        self._impl.ack(ONE)
+        self.assertAck(ONE, b'common')
+
+        self.assertNextEquals(THREE)
+        self._impl.ack(THREE)
+        self.assertAck(THREE, b'common')
+
+        # no done is read, but pretend it is (last 'ACK 'commit_id' '')
+        self._walker.wants_satisified = True
+        self.assertNextEmpty()
+        self.assertAcks([(THREE, b'ready'), (None, b'nak'), (THREE, b'')])
+        # PACK is sent
+        self.assertTrue(self._walker.pack_sent)
 
     def test_multi_ack_partial(self):
         self.assertNextEquals(TWO)
@@ -744,7 +847,7 @@ class MultiAckDetailedGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertNoAck()
 
         self.assertNextEquals(None)
-        # done, re-send ack of last common
+        self.assertNextEmpty()
         self.assertAck(ONE)
 
     def test_multi_ack_flush(self):
@@ -755,6 +858,7 @@ class MultiAckDetailedGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
           (b'have', ONE),
           (b'have', THREE),
           (b'done', None),
+          (None, None),
           ]
         self.assertNextEquals(TWO)
         self.assertNoAck()
@@ -762,16 +866,17 @@ class MultiAckDetailedGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertNextEquals(ONE)
         self.assertNak()  # nak the flush-pkt
 
-        self._walker.done = True
         self._impl.ack(ONE)
-        self.assertAcks([(ONE, b'common'), (ONE, b'ready')])
+        self.assertAck(ONE, b'common')
 
         self.assertNextEquals(THREE)
         self._impl.ack(THREE)
-        self.assertAck(THREE, b'ready')
+        self.assertAck(THREE, b'common')
 
+        self._walker.wants_satisified = True
         self.assertNextEquals(None)
-        self.assertAck(THREE)
+        self.assertNextEmpty()
+        self.assertAcks([(THREE, b'ready'), (None, b'nak'), (THREE, b'')])
 
     def test_multi_ack_nak(self):
         self.assertNextEquals(TWO)
@@ -783,8 +888,31 @@ class MultiAckDetailedGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertNextEquals(THREE)
         self.assertNoAck()
 
+        # Done is sent here.
+        self.assertNextEquals(None)
+        self.assertNextEmpty()
+        self.assertNak()
+        self.assertNextEmpty()
+        self.assertTrue(self._walker.pack_sent)
+
+    def test_multi_ack_nak_nodone(self):
+        self._walker.done_required = False
+        self.assertNextEquals(TWO)
+        self.assertNoAck()
+
+        self.assertNextEquals(ONE)
+        self.assertNoAck()
+
+        self.assertNextEquals(THREE)
+        self.assertNoAck()
+
+        # Done is sent here.
+        self.assertFalse(self._walker.pack_sent)
         self.assertNextEquals(None)
+        self.assertNextEmpty()
+        self.assertTrue(self._walker.pack_sent)
         self.assertNak()
+        self.assertNextEmpty()
 
     def test_multi_ack_nak_flush(self):
         # same as nak test but contains a flush-pkt in the middle
@@ -805,6 +933,7 @@ class MultiAckDetailedGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertNoAck()
 
         self.assertNextEquals(None)
+        self.assertNextEmpty()
         self.assertNak()
 
     def test_multi_ack_stateless(self):
@@ -821,9 +950,38 @@ class MultiAckDetailedGraphWalkerImplTestCase(AckGraphWalkerImplTestCase):
         self.assertNextEquals(THREE)
         self.assertNoAck()
 
+        self.assertFalse(self._walker.pack_sent)
+        self.assertNextEquals(None)
+        self.assertNak()
+
+        self.assertNextEmpty()
+        self.assertNoAck()
+        self.assertFalse(self._walker.pack_sent)
+
+    def test_multi_ack_stateless_nodone(self):
+        self._walker.done_required = False
+        # transmission ends with a flush-pkt
+        self._walker.lines[-1] = (None, None)
+        self._walker.http_req = True
+
+        self.assertNextEquals(TWO)
+        self.assertNoAck()
+
+        self.assertNextEquals(ONE)
+        self.assertNoAck()
+
+        self.assertNextEquals(THREE)
+        self.assertNoAck()
+
+        self.assertFalse(self._walker.pack_sent)
         self.assertNextEquals(None)
         self.assertNak()
 
+        self.assertNextEmpty()
+        self.assertNoAck()
+        # PACK will still not be sent.
+        self.assertFalse(self._walker.pack_sent)
+
 
 class FileSystemBackendTests(TestCase):
     """Tests for FileSystemBackend."""
@@ -831,6 +989,7 @@ class FileSystemBackendTests(TestCase):
     def setUp(self):
         super(FileSystemBackendTests, self).setUp()
         self.path = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.path)
         self.repo = Repo.init(self.path)
         self.backend = FileSystemBackend()
 
@@ -840,7 +999,9 @@ class FileSystemBackendTests(TestCase):
 
     def test_absolute(self):
         repo = self.backend.open_repository(self.path)
-        self.assertEqual(os.path.abspath(repo.path), os.path.abspath(self.repo.path))
+        self.assertEqual(
+            os.path.normcase(os.path.abspath(repo.path)),
+            os.path.normcase(os.path.abspath(self.repo.path)))
 
     def test_child(self):
         self.assertRaises(NotGitRepository,
@@ -901,6 +1062,7 @@ class UpdateServerInfoTests(TestCase):
     def setUp(self):
         super(UpdateServerInfoTests, self).setUp()
         self.path = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.path)
         self.repo = Repo.init(self.path)
 
     def test_empty(self):

dulwich/tests/utils.py

@@ -73,7 +73,8 @@ def open_repo(name, temp_dir=None):
         temporary directory will be created.
     :returns: An initialized Repo object that lives in a temporary directory.
     """
-    temp_dir = tempfile.mkdtemp()
+    if temp_dir is None:
+        temp_dir = tempfile.mkdtemp()
     repo_dir = os.path.join(os.path.dirname(__file__), 'data', 'repos', name)
     temp_repo_dir = os.path.join(temp_dir, name)
     shutil.copytree(repo_dir, temp_repo_dir, symlinks=True)
@@ -82,7 +83,8 @@ def open_repo(name, temp_dir=None):
 
 def tear_down_repo(repo):
     """Tear down a test repository."""
-    temp_dir = os.path.dirname(repo._path_bytes.rstrip(os.sep.encode(sys.getfilesystemencoding())))
+    repo.close()
+    temp_dir = os.path.dirname(repo.path.rstrip(os.sep))
     shutil.rmtree(temp_dir)
 
 

relicensing-apachev2.txt

@@ -0,0 +1,110 @@
+At the moment, Dulwich is licensed under the GNU General Public License,
+version 2 or later.
+
+We'd like to relicense Dulwich under the Apache v2 (or later) license, as
+the GPL is problematic for many free software Python projects that are under
+BSD-style licenses. See also https://github.com/jelmer/dulwich/issues/153
+
+For reference, a full copy of the Apachev2 license can be found here:
+https://www.apache.org/licenses/LICENSE-2.0
+
+New contributions to Dulwich should be dual licensed under the GNU GPLv2 (or
+later) and the Apachev2 (or later) licenses.
+
+Contributions made prior were contributed under the GPLv2 (or later) license
+alone. The following contributors have not (yet) relicensed their code under
+dual Apachev2/GPLv2:
+
+Aaron O'Mullan <aaron.omullan@friendco.de>
+Abderrahim Kitouni <a.kitouni@gmail.com>
+Adam "Cezar" Jenkins <emperorcezar@gmail.com>
+Alberto Ruiz <aruiz@gnome.org>
+Alexander Belchenko <bialix@ukr.net>
+Alex Holmes <alex@alex-holmes.com>
+Ali Sabil <ali.sabil@gmail.com>
+Andi McClure <andi.m.mcclure@gmail.com>
+André Roth <neolynx@gmail.com>
+Andres Lowrie <andres.lowrie@gmail.com>
+Artem Tikhomirov <artem.tikhomirov@syntevo.com>
+Augie Fackler <durin42@gmail.com> <raf@durin42.com>
+Benjamin Pollack <benjamin@bitquabit.com> <benjamin@fogcreek.com>
+Brendan Cully <brendan@kublai.com>
+Brian Visel <eode@eptitude.net>
+Bruce Duncan <Bruce.Duncan@ed.ac.uk>
+Bruno Renié <buburno@gmail.com>
+Chaiwat Suttipongsakul <cwt@bashell.com>
+Chow Loong Jin <hyperair@debian.org>
+Chris Eberle <eberle1080@gmail.com>
+Chris Reid <chris@reidsy.com>
+codingtony <tony.bussieres@gmail.com>
+dak180 <dak180@users.sourceforge.net>
+Damien Tournoud <damien@commerceguys.com>
+Dan Callaghan <dcallagh@redhat.com>
+Daniele Sluijters <daniele.sluijters@gmail.com>
+David Bennett <davbennett@google.com>
+David Blewett <davidb@sixfeetup.com>
+David Borowitz <dborowitz@google.com>
+David Carr <david@carrclan.us>
+David Keijser <david.keijser@klarna.com>
+David Ostrovsky <david@ostrovsky.org>
+David Pursehouse <david.pursehouse@gmail.com>
+DeeKey <dkomarov@gmail.com>
+Dirk <dirk@opani.com>
+diryboy <lancevdance@gmail.com>
+D-Key <dkomarov@gmail.com>
+Dmitrij D. Czarkoff <czarkoff@gmail.com>
+Dmitriy <dkomarov@gmail.com>
+Dov Feldstern <dovdevel@gmail.com>
+Fabien Boucher <fabien.boucher@enovance.com>
+Gary van der Merwé <garyvdm@gmail.com>
+Hal Wine <hal.wine@gmail.com>
+Hannu Valtonen <hannu.valtonen@ohmu.fi>
+Hans Kolek <hkolek@gmail.com>
+Hervé Cauwelier <herve@oursours.net> <herve@itaapy.com>
+Hwee Miin Koh <hwee-miin.koh@ubisoft.com>
+Jameson Nash <jameson@mit.edu>
+James Westby <jw+debian@jameswestby.net>
+Jason R. Coombs <jaraco@jaraco.com>
+John Arbash Meinel <john@arbash-meinel.com>
+John Carr <john.carr@unrouted.co.uk>
+Jonathan Chu <jchonphoenix@gmail.com>
+kwatters <kwatters@tagged.com>
+Lukasz Balcerzak <lukasz.balcerzak@python-center.org>
+Marc Brinkmann <git@marcbrinkmann.de>
+Marcin Kuźmiński <marcin@python-blog.com> <marcin@python-works.com>
+Martin Packman <gzlist@googlemail.com>
+Max Bowsher <maxb@f2s.com>
+max <max0d41@github.com>
+Max Shawabkeh <max99x@gmail.com>
+Michael K <michael-k@users.noreply.github.com>
+Mike Edgar <adgar@google.com>
+Mike Williams <miwilliams@google.com>
+Nick Stenning <nick@whiteink.com>
+Nick Ward <ward.nickjames@gmail.com>
+Nix <nix@esperi.co.uk>
+Pascal Quantin <pascal.quantin@gmail.com>
+Paul Chen <lancevdance@gmail.com>
+Paul Hummer <paul@eventuallyanyway.com>
+rfaulk <rfaulkner@wikimedia.org>
+Ricardo Salveti <ricardo.salveti@openbossa.org>
+Risto Kankkunen <risto.kankkunen@f-secure.com> <risto.kankkunen@iki.fi>
+Robert Brown <robert.brown@gmail.com>
+Rod Cloutier <rodcloutier@gmail.com>
+Roland Mas <lolando@debian.org>
+Ronald Blaschke <ron@rblasch.org>
+Ross Light <rlight2@gmail.com> <ross@zombiezen.com>
+Ryan Faulkner <rfaulk@yahoo-inc.com>
+Ryan McKern <ryan@orangefort.com>
+Sam Vilain <svilain@saymedia.com>
+Siddharth Agarwal <sid0@fb.com>
+Stefan Zimmermann <zimmermann.code@gmail.com>
+Takeshi Kanemoto <tak.kanemoto@gmail.com>
+Tay Ray Chuan <rctay89@gmail.com>
+Ted Horst <ted.horst@earthlink.net>
+Timo Schmid <info@bluec0re.eu>
+Tommy Yu <tommy.yu@auckland.ac.nz>
+Travis Cline <travis.cline@gmail.com>
+Víðir Valberg Guðmundsson <vidir.valberg@orn.li>
+William Grant <william.grant@canonical.com>
+Yifan Zhang <yifan@wavii.com>
+Yuval Langer <yuval.langer@gmail.com>

setup.py

@@ -97,6 +97,9 @@ setup(name='dulwich',
           'License :: OSI Approved :: GNU General Public License v2 or later (GPLv2+)',
           'Programming Language :: Python :: 2.6',
           'Programming Language :: Python :: 2.7',
+          'Programming Language :: Python :: 3.4',
+          'Programming Language :: Python :: Implementation :: CPython',
+          'Programming Language :: Python :: Implementation :: PyPy',
           'Operating System :: POSIX',
           'Topic :: Software Development :: Version Control',
       ],