Repository: gitpython-developers/GitPython
Pull request #638 (2017-06-26 19:29:31)

Repo: handle worktrees better
This makes Repo("foo") work when foo/.git is a file of the form created
by "git worktree add", i.e. it's a text file that says:

gitdir: /home/me/project/.git/worktrees/bar

and where /home/me/project/.git/ is the nominal gitdir, but
/home/me/project/.git/worktrees/bar has this worktree's HEAD etc and a
"gitdir" file that contains the path of foo/.git .

Signed-off-by: Peter Jones <pjones@redhat.com>

codecov-io: # [Codecov](https://codecov.io/gh/gitpython-developers/GitPython/pull/638?src=pr&el=h1) Report
> Merging [#638](https://codecov.io/gh/gitpython-developers/GitPython/pull/638?src=pr&el=desc) into [master](https://codecov.io/gh/gitpython-developers/GitPython/commit/4bd708d41090fbe00acb41246eb22fa8b5632967?src=pr&el=desc) will **decrease** coverage by `0.04%`.
> The diff coverage is `46.15%`.

[![Impacted file tree graph](https://codecov.io/gh/gitpython-developers/GitPython/pull/638/graphs/tree.svg?width=650&height=150&src=pr&token=mTmKwYbezP)](https://codecov.io/gh/gitpython-developers/GitPython/pull/638?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #638      +/-   ##
==========================================
- Coverage   92.76%   92.71%   -0.05%     
==========================================
  Files          61       61              
  Lines        9910     9927      +17     
==========================================
+ Hits         9193     9204      +11     
- Misses        717      723       +6
```


| [Impacted Files](https://codecov.io/gh/gitpython-developers/GitPython/pull/638?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [git/test/test\_repo.py](https://codecov.io/gh/gitpython-developers/GitPython/pull/638?src=pr&el=tree#diff-Z2l0L3Rlc3QvdGVzdF9yZXBvLnB5) | `95.43% <0%> (+0.65%)` | :arrow_up: |
| [git/repo/base.py](https://codecov.io/gh/gitpython-developers/GitPython/pull/638?src=pr&el=tree#diff-Z2l0L3JlcG8vYmFzZS5weQ==) | `95.12% <100%> (+0.03%)` | :arrow_up: |
| [git/repo/fun.py](https://codecov.io/gh/gitpython-developers/GitPython/pull/638?src=pr&el=tree#diff-Z2l0L3JlcG8vZnVuLnB5) | `89.36% <37.5%> (-4.83%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/gitpython-developers/GitPython/pull/638?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/gitpython-developers/GitPython/pull/638?src=pr&el=footer). Last update [4bd708d...3133287](https://codecov.io/gh/gitpython-developers/GitPython/pull/638?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

vathpela: I'm really not sure what's going on in the AppVeyor test cases here.  It's not obvious to me that these are related to this change set, and locally on a Linux machine all tests with tox pass except all py34 ones, in which pip fails to install the test prerequisites correctly.  That's clearly just py34 not working, rather than being this patch.

I have no idea why test_git_submodules_and_add_sm_with_new_commit() and test_diff_with_staged_file() failures would be caused by this, and they do seem to be present in AppVeyor runs on the PRs before this one.  Staring at the code and the backtrace, my best guess is that polish_url() should be returning a backslash-version instead of slashes, and so the filesystem paths aren't working?  But I don't know how that all works in cygwin/windows, and I don't have a windows machine to reproduce locally.
Byron: Hi @vathpela , thanks a lot for your contribution, and for making it such a gorgeous one!

I can feel the pain that appveyor can cause, which is why I chose to go down the easy path and try to skip the test there, just to save time to be spent more productively :). Also it looks like Appveyor was breaking for a while, as you noted, which shouldn't have happened in the first place.

Once the CI's are happy, this one will be merged.
Thanks again

Patch:
diff --git a/AUTHORS b/AUTHORS
index 781695ba..ad7c452c 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -18,5 +18,6 @@ Contributors are:
 -Bernard `Guyzmo` Pratz <guyzmo+gitpython+pub@m0g.net>
 -Timothy B. Hartman <tbhartman _at_ gmail.com>
 -Konstantin Popov <konstantin.popov.89 _at_ yandex.ru>
+-Peter Jones <pjones _at_ redhat.com>
 
 Portions derived from other open source works and are clearly marked.
diff --git a/doc/source/changes.rst b/doc/source/changes.rst
index 4ef40f62..4aedf936 100644
--- a/doc/source/changes.rst
+++ b/doc/source/changes.rst
@@ -2,6 +2,11 @@
 Changelog
 =========
 
+2.1.6 - Bugfixes
+====================================
+
+* support for worktrees
+
 2.1.3 - Bugfixes
 ====================================
 
diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py
index 22b7c53e..90ecb62c 100644
--- a/git/refs/symbolic.py
+++ b/git/refs/symbolic.py
@@ -75,7 +75,12 @@ class SymbolicReference(object):
 
     @classmethod
     def _get_packed_refs_path(cls, repo):
-        return osp.join(repo.git_dir, 'packed-refs')
+        try:
+            commondir = open(osp.join(repo.git_dir, 'commondir'), 'rt').readlines()[0].strip()
+        except (OSError, IOError):
+            commondir = '.'
+        repodir = osp.join(repo.git_dir, commondir)
+        return osp.join(repodir, 'packed-refs')
 
     @classmethod
     def _iter_packed_refs(cls, repo):
@@ -122,13 +127,13 @@ class SymbolicReference(object):
         # END recursive dereferencing
 
     @classmethod
-    def _get_ref_info(cls, repo, ref_path):
+    def _get_ref_info_helper(cls, repo, repodir, ref_path):
         """Return: (str(sha), str(target_ref_path)) if available, the sha the file at
         rela_path points to, or None. target_ref_path is the reference we
         point to, or None"""
         tokens = None
         try:
-            with open(osp.join(repo.git_dir, ref_path), 'rt') as fp:
+            with open(osp.join(repodir, ref_path), 'rt') as fp:
                 value = fp.read().rstrip()
             # Don't only split on spaces, but on whitespace, which allows to parse lines like
             # 60b64ef992065e2600bfef6187a97f92398a9144                branch 'master' of git-server:/path/to/repo
@@ -159,6 +164,22 @@ class SymbolicReference(object):
 
         raise ValueError("Failed to parse reference information from %r" % ref_path)
 
+    @classmethod
+    def _get_ref_info(cls, repo, ref_path):
+        """Return: (str(sha), str(target_ref_path)) if available, the sha the file at
+        rela_path points to, or None. target_ref_path is the reference we
+        point to, or None"""
+        try:
+            return cls._get_ref_info_helper(repo, repo.git_dir, ref_path)
+        except ValueError:
+            try:
+                commondir = open(osp.join(repo.git_dir, 'commondir'), 'rt').readlines()[0].strip()
+            except (OSError, IOError):
+                commondir = '.'
+
+            repodir = osp.join(repo.git_dir, commondir)
+            return cls._get_ref_info_helper(repo, repodir, ref_path)
+
     def _get_object(self):
         """
         :return:
diff --git a/git/repo/base.py b/git/repo/base.py
index 2f67a341..28bb2a5d 100644
--- a/git/repo/base.py
+++ b/git/repo/base.py
@@ -32,7 +32,7 @@ from git.remote import Remote, add_progress, to_progress_instance
 from git.util import Actor, finalize_process, decygpath, hex_to_bin
 import os.path as osp
 
-from .fun import rev_parse, is_git_dir, find_submodule_git_dir, touch
+from .fun import rev_parse, is_git_dir, find_submodule_git_dir, touch, find_worktree_git_dir
 import gc
 import gitdb
 
@@ -138,10 +138,15 @@ class Repo(object):
                 self._working_tree_dir = os.getenv('GIT_WORK_TREE', os.path.dirname(self.git_dir))
                 break
 
-            sm_gitpath = find_submodule_git_dir(osp.join(curpath, '.git'))
+            dotgit = osp.join(curpath, '.git')
+            sm_gitpath = find_submodule_git_dir(dotgit)
             if sm_gitpath is not None:
                 self.git_dir = osp.normpath(sm_gitpath)
-            sm_gitpath = find_submodule_git_dir(osp.join(curpath, '.git'))
+
+            sm_gitpath = find_submodule_git_dir(dotgit)
+            if sm_gitpath is None:
+                sm_gitpath = find_worktree_git_dir(dotgit)
+
             if sm_gitpath is not None:
                 self.git_dir = _expand_path(sm_gitpath)
                 self._working_tree_dir = curpath
diff --git a/git/repo/fun.py b/git/repo/fun.py
index 39e55880..6aefd9d6 100644
--- a/git/repo/fun.py
+++ b/git/repo/fun.py
@@ -1,5 +1,6 @@
 """Package with general repository related functions"""
 import os
+import stat
 from string import digits
 
 from git.compat import xrange
@@ -17,7 +18,7 @@ from git.cmd import Git
 
 
 __all__ = ('rev_parse', 'is_git_dir', 'touch', 'find_submodule_git_dir', 'name_to_object', 'short_to_long', 'deref_tag',
-           'to_commit')
+           'to_commit', 'find_worktree_git_dir')
 
 
 def touch(filename):
@@ -47,6 +48,25 @@ def is_git_dir(d):
     return False
 
 
+def find_worktree_git_dir(dotgit):
+    """Search for a gitdir for this worktree."""
+    try:
+        statbuf = os.stat(dotgit)
+    except OSError:
+        return None
+    if not stat.S_ISREG(statbuf.st_mode):
+        return None
+
+    try:
+        lines = open(dotgit, 'r').readlines()
+        for key, value in [line.strip().split(': ') for line in lines]:
+            if key == 'gitdir':
+                return value
+    except ValueError:
+        pass
+    return None
+
+
 def find_submodule_git_dir(d):
     """Search for a submodule repo."""
     if is_git_dir(d):


Test patch:
diff --git a/git/test/test_fun.py b/git/test/test_fun.py
index b472fe19..5e32a1f9 100644
--- a/git/test/test_fun.py
+++ b/git/test/test_fun.py
@@ -1,10 +1,14 @@
 from io import BytesIO
 from stat import S_IFDIR, S_IFREG, S_IFLNK
+from os import stat
+import os.path as osp
+
 try:
-    from unittest import skipIf
+    from unittest import skipIf, SkipTest
 except ImportError:
-    from unittest2 import skipIf
+    from unittest2 import skipIf, SkipTest
 
+from git import Git
 from git.compat import PY3
 from git.index import IndexFile
 from git.index.fun import (
@@ -14,13 +18,18 @@ from git.objects.fun import (
     traverse_tree_recursive,
     traverse_trees_recursive,
     tree_to_stream,
-    tree_entries_from_data
+    tree_entries_from_data,
+)
+from git.repo.fun import (
+    find_worktree_git_dir
 )
 from git.test.lib import (
+    assert_true,
     TestBase,
-    with_rw_repo
+    with_rw_repo,
+    with_rw_directory
 )
-from git.util import bin_to_hex
+from git.util import bin_to_hex, cygpath, join_path_native
 from gitdb.base import IStream
 from gitdb.typ import str_tree_type
 
@@ -254,6 +263,29 @@ class TestFun(TestBase):
             assert entries
         # END for each commit
 
+    @with_rw_directory
+    def test_linked_worktree_traversal(self, rw_dir):
+        """Check that we can identify a linked worktree based on a .git file"""
+        git = Git(rw_dir)
+        if git.version_info[:3] < (2, 5, 1):
+            raise SkipTest("worktree feature unsupported")
+
+        rw_master = self.rorepo.clone(join_path_native(rw_dir, 'master_repo'))
+        branch = rw_master.create_head('aaaaaaaa')
+        worktree_path = join_path_native(rw_dir, 'worktree_repo')
+        if Git.is_cygwin():
+            worktree_path = cygpath(worktree_path)
+        rw_master.git.worktree('add', worktree_path, branch.name)
+
+        dotgit = osp.join(worktree_path, ".git")
+        statbuf = stat(dotgit)
+        assert_true(statbuf.st_mode & S_IFREG)
+
+        gitdir = find_worktree_git_dir(dotgit)
+        self.assertIsNotNone(gitdir)
+        statbuf = stat(gitdir)
+        assert_true(statbuf.st_mode & S_IFDIR)
+
     @skipIf(PY3, 'odd types returned ... maybe figure it out one day')
     def test_tree_entries_from_data_with_failing_name_decode_py2(self):
         r = tree_entries_from_data(b'100644 \x9f\0aaa')
diff --git a/git/test/test_repo.py b/git/test/test_repo.py
index 86019b73..a6be4e66 100644
--- a/git/test/test_repo.py
+++ b/git/test/test_repo.py
@@ -22,6 +22,7 @@ from git import (
     NoSuchPathError,
     Head,
     Commit,
+    Object,
     Tree,
     IndexFile,
     Git,
@@ -911,22 +912,28 @@ class TestRepo(TestBase):
             self.assertRaises(GitCommandError, repo.is_ancestor, i, j)
 
     @with_rw_directory
-    def test_work_tree_unsupported(self, rw_dir):
+    def test_git_work_tree_dotgit(self, rw_dir):
+        """Check that we find .git as a worktree file and find the worktree
+        based on it."""
         git = Git(rw_dir)
         if git.version_info[:3] < (2, 5, 1):
             raise SkipTest("worktree feature unsupported")
 
         rw_master = self.rorepo.clone(join_path_native(rw_dir, 'master_repo'))
-        rw_master.git.checkout('HEAD~10')
+        branch = rw_master.create_head('aaaaaaaa')
         worktree_path = join_path_native(rw_dir, 'worktree_repo')
         if Git.is_cygwin():
             worktree_path = cygpath(worktree_path)
-        try:
-            rw_master.git.worktree('add', worktree_path, 'master')
-        except Exception as ex:
-            raise AssertionError(ex, "It's ok if TC not running from `master`.")
+        rw_master.git.worktree('add', worktree_path, branch.name)
+
+        # this ensures that we can read the repo's gitdir correctly
+        repo = Repo(worktree_path)
+        self.assertIsInstance(repo, Repo)
 
-        self.failUnlessRaises(InvalidGitRepositoryError, Repo, worktree_path)
+        # this ensures we're able to actually read the refs in the tree, which
+        # means we can read commondir correctly.
+        commit = repo.head.commit
+        self.assertIsInstance(commit, Object)
 
     @with_rw_directory
     def test_git_work_tree_env(self, rw_dir):
diff --git a/git/test/test_submodule.py b/git/test/test_submodule.py
index 9e79a72c..e667ae17 100644
--- a/git/test/test_submodule.py
+++ b/git/test/test_submodule.py
@@ -663,12 +663,13 @@ class TestSubmodule(TestBase):
                                   url=empty_repo_dir, no_checkout=checkout_mode and True or False)
         # end for each checkout mode
 
-    @skipIf(HIDE_WINDOWS_KNOWN_ERRORS and Git.is_cygwin(),
-            """FIXME: ile "C:\\projects\\gitpython\\git\\cmd.py", line 671, in execute
+    @skipIf(HIDE_WINDOWS_KNOWN_ERRORS,
+            """FIXME on cygwin: File "C:\\projects\\gitpython\\git\\cmd.py", line 671, in execute
                 raise GitCommandError(command, status, stderr_value, stdout_value)
             GitCommandError: Cmd('git') failed due to: exit code(128)
               cmdline: git add 1__Xava verbXXten 1_test _myfile 1_test_other_file 1_XXava-----verbXXten
               stderr: 'fatal: pathspec '"1__çava verböten"' did not match any files'
+             FIXME on appveyor: see https://ci.appveyor.com/project/Byron/gitpython/build/1.0.185
                 """)
     @with_rw_directory
     def test_git_submodules_and_add_sm_with_new_commit(self, rwdir):
@@ -698,6 +699,9 @@ class TestSubmodule(TestBase):
 
         parent.index.commit("moved submodules")
 
+        with sm.config_writer() as writer:
+            writer.set_value('user.email', 'example@example.com')
+            writer.set_value('user.name', 'me')
         smm = sm.module()
         fp = osp.join(smm.working_tree_dir, 'empty-file')
         with open(fp, 'w'):