Merge pull request #2231 from gitpython-developers/detached-head

Clarify detached HEAD access and isolate test branch state

Author: Byron

doc/source/tutorial.rst

@@ -78,6 +78,12 @@ Query relevant repository paths ...
 
 :class:`Heads <git.refs.head.Head>` Heads are branches in git-speak. :class:`References <git.refs.reference.Reference>` are pointers to a specific commit or to other references. Heads and :class:`Tags <git.refs.tag.TagReference>` are a kind of references. GitPython allows you to query them rather intuitively.
 
+To obtain the current commit ID, use ``repo.head.commit.hexsha``. This works both
+on a branch and with a detached HEAD, provided HEAD resolves to an existing commit.
+When ``repo.head.is_detached`` is true, HEAD points directly to a commit and there
+is no active branch: reading ``repo.head.reference`` or ``repo.active_branch``
+raises :exc:`TypeError`. The branch examples below assume an attached HEAD.
+
 .. literalinclude:: ../../test/test_docs.py
     :language: python
     :dedent: 8
@@ -152,7 +158,7 @@ Examining References
     :start-after: # [2-test_references_and_objects]
     :end-before: # ![2-test_references_and_objects]
 
-A :class:`symbolic reference <git.refs.symbolic.SymbolicReference>` is a special case of a reference as it points to another reference instead of a commit.
+A :class:`symbolic reference <git.refs.symbolic.SymbolicReference>` can point to another reference. When detached, it points directly to a commit instead. Reading its ``commit`` property resolves the commit in either state. Assigning a commit to ``reference`` detaches it; reading ``reference`` then raises :exc:`TypeError`.
 
 .. literalinclude:: ../../test/test_docs.py
     :language: python

git/refs/symbolic.py

@@ -59,12 +59,14 @@ def _git_dir(repo: "Repo", path: Union[PathLike, None]) -> PathLike:
 
 
 class SymbolicReference:
-    """Special case of a reference that is symbolic.
+    """A reference that can point to another reference or be detached.
 
-    This does not point to a specific commit, but to another
-    :class:`~git.refs.head.Head`, which itself specifies a commit.
+    An attached :class:`~git.refs.head.HEAD` usually points to a
+    :class:`~git.refs.head.Head`, which itself specifies a commit. A detached
+    :class:`~git.refs.head.HEAD` points directly to a commit instead.
 
-    A typical example for a symbolic reference is :class:`~git.refs.head.HEAD`.
+    Use :attr:`commit` to access the commit in either case, and :attr:`reference`
+    to access the target reference when attached.
     """
 
     __slots__ = ("repo", "path")
@@ -416,7 +418,15 @@ def set_object(
 
     @property
     def commit(self) -> "Commit":
-        """Query or set commits directly"""
+        """The commit this reference resolves to, whether detached or symbolic.
+
+        For example, ``repo.head.commit.hexsha`` returns the current commit ID
+        both on a branch and with a detached HEAD. HEAD must resolve to an
+        existing commit; an unborn branch in an empty repository has none.
+
+        Assigning updates the commit without changing whether this reference
+        is detached.
+        """
         return self._get_commit()
 
     @commit.setter
@@ -443,7 +453,10 @@ def _get_reference(self) -> "Reference":
         """
         sha, target_ref_path = self._get_ref_info(self.repo, self.path)
         if target_ref_path is None:
-            raise TypeError("%s is a detached symbolic reference as it points to %r" % (self, sha))
+            raise TypeError(
+                "%s is a detached symbolic reference as it points to %r. "
+                "Use .commit or .object to access the target directly." % (self, sha)
+            )
         return cast("Reference", self.from_path(self.repo, target_ref_path))
 
     def set_reference(
@@ -531,6 +544,18 @@ def set_reference(
     # Aliased reference
     @property
     def reference(self) -> "Reference":
+        """The reference we point to, available only when not detached.
+
+        Check :attr:`is_detached` before reading this property if a target
+        reference is required. To access the target commit or object in either
+        state, use :attr:`commit` or :attr:`object` instead.
+
+        Assigning a reference keeps this reference symbolic. Assigning a git
+        object or revision string detaches it; reading this property then raises.
+
+        :raise TypeError:
+            If this reference is detached when reading the property.
+        """
         return self._get_reference()
 
     @reference.setter

git/repo/base.py

@@ -1150,7 +1150,11 @@ def ignored(self, *paths: PathLike) -> List[str]:
 
     @property
     def active_branch(self) -> Head:
-        """The name of the currently active branch.
+        """The currently active branch.
+
+        Check ``repo.head.is_detached`` before accessing this property if HEAD
+        may be detached. To access the current commit in either state, use
+        ``repo.head.commit`` instead.
 
         :raise TypeError:
             If HEAD is detached.

test/lib/helper.py

@@ -139,7 +139,8 @@ def wrapper(self, *args, **kwargs):
 
 def with_rw_repo(working_tree_ref, bare=False):
     """Same as with_bare_repo, but clones the rorepo as non-bare repository, checking
-    out the working tree at the given working_tree_ref.
+    out the working tree at the given working_tree_ref with an attached HEAD,
+    regardless of the source repository's HEAD state.
 
     This repository type is more costly due to the working copy checkout.
 
@@ -158,7 +159,12 @@ def repo_creator(self):
             repo_dir = tempfile.mktemp(prefix="%sbare_%s" % (prefix, func.__name__))
             rw_repo = self.rorepo.clone(repo_dir, shared=True, bare=bare, n=True)
 
-            rw_repo.head.commit = rw_repo.commit(working_tree_ref)
+            if rw_repo.head.is_detached:
+                rw_repo.head.reference = rw_repo.create_head(
+                    "master", working_tree_ref, force=True, logmsg="Create test branch"
+                )
+            else:
+                rw_repo.head.commit = rw_repo.commit(working_tree_ref)
             if not bare:
                 rw_repo.head.reference.checkout()
             # END handle checkout
@@ -294,6 +300,7 @@ def remote_repo_creator(self):
             rw_repo_dir = tempfile.mktemp(prefix="daemon_cloned_repo-%s-" % func.__name__)
 
             rw_daemon_repo = self.rorepo.clone(rw_daemon_repo_dir, shared=True, bare=True)
+            rw_daemon_repo.head.reference = rw_daemon_repo.create_head("master", force=True)
             # Recursive alternates info?
             rw_repo = rw_daemon_repo.clone(rw_repo_dir, shared=True, bare=False, n=True)
             try: