| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent b44be6d commit 0709904
6 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -906,7 +906,7 @@ identify the historical fixture where needed. | |||
| 906 | 906 | | 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. | | |
| 907 | 907 | | GIX-21 | Bug; missing API | `Repository.committer()` preserves a process identity of ` Name. ` / ` email@example.invalid ` while `git var GIT_COMMITTER_IDENT` returns `Name.` / `email@example.invalid`. Native snapshot overrides for name, email and date work, but modifying the retained snapshot would leak per-command identity across threads. GixPython 0.1.0 has no general independent repository clone: Python copying fails and internal `RepoHandle::clone()` shares its `Arc` state. | Expose Git-compatible identity cleanup and a cheap independent native clone with isolated config, including per-command removal semantics. Reopening isolates state but repeats setup; `with_object_memory()` changes object-write behavior and is not a general clone. Keep `_checked_signature` validation and CLI fallback for cleanup/overrides. Never clean identity fields in Python or mutate shared snapshots/process environment to handle a command. | | |
| 908 | 908 | | GIX-22 | Missing API | The native Git directory is sufficient for the two fixed metadata leaves used here: `modules` and `COMMIT_EDITMSG`. Git's `path.c` applies no special relocation to either, including in linked worktrees. The adapter appends only these names to the canonical native Git directory. | No new binding is needed for these ordinary paths. Other `--git-path` names and symlinked metadata leaves retain CLI resolution; a general resolver must handle config/environment overrides, common/private storage and canonical targets. `Repository.modules_path()` means `.gitmodules`, not the `modules` storage directory. | | |
| 909 | - | GIX-23 | Missing API | No hook lookup or execution API is bound in GixPython 0.1.0. The removed absence fast path read config through Gix but used Python `os.stat()` to declare success. | Bind native hook lookup with configured/default path, linked-worktree, missing/nonexecutable-hook and diagnostic semantics, plus execution where needed. Until then all managed hook calls use Git, including `--ignore-missing` no-ops. A Python filesystem check is not a Gix implementation. | | ||
| 909 | + | GIX-23 | Missing API | No hook lookup or execution API is bound in GixPython 0.1.0. The removed absence fast path read config through Gix but used Python `os.stat()` to declare success. | Bind native hook lookup with configured/default path, linked-worktree, missing/nonexecutable-hook and diagnostic semantics, plus execution where needed. Until then all managed hook calls use Git, including `--ignore-missing` no-ops, preceded by a Git discovery/ownership check that removes only `Repo`'s automatic environment bindings. Each valid hook request therefore makes two CLI calls. Explicit caller overrides retain Git's behavior. A Python filesystem check is not a Gix implementation. | | ||
| 910 | 910 | | GIX-24 | Bug | In both object formats, `new_commit_as()` accepts a blob as the tree or a parent where `git commit-tree` rejects it. Tree-editor `upsert()`/`write()` accepts object IDs whose actual kinds disagree with blob/tree/gitlink modes; `git mktree --missing` rejects all three tested mismatches. `edit_references_as()` accepts a blob target under `refs/heads/`, rejected by `git update-ref`. | Provide checked commit/tree construction and reference edits, at least in Git-strict mode. Match Git's kind and direct-branch target validation before writing and under the required ref locks, including its distinct handling of symbolic aliases. Existing `_commit_tree`, `_write_tree` and `_update_ref` guards query Gix headers and select CLI on invalid inputs; they do not replace Gix writes with Python. | | |
| 911 | 911 | | GIX-25 | Bug; missing API | After `git symbolic-ref refs/heads/dangling refs/heads/missing`, `Repository.references().all()` enumerates the dangling name, while `git for-each-ref --format=%(refname)` omits it. Valid symbolic aliases must remain present. Both SHA-1 and SHA-256 probes reproduce this difference. | Expose Git-compatible enumeration with the same dangling-reference and diagnostic behavior, retaining the raw iterator for callers that need it. `_for_each_ref` forces Gix to resolve symbolic targets and falls back to Git on failure; keep that fallback until a compatible Gix API/mode is verified. | | |
| 912 | 912 | | GIX-26 | Bug; missing API | `dangerously_push_entry(IndexStat(), oid, 0, 0o120000, b".gitmodules")`, followed by `sort_entries()`, `verify_entries()` and `write()`, succeeds and retains the unsafe symlink. Protected Git `update-index -z --index-info` ignores it with exit 0 and `Ignoring path .gitmodules`; protected `read-tree` rejects it. The native fresh `index_from_tree` also rejects it. | A Git-strict checked index batch editor that validates names/modes, preserves metadata and reports rejected records. `verify_entries()` verifies ordering, not pathname safety. All private-index edits use Git with `core.protectHFS=true` and `core.protectNTFS=true`, followed by requested-entry readback; no Python dictionary reconstruction or native dangerous insertion remains. | | |
@@ -916,6 +916,17 @@ identify the historical fixture where needed. | |||
| 916 | 916 | | 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. | | |
| 917 | 917 | | 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. | | |
| 918 | 918 | ||
| 919 | + The PR #2306 regression exposed an adapter ownership-check bypass in both | ||
| 920 | + backends: `Repo`'s automatic `GIT_DIR` made Git 2.54.0 treat hook execution as | ||
| 921 | + explicitly selecting the repository, so it ran a hook where an ordinary | ||
| 922 | + `git commit` rejected dubious ownership. `run_commit_hook()` now asks Git to | ||
| 923 | + rediscover the repository from its original location before executing hooks. | ||
| 924 | + `test_pre_commit_hook_not_run_in_repository_git_refuses` uses Git's | ||
| 925 | + `GIT_TEST_ASSUME_DIFFERENT_OWNER` switch and verifies rejection, `safe.directory` | ||
| 926 | + exemptions and explicit command-environment behavior across ordinary, bare, | ||
| 927 | + linked, separate-directory and configured-worktree layouts. This fixes the | ||
| 928 | + adapter bug without claiming a native Gix trust check; GIX-23 remains open. | ||
| 929 | + | ||
| 919 | 930 | The October 22 Windows Gix CI run on Git 2.56.0.windows.2, CPython 3.12.10, | |
| 920 | 931 | and GixPython 0.2.0 exposed an adapter failure in | |
| 921 | 932 | `test_ignored_items_reported_verbatim`: Python's path metadata lookup raises | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -6,7 +6,7 @@ | |||
| 6 | 6 | from typing import TYPE_CHECKING, Tuple, Union, cast | |
| 7 | 7 | ||
| 8 | 8 | from git.cmd import Git | |
| 9 | - from git.exc import GitCommandError, HookExecutionError | ||
| 9 | + from git.exc import GitCommandError, HookExecutionError, UnsafeOptionError | ||
| 10 | 10 | from git.types import PathLike | |
| 11 | 11 | from .typ import BaseIndexEntry | |
| 12 | 12 | ||
@@ -27,10 +27,37 @@ def hook_path(name: str, git_dir: PathLike) -> str: | |||
| 27 | 27 | ||
| 28 | 28 | ||
| 29 | 29 | def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None: | |
| 30 | - """Run a Git commit hook, ignoring missing hooks.""" | ||
| 30 | + """Run a Git commit hook, ignoring missing hooks. | ||
| 31 | + | ||
| 32 | + :raise GitCommandError: If Git rejects the repository before executing its hook. | ||
| 33 | + """ | ||
| 31 | 34 | Git._check_operand(name, "hook name") | |
| 32 | 35 | if name not in ("pre-commit", "commit-msg", "post-commit"): | |
| 33 | 36 | raise ValueError("Only commit hooks are supported") | |
| 37 | + if any("\0" in arg for arg in Git._unpack_args(args)): | ||
| 38 | + raise UnsafeOptionError("Git arguments cannot contain NUL bytes") | ||
| 39 | + command = index.repo.git | ||
| 40 | + # Repo's automatic GIT_DIR bypasses Git's discovery-time ownership check. | ||
| 41 | + one_shot_options, command._git_options = command._git_options, () | ||
| 42 | + options = [*command._persistent_git_options, *one_shot_options] | ||
| 43 | + discovery_env = {} | ||
| 44 | + if ( | ||
| 45 | + "GIT_DIR" in command._repository_environment | ||
| 46 | + and command.environment().get("GIT_DIR") == command._repository_environment["GIT_DIR"] | ||
| 47 | + and not any(option == "-C" or option == "--git-dir" or option.startswith("--git-dir=") for option in options) | ||
| 48 | + ): | ||
| 49 | + options[:0] = ["-C", index.repo._discovery_path] | ||
| 50 | + discovery_env = { | ||
| 51 | + key: None | ||
| 52 | + for key, value in command._repository_environment.items() | ||
| 53 | + if command.environment().get(key) == value | ||
| 54 | + } | ||
| 55 | + command.execute( | ||
| 56 | + [command.GIT_PYTHON_GIT_EXECUTABLE, *options, "rev-parse", "--git-dir"], | ||
| 57 | + env=discovery_env, | ||
| 58 | + _repository_redirected="-C" in options, | ||
| 59 | + ) | ||
| 60 | + command._git_options = one_shot_options | ||
| 34 | 61 | try: | |
| 35 | 62 | index.repo.git._call_process_safe( | |
| 36 | 63 | "hook", | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -375,6 +375,7 @@ def __init__( | |||
| 375 | 375 | checkout = checkout_probe._call_process_safe("rev_parse", "--show-toplevel") | |
| 376 | 376 | ||
| 377 | 377 | self.git_dir = git_dir | |
| 378 | + self._discovery_path = git_dir if candidate == curpath else curpath | ||
| 378 | 379 | probe.update_environment(GIT_DIR=git_dir) | |
| 379 | 380 | try: | |
| 380 | 381 | if native is not NotImplemented: | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -342,18 +342,18 @@ def test_hooks_retain_cli_even_when_missing(repo): | |||
| 342 | 342 | run_commit_hook(name, current.index) | |
| 343 | 343 | with pytest.raises(UnsafeOptionError, match="NUL"): | |
| 344 | 344 | run_commit_hook("commit-msg", repo.index, "message\0path") | |
| 345 | - assert cli.call_count == 6 | ||
| 346 | - assert _backend.statistics().get(("Git.execute", "CLI process"), 0) == count + 6 | ||
| 345 | + assert cli.call_count == 12 | ||
| 346 | + assert _backend.statistics().get(("Git.execute", "CLI process"), 0) == count + 12 | ||
| 347 | 347 | hook = Path(repo.common_dir, "hooks", "pre-commit") | |
| 348 | 348 | hook.parent.mkdir(exist_ok=True) | |
| 349 | 349 | hook.write_text("present, even without executable permission") | |
| 350 | 350 | with patch.object(Git, "execute", return_value="") as cli: | |
| 351 | 351 | run_commit_hook("pre-commit", linked.index) | |
| 352 | - cli.assert_called_once() | ||
| 352 | + assert cli.call_count == 2 | ||
| 353 | 353 | repo.git.config("core.hooksPath", "custom-hooks") | |
| 354 | 354 | with patch.object(Git, "execute", return_value="") as cli: | |
| 355 | 355 | run_commit_hook("post-commit", repo.index) | |
| 356 | - cli.assert_called_once() | ||
| 356 | + assert cli.call_count == 2 | ||
| 357 | 357 | ||
| 358 | 358 | ||
| 359 | 359 | @pytest.mark.parametrize("config_variant", ["normal", "duplicate", "include"]) | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -16,7 +16,7 @@ | |||
| 16 | 16 | import ddt | |
| 17 | 17 | import pytest | |
| 18 | 18 | ||
| 19 | - from git import NULL_TREE, BlobFilter, Diff, Git, IndexFile, Object, Tree | ||
| 19 | + from git import NULL_TREE, BlobFilter, Diff, Git, IndexFile, Object, Repo, Tree | ||
| 20 | 20 | from git.diff import NULL_TREE_SHA | |
| 21 | 21 | from git.exc import ( | |
| 22 | 22 | CheckoutError, | |
@@ -1189,6 +1189,86 @@ def test_pre_commit_hook_outside_worktree(self): | |||
| 1189 | 1189 | output = Path(repo.working_dir, "outside-hook-output.txt").read_text(encoding="utf-8") | |
| 1190 | 1190 | self.assertEqual(output, "ran outside hook\n") | |
| 1191 | 1191 | ||
| 1192 | + def test_pre_commit_hook_not_run_in_repository_git_refuses(self): | ||
| 1193 | + for layout in ("regular", "bare", "linked", "separate", "configured"): | ||
| 1194 | + # Linked worktree registrations contain absolute paths. | ||
| 1195 | + with self.subTest(layout=layout), fixture_repo( | ||
| 1196 | + "hook-ownership.sh", | ||
| 1197 | + layout, | ||
| 1198 | + write=True, | ||
| 1199 | + execute=layout == "linked", | ||
| 1200 | + subdir="metadata" if layout == "configured" else "repo", | ||
| 1201 | + ) as repo: | ||
| 1202 | + output = Path(repo.working_dir, "output.txt") | ||
| 1203 | + discovery = repo.git_dir if layout == "configured" else repo.working_dir | ||
| 1204 | + baseline_output = Path(discovery, "output.txt") | ||
| 1205 | + baseline = Git(discovery) | ||
| 1206 | + # Reset any configured exemptions, as in Git's t0033-safe-directory.sh. | ||
| 1207 | + with mock.patch.dict( | ||
| 1208 | + os.environ, | ||
| 1209 | + { | ||
| 1210 | + "GIT_TEST_ASSUME_DIFFERENT_OWNER": "1", | ||
| 1211 | + "GIT_CONFIG_COUNT": "1", | ||
| 1212 | + "GIT_CONFIG_KEY_0": "safe.directory", | ||
| 1213 | + "GIT_CONFIG_VALUE_0": "", | ||
| 1214 | + }, | ||
| 1215 | + ): | ||
| 1216 | + if layout == "regular": | ||
| 1217 | + with self.assertRaisesRegex(GitCommandError, "dubious ownership"): | ||
| 1218 | + baseline.commit("--allow-empty", "--no-gpg-sign", "-m", "Git refuses this repository") | ||
| 1219 | + self.assertFalse(output.exists()) | ||
| 1220 | + with self.assertRaisesRegex(GitCommandError, "dubious ownership"): | ||
| 1221 | + repo.index.commit("Git refuses this repository") | ||
| 1222 | + self.assertFalse(output.exists()) | ||
| 1223 | + with self.assertRaises(GitCommandError): | ||
| 1224 | + baseline.hook("run", "pre-commit") | ||
| 1225 | + self.assertFalse(baseline_output.exists()) | ||
| 1226 | + with self.assertRaisesRegex(GitCommandError, "dubious ownership"): | ||
| 1227 | + run_commit_hook("pre-commit", repo.index) | ||
| 1228 | + self.assertFalse(output.exists()) | ||
| 1229 | + | ||
| 1230 | + for environment in ( | ||
| 1231 | + {"GIT_CONFIG_VALUE_0": str(discovery)}, | ||
| 1232 | + {"GIT_DIR": str(repo.git_dir), "GIT_WORK_TREE": str(repo.working_dir)}, | ||
| 1233 | + ): | ||
| 1234 | + with baseline.custom_environment(**environment), repo.git.custom_environment(**environment): | ||
| 1235 | + baseline.hook("run", "pre-commit") | ||
| 1236 | + expected = baseline_output.read_text(encoding="utf-8") | ||
| 1237 | + baseline_output.unlink() | ||
| 1238 | + run_commit_hook("pre-commit", repo.index) | ||
| 1239 | + self.assertEqual(output.read_text(encoding="utf-8"), expected) | ||
| 1240 | + output.unlink() | ||
| 1241 | + | ||
| 1242 | + def test_commit_hook_preserves_explicit_repository_selection(self): | ||
| 1243 | + for selection in ("one-shot", "persistent", "environment", "git-dir-option"): | ||
| 1244 | + with self.subTest(selection=selection), fixture_repo("hook-ownership.sh", "regular", write=True) as repo: | ||
| 1245 | + with Repo(repo.git_dir) as metadata: | ||
| 1246 | + index = metadata.index | ||
| 1247 | + baseline = Git(metadata.working_dir) | ||
| 1248 | + metadata.git.version_info | ||
| 1249 | + environment = {"GIT_DIR": ".git"} if selection == "environment" else {} | ||
| 1250 | + with baseline.custom_environment(**environment), metadata.git.custom_environment(**environment): | ||
| 1251 | + for command in (baseline, metadata.git): | ||
| 1252 | + if selection == "one-shot": | ||
| 1253 | + command(C=".git") | ||
| 1254 | + elif selection == "persistent": | ||
| 1255 | + command.set_persistent_git_options(C=".git") | ||
| 1256 | + elif selection == "git-dir-option": | ||
| 1257 | + command(git_dir=".git") | ||
| 1258 | + output = Path(repo.git_dir if selection in ("one-shot", "persistent") else repo.working_dir) | ||
| 1259 | + output /= "output.txt" | ||
| 1260 | + baseline.hook("run", "pre-commit") | ||
| 1261 | + expected = output.read_text(encoding="utf-8") | ||
| 1262 | + output.unlink() | ||
| 1263 | + run_commit_hook("pre-commit", index) | ||
| 1264 | + self.assertEqual(output.read_text(encoding="utf-8"), expected) | ||
| 1265 | + | ||
| 1266 | + metadata.git.set_persistent_git_options() | ||
| 1267 | + metadata.git(C="missing") | ||
| 1268 | + with self.assertRaises(GitCommandError): | ||
| 1269 | + run_commit_hook("pre-commit", index) | ||
| 1270 | + self.assertEqual(metadata.git.rev_parse("--git-dir"), str(metadata.git_dir)) | ||
| 1271 | + | ||
| 1192 | 1272 | def test_pre_commit_hook_fail(self): | |
| 1193 | 1273 | with fixture_repo("index.sh", "hook-fail", write=True) as repo: | |
| 1194 | 1274 | with self.assertRaises(HookExecutionError) as caught: | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,33 @@ | |||
| 1 | + #!/usr/bin/env bash | ||
| 2 | + set -eu | ||
| 3 | + | ||
| 4 | + case "$1" in | ||
| 5 | + bare) | ||
| 6 | + git init --bare --initial-branch=master repo | ||
| 7 | + hooks=repo/hooks | ||
| 8 | + ;; | ||
| 9 | + linked) | ||
| 10 | + git init --initial-branch=master main | ||
| 11 | + git -C main -c commit.gpgsign=false commit --allow-empty -m initial | ||
| 12 | + git -C main worktree add -b linked ../repo | ||
| 13 | + hooks=main/.git/hooks | ||
| 14 | + ;; | ||
| 15 | + separate) | ||
| 16 | + git init --initial-branch=master --separate-git-dir=metadata repo | ||
| 17 | + printf 'gitdir: ../metadata\n' >repo/.git | ||
| 18 | + hooks=metadata/hooks | ||
| 19 | + ;; | ||
| 20 | + configured) | ||
| 21 | + git init --initial-branch=master repo | ||
| 22 | + git -C repo config core.worktree ../repo | ||
| 23 | + mv repo/.git metadata | ||
| 24 | + hooks=metadata/hooks | ||
| 25 | + ;; | ||
| 26 | + regular) | ||
| 27 | + git init --initial-branch=master repo | ||
| 28 | + hooks=repo/.git/hooks | ||
| 29 | + ;; | ||
| 30 | + *) exit 1 ;; | ||
| 31 | + esac | ||
| 32 | + printf "#!/usr/bin/env sh\necho 'ran hook' >output.txt\n" >"$hooks/pre-commit" | ||
| 33 | + chmod +x "$hooks/pre-commit" | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments