worktrees: make non-packed refs also work correctly.

Turns out aec58a9 did the right thing for /packed/ refs, but didn't work
correctly on /unpacked/ refs.  So this patch gives unpacked refs the
same treatment.

Without the fix here, the test added will cause this traceback:

======================================================================
ERROR: Check that we find .git as a worktree file and find the worktree
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pjones/devel/github.com/GitPython/git/test/lib/helper.py", line 92, in wrapper
    return func(self, path)
  File "/home/pjones/devel/github.com/GitPython/git/test/test_repo.py", line 938, in test_git_work_tree_dotgit
    self.assertIsInstance(repo.heads['aaaaaaaa'], Head)
  File "/home/pjones/devel/github.com/GitPython/git/util.py", line 893, in __getitem__
    raise IndexError("No item found with id %r" % (self._prefix + index))
IndexError: No item found with id 'aaaaaaaa'

Woops.

Things I've learned:
- test_remote doesn't work currently if you start on a branch.  I think
  it never did?
- Because of 346424d, all *sorts* of stuff in the test suite doesn't
  work if you name your development branch "packed-refs"
  (This seems like a bug...)

Signed-off-by: Peter Jones <pjones@redhat.com>

Author: vathpela

git/refs/remote.py

@@ -36,6 +36,10 @@ def delete(cls, repo, *refs, **kwargs):
         # are generally ignored in the refs/ folder. We don't though
         # and delete remainders manually
         for ref in refs:
+            try:
+                os.remove(osp.join(repo.common_dir, ref.path))
+            except OSError:
+                pass
             try:
                 os.remove(osp.join(repo.git_dir, ref.path))
             except OSError:

git/refs/symbolic.py

@@ -26,6 +26,14 @@
 __all__ = ["SymbolicReference"]
 
 
+def _git_dir(repo, path):
+    """ Find the git dir that's appropriate for the path"""
+    name = "%s" % (path,)
+    if name in ['HEAD', 'ORIG_HEAD', 'FETCH_HEAD', 'index', 'logs']:
+        return repo.git_dir
+    return repo.common_dir
+
+
 class SymbolicReference(object):
 
     """Represents a special case of a reference such that this reference is symbolic.
@@ -71,16 +79,11 @@ def name(self):
 
     @property
     def abspath(self):
-        return join_path_native(self.repo.git_dir, self.path)
+        return join_path_native(_git_dir(self.repo, self.path), self.path)
 
     @classmethod
     def _get_packed_refs_path(cls, repo):
-        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')
+        return osp.join(repo.common_dir, 'packed-refs')
 
     @classmethod
     def _iter_packed_refs(cls, repo):
@@ -127,11 +130,12 @@ def dereference_recursive(cls, repo, ref_path):
         # END recursive dereferencing
 
     @classmethod
-    def _get_ref_info_helper(cls, repo, repodir, ref_path):
+    def _get_ref_info_helper(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"""
         tokens = None
+        repodir = _git_dir(repo, ref_path)
         try:
             with open(osp.join(repodir, ref_path), 'rt') as fp:
                 value = fp.read().rstrip()
@@ -169,16 +173,7 @@ 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)
+        return cls._get_ref_info_helper(repo, ref_path)
 
     def _get_object(self):
         """
@@ -433,7 +428,7 @@ def delete(cls, repo, path):
             or just "myreference", hence 'refs/' is implied.
             Alternatively the symbolic reference to be deleted"""
         full_ref_path = cls.to_full_path(path)
-        abs_path = osp.join(repo.git_dir, full_ref_path)
+        abs_path = osp.join(repo.common_dir, full_ref_path)
         if osp.exists(abs_path):
             os.remove(abs_path)
         else:
