Skip to content

Commit fb9fe55

Browse files
codexByron
authored andcommitted
Clarify detached HEAD access and isolate test branch state (#2230)
Reading head.reference intentionally returns another reference, so it raises TypeError when HEAD contains a commit ID directly. That contract is useful, but the public properties did not explain how to obtain the commit instead. The package tests also assumed the source checkout was attached, sometimes specifically to master. The seven tests reported in #2230 passed attached and failed after detaching at the same commit. Document head.commit.hexsha for both HEAD states, the reference setter/getter asymmetry, and active_branch's detached-HEAD restriction. Keep TypeError and the existing exception message prefix, appending a hint to use .commit or .object. The full diagnostic changes; return types and exception types do not. The extended API regression failed on the missing hint before this fix and verifies that commit access still works after detaching. Make writable test clones attach a branch when cloning leaves HEAD detached, while retaining clone-created branch/tracking configuration otherwise. Give the temporary bare remote its own master branch. Move branch-dependent assertions and tutorial setup to writable fixtures, make remote tests supply their refspecs, and establish tracking configuration before testing its removal. Compare @{1} against git rev-parse instead of assuming successive reflog entries contain different commits. Add a helper regression using a detached commit that no branch points to, covering both bare and working clones without reattaching the source. The Ubuntu/Python 3.14 CI job now also runs the full suite after detaching the source and after renaming master to another branch, leaving master absent. Git baseline: /Users/byron/dev/github.com/git/git at 1630431f326e15fcde608827b5ff38422528eb59, Documentation/git-symbolic-ref.adoc, documents exit status 1 when the requested name is not symbolic. Apple Git 2.50.1 confirmed that symbolic-ref -q HEAD exits 1 for detached HEAD while rev-parse HEAD still returns its commit ID. Validation on Python 3.14.7: - Full pytest suite using GIT_PYTHON_TEST_GIT_REPO_BASE with non-master and detached source clones, each without a local master: 760 passed, 81 skipped, 1 xfailed and 14 subtests passed per state. Six tests needing network or process access were run separately outside the sandbox and all passed. - Ruff check and format --check passed for the repository. - Sphinx HTML build succeeded; its 12 Python 3.14 annotation warnings are byte-for-byte identical to the unchanged baseline's warnings.
1 parent b62e91b commit fb9fe55

10 files changed

Lines changed: 122 additions & 40 deletions

File tree

.github/workflows/pythonpackage.yml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,15 @@ jobs:
170170
pytest --color=yes -p no:sugar --instafail -vv
171171
continue-on-error: false
172172

173+
- name: Test detached and non-master source checkouts
174+
if: matrix.os-type == 'ubuntu' && matrix.python-version == '3.14'
175+
run: |
176+
git checkout --detach HEAD
177+
pytest --color=yes -p no:sugar --instafail -q
178+
git branch -m master gitpython-test-checkout
179+
git checkout gitpython-test-checkout
180+
pytest --color=yes -p no:sugar --instafail -q
181+
173182
- name: Documentation
174183
if: matrix.build-docs
175184
run: |

doc/source/tutorial.rst

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ The first step is to create a :class:`git.Repo <git.repo.base.Repo>` object to r
2323
:start-after: # [1-test_init_repo_object]
2424
:end-before: # ![1-test_init_repo_object]
2525

26-
In the above example, the directory ``self.rorepo.working_tree_dir`` equals ``/Users/mtrier/Development/git-python`` and is my working repository which contains the ``.git`` directory. You can also initialize GitPython with a *bare* repository.
26+
In the above example, ``rw_repo.working_tree_dir`` is the path to a working repository which contains the ``.git`` directory. You can also initialize GitPython with a *bare* repository.
2727

2828
.. literalinclude:: ../../test/test_docs.py
2929
:language: python
@@ -78,6 +78,12 @@ Query relevant repository paths ...
7878

7979
: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.
8080

81+
To obtain the current commit ID, use ``repo.head.commit.hexsha``. This works both
82+
on a branch and with a detached HEAD, provided HEAD resolves to an existing commit.
83+
When ``repo.head.is_detached`` is true, HEAD points directly to a commit and there
84+
is no active branch: reading ``repo.head.reference`` or ``repo.active_branch``
85+
raises :exc:`TypeError`. The branch examples below assume an attached HEAD.
86+
8187
.. literalinclude:: ../../test/test_docs.py
8288
:language: python
8389
:dedent: 8
@@ -152,7 +158,7 @@ Examining References
152158
:start-after: # [2-test_references_and_objects]
153159
:end-before: # ![2-test_references_and_objects]
154160

155-
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.
161+
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`.
156162

157163
.. literalinclude:: ../../test/test_docs.py
158164
:language: python

git/refs/symbolic.py

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,14 @@ def _git_dir(repo: "Repo", path: Union[PathLike, None]) -> PathLike:
5959

6060

6161
class SymbolicReference:
62-
"""Special case of a reference that is symbolic.
62+
"""A reference that can point to another reference or be detached.
6363
64-
This does not point to a specific commit, but to another
65-
:class:`~git.refs.head.Head`, which itself specifies a commit.
64+
An attached :class:`~git.refs.head.HEAD` usually points to a
65+
:class:`~git.refs.head.Head`, which itself specifies a commit. A detached
66+
:class:`~git.refs.head.HEAD` points directly to a commit instead.
6667
67-
A typical example for a symbolic reference is :class:`~git.refs.head.HEAD`.
68+
Use :attr:`commit` to access the commit in either case, and :attr:`reference`
69+
to access the target reference when attached.
6870
"""
6971

7072
__slots__ = ("repo", "path")
@@ -416,7 +418,15 @@ def set_object(
416418

417419
@property
418420
def commit(self) -> "Commit":
419-
"""Query or set commits directly"""
421+
"""The commit this reference resolves to, whether detached or symbolic.
422+
423+
For example, ``repo.head.commit.hexsha`` returns the current commit ID
424+
both on a branch and with a detached HEAD. HEAD must resolve to an
425+
existing commit; an unborn branch in an empty repository has none.
426+
427+
Assigning updates the commit without changing whether this reference
428+
is detached.
429+
"""
420430
return self._get_commit()
421431

422432
@commit.setter
@@ -443,7 +453,10 @@ def _get_reference(self) -> "Reference":
443453
"""
444454
sha, target_ref_path = self._get_ref_info(self.repo, self.path)
445455
if target_ref_path is None:
446-
raise TypeError("%s is a detached symbolic reference as it points to %r" % (self, sha))
456+
raise TypeError(
457+
"%s is a detached symbolic reference as it points to %r. "
458+
"Use .commit or .object to access the target directly." % (self, sha)
459+
)
447460
return cast("Reference", self.from_path(self.repo, target_ref_path))
448461

449462
def set_reference(
@@ -531,6 +544,18 @@ def set_reference(
531544
# Aliased reference
532545
@property
533546
def reference(self) -> "Reference":
547+
"""The reference we point to, available only when not detached.
548+
549+
Check :attr:`is_detached` before reading this property if a target
550+
reference is required. To access the target commit or object in either
551+
state, use :attr:`commit` or :attr:`object` instead.
552+
553+
Assigning a reference keeps this reference symbolic. Assigning a git
554+
object or revision string detaches it; reading this property then raises.
555+
556+
:raise TypeError:
557+
If this reference is detached when reading the property.
558+
"""
534559
return self._get_reference()
535560

536561
@reference.setter

git/repo/base.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1150,7 +1150,11 @@ def ignored(self, *paths: PathLike) -> List[str]:
11501150

11511151
@property
11521152
def active_branch(self) -> Head:
1153-
"""The name of the currently active branch.
1153+
"""The currently active branch.
1154+
1155+
Check ``repo.head.is_detached`` before accessing this property if HEAD
1156+
may be detached. To access the current commit in either state, use
1157+
``repo.head.commit`` instead.
11541158
11551159
:raise TypeError:
11561160
If HEAD is detached.

test/lib/helper.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,8 @@ def wrapper(self, *args, **kwargs):
139139

140140
def with_rw_repo(working_tree_ref, bare=False):
141141
"""Same as with_bare_repo, but clones the rorepo as non-bare repository, checking
142-
out the working tree at the given working_tree_ref.
142+
out the working tree at the given working_tree_ref with an attached HEAD,
143+
regardless of the source repository's HEAD state.
143144
144145
This repository type is more costly due to the working copy checkout.
145146
@@ -158,7 +159,12 @@ def repo_creator(self):
158159
repo_dir = tempfile.mktemp(prefix="%sbare_%s" % (prefix, func.__name__))
159160
rw_repo = self.rorepo.clone(repo_dir, shared=True, bare=bare, n=True)
160161

161-
rw_repo.head.commit = rw_repo.commit(working_tree_ref)
162+
if rw_repo.head.is_detached:
163+
rw_repo.head.reference = rw_repo.create_head(
164+
"master", working_tree_ref, force=True, logmsg="Create test branch"
165+
)
166+
else:
167+
rw_repo.head.commit = rw_repo.commit(working_tree_ref)
162168
if not bare:
163169
rw_repo.head.reference.checkout()
164170
# END handle checkout
@@ -294,6 +300,7 @@ def remote_repo_creator(self):
294300
rw_repo_dir = tempfile.mktemp(prefix="daemon_cloned_repo-%s-" % func.__name__)
295301

296302
rw_daemon_repo = self.rorepo.clone(rw_daemon_repo_dir, shared=True, bare=True)
303+
rw_daemon_repo.head.reference = rw_daemon_repo.create_head("master", force=True)
297304
# Recursive alternates info?
298305
rw_repo = rw_daemon_repo.clone(rw_repo_dir, shared=True, bare=False, n=True)
299306
try:

test/test_base.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import os.path as osp
99
import sys
1010
import tempfile
11-
from unittest import skipIf
11+
from unittest import mock, skipIf
1212

1313
from git import Repo
1414
from git.objects import Blob, Commit, TagObject, Tree
@@ -92,7 +92,7 @@ def test_get_object_type_by_name(self):
9292

9393
def test_object_resolution(self):
9494
# Objects must be resolved to shas so they compare equal.
95-
self.assertEqual(self.rorepo.head.reference.object, self.rorepo.active_branch.object)
95+
self.assertEqual(self.rorepo.head.object, self.rorepo.commit("HEAD"))
9696

9797
@with_rw_repo("HEAD", bare=True)
9898
def test_with_bare_rw_repo(self, bare_rw_repo: Repo):
@@ -108,6 +108,22 @@ def test_with_rw_repo(self, rw_repo: Repo):
108108
assert osp.isdir(osp.join(rw_repo.working_tree_dir, "lib"))
109109
assert osp.isdir(rw_repo.working_dir)
110110

111+
@with_rw_repo("HEAD")
112+
def test_with_rw_repo_from_detached_source(self, source_repo):
113+
source_repo.active_branch.rename("test-source")
114+
commit = source_repo.head.commit.parents[0]
115+
source_repo.head.reference = commit
116+
117+
def check_repo(_self, repo):
118+
assert not repo.head.is_detached
119+
self.assertEqual(repo.head.commit, commit)
120+
assert repo.head.reference.log()
121+
122+
with mock.patch.object(self, "rorepo", source_repo):
123+
for bare in (False, True):
124+
with_rw_repo("HEAD", bare=bare)(check_repo)(self)
125+
assert source_repo.head.is_detached
126+
111127
@skipIf(HIDE_WINDOWS_FREEZE_ERRORS, "FIXME: Freezes! sometimes...")
112128
@with_rw_and_rw_remote_repo("0.1.6")
113129
def test_with_rw_remote_and_rw_repo(self, rw_repo, rw_remote_repo):

test/test_docs.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,16 @@ def tearDown(self):
2222
# @skipIf(HIDE_WINDOWS_KNOWN_ERRORS,
2323
# "FIXME: helper.wrapper fails with: PermissionError: [WinError 5] Access is denied: "
2424
# "'C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\test_work_tree_unsupportedryfa60di\\master_repo\\.git\\objects\\pack\\pack-bc9e0787aef9f69e1591ef38ea0a6f566ec66fe3.idx") # noqa: E501
25+
@with_rw_repo("HEAD")
2526
@with_rw_directory
26-
def test_init_repo_object(self, rw_dir):
27+
def test_init_repo_object(self, rw_dir, rw_repo):
2728
# [1-test_init_repo_object]
2829
from git import Repo
2930

30-
# rorepo is a Repo instance pointing to the git-python repository.
31+
# rw_repo is a Repo instance pointing to a clone of the git-python repository.
3132
# For all you know, the first argument to Repo is a path to the repository you
3233
# want to work with.
33-
repo = Repo(self.rorepo.working_tree_dir)
34+
repo = Repo(rw_repo.working_tree_dir)
3435
assert not repo.bare
3536
# ![1-test_init_repo_object]
3637

@@ -457,6 +458,7 @@ def test_references_and_objects(self, rw_dir):
457458
# To detach your head, you have to point to a commit directly.
458459
repo.head.reference = repo.commit("HEAD~5")
459460
assert repo.head.is_detached
461+
assert repo.head.commit.hexsha == repo.commit("HEAD").hexsha
460462
# Now our head points 15 commits into the past, whereas the working tree
461463
# and index are 10 commits in the past.
462464
# ![29-test_references_and_objects]

test/test_refs.py

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,6 @@ def test_set_tracking_branch_with_import(self, rwrepo):
249249
writer.set_value("include", "path", included_config)
250250

251251
for head in rwrepo.heads:
252-
head.set_tracking_branch(None)
253-
assert head.tracking_branch() is None
254252
remote_ref = rwrepo.remotes[0].refs[0]
255253
assert head.set_tracking_branch(remote_ref) is head
256254
assert head.tracking_branch() == remote_ref
@@ -263,18 +261,20 @@ def test_refs(self):
263261
types_found.add(type(ref))
264262
assert len(types_found) >= 3
265263

266-
def test_is_valid(self):
267-
assert not Reference(self.rorepo, "refs/doesnt/exist").is_valid()
268-
assert self.rorepo.head.is_valid()
269-
assert self.rorepo.head.reference.is_valid()
270-
assert not SymbolicReference(self.rorepo, "hellothere").is_valid()
264+
@with_rw_repo("HEAD")
265+
def test_is_valid(self, rw_repo):
266+
assert not Reference(rw_repo, "refs/doesnt/exist").is_valid()
267+
assert rw_repo.head.is_valid()
268+
assert rw_repo.head.reference.is_valid()
269+
assert not SymbolicReference(rw_repo, "hellothere").is_valid()
271270

272271
def test_orig_head(self):
273272
assert type(self.rorepo.head.orig_head()) is SymbolicReference
274273

275274
@with_rw_repo("0.1.6")
276275
def test_head_checkout_detached_head(self, rw_repo):
277-
res = rw_repo.remotes.origin.refs.HEAD.reference.checkout()
276+
remote_branch = next(ref for ref in rw_repo.remotes.origin.refs if ref.remote_head != "HEAD")
277+
res = remote_branch.checkout()
278278
assert isinstance(res, SymbolicReference)
279279
assert res.name == "HEAD"
280280

@@ -495,6 +495,7 @@ def test_head_reset(self, rw_repo):
495495
# We allow heads to point to any object.
496496
head.object = head_tree
497497
assert head.object == head_tree
498+
self.assertRaisesRegex(TypeError, r"\.object", getattr, head, "reference")
498499
# Cannot query tree as commit.
499500
self.assertRaises(TypeError, getattr, head, "commit")
500501

@@ -721,8 +722,9 @@ def test_dereference_recursive(self):
721722
# For now, just test the HEAD.
722723
assert SymbolicReference.dereference_recursive(self.rorepo, "HEAD")
723724

724-
def test_reflog(self):
725-
assert isinstance(self.rorepo.active_branch.log(), RefLog)
725+
@with_rw_repo("HEAD")
726+
def test_reflog(self, rw_repo):
727+
assert isinstance(rw_repo.active_branch.log(), RefLog)
726728

727729
def test_refs_outside_repo(self):
728730
# Create a file containing a valid reference outside the repository. Attempting

test/test_remote.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,7 +1022,7 @@ def test_push_unsafe_options_allowed(self, rw_repo):
10221022
# The options will be allowed, but the command will fail.
10231023
assert not tmp_file.exists()
10241024
with self.assertRaises(GitCommandError):
1025-
remote.push(**unsafe_option, allow_unsafe_options=True)
1025+
remote.push("HEAD:refs/heads/test-push", **unsafe_option, allow_unsafe_options=True)
10261026
assert tmp_file.exists()
10271027
tmp_file.unlink()
10281028

@@ -1096,8 +1096,8 @@ def test_timeout_funcs(self, repo):
10961096
for function in ["pull", "fetch"]: # Can't get push to time out.
10971097
f = getattr(repo.remotes.origin, function)
10981098
assert f is not None # Make sure these functions exist.
1099-
_ = f() # Make sure the function runs.
1099+
_ = f("HEAD") # Make sure the function runs without requiring an upstream branch.
11001100
with pytest.raises(GitCommandError, match="kill_after_timeout=0 s"):
1101-
f(kill_after_timeout=0)
1101+
f("HEAD", kill_after_timeout=0)
11021102

11031103
Git.AutoInterrupt._status_code_if_terminate = default

test/test_repo.py

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -322,15 +322,16 @@ def test_heads_should_return_array_of_head_objects(self):
322322
for head in self.rorepo.heads:
323323
self.assertEqual(Head, head.__class__)
324324

325-
def test_heads_should_populate_head_data(self):
326-
for head in self.rorepo.heads:
325+
@with_rw_repo("HEAD")
326+
def test_heads_should_populate_head_data(self, rw_repo):
327+
for head in rw_repo.heads:
327328
assert head.name
328329
self.assertIsInstance(head.commit, Commit)
329330
# END for each head
330331

331-
active_branch = self.rorepo.active_branch
332-
self.assertIsInstance(self.rorepo.heads[active_branch.name], Head)
333-
self.assertEqual(self.rorepo.heads[active_branch.name], active_branch)
332+
active_branch = rw_repo.active_branch
333+
self.assertIsInstance(rw_repo.heads[active_branch.name], Head)
334+
self.assertEqual(rw_repo.heads[active_branch.name], active_branch)
334335

335336
def test_tree_from_revision(self):
336337
tree = self.rorepo.tree("0.1.6")
@@ -634,8 +635,9 @@ def test_is_dirty_with_pathlib_and_pathlike(self, rwrepo):
634635
assert rwrepo.is_dirty(path=Path("doc")) is False
635636
assert rwrepo.is_dirty(path=PathLikeMock("doc")) is False
636637

637-
def test_head(self):
638-
self.assertEqual(self.rorepo.head.reference.object, self.rorepo.active_branch.object)
638+
@with_rw_repo("HEAD")
639+
def test_head(self, rw_repo):
640+
self.assertEqual(rw_repo.head.reference.object, rw_repo.active_branch.object)
639641

640642
def test_index(self):
641643
index = self.rorepo.index
@@ -1185,7 +1187,8 @@ def test_rev_parse(self):
11851187
except IndexError:
11861188
pass
11871189
else:
1188-
self.assertNotEqual(previous, head.commit)
1190+
# A checkout can record the same commit in consecutive reflog entries.
1191+
self.assertEqual(previous.hexsha, self.rorepo.git.rev_parse("@{1}"))
11891192

11901193
def test_repo_odbtype(self):
11911194
target_type = GitCmdObjectDB
@@ -1351,9 +1354,17 @@ def test_active_branch_raises_type_error_when_head_is_detached(self, rw_dir):
13511354
with open(osp.join(rw_dir, "a.txt"), "w") as f:
13521355
f.write("a")
13531356
repo.index.add(["a.txt"])
1354-
repo.index.commit("initial commit")
1355-
repo.git.checkout(repo.head.commit.hexsha)
1356-
self.assertRaisesRegex(TypeError, "detached symbolic reference", lambda: repo.active_branch)
1357+
commit = repo.index.commit("initial commit")
1358+
self.assertEqual(repo.head.reference.commit, commit)
1359+
self.assertEqual(repo.head.commit.hexsha, commit.hexsha)
1360+
1361+
repo.git.checkout(commit.hexsha)
1362+
assert repo.head.is_detached
1363+
self.assertEqual(repo.head.commit.hexsha, repo.git.rev_parse("HEAD"))
1364+
self.assertEqual(repo.commit().hexsha, commit.hexsha)
1365+
self.assertEqual(repo.commit("HEAD").hexsha, commit.hexsha)
1366+
self.assertRaisesRegex(TypeError, r"detached symbolic reference.*\.commit", lambda: repo.head.reference)
1367+
self.assertRaisesRegex(TypeError, r"detached symbolic reference.*\.commit", lambda: repo.active_branch)
13571368

13581369
def test_merge_base(self):
13591370
repo = self.rorepo

0 commit comments

Comments
 (0)