Skip to content

Commit d03d3d7

Browse files
committed
fix: read reference arguments by exact name in revision APIs
<!-- agent --> `Reference.__str__` returns the short name, so `refs/heads/main` becomes `main` and `refs/remotes/origin/main` becomes `origin/main`. The revision APIs turned reference objects into revisions with `str(ref)` or `Git._check_operand(ref, "revision")`, and Git's revision lookup (gitrevisions(7)) tries `refs/tags/<name>` before `refs/heads/<name>` and `refs/remotes/<name>`, only warning about the ambiguity on stderr. A tag that shares a branch's short name therefore took the branch's place. Whoever controls a remote can supply such tags: `git fetch` auto-follows tags that point into fetched history, and a clone brings in every tag. With a branch `main` and fetched tags `main` and `origin/main` on another commit, `repo.heads.main.commit` still reported the branch, but these APIs used the tag's commit on both backends: - `Repo.commit()`, `Repo.tree()`, `Object.new()`, `Repo.iter_commits()` - `Repo.merge_base()`, `Repo.is_ancestor()`, `Diffable.diff()` - `Repo.blame()`, `Repo.blame_incremental()`, `Repo.archive()` - `IndexFile.from_tree()`, `IndexFile.merge_tree()`, `IndexFile.reset()` - `HEAD.reset()`, `Repo.create_head()`, `Repo.create_tag()` In the documented merge workflow, `merge_base(new_branch, original_branch)` followed by `index.merge_tree(original_branch, base=...)`, the merge commit received the tag's tree while its parents named the branch commits. `repo.head.reset(repo.heads.main, working_tree=True)` and `repo.index.reset(ref, head=True)` moved the current branch to the tag's commit and wrote its files into the working tree. Add `Git._check_revision()`. For a `SymbolicReference` it reads the exact reference through `dereference_recursive()`, the `show-ref`/Gix read behind `ref.object`, and passes on the object ID, which is how `SymbolicReference.set_object()` already resolves reference arguments. Strings keep Git's name guessing, as the changelog already documents. `merge_base()` resolves its top-level arguments before `_unpack_args()` flattens them. A reference that does not exist now raises `ValueError`, as `ref.object` does, instead of resolving to whatever Git guesses for its short name. On the CLI backend each reference argument costs one or two extra short Git calls; the Gix backend reads references natively, and `doc/gix-backend.md` is unchanged. Validation: the new `shadowed` case in `tests/fixtures/exact-refs.sh` backs `test_reference_arguments_do_not_resolve_same_named_tags` in `test/test_refs.py`, which checks the APIs above for a branch and a remote-tracking branch. It fails before the change and passes after it with `GIT_PYTHON_BACKEND=cli` and `GIT_PYTHON_BACKEND=gix` for SHA-1/SHA-256 and files/reftable (Git 2.52.0, GixPython 0.3.0, Python 3.11, macOS). Full suites with the change: CLI 1,915 passed and Gix 2,018 passed. Each run also had 116 failures, and the same 116 tests fail on the unpatched tree; they need history and tags such as `0.1.5`, which this shallow clone lacks. `ruff check`, `ruff format --check`, ShellCheck, `mypy --python-version=3.11` and `basedpyright --warnings` pass. Agent-Model: claude-opus-5-5
1 parent 9689fb2 commit d03d3d7

12 files changed

Lines changed: 91 additions & 19 deletions

File tree

‎doc/source/changes.rst‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,12 @@ API changes
5757
* Reference reads require the exact name. A missing branch no longer resolves to
5858
a similarly named tag, so an unborn branch's initial commit has no parent.
5959
A tag named ``REBASE_HEAD`` no longer reports a rebase in progress. Revision
60-
APIs such as ``Repo.commit(rev)`` retain Git's name guessing.
60+
APIs such as ``Repo.commit(rev)`` retain Git's name guessing for strings.
61+
A reference object passed as a revision, for example to ``Repo.commit()``,
62+
``Repo.iter_commits()``, ``Repo.merge_base()``, ``IndexFile.from_tree()`` or
63+
``HEAD.reset()``, is read by its exact name, so a tag named like a branch or
64+
remote-tracking branch no longer takes its place. A missing reference raises
65+
``ValueError``, as its ``object`` property does.
6166
* Packed tag names containing non-UTF-8 bytes are represented losslessly as
6267
surrogate-escaped strings. ``safe_encode()`` recovers their original bytes,
6368
and Gix reference validation and resolution receive those bytes directly.