@@ -484,8 +479,9 @@ def _create(cls, repo, path, resolve, reference, force, logmsg=None):
         a proper symbolic reference. Otherwise it will be resolved to the
         corresponding object and a detached symbolic reference will be created
         instead"""
+        git_dir = _git_dir(repo, path)
         full_ref_path = cls.to_full_path(path)
-        abs_ref_path = osp.join(repo.git_dir, full_ref_path)
+        abs_ref_path = osp.join(git_dir, full_ref_path)
 
         # figure out target data
         target = reference
@@ -559,8 +555,8 @@ def rename(self, new_path, force=False):
         if self.path == new_path:
             return self
 
-        new_abs_path = osp.join(self.repo.git_dir, new_path)
-        cur_abs_path = osp.join(self.repo.git_dir, self.path)
+        new_abs_path = osp.join(_git_dir(self.repo, new_path), new_path)
+        cur_abs_path = osp.join(_git_dir(self.repo, self.path), self.path)
         if osp.isfile(new_abs_path):
             if not force:
                 # if they point to the same file, its not an error
@@ -594,7 +590,7 @@ def _iter_items(cls, repo, common_path=None):
 
         # walk loose refs
         # Currently we do not follow links
-        for root, dirs, files in os.walk(join_path_native(repo.git_dir, common_path)):
+        for root, dirs, files in os.walk(join_path_native(repo.common_dir, common_path)):
             if 'refs' not in root.split(os.sep):  # skip non-refs subfolders
                 refs_id = [d for d in dirs if d == 'refs']
                 if refs_id:
@@ -605,7 +601,7 @@ def _iter_items(cls, repo, common_path=None):
                 if f == 'packed-refs':
                     continue
                 abs_path = to_native_path_linux(join_path(root, f))
-                rela_paths.add(abs_path.replace(to_native_path_linux(repo.git_dir) + '/', ""))
+                rela_paths.add(abs_path.replace(to_native_path_linux(repo.common_dir) + '/', ""))
             # END for each file in root directory
         # END for each directory to walk
 

git/remote.py

@@ -652,7 +652,7 @@ def _get_fetch_info_from_stderr(self, proc, progress):
                     continue
 
         # read head information
-        with open(osp.join(self.repo.git_dir, 'FETCH_HEAD'), 'rb') as fp:
+        with open(osp.join(self.repo.common_dir, 'FETCH_HEAD'), 'rb') as fp:
             fetch_head_info = [l.decode(defenc) for l in fp.readlines()]
 
         l_fil = len(fetch_info_lines)

git/repo/base.py

@@ -74,6 +74,7 @@ class Repo(object):
     working_dir = None
     _working_tree_dir = None
     git_dir = None
+    _common_dir = None
 
     # precompiled regex
     re_whitespace = re.compile(r'\s+')
@@ -169,17 +170,23 @@ def __init__(self, path=None, odbt=DefaultDBType, search_parent_directories=Fals
             # lets not assume the option exists, although it should
             pass
 
+        try:
+            common_dir = open(osp.join(self.git_dir, 'commondir'), 'rt').readlines()[0].strip()
+            self._common_dir = osp.join(self.git_dir, common_dir)
+        except (OSError, IOError):
+            self._common_dir = None
+
         # adjust the wd in case we are actually bare - we didn't know that
         # in the first place
         if self._bare:
             self._working_tree_dir = None
         # END working dir handling
 
-        self.working_dir = self._working_tree_dir or self.git_dir
+        self.working_dir = self._working_tree_dir or self.common_dir
         self.git = self.GitCommandWrapperType(self.working_dir)
 
         # special handling, in special times
-        args = [osp.join(self.git_dir, 'objects')]
+        args = [osp.join(self.common_dir, 'objects')]
         if issubclass(odbt, GitCmdObjectDB):
             args.append(self.git)
         self.odb = odbt(*args)
@@ -236,6 +243,13 @@ def working_tree_dir(self):
         """
         return self._working_tree_dir
 
+    @property
+    def common_dir(self):
+        """:return: The git dir that holds everything except possibly HEAD,
+        FETCH_HEAD, ORIG_HEAD, COMMIT_EDITMSG, index, and logs/ .
+        """
+        return self._common_dir or self.git_dir
+
     @property
     def bare(self):
         """:return: True if the repository is bare"""
@@ -574,7 +588,7 @@ def _set_alternates(self, alts):
         :note:
             The method does not check for the existence of the paths in alts
             as the caller is responsible."""
-        alternates_path = osp.join(self.git_dir, 'objects', 'info', 'alternates')
+        alternates_path = osp.join(self.common_dir, 'objects', 'info', 'alternates')
         if not alts:
             if osp.isfile(alternates_path):
                 os.remove(alternates_path)
@@ -932,7 +946,7 @@ def clone(self, path, progress=None, **kwargs):
             * All remaining keyword arguments are given to the git-clone command
 
         :return: ``git.Repo`` (the newly cloned repo)"""
-        return self._clone(self.git, self.git_dir, path, type(self.odb), progress, **kwargs)
+        return self._clone(self.git, self.common_dir, path, type(self.odb), progress, **kwargs)
 
     @classmethod
     def clone_from(cls, url, to_path, progress=None, env=None, **kwargs):

git/test/test_repo.py

@@ -935,6 +935,8 @@ def test_git_work_tree_dotgit(self, rw_dir):
         commit = repo.head.commit
         self.assertIsInstance(commit, Object)
 
+        self.assertIsInstance(repo.heads['aaaaaaaa'], Head)
+
     @with_rw_directory
     def test_git_work_tree_env(self, rw_dir):
         """Check that we yield to GIT_WORK_TREE"""