Some refactoring, simplifications.

dulwich/refs.py

@@ -405,7 +405,7 @@ class DiskRefsContainer(RefsContainer):
 
     def __init__(self, path, worktree_path=None):
         self.path = path
-        self.worktree_path = worktree_path or path;
+        self.worktree_path = worktree_path or path
         self._packed_refs = None
         self._peeled_refs = None
 
@@ -451,7 +451,7 @@ class DiskRefsContainer(RefsContainer):
             name = name.decode(sys.getfilesystemencoding())
         if os.path.sep != "/":
             name = name.replace("/", os.path.sep)
-        #TODO: as the 'HEAD' reference is working tree specific, it
+        # TODO: as the 'HEAD' reference is working tree specific, it
         # should actually not be a part of RefsContainer
         if name == 'HEAD':
             return os.path.join(self.worktree_path, name)

dulwich/repo.py

@@ -679,15 +679,15 @@ class Repo(BaseRepo):
             raise NotGitRepository(
                 "No git repository was found at %(path)s" % dict(path=root)
             )
-        commondir = self.get_named_file(COMMONDIR, mode='r')
-        if commondir:
-            with commondir:
-                self._commondir = os.path.join(self.controldir(), commondir.read().rstrip("\n"))
+        commondir = self.get_named_file(COMMONDIR)
+        if commondir is not None:
+            self._commondir = os.path.join(
+                self.controldir(), commondir.read().rstrip("\r\n"))
         else:
             self._commondir = self._controldir
         self.path = root
-        object_store = DiskObjectStore(os.path.join(self.commondir(),
-                                                    OBJECTDIR))
+        object_store = DiskObjectStore(
+            os.path.join(self.commondir(), OBJECTDIR))
         refs = DiskRefsContainer(self.commondir(), self._controldir)
         BaseRepo.__init__(self, object_store, refs)
 
@@ -751,7 +751,7 @@ class Repo(BaseRepo):
         with GitFile(os.path.join(self.controldir(), path), 'wb') as f:
             f.write(contents)
 
-    def get_named_file(self, path, **kwargs):
+    def get_named_file(self, path, basedir=None):
         """Get a file from the control dir with a specific name.
 
         Although the filename should be interpreted as a filename relative to
@@ -764,11 +764,11 @@ class Repo(BaseRepo):
         """
         # TODO(dborowitz): sanitize filenames, since this is used directly by
         # the dumb web serving code.
-        basedir_ = kwargs.get('basedir', self.controldir())
-        mode = kwargs.get('mode', 'rb')
+        if basedir is None:
+            basedir = self.controldir()
         path = path.lstrip(os.path.sep)
         try:
-            return open(os.path.join(basedir_, path), mode)
+            return open(os.path.join(basedir, path), 'rb')
         except (IOError, OSError) as e:
             if e.errno == errno.ENOENT:
                 return None
@@ -958,24 +958,26 @@ class Repo(BaseRepo):
         return cls(path)
 
     @classmethod
-    def init_new_working_directory(cls, path, main_path, mkdir=False):
+    def _init_new_working_directory(cls, path, main_repo, identifier=None, mkdir=False):
         """Create a new working directory linked to a repository.
 
         :param path: Path in which to create the working tree.
-        :param main_path: Path to the main repository (that can be bare or not).
+        :param main_repo: Main repository to reference
+        :param identifier: Worktree identifier
         :param mkdir: Whether to create the directory
         :return: `Repo` instance
         """
         if mkdir:
             os.mkdir(path)
-        worktree_id = os.path.basename(path)
+        if identifier is None:
+            identifier = os.path.basename(path)
+        main_worktreesdir = os.path.join(main_repo.controldir(), WORKTREES)
+        worktree_controldir = os.path.join(main_worktreesdir, identifier)
         gitdirfile = os.path.join(path, CONTROLDIR)
-        main = Repo(main_path)
-        main_controldir = main.controldir()
-        main_worktreesdir = os.path.join(main_controldir, WORKTREES)
-        worktree_controldir = os.path.join(main_worktreesdir, worktree_id)
-        with open(gitdirfile, 'w') as f:
-            f.write('gitdir: {}\n'.format(worktree_controldir))
+        with open(gitdirfile, 'wb') as f:
+            f.write('gitdir: ' +
+                    worktree_controldir.encode(sys.getfilesystemencoding()) +
+                    b'\n')
         try:
             os.mkdir(main_worktreesdir)
         except OSError as e:
@@ -986,20 +988,14 @@ class Repo(BaseRepo):
         except OSError as e:
             if e.errno != errno.EEXIST:
                 raise
