Skip to content

Commit 6de90f1

Browse files
Byroncodex
andcommitted
fix(refs): preserve full reference names in revision APIs
<!-- agent: Codex --> Converting reference arguments to object IDs prevents same-named tags from replacing branches, but loses the reference identity required by several Git operations. Reflog walks then return no commits, fork-point lookup fails with `No such ref`, and the default `upload-archive` server rejects the literal ID. References inside nested `merge_base()` arguments also still became short names and could select a shadowing tag. Keep the exact-reference existence and symbolic-chain checks in `Git._check_revision()`, then pass the validated full ref name. This preserves Git's ref-dependent behavior without operation-specific conversion switches. Strings retain Git's revision interpretation and commit objects retain their IDs. Normalize `os.PathLike` operands through `os.fsdecode()` before validation so their filesystem paths, including byte paths, are used instead of their display strings. Flatten nested list/tuple merge-base arguments while preserving reference objects until each operand is validated. Extend `tests/fixtures/exact-refs.sh` with an upstream advance and rewind plus a topic based on its old tip. Regressions compare branch, remote-tracking and HEAD behavior with Git 2.54.0 across SHA-1/SHA-256 and files/reftable repositories. They cover shadowing tags, missing refs, nested operands, option spellings, remote archives and safe/unsafe `PathLike` values. Update the changelog and document the retained CLI fallbacks: GixPython 0.3.0 exposes neither the required reflog-walk/fork-point operations nor an upload-archive client. Assisted-by: GPT 6.1 Sol Co-authored-by: GPT 6.1 Sol <codex@openai.com>
1 parent d03d3d7 commit 6de90f1

6 files changed

Lines changed: 148 additions & 12 deletions

File tree

‎doc/gix-backend.md‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ and predate the current fallback; they do not establish native coverage.
374374
| Worktree inventory, `worktree list` | Main and linked worktree metadata, including bare main repositories and locks | Prunable entries; GIX-14 |
375375
| `Repo.merge_base`, `Repo.is_ancestor` | Native graph queries for two revisions | Octopus/fork-point and other options |
376376
| `Commit.count` | Reachable commit count, skip/limit/first-parent | Path filters and other revision options |
377-
| `Commit.iter_items`, `Repo.iter_commits` | First-parent walks and a single tip | General history ordering; GIX-8 |
377+
| `Commit.iter_items`, `Repo.iter_commits` | First-parent walks and a single tip | Reflog walks, general history ordering; GIX-8 |
378378
| Index entry reads, `ls_files` | Stage/mode/OID/path and exposed index flags | Custom and sparse indexes; GIX-3/4 |
379379
| `IndexFile.version`, `update_index` query | Native index version | Actual `update-index` mutations |
380380
| Index editing and `IndexFile.write` | Opaque byte snapshots; atomic locked publication | Private-file edits always use Git: arbitrary native index loading and strict pathname validation are missing; GIX-3/4/13/26 |
@@ -392,6 +392,25 @@ and predate the current fallback; they do not establish native coverage.
392392
| `Repo.untracked_files` | Native directory walk | Extra status options and non-root command directories |
393393
| `Repo.ignored` | Native excludes with tracked-file suppression | Symlink/submodule traversal, unsupported path normalization, and unavailable path metadata |
394394

395+
Revision APIs preserve a reference object's validated full name. The shared
396+
`Git._check_revision()` first uses the exact reference reader to verify existence
397+
and the symbolic-reference chain; it passes the full name rather than the resolved
398+
object ID. Strings retain Git's name guessing, and commit objects retain their IDs.
399+
This prevents same-named tags from replacing branches without losing the ref
400+
identity needed by reflog walks, fork-point lookups and remote archives.
401+
`merge_base()` validates nested list/tuple operands before string conversion.
402+
`exact-refs.sh` covers these paths with shadowing tags and missing references in
403+
SHA-1/SHA-256 and files/reftable repositories on both backends.
404+
405+
These name-sensitive operations retain CLI fallback. GixPython 0.3.0's
406+
`Repository.rev_walk()` has no Git-compatible reflog-walk mode,
407+
`Repository.merge_base()` exposes graph merge bases rather than reflog-based
408+
fork-point selection, and no upload-archive client API is exposed. The regressions
409+
compare Git 2.54.0's reflog history, old fork point after an upstream rewind,
410+
and default upload-archive server behavior, which rejects literal object IDs.
411+
Native replacements must expose matching reflog walking and fork-point selection
412+
and the upload-archive protocol; no Python implementation substitutes for them.
413+
395414
Reference reads require an exact name, unlike revision APIs that intentionally
396415
apply Git's name guessing. `reference_info()` uses
397416
`Repository.try_find_reference()` and verifies the returned name for `HEAD` and

