diff --git a/AGENTS.md b/AGENTS.md index 56d2eee28..3edeea1e3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,6 +9,12 @@ when communicating through a person's account. issue with a fix attached. Do not create an issue solely to follow it immediately with a PR that fixes it; describe the problem in the PR instead. +Agents creating a PR must state the underlying motivation in its description, such +as the project that will benefit or where the bug was first discovered. Any +motivation is valid; explain why the work was pursued rather than restating what +the PR does. "Want to fix a bug" is insufficient because that is already evident +from the PR itself. + # Gix backend implementations Gix backend operations must use Gix through the supported GixPython (`gix`) diff --git a/doc/gix-backend.md b/doc/gix-backend.md index 3d2f39238..a1525be6e 100644 --- a/doc/gix-backend.md +++ b/doc/gix-backend.md @@ -392,6 +392,20 @@ and predate the current fallback; they do not establish native coverage. | `Repo.untracked_files` | Native directory walk | Extra status options and non-root command directories | | `Repo.ignored` | Native excludes with tracked-file suppression | Symlink/submodule traversal, unsupported path normalization, and unavailable path metadata | +Reference reads require an exact name, unlike revision APIs that intentionally +apply Git's name guessing. `reference_info()` uses +`Repository.try_find_reference()` and verifies the returned name for `HEAD` and +fully qualified refs. Other root names, including `REBASE_HEAD`, retain the CLI +fallback: GixPython's lookup accepts partial names, including symbolic lookalike +tags for missing root names (GIX-33). Native `symbolic_ref` dispatch verifies the +returned name and falls back for a lookalike. A native root-reference conversion +needs verified existence and error behavior, including Git's distinction between +an absent and an invalid reference. `Repo.currently_rebasing_on()` probes +`git show-ref --exists` and then uses the shared exact reference reader; the +existence probe remains CLI on both backends. `exact-refs.sh` tests missing names beside fetched tags with both object +and reference formats. Reftable still uses GIX-1; this change claims no new native +coverage. The symbolic lookalike mismatch remains open upstream as GIX-33. + Reference names and revisions are encoded with `surrogateescape` before calling GixPython's byte-capable `Target.Symbolic`, `Repository.try_find_reference`, `Repository.rev_parse_single`, `Repository.rev_parse` and `RefEdit` APIs. @@ -935,7 +949,7 @@ identify the historical fixture where needed. | GIX-14 | Bug; missing API | For a linked worktree of a bare main repository, `is_bare()` returns true while `git rev-parse --is-bare-repository` returns false; `workdir()` does expose the worktree. Initial gitfile opening can retain a noncanonical target and use an arbitrary gitfile's location as the worktree, unlike Git. Discovery accepts undecodable HEADs until explicit `head()` decoding and ignores dangling `commondir` symlinks that Git rejects. Bindings flatten failure kinds into `gix.Error`. Windows also exposes adapter path-formatting differences and a worker panic on an undecodable `commondir`; see the Windows reproduction below. | Provide Git-compatible classification, canonical gitfile locations and layout validation, plus typed errors instead of panics. Format native locations compatibly with Git's command output. The adapter combines Gix metadata, reopens through Gix and forces HEAD decoding, retaining CLI for remaining validation/diagnostic gaps. Those workarounds do not close these compatibility bugs. A Git-strict mode must cover these cases; strict config and trust settings alone do not. | | GIX-15 | Bug | Native blame disagrees with Git even with Myers and rewrite tracking selected. In the fixture's `README.md`, lines 150 and 158 are attributed to the opposite commits. Incremental order also differs. | Attribution and incremental-output parity before replacing `Repo.blame` / `blame_incremental`. | | GIX-16 | Bug; missing API | Native archive streaming takes a tree rather than a commit. Its TAR omits Git's global PAX commit comment, leaves `export-subst` placeholders literal, and writes ordinary modes as `0644` where Git uses `0664`. | Commit-aware export substitution, metadata and permission parity. Both engines respected `export-ignore` in the probe. | -| GIX-17 | Bug | Revision parsing accepts abbreviated IDs with `-dirty`, prefers the OID suffix over an exact describe-shaped tag, and treats escaped braces in message searches differently. | Git-compatible revision grammar and regex semantics. Existing `test_rev_parse.py` cases reproduce all three. | +| GIX-17 | Bug (open) | Revision parsing accepts abbreviated IDs with `-dirty`, prefers the OID suffix over an exact describe-shaped tag, and treats escaped braces in message searches differently. With GixPython 0.3.0 and Git 2.54.0, `2313-supersede` resolves to blob `23136a1d1dd1f02de7cde0f7b55f49d76ac77d55` through Gix while Git resolves the existing branch's commit. The deterministic `rev-parse.sh hex-prefix-collision` fixture creates a branch named `-topic`: Gix returns the blob and also accepts `-missing`, which Git rejects. | Provide Git-compatible revision grammar and regex semantics. `_check_revision_grammar()` retains CLI for hexadecimal prefixes followed by a dash, as well as the existing guarded forms; this adapter workaround does not close the upstream bug. `test_rev_parse.py` reproduces these differences, including `test_rev_parse_hex_prefix_with_arbitrary_suffix`. | | GIX-18 | Bug; missing API | The tree-diff resource cache reads attributes from the index; Git also consults worktree attributes. Native binary classification with textconv configured can also differ even in `to_git` mode. | Independent attribute-source control and `--no-textconv` parity. Statistics currently fall back for any effective `diff` attribute. | | GIX-19 | Unverified | GixPython opens repositories with `extensions.compatObjectFormat`, but translation and object-write compatibility have not been validated. The installed Git reports `compatibility hash algorithm support requires Rust` when asked to translate an ID. This is an unvalidated capability guard, not evidence of missing native object mappings. | Verify compatibility-format reads, writes and OID translation against a Git build that supports them. All operations use CLI in the meantime. | | GIX-20 | Bug; missing API | `Target.Symbolic(name)` validates full names but rejects standalone names accepted by `git check-ref-format --allow-onelevel`. Four of 430 audited calls differed: `refs` twice, `hellothere`, and `valid_one_level_refname`. Further probes include digits, punctuation and Unicode (`1`, `A1`, `HEAD_1`, `A-B`, `A.B`, `Ä`). | Expose Git-compatible partial/one-level name validation, or correct the native restriction where appropriate. This is a binding/behavior shortcoming to revisit upstream; standalone names rejected natively retain CLI validation. | @@ -951,6 +965,7 @@ identify the historical fixture where needed. | GIX-30 | Bug (open) | GixPython 0.2.0 `Commit.decode()` reports `encoding=None` when `encoding ISO-8859-1` follows `x-extra value`; Git 2.54.0 selects `ISO-8859-1` and renders Latin-1 identity/message bytes correctly. Gix retains `(b"encoding", b"ISO-8859-1")` in `extra_headers`, but its encoding field disagrees with Git. The `late-latin1` read-only `commit.sh` fixture in `test_deserialization_declared_encoding` reproduces this and asserts the fallback. | Recognize encoding headers in Git-compatible order. `object_metadata()` conservatively retains CLI when an encoding appears in `extra_headers`; it does not select the header in Python. Keep this bug open until an upstream fix or compatible mode is verified. | | GIX-31 | Bug (open) | GixPython 0.2.0 `Commit.decode()` rejects commit headers containing a CR-only or tab-only line with `object parsing failed, at gix-object/src/commit/decode.rs:20`. Git 2.54.0 accepts both objects with `fsck --strict`, continues through their header block and renders the later Latin-1 declaration and message correctly. A space-only continuation line decodes natively but retains the late encoding declaration in `extra_headers` (GIX-30). `test_deserialization_headers_end_at_empty_line` reproduces all three using the `commit.sh whitespace-headers` fixtures and checks Git's projection. | Accept Git-compatible header lines and end headers only at an empty line. The existing `object_metadata()` native decoder diagnostics fallback retains Git for CR/tab inputs; the existing GIX-30 fallback covers the space input. Neither fallback closes these bugs. | | GIX-32 | Missing capability (open); no demonstrated Git compatibility mismatch | Windows CI on GixPython 0.3.0, Git 2.56.0.windows.2 and CPython 3.12.10 enumerates the non-UTF-8 packed names in `legacy-refs.sh`, but exact byte-name lookup selects `Reference.read: CLI: native read diagnostics`; the no-CLI guard fails for SHA-1 and SHA-256. POSIX lookup succeeds. Git's Windows CLI likewise cannot address those original bytes through UTF-16 arguments, so there is no demonstrated equivalent Windows CLI operand. Git enumeration does preserve both names. | Provide native Windows packed-reference lookup for byte names without first converting the name to a loose-reference filesystem path. Keep `reference_info()`'s existing diagnostic fallback; `test_non_utf8_tag_names_preserve_bytes` checks it on Windows. Resolution coverage and native claims remain POSIX-only. The upstream lookup source attempts loose-path conversion before consulting packed refs; CI demonstrates the fallback, but does not capture the underlying Gix error text. | +| GIX-33 | Bug (open); missing exact-name mode | With Git 2.54.0 and GixPython 0.3.0, `exact-refs.sh sha1 files symbolic` makes `refs/tags/REBASE_HEAD` a symbolic ref to another tag while the root `REBASE_HEAD` is absent. `Repository.try_find_reference("REBASE_HEAD")` returns that tag; native `_symbolic_ref` formerly returned its target, whereas `git symbolic-ref --quiet --no-recurse -- REBASE_HEAD` exits 1. GitPython's root-reference object read consequently returned a commit through Gix and raised `ValueError` through CLI. SHA-256 reproduces the same mismatch. | Expose an exact-name lookup mode, including root references, whose absence and error behavior matches Git. `_symbolic_ref()` compares the native reference's name with the requested bytes and selects CLI for a lookalike. `test_missing_root_reference_does_not_follow_symbolic_lookalike_tag` verifies parity and forced CLI fallback. This adapter guard does not close the upstream compatibility bug. | The PR #2306 regression exposed an adapter ownership-check bypass in both backends: `Repo`'s automatic `GIT_DIR` made Git 2.54.0 treat hook execution as diff --git a/doc/source/changes.rst b/doc/source/changes.rst index c8c71e9f8..0da1a81c4 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -2,11 +2,11 @@ Changelog ========= -Unreleased: Git CLI migration -============================= +Unreleased: Git CLI migration - back to the roots +================================================= GitPython now delegates repository discovery, references, configuration, object -storage, tree construction, index operations, and revision parsing to Git. The +storage, tree construction, index operations, and revision parsing to Git.The default backend supports SHA-1 and SHA-256 repositories with either files or reftable reference storage. Repository object IDs must no longer be assumed to contain 20 binary bytes or 40 hexadecimal characters. @@ -54,6 +54,10 @@ Git executable and safety API changes ----------- +* Reference reads require the exact name. A missing branch no longer resolves to + a similarly named tag, so an unborn branch's initial commit has no parent. + A tag named ``REBASE_HEAD`` no longer reports a rebase in progress. Revision + APIs such as ``Repo.commit(rev)`` retain Git's name guessing. * Packed tag names containing non-UTF-8 bytes are represented losslessly as surrogate-escaped strings. ``safe_encode()`` recovers their original bytes, and Gix reference validation and resolution receive those bytes directly. diff --git a/git/_backend.py b/git/_backend.py index 556b615c7..82c46b8b8 100644 --- a/git/_backend.py +++ b/git/_backend.py @@ -166,7 +166,7 @@ def _canonical_repository(repo: Any) -> Any: def _check_revision_grammar(ref: str) -> None: - if ref.startswith(":/") or "^{/" in ref or "-dirty" in ref or re.search(r"-\d+-g[0-9a-fA-F]+", ref): + if ref.startswith(":/") or "^{/" in ref or "-dirty" in ref or re.search(r"-\d+-g[0-9a-fA-F]+|^[0-9a-fA-F]+-", ref): raise _Unsupported("revision grammar differences (GIX-17)") @@ -363,7 +363,7 @@ def _symbolic_ref(repo: Any, args: List[str], kwargs: Dict[str, Any]) -> bytes: if len(args) != 4 or args[:3] != ["--quiet", "--no-recurse", "--"]: raise _Unsupported("reference mutation or options") reference = repo.try_find_reference(safe_encode(args[-1])) - if reference is None: + if reference is None or reference.name() != safe_encode(args[-1]): raise _Unsupported("missing reference diagnostics") target = reference.target().try_name() if target is None: diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py index 68552f672..3924fb2ca 100644 --- a/git/refs/symbolic.py +++ b/git/refs/symbolic.py @@ -198,7 +198,8 @@ def _get_ref_info_helper( if exc.status != 1: raise ValueError("Invalid symbolic reference %r" % path) from exc try: - oid = repo.git._call_process_safe("rev_parse", "--verify", "--end-of-options", path) + # Revision parsing can guess a tag when the exact reference is absent. + oid = repo.git._call_process_safe("show_ref", "--verify", "--hash", "--", path) except GitCommandError as exc: raise ValueError("Reference at %r does not exist or is invalid" % path) from exc if not repo.re_hexsha_only.fullmatch(oid): diff --git a/git/repo/base.py b/git/repo/base.py index 0de87d01c..bd24f8530 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -35,7 +35,7 @@ ) from git.index import IndexFile from git.objects import Submodule, RootModule, Commit -from git.refs import HEAD, Head, Reference, TagReference +from git.refs import HEAD, Head, Reference, SymbolicReference, TagReference from git.remote import Remote, _T_RemoteName, add_progress, to_progress_instance from git.util import ( Actor, @@ -88,7 +88,6 @@ if TYPE_CHECKING: from git.objects import Tree from git.objects.submodule.base import UpdateProgress - from git.refs.symbolic import SymbolicReference from git.remote import RemoteProgress from git.util import IterableList @@ -1964,11 +1963,11 @@ def currently_rebasing_on(self) -> Commit | None: """ if not self.git_dir: return None - status, oid, stderr = self.git._call_process_safe( - "rev_parse", "--verify", "--quiet", "REBASE_HEAD", with_extended_output=True, with_exceptions=False + status, stdout, stderr = self.git._call_process_safe( + "show_ref", "--exists", "--", "REBASE_HEAD", with_extended_output=True, with_exceptions=False ) - if status == 1: + if status == 2: return None if status: - raise GitCommandError("git rev-parse", status, stderr, oid) - return self.commit(oid) + raise GitCommandError("git show-ref", status, stderr, stdout) + return self.commit(SymbolicReference.dereference_recursive(self, "REBASE_HEAD")) diff --git a/test/test_git.py b/test/test_git.py index 950e83914..dac9c11ed 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -518,7 +518,7 @@ def test_it_redacts_authorization_extra_header_from_error(self): def test_clone_redacts_config_urls_from_trace_and_error(self, option, expected): for trace in (True, "full"): with self.subTest(trace=trace), writable_fixture("repo.sh", "empty") as temporary: - directory = temporary.path() + directory = temporary.path() / "alice-bob-user-token" with mock.patch.object(Git, "GIT_PYTHON_TRACE", trace): with self.assertLogs("git.cmd", level="INFO") as logs: with self.assertRaises(GitCommandError) as context: @@ -532,6 +532,9 @@ def test_clone_redacts_config_urls_from_trace_and_error(self, option, expected): message = str(context.exception) assert logs is not None log_output = " ".join(logs.output) + for path in (str(directory), directory.as_posix()): + message = message.replace(path, "") + log_output = log_output.replace(path, "") self.assertIn(expected, log_output) for credential in ("alice", "bob", "user", "token"): self.assertNotIn(credential, message) diff --git a/test/test_refs.py b/test/test_refs.py index 1e47101c1..69e04fff1 100644 --- a/test/test_refs.py +++ b/test/test_refs.py @@ -163,6 +163,113 @@ def test_head_rename_retains_literal_prefixed_branch_name(object_format): assert repo.heads.topic.commit == original_topic +@pytest.mark.parametrize("object_format", ["sha1", "sha256"]) +@pytest.mark.parametrize("ref_format", ["files", "reftable"]) +@pytest.mark.parametrize("force_cli", [False, True]) +def test_reference_lookup_does_not_guess_missing_names(object_format, ref_format, force_cli): + with fixture_repo("exact-refs.sh", object_format, ref_format) as repo: + status, _, _ = repo.git.rev_parse( + "--verify", "--quiet", "HEAD", with_extended_output=True, with_exceptions=False + ) + assert status == 1 + with patch.object(_backend, "gix", None) if force_cli else contextlib.nullcontext(): + for path in ( + "refs/heads/unborn", + "REBASE_HEAD", + "ORIG_HEAD", + "refs/tags/missing", + "refs/remotes/origin/missing", + ): + tag = TagReference(repo, "refs/tags/" + path) + assert tag.commit == repo.commit(tag.path) + status, _, _ = repo.git.show_ref( + "--exists", "--", path, with_extended_output=True, with_exceptions=False + ) + assert status == 2 + with pytest.raises(ValueError, match="does not exist or is invalid"): + SymbolicReference(repo, path).object + assert not repo.head.is_valid() + with pytest.raises(ValueError): + repo.head.commit + # Revision APIs deliberately retain Git's name guessing. + assert repo.commit("REBASE_HEAD") == repo.tags.REBASE_HEAD.commit + assert repo.tags["nested/release"].object.type == "tag" + + +@pytest.mark.parametrize("object_format", ["sha1", "sha256"]) +def test_missing_root_reference_does_not_follow_symbolic_lookalike_tag(object_format): + with fixture_repo("exact-refs.sh", object_format, "files", "symbolic") as repo: + assert repo.tags.REBASE_HEAD.commit == repo.tags["refs/heads/unborn"].commit + status, _, _ = repo.git.symbolic_ref( + "--quiet", "--no-recurse", "--", "REBASE_HEAD", with_extended_output=True, with_exceptions=False + ) + assert status == 1 + for force_cli in (False, True): + with patch.object(_backend, "gix", None) if force_cli else contextlib.nullcontext(): + with pytest.raises(ValueError, match="does not exist or is invalid"): + SymbolicReference(repo, "REBASE_HEAD").object + + +@pytest.mark.parametrize("object_format", ["sha1", "sha256"]) +@pytest.mark.parametrize("ref_format", ["files", "reftable"]) +def test_initial_commit_does_not_inherit_lookalike_tag_history(object_format, ref_format): + with fixture_repo("exact-refs.sh", object_format, ref_format, write=True) as repo: + foreign = repo.tags["refs/heads/unborn"].commit + initial = repo.index.commit("Local initial commit") + assert initial.parents == [] + assert repo.head.commit == initial + assert repo.tags["refs/heads/unborn"].commit == foreign + assert repo.git.rev_list("--count", "HEAD") == "1" + + +@pytest.mark.parametrize("object_format", ["sha1", "sha256"]) +@pytest.mark.parametrize("ref_format", ["files", "reftable"]) +def test_rebase_detection_ignores_tag_named_rebase_head(object_format, ref_format): + with fixture_repo("exact-refs.sh", object_format, ref_format, write=True) as repo: + assert repo.currently_rebasing_on() is None + replayed = repo.tags.REBASE_HEAD.commit + repo.git.update_ref("REBASE_HEAD", replayed.hexsha) + assert repo.currently_rebasing_on() == replayed + repo.git.update_ref("-d", "REBASE_HEAD") + assert repo.currently_rebasing_on() is None + + +@pytest.mark.parametrize("object_format", ["sha1", "sha256"]) +@pytest.mark.parametrize("ref_format", ["files", "reftable"]) +def test_rebase_detection_peels_nested_annotated_tags(object_format, ref_format): + with fixture_repo("exact-refs.sh", object_format, ref_format, "nested", write=True) as repo: + tag = repo.tags["nested/outer"].object + assert tag.object.type == "tag" + repo.git.update_ref("REBASE_HEAD", tag.hexsha) + expected = repo.tags.REBASE_HEAD.commit + assert repo.git.rev_parse("REBASE_HEAD^{commit}") == expected.hexsha + assert repo.currently_rebasing_on() == expected + + +@pytest.mark.parametrize("object_format", ["sha1", "sha256"]) +@pytest.mark.parametrize("ref_format", ["files", "reftable"]) +def test_exact_reference_reads_preserve_existing_targets(object_format, ref_format): + with fixture_repo("exact-refs.sh", object_format, ref_format, "existing", write=True) as repo: + commit = repo.tags.REBASE_HEAD.commit + paths = [ + "HEAD", + "refs/heads/unborn", + "FETCH_HEAD", + "refs/tags/nested/release", + "refs/stash", + "ORIG_HEAD", + "MERGE_HEAD", + "REBASE_HEAD", + ] + repo.git.pack_refs("--all") + for path in paths: + expected = repo.git.rev_parse("--verify", path) + assert SymbolicReference(repo, path).object.hexsha == expected + repo.git.update_ref("--no-deref", "HEAD", commit.hexsha) + assert repo.head.is_detached + assert repo.head.commit == commit + + class TestRefs(TestBase): def test_from_path(self): # Should be able to create any reference directly. diff --git a/test/test_rev_parse.py b/test/test_rev_parse.py index 39c6768fd..d86fe832d 100644 --- a/test/test_rev_parse.py +++ b/test/test_rev_parse.py @@ -80,6 +80,18 @@ def test_rev_parse_names_hex_and_describe_forms(rev_parse_repo): assert collision.rev_parse(describe_name).hexsha == release.hexsha +def test_rev_parse_hex_prefix_with_arbitrary_suffix(): + with fixture_repo("rev-parse.sh", "hex-prefix-collision") as repo: + prefix = (Path(repo.working_tree_dir).parent / "hex-prefix").read_text().strip() + name = prefix + "-topic" + assert repo.rev_parse(name).hexsha == repo.git.rev_parse("--verify", name) + missing = prefix + "-missing" + with pytest.raises(GitCommandError): + repo.git.rev_parse("--verify", missing) + with pytest.raises(BadName): + repo.rev_parse(missing) + + def test_rev_parse_navigation_and_peeling(rev_parse_repo): repo = rev_parse_repo["repo"] root = rev_parse_repo["root"] diff --git a/tests/fixtures/exact-refs.sh b/tests/fixtures/exact-refs.sh new file mode 100644 index 000000000..71419c138 --- /dev/null +++ b/tests/fixtures/exact-refs.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +set -eu + +git init --object-format="$1" --ref-format="$2" --initial-branch=main source +git -C source commit --allow-empty -m 'Remote history' +for name in refs/heads/unborn REBASE_HEAD ORIG_HEAD refs/tags/missing refs/remotes/origin/missing; do + git -C source tag "$name" +done +git -C source tag -a nested/release -m 'Annotated release' +if [ "${3-}" = nested ]; then + git -C source tag -a nested/outer nested/release -m 'Nested annotated release' +fi +git init --object-format="$1" --ref-format="$2" --initial-branch=unborn repo +git -C repo fetch ../source 'refs/tags/*:refs/tags/*' +if [ "${3-}" = symbolic ]; then + git -C repo symbolic-ref refs/tags/REBASE_HEAD refs/tags/refs/heads/unborn +fi +if [ "${3-}" = existing ]; then + commit=$(git -C source rev-parse HEAD) + for path in refs/heads/unborn refs/stash ORIG_HEAD REBASE_HEAD; do + git -C repo update-ref "$path" "$commit" + done + printf '%s\n' "$commit" >repo/.git/MERGE_HEAD +fi diff --git a/tests/fixtures/rev-parse.sh b/tests/fixtures/rev-parse.sh index e63dcc685..ab87a8ea1 100644 --- a/tests/fixtures/rev-parse.sh +++ b/tests/fixtures/rev-parse.sh @@ -40,6 +40,12 @@ describe-collision) abbrev=$(git rev-parse --short=7 "$merge") git tag "anything-9-g$abbrev" "$release" ;; +hex-prefix-collision) + blob=$(git rev-parse HEAD:README.md) + prefix=$(git rev-parse --short=7 "$blob") + git branch "$prefix-topic" "$release" + printf '%s\n' "$prefix" > ../hex-prefix + ;; rev_parse_preserves_path_metadata_with_colons) blob=$(git rev-parse HEAD:dir/file.txt) subtree=$(printf '100644 blob %s\tname:with:colons\n' "$blob" | git mktree)