-        with open(os.path.join(worktree_controldir, GITDIR), 'w') as f:
-            f.write(gitdirfile + '\n')
-        with open(os.path.join(worktree_controldir, COMMONDIR), 'w') as f:
-            f.write('../..\n')
-        with open(os.path.join(worktree_controldir, 'HEAD'), 'w') as f:
-            f.write('{}\n'.format(main.refs[b'HEAD'].decode('ascii')))
-        main.close()
+        with open(os.path.join(worktree_controldir, GITDIR), 'wb') as f:
+            f.write(gitdirfile + b'\n')
+        with open(os.path.join(worktree_controldir, COMMONDIR), 'wb') as f:
+            f.write(b'../..\n')
+        with open(os.path.join(worktree_controldir, 'HEAD'), 'wb') as f:
+            f.write(main_repo.head() + b'\n')
         r = cls(path)
-        from dulwich.index import (
-            build_index_from_tree,
-            )
-        indexfile = r.index_path()
-        tree = r[b'HEAD'].tree
-        build_index_from_tree(r.path, indexfile, r.object_store, tree)
+        r.reset_index()
         return r
 
     @classmethod

dulwich/tests/compat/test_client.py

@@ -62,13 +62,11 @@ from dulwich.tests import (
     SkipTest,
     expectedFailure,
     )
-from dulwich.tests.utils import (
-    rmtree_ro,
-)
 from dulwich.tests.compat.utils import (
     CompatTestCase,
     check_for_daemon,
     import_repo_to_dir,
+    rmtree_ro,
     run_git_or_fail,
     _DEFAULT_GIT,
     )

dulwich/tests/compat/test_repository.py

@@ -23,7 +23,8 @@
 
 from io import BytesIO
 from itertools import chain
-import os, tempfile
+import os
+import tempfile
 
 from dulwich.objects import (
     hex_to_sha,
@@ -32,15 +33,11 @@ from dulwich.repo import (
     check_ref_format,
     Repo,
     )
-from dulwich.tests.utils import (
-    rmtree_ro,
-    create_empty_commit,
-)
 from dulwich.tests.compat.utils import (
+    rmtree_ro,
     run_git_or_fail,
     CompatTestCase,
-    git_version
-)
+    )
 
 
 class ObjectStoreTestCase(CompatTestCase):
@@ -50,9 +47,6 @@ class ObjectStoreTestCase(CompatTestCase):
         super(ObjectStoreTestCase, self).setUp()
         self._repo = self.import_repo('server_new.export')
 
-    def repo_path(self):
-        return self._repo.path
-
     def _run_git(self, args):
         return run_git_or_fail(args, cwd=self._repo.path)
 
@@ -141,9 +135,22 @@ class WorkingTreeTestCase(ObjectStoreTestCase):
 
     min_git_version = (2, 5, 0)
 
+    def create_new_worktree(self, repo_dir, branch):
+        """Create a new worktree using git-worktree.
+
+        :param repo_dir: The directory of the main working tree.
+        :param branch: The branch or commit to checkout in the new worktree.
+
+        :returns: The path to the new working tree.
+        """
+        temp_dir = tempfile.mkdtemp()
+        run_git_or_fail(['worktree', 'add', temp_dir, branch],
+                        cwd=repo_dir)
+        return temp_dir
+
     def setUp(self):
         super(WorkingTreeTestCase, self).setUp()
-        self._worktree_path = self.create_new_worktree(self.repo_path(), 'branch')
+        self._worktree_path = self.create_new_worktree(self._repo.path, 'branch')
         self._worktree_repo = Repo(self._worktree_path)
         self._mainworktree_repo = self._repo
         self._number_of_working_tree = 2
@@ -174,13 +181,14 @@ class WorkingTreeTestCase(ObjectStoreTestCase):
         self.assertEqual(len(worktrees), self._number_of_working_tree)
         self.assertEqual(worktrees[0][1], '(bare)')
         self.assertEqual(worktrees[0][0], self._mainworktree_repo.path)
-        
+
         output = run_git_or_fail(['worktree', 'list'], cwd=self._mainworktree_repo.path)
         worktrees = self._parse_worktree_list(output)
         self.assertEqual(len(worktrees), self._number_of_working_tree)
         self.assertEqual(worktrees[0][1], '(bare)')
         self.assertEqual(worktrees[0][0], self._mainworktree_repo.path)
 
+
 class InitNewWorkingDirectoryTestCase(WorkingTreeTestCase):
     """Test compatibility of Repo.init_new_working_directory."""
 
@@ -189,14 +197,13 @@ class InitNewWorkingDirectoryTestCase(WorkingTreeTestCase):
     def setUp(self):
         super(InitNewWorkingDirectoryTestCase, self).setUp()
         self._other_worktree = self._repo
-        self._repo = Repo.init_new_working_directory(tempfile.mkdtemp(),
-                                                     self._mainworktree_repo.path)
+        self._repo = Repo._init_new_working_directory(
+            tempfile.mkdtemp(), self._mainworktree_repo)
         self._number_of_working_tree = 3
-        
+
     def tearDown(self):
         self._repo.close()
         rmtree_ro(self._repo.path)
-        self._repo = self._other_worktree
         super(InitNewWorkingDirectoryTestCase, self).tearDown()
 
     def test_head_equality(self):