‎doc/source/changes.rst‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,11 @@ API changes
6060
APIs such as ``Repo.commit(rev)`` retain Git's name guessing for strings.
6161
A reference object passed as a revision, for example to ``Repo.commit()``,
6262
``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.
63+
``HEAD.reset()``, is validated by its exact name and passed using its full ref
64+
name, so a tag named like a branch or remote-tracking branch no longer takes
65+
its place. The ref name also preserves reflog walks, fork-point lookups and
66+
remote archives. A missing reference raises ``ValueError``, as its ``object``
67+
property does.
6668
* Packed tag names containing non-UTF-8 bytes are represented losslessly as
6769
surrogate-escaped strings. ``safe_encode()`` recovers their original bytes,
6870
and Gix reference validation and resolution receive those bytes directly.

‎git/cmd.py‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,17 +1025,20 @@ def _check_operand(value: Any, label: str = "operand") -> str:
10251025

10261026
@staticmethod
10271027
def _check_revision(value: Any, label: str = "revision") -> str:
1028-
"""Validate a revision, passing a reference object as the object ID it holds.
1028+
"""Validate a revision, preserving a reference object's full name.
10291029
10301030
A reference converts to its short name, for which Git's revision lookup tries
10311031
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.
1032+
``refs/remotes/``. Verify the exact reference exists before passing its full
1033+
name, retaining its identity for reflog walks and remote archives.
10341034
"""
10351035
from git.refs.symbolic import SymbolicReference
10361036

10371037
if isinstance(value, SymbolicReference):
1038-
value = value.dereference_recursive(value.repo, value.path)
1038+
value.dereference_recursive(value.repo, value.path)
1039+
value = value.path
1040+
if isinstance(value, os.PathLike):
1041+
value = os.fsdecode(value)
10391042
return Git._check_operand(value, label)
10401043

10411044
def _require_version(self) -> None:

‎git/repo/base.py‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,12 +1029,18 @@ 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]
1032+
def checked_revisions(items: Sequence[Any]) -> Iterator[str]:
1033+
# Preserve reference objects until each flattened operand is validated.
1034+
for item in items:
1035+
if isinstance(item, (list, tuple)):
1036+
yield from checked_revisions(item)
1037+
else:
1038+
yield Git._check_revision(item)
1039+
10341040
res: List[Commit] = []
10351041
try:
10361042
lines: List[str] = self.git._call_process_safe(
1037-
"merge_base", "--", *(Git._check_operand(item, "revision") for item in Git._unpack_args(revs)), **kwargs
1043+
"merge_base", "--", *checked_revisions(rev), **kwargs
10381044
).splitlines()
10391045
except GitCommandError as err:
10401046
if err.status != 1:

‎test/test_refs.py‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,102 @@ def test_reference_arguments_do_not_resolve_same_named_tags(object_format, ref_f
259259
assert repo.index.write_tree() == local.tree
260260

261261

262+
@pytest.mark.parametrize("object_format", ["sha1", "sha256"])
263+
@pytest.mark.parametrize("ref_format", ["files", "reftable"])
264+
def test_merge_base_preserves_nested_reference_arguments(object_format, ref_format):
265+
with fixture_repo("exact-refs.sh", object_format, ref_format, "shadowed") as repo:
266+
for ref in (repo.heads.main, RemoteReference(repo, "refs/remotes/origin/main")):
267+
for left, right in (([ref], ref), (([ref],), ref), (ref, [ref]), ([ref.path], (ref.path,))):
268+
assert repo.merge_base(left, right) == [ref.commit]
269+
270+
271+
@pytest.mark.parametrize("object_format", ["sha1", "sha256"])
272+
@pytest.mark.parametrize("ref_format", ["files", "reftable"])
273+
@pytest.mark.parametrize("path_type", [Path, PathLikeMock])
274+
def test_revision_arguments_preserve_pathlike_paths(object_format, ref_format, path_type):
275+
with fixture_repo("exact-refs.sh", object_format, ref_format, "shadowed") as repo:
276+
local = repo.head.commit
277+
for name in ("HEAD", "refs/heads/main", "refs/remotes/origin/main"):
278+
path = path_type(name)
279+
ref = SymbolicReference(repo, path)
280+
assert repo.commit(ref) == ref.object == local
281+
assert repo.merge_base(ref, local) == [local]
282+
assert repo.merge_base(path, local) == [local]
283+
assert repo.merge_base(([path],), [local]) == [local]
284+
285+
286+
@pytest.mark.parametrize("revision", ["HEAD", "--all", "HEAD\0", "HEAD\r", "HEAD\n"])
287+
@pytest.mark.parametrize("as_bytes", [False, True])
288+
def test_check_revision_normalizes_pathlike_before_validation(revision, as_bytes):
289+
path = Mock(spec=os.PathLike, __fspath__=Mock(return_value=os.fsencode(revision) if as_bytes else revision))
290+
if revision == "HEAD":
291+
assert Git._check_revision(path) == revision
292+
else:
293+
with pytest.raises(UnsafeOptionError, match="Invalid revision"):
294+
Git._check_revision(path)
295+
296+
297+
@pytest.mark.parametrize("object_format", ["sha1", "sha256"])
298+
@pytest.mark.parametrize("ref_format", ["files", "reftable"])
299+
def test_revision_arguments_validate_missing_references(object_format, ref_format):
300+
with fixture_repo("exact-refs.sh", object_format, ref_format) as repo:
301+
for path in ("refs/heads/unborn", "REBASE_HEAD"):
302+
ref = SymbolicReference(repo, path)
303+
foreign = repo.tags[path].commit
304+
assert repo.git.rev_parse("--verify", path) == foreign.hexsha
305+
with pytest.raises(ValueError, match="does not exist or is invalid"):
306+
repo.commit(ref)
307+
with pytest.raises(ValueError, match="does not exist or is invalid"):
308+
repo.merge_base([ref], foreign)
309+
310+
311+
@pytest.mark.parametrize("object_format", ["sha1", "sha256"])
312+
@pytest.mark.parametrize("ref_format", ["files", "reftable"])
313+
def test_reflog_walk_preserves_reference_names(object_format, ref_format):
314+
with fixture_repo("exact-refs.sh", object_format, ref_format, "reflog") as repo:
315+
for ref in (repo.heads.main, RemoteReference(repo, "refs/remotes/origin/main"), repo.head):
316+
expected = repo.git.rev_list("--walk-reflogs", ref.path, "--").splitlines()
317+
assert len(expected) >= 2
318+
for options in ({"walk_reflogs": True}, {"walk-reflogs": True}):
319+
assert [commit.hexsha for commit in repo.iter_commits(ref, **options)] == expected
320+
assert [commit.hexsha for commit in Commit.iter_items(repo, ref, max_count=1, **options)] == expected[
321+
:1
322+
]
323+
with pytest.raises(ValueError, match="does not exist or is invalid"):
324+
list(repo.iter_commits(SymbolicReference(repo, "REBASE_HEAD"), walk_reflogs=True))
325+
326+
327+
@pytest.mark.parametrize("object_format", ["sha1", "sha256"])
328+
@pytest.mark.parametrize("ref_format", ["files", "reftable"])
329+
def test_fork_point_preserves_upstream_reference_name(object_format, ref_format):
330+
with fixture_repo("exact-refs.sh", object_format, ref_format, "reflog") as repo:
331+
topic = repo.heads.topic
332+
old_tip = topic.commit.parents[0]
333+
for upstream in (repo.heads.main, RemoteReference(repo, "refs/remotes/origin/main")):
334+
assert repo.merge_base(upstream, topic) == [upstream.commit]
335+
assert repo.git.merge_base("--fork-point", upstream.path, topic.path) == old_tip.hexsha
336+
for options in ({"fork_point": True}, {"fork_p": True}):
337+
assert repo.merge_base(upstream, topic, **options) == [old_tip]
338+
with pytest.raises(ValueError, match="does not exist or is invalid"):
339+
repo.merge_base(SymbolicReference(repo, "REBASE_HEAD"), topic, fork_point=True)
340+
341+
342+
@pytest.mark.parametrize("object_format", ["sha1", "sha256"])
343+
@pytest.mark.parametrize("ref_format", ["files", "reftable"])
344+
def test_remote_archive_preserves_reference_name(object_format, ref_format):
345+
with fixture_repo("exact-refs.sh", object_format, ref_format, "shadowed") as repo:
346+
for ref in (repo.heads.main, RemoteReference(repo, "refs/remotes/origin/main"), repo.head):
347+
expected = io.BytesIO()
348+
repo.git.archive("--remote=" + repo.git_dir, ref.path, output_stream=expected)
349+
stream = io.BytesIO()
350+
repo.archive(stream, ref, remote=repo.git_dir)
351+
assert stream.getvalue() == expected.getvalue()
352+
with tarfile.open(fileobj=io.BytesIO(stream.getvalue())) as archive:
353+
assert archive.getnames() == ["file"]
354+
with pytest.raises(ValueError, match="does not exist or is invalid"):
355+
repo.archive(io.BytesIO(), SymbolicReference(repo, "REBASE_HEAD"), remote=repo.git_dir)
356+
357+
262358
@pytest.mark.parametrize("object_format", ["sha1", "sha256"])
263359
@pytest.mark.parametrize("ref_format", ["files", "reftable"])
264360
def test_rebase_detection_ignores_tag_named_rebase_head(object_format, ref_format):

‎tests/fixtures/exact-refs.sh‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ git -C repo fetch ../source 'refs/tags/*:refs/tags/*'
1515
if [ "${3-}" = symbolic ]; then
1616
git -C repo symbolic-ref refs/tags/REBASE_HEAD refs/tags/refs/heads/unborn
1717
fi
18-
if [ "${3-}" = shadowed ]; then
18+
if [ "${3-}" = shadowed ] || [ "${3-}" = reflog ]; then
1919
# Fetched tags share the short names of a local branch and a remote-tracking branch.
2020
git -C repo fetch ../source 'refs/heads/main:refs/tags/main' 'refs/heads/main:refs/tags/origin/main'
2121
blob=$(printf 'local\n' | git -C repo hash-object -w --stdin)
@@ -25,6 +25,16 @@ if [ "${3-}" = shadowed ]; then
2525
git -C repo update-ref "$path" "$commit"
2626
done
2727
git -C repo symbolic-ref HEAD refs/heads/main
28+
if [ "${3-}" = reflog ]; then
29+
old_tip=$(git -C repo commit-tree -p "$commit" -m 'Old tip' "$tree")
30+
topic=$(git -C repo commit-tree -p "$old_tip" -m 'Topic history' "$tree")
31+
git -C repo update-ref refs/heads/topic "$topic"
32+
git -C repo tag topic refs/tags/main
33+
for path in refs/heads/main refs/remotes/origin/main; do
34+
git -C repo update-ref --create-reflog -m 'Advance upstream' "$path" "$old_tip"
35+
git -C repo update-ref -m 'Rewind upstream' "$path" "$commit"
36+
done
37+
fi
2838
fi
2939
if [ "${3-}" = existing ]; then
3040
commit=$(git -C source rev-parse HEAD)

0 commit comments

Comments
 (0)