‎git/cmd.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,6 +1023,21 @@ def _check_operand(value: Any, label: str = "operand") -> str:
10231023
raise UnsafeOptionError(f"Invalid {label}: {value!r}")
10241024
return value
10251025

1026+
@staticmethod
1027+
def _check_revision(value: Any, label: str = "revision") -> str:
1028+
"""Validate a revision, passing a reference object as the object ID it holds.
1029+
1030+
A reference converts to its short name, for which Git's revision lookup tries
1031+
other references, such as a same-named tag, before ``refs/heads/`` and
1032+
``refs/remotes/``. Read the exact reference instead, as
1033+
``SymbolicReference.object`` does.
1034+
"""
1035+
from git.refs.symbolic import SymbolicReference
1036+
1037+
if isinstance(value, SymbolicReference):
1038+
value = value.dereference_recursive(value.repo, value.path)
1039+
return Git._check_operand(value, label)
1040+
10261041
def _require_version(self) -> None:
10271042
key = None
10281043
if self._version_info_token is not self._refresh_token:

‎git/diff.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,8 +261,10 @@ def diff(
261261
clusterable_short_options="46abceflmnpqrstuvwzBCDMNRW",
262262
)
263263

264-
if isinstance(other, str):
265-
other = self.repo.rev_parse(Git._check_operand(other, "revision")).hexsha
264+
from git.refs.symbolic import SymbolicReference
265+
266+
if isinstance(other, (str, SymbolicReference)):
267+
other = self.repo.rev_parse(Git._check_revision(other)).hexsha
266268
native = _backend.tree_diff(self.repo, self, other, paths, create_patch, kwargs)
267269
if native is not NotImplemented:
268270
return native

‎git/index/base.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -280,8 +280,7 @@ def _tree_oid(self, tree: Treeish) -> str:
280280
tree = tree[0]
281281
if isinstance(tree, bytes):
282282
tree = tree.hex() if len(tree) == self.repo._oid_size else tree.decode("ascii")
283-
value = str(tree)
284-
Git._check_operand(value, "tree revision")
283+
value = Git._check_revision(tree, "tree revision")
285284
return self.repo.git._call_process_safe("rev_parse", "--verify", "--end-of-options", value + "^{tree}")
286285

287286
def _index_path(self) -> PathLike:

‎git/objects/base.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import gitdb.typ as dbtyp
1111

12+
from git.cmd import Git
1213
from git.compat import force_text
1314
from git.exc import WorkTreeRepositoryUnsupported
1415
from git.util import LazyMixin, bin_to_hex, join_path_native, stream_copy
@@ -138,7 +139,7 @@ def new(cls, repo: "Repo", id: Union[str, "Reference"]) -> AnyGitObject:
138139
This cannot be a ``__new__`` method as it would always call :meth:`__init__`
139140
with the input id which is not necessarily a binsha.
140141
"""
141-
return repo.rev_parse(str(id))
142+
return repo.rev_parse(Git._check_revision(id))
142143

143144
@classmethod
144145
def new_from_sha(cls, repo: "Repo", sha1: bytes) -> AnyGitObject:

‎git/objects/commit.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,8 +465,8 @@ def iter_items(
465465
args_list.extend(paths_tup)
466466
# END if paths
467467

468-
Git._check_operand(str(rev), "revision")
469-
native = _backend.history(repo.git, str(rev), paths, kwargs)
468+
rev = Git._check_revision(rev)
469+
native = _backend.history(repo.git, rev, paths, kwargs)
470470
if native is not NotImplemented:
471471
return (cls(repo, hex_to_bin(oid)) for oid in native)
472472
proc = repo.git._call_process_safe("rev_list", rev, args_list, as_process=True, **kwargs)

‎git/refs/head.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ def reset(
121121
# END working tree handling
122122

123123
try:
124-
oid = self.repo.rev_parse(Git._check_operand(commit, "revision")).hexsha
124+
oid = self.repo.rev_parse(Git._check_revision(commit)).hexsha
125125
self.repo.git._call_process_safe("reset", mode, oid, "--", paths, **kwargs)
126126
except GitCommandError as e:
127127
# git nowadays may use 1 as status to indicate there are still unstaged

‎git/refs/symbolic.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ def _create(
575575
) -> T_References:
576576
full_path = cls.to_full_path(path)
577577
cls._get_validated_ref_path(repo, full_path)
578-
target = repo.rev_parse(str(reference)) if resolve else reference
578+
target = repo.rev_parse(Git._check_revision(reference)) if resolve else reference
579579
ref = cls(repo, full_path)
580580
if force:
581581
ref.set_reference(target, logmsg)

‎git/refs/tag.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ def create(
170170

171171
name = Git._check_operand(path, "tag")
172172
cls._get_validated_ref_path(repo, cls.to_full_path(name))
173-
reference = Git._check_operand(reference, "revision")
173+
reference = Git._check_revision(reference)
174174
oid = repo.rev_parse(reference).hexsha
175175
options = [] if allow_unsafe_options else ["--no-sign"]
176176
repo.git._call_process_safe("tag", *options, "--", name, oid, **kwargs)

‎git/repo/base.py‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -909,7 +909,7 @@ def commit(self, rev: Union[str, Commit_ish, None] = None) -> Commit:
909909
"""
910910
if rev is None:
911911
return self.head.commit
912-
return to_commit(self.rev_parse(str(rev)))
912+
return to_commit(self.rev_parse(Git._check_revision(rev)))
913913