dulwich/tests/compat/utils.py

@@ -21,20 +21,19 @@
 """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
 
 from dulwich.repo import Repo
 from dulwich.protocol import TCP_GIT_PORT
 
-from dulwich.tests.utils import (
-    rmtree_ro,
-)
 from dulwich.tests import (
     SkipTest,
     TestCase,
@@ -244,23 +243,12 @@ class CompatTestCase(TestCase):
         self.addCleanup(cleanup)
         return repo
 
-    def create_new_worktree(self, repo_dir, branch):
-        """Create a new worktree using git-worktree.
 
-        :param repo_dir: The directory of the main working tree.
-        :param branch: The branch or commit to checkout in the new worktree.
-
-        :returns: The path to the new working tree.
-        """
-        temp_dir = tempfile.mkdtemp()
-        run_git_or_fail(['worktree', 'add', temp_dir, branch],
-                        cwd=repo_dir)
-        return temp_dir
-
-    def debug_repo(self, s, repo):
-        print('{}: {}'.format(s, repo))
-        print('controldir: {}'.format(repo.controldir()))
-        print(' commondir: {}'.format(repo.commondir()))
-        for r in repo.get_refs().keys():
-            print('{}: {}'.format(r, repo.refs[r]))
+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/a.git/worktrees/b/HEAD

@@ -1 +0,0 @@
-2a72d929692c41d8554c07f6301757ba18a65d91

dulwich/tests/data/repos/a.git/worktrees/b/commondir

@@ -1 +0,0 @@
-../..

dulwich/tests/data/repos/a.git/worktrees/b/gitdir

@@ -1 +0,0 @@
-../../../b

dulwich/tests/test_repository.py

@@ -47,11 +47,8 @@ from dulwich.tests import (
     )
 from dulwich.tests.utils import (
     open_repo,
-    rmtree_ro,
     tear_down_repo,
     setup_warning_catcher,
-    create_empty_commit,
-    create_repo_and_worktree,
     )
 
 missing_sha = b'b91fa4d900e17e99b433218e988c4eb4a3e9a097'
@@ -525,14 +522,31 @@ exit 1
             check(bare)
 
     def test_working_tree(self):
-        (r, w) = create_repo_and_worktree()
-        self.addCleanup(rmtree_ro, r.path)
-        self.addCleanup(rmtree_ro, w.path)
+        temp_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, temp_dir)
+        worktree_temp_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, worktree_temp_dir)
+        r = Repo.init(temp_dir)
+        root_sha = r.do_commit(
+                b'empty commit',
+                committer=b'Test Committer <test@nodomain.com>',
+                author=b'Test Author <test@nodomain.com>',
+                commit_timestamp=12345, commit_timezone=0,
+                author_timestamp=12345, author_timezone=0)
+        r.refs[b'refs/heads/master'] = root_sha
+        w = Repo._init_new_working_directory(worktree_temp_dir, r)
+        new_sha = w.do_commit(
+                b'new commit',
+                committer=b'Test Committer <test@nodomain.com>',
+                author=b'Test Author <test@nodomain.com>',
+                commit_timestamp=12345, commit_timezone=0,
+                author_timestamp=12345, author_timezone=0)
+        w.refs[b'HEAD'] = new_sha
         self.assertEqual(os.path.abspath(r.controldir()),
                          os.path.abspath(w.commondir()))
         self.assertEqual(r.refs.keys(), w.refs.keys())
         self.assertNotEqual(r.head(), w.head())
-        self.assertEqual(w.get_parents(w.head()), [r.head()])
+
 
 class BuildRepoRootTests(TestCase):
     """Tests that build on-disk repos from scratch.

dulwich/tests/utils.py

@@ -22,10 +22,8 @@
 
 
 import datetime
-import functools
 import os
 import shutil
-import sys
 import tempfile
 import time
 import types
@@ -362,32 +360,3 @@ def setup_warning_catcher():
         warnings.showwarning = original_showwarning
 
     return caught_warnings, restore_showwarning
-
-def create_empty_commit(r):
-    return r.do_commit(
-        b'empty commit',
-        committer=b'Test Committer <test@nodomain.com>',
-        author=b'Test Author <test@nodomain.com>',
-        commit_timestamp=12345, commit_timezone=0,
-        author_timestamp=12345, author_timezone=0)
-
-def create_repo_and_worktree():
-    temp_dir = tempfile.mkdtemp()
-    worktree_temp_dir = tempfile.mkdtemp()
-    r = Repo.init(temp_dir)
-    root_sha = create_empty_commit(r)
-    second_sha = create_empty_commit(r)
-    r.refs[b'refs/heads/master'] = second_sha
-    w = Repo.init_new_working_directory(worktree_temp_dir, temp_dir)
-    new_sha = create_empty_commit(w)
-    w.refs[b'refs/heads/branch'] = new_sha
-    return (r, w)
-
-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