914914
def iter_trees(self, *args: Any, **kwargs: Any) -> Iterator["Tree"]:
915915
""":return: Iterator yielding :class:`~git.objects.tree.Tree` objects
@@ -939,7 +939,7 @@ def tree(self, rev: Union[Tree_ish, str, None] = None) -> "Tree":
939939
"""
940940
if rev is None:
941941
return self.head.commit.tree
942-
obj = self.rev_parse(str(rev))
942+
obj = self.rev_parse(Git._check_revision(rev))
943943
if obj.type == "tree":
944944
obj.path = getattr(obj, "path", "")
945945
return obj
@@ -1029,10 +1029,12 @@ def merge_base(self, *rev: TBD, allow_unsafe_options: bool = False, **kwargs: An
10291029
options=Git._option_candidates(rev, kwargs), unsafe_options=self.unsafe_git_revision_options
10301030
)
10311031

1032+
# Resolve reference objects before flattening turns them into short names.
1033+
revs = [item if isinstance(item, (list, tuple)) else Git._check_revision(item) for item in rev]
10321034
res: List[Commit] = []
10331035
try:
10341036
lines: List[str] = self.git._call_process_safe(
1035-
"merge_base", "--", *(Git._check_operand(item, "revision") for item in Git._unpack_args(rev)), **kwargs
1037+
"merge_base", "--", *(Git._check_operand(item, "revision") for item in Git._unpack_args(revs)), **kwargs
10361038
).splitlines()
10371039
except GitCommandError as err:
10381040
if err.status != 1:
@@ -1065,8 +1067,8 @@ def is_ancestor(self, ancestor_rev: Commit, rev: Commit) -> bool:
10651067
"merge_base",
10661068
"--is-ancestor",
10671069
"--",
1068-
Git._check_operand(ancestor_rev, "revision"),
1069-
Git._check_operand(rev, "revision"),
1070+
Git._check_revision(ancestor_rev),
1071+
Git._check_revision(rev),
10701072
)
10711073
except GitCommandError as err:
10721074
if err.status == 1:
@@ -1303,7 +1305,7 @@ def blame_incremental(
13031305

13041306
data: bytes = self.git._call_process_safe(
13051307
"blame",
1306-
Git._check_operand(rev, "revision") if rev is not None else None,
1308+
Git._check_revision(rev) if rev is not None else None,
13071309
"--no-textconv" if not allow_unsafe_options else None,
13081310
"--",
13091311
file,
@@ -1424,7 +1426,7 @@ def blame(
14241426
)
14251427
data: bytes = self.git._call_process_safe(
14261428
"blame",
1427-
Git._check_operand(rev, "revision") if rev is not None else None,
1429+
Git._check_revision(rev) if rev is not None else None,
14281430
*rev_opts_list,
14291431
"--no-textconv" if not allow_unsafe_options else None,
14301432
"--",
@@ -1925,7 +1927,7 @@ def archive(
19251927
self.git._call_process_safe(
19261928
"archive",
19271929
"--",
1928-
Git._check_operand(treeish, "revision"),
1930+
Git._check_revision(treeish),
19291931
*path,
19301932
_allow_network=bool(kwargs.get("remote")),
19311933
_config=()

0 commit comments

Comments
 (0)