Skip to content

Commit 8b8f944

Browse files
codexByron
authored andcommitted
refactor(gix): open fresh repository state for each operation
Retained native repositories required file and environment fingerprints, conditional reloads and a per-Repo refresh lock, while included configuration still needed unconditional refresh. Open a new `gix.Repository` through the shared accessor for each operation and remove `_gix_state`, the cached handle, the refresh lock and CLI-triggered invalidation. Keep execution-only configuration overrides out of configuration queries and preserve storage compatibility fallbacks. Update pickling, cleanup assertions and documentation for operation-local native handles. Reopening repeats native initialization; the historical retained-handle timings do not measure this policy. The ten focused Gix lifetime/discovery cases pass. Cleanup, fixture lifetime and the documented close-before-removal example pass on both backends (27 tests and one skip each), with localhost access for the Git daemon cases. Ruff lint and format checks pass. Benchmark timings were not rerun.
1 parent 00f05f7 commit 8b8f944

10 files changed

Lines changed: 70 additions & 149 deletions

File tree

‎README.md‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,9 @@ bindings to execute supported operations through Gitoxide while exposing GitPyth
1111
high-level repository, commit, tree and index interfaces.
1212

1313
Supported object reads, tree traversal, reference/index queries and commit history
14-
can run without launching Git subprocesses. GitPython also retains and reuses a Gix
15-
repository handle across operations, reducing repeated initialization work. These
16-
changes reduce subprocess overhead, particularly for repeated repository queries;
17-
the [backend benchmarks](test/performance/README.md) measure results and CLI launch
18-
counts for both backends.
14+
can run without launching Git subprocesses. Each native operation opens a fresh
15+
Gix repository handle. The [backend benchmarks](test/performance/README.md) measure
16+
results and CLI launch counts for both backends.
1917

2018
### Activate the Gix backend
2119

‎doc/gix-backend.md‎

Lines changed: 32 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -453,9 +453,9 @@ environment metadata.
453453
operations and their complete journey on one already-open `git.Repo`, plus
454454
separate direct opening and discovery from `git/objects`. Fresh high-level
455455
wrappers preserve the cost of actual operations; imports, fixture preparation
456-
and parity preflight are outside timing. Native repository refresh remains
457-
inside operation timing, so these measurements include the cost of keeping
458-
retained state current.
456+
and parity preflight are outside timing. Native repository opening remains inside
457+
operation timing. Historical retained-handle results below included refresh
458+
costs instead.
459459

460460
Submodule inventory, revision path/mode lookup and raw commit/tree readback
461461
extend the already-open journey, each with a zero-CLI ceiling. The readback
@@ -632,8 +632,8 @@ used Gix; CLI validation used the affected selections. Repository-wide Ruff
632632
lint/format, mypy and basedpyright passed. The benchmark comparison independently
633633
checked result parity, fixture/source identity and all process ceilings.
634634
Generic configuration and standalone reference-name gaps remain documented
635-
under GIX-12/20. `Repo._get_gix_repository(recreate=True)` still explicitly
636-
recreates retained native state; its snapshot limitations are documented below.
635+
under GIX-12/20. That snapshot retained native repository state; the current
636+
accessor opens a fresh handle for each operation, as documented below.
637637

638638
Raw benchmarks, comparison tables and logs from that snapshot are under
639639
`.cache/gix-only-audit/benchmark-*` and `.cache/gix-only-audit/pyperf-comparison.md`.
@@ -785,39 +785,34 @@ expected failure. Operation reporting counted 114,800 native operations and
785785
39,002 CLI fallback decisions. Those decisions are not a complete subprocess
786786
count: raw `repo.git` calls and subprocesses started by Git are not all counted.
787787

788-
### Retain native repository state (implemented)
789-
790-
Each `git.Repo` owns a native `gix.Repository`. Managed operations access it through
791-
`Repo._get_gix_repository()`, which reuses the instance by default.
792-
`Repo._get_gix_repository(recreate=True)` replaces it explicitly; this method is
793-
the control point for a future configurable reuse policy. Managed operations reuse it;
794-
`close()` releases it, and a later operation can reopen it. Pickling excludes
795-
native resources and restores the command's weak owner reference. Gix provides
796-
thread-safe handle access and automatic index/ODB snapshot refresh. A per-Repo
797-
lock serializes handle refresh, without serializing read operations.
798-
799-
Retaining native state is an intentional deviation from Git's fresh process per
800-
command. Configuration/environment and observed CLI changes trigger best-effort
801-
refresh, but this is not a promise that every external filesystem change is
802-
immediately visible in every native cache. Explicit recreation or closing and
803-
reopening the `Repo` provides fresh native state. Configuration queries currently
804-
use a separate fresh handle because the retained execution handle overrides
805-
hooks, fsmonitor and automatic maintenance; those overrides must not leak into
806-
configuration results. The accessor itself removes no CLI calls: ten supported
807-
metadata queries launch zero processes before and after this refactor.
808-
809-
Configuration and storage metadata changes, environment changes and raw CLI
810-
launches cause `reload()` before reuse. Configuration queries use a separate
811-
fresh handle so synthetic safety settings never appear as user settings.
812-
Configurations with includes conservatively reload on each operation because
813-
the binding does not expose all included source paths. Storage overrides,
814-
reftable and compatibility object formats retain their existing CLI guards.
788+
### Open fresh native repository state
789+
790+
Managed operations access Gix through `Repo._get_gix_repository()`, which opens
791+
a fresh `gix.Repository` for each call. Standalone commands use the same opening
792+
path. No native handle, environment/file fingerprint or refresh lock is cached
793+
on `Repo`; configuration, includes and storage metadata are read by Gix again
794+
on each open. Concurrent operations therefore receive independent handles.
795+
796+
Execution handles override hooks, fsmonitor and automatic maintenance.
797+
Configuration queries omit those overrides so synthetic settings do not appear
798+
as user settings. Storage overrides, reftable and compatibility object formats
799+
retain their existing CLI guards. Reopening itself adds no Git CLI calls.
800+
801+
`Repo.close()` releases persistent CLI object readers. Native handles remain
802+
local to operations; callers holding separately opened handles or iterators
803+
must release them before deleting their repository, especially on Windows.
804+
Pickling restores the command's weak owner reference without native resources.
805+
806+
This replaces the retained-handle policy and its best-effort invalidation via
807+
file metadata, environment changes and CLI launches. Reopening repeats native
808+
initialization work but avoids fingerprint construction and unconditional
809+
reloads for configurations with includes.
815810

816811
An earlier 1,000-call benchmark without coverage measured an object read using
817-
one native handle at 0.046 ms per call, compared with 0.635 ms through the
818-
fresh-open stream adapter. Opening and validating a native repository alone
819-
took 0.418 ms per call. These small repeated-read measurements do not estimate
820-
whole-suite speedup.
812+
one retained native handle at 0.046 ms per call, compared with 0.635 ms through
813+
the fresh-open stream adapter. Opening and validating a native repository alone
814+
took 0.418 ms per call. Those historical measurements do not describe the current
815+
accessor or estimate whole-suite speedup; rerun the backend harness to measure it.
821816

822817
### Reuse prepared test fixtures (implemented)
823818

@@ -1230,8 +1225,7 @@ can use Git to preserve its public diagnostics. Once `_write()` begins,
12301225
`gix.Error` failures become `GitCommandError`; other exceptions propagate.
12311226
Neither may trigger a second CLI mutation.
12321227
Access native repository state through `Repo._get_gix_repository()` for bound
1233-
commands. Reuse is the default, with explicit recreation available; keep the
1234-
snapshot deviation and refresh behavior above documented. Close native iterators
1228+
commands. Each access opens a fresh handle through Gix. Close native iterators
12351229
when partially consumed.
12361230

12371231
Add each conversion with a no-subprocess check and a Git parity check where

‎git/_backend.py‎

Lines changed: 2 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -144,56 +144,17 @@ def _open_repository(command: Any, env: Dict[str, Any], *, query_config: bool =
144144
options = options.config_overrides(
145145
["core.fsmonitor=false", "gc.auto=0", "maintenance.auto=false", "core.hooksPath=" + os.devnull]
146146
)
147-
owner = command._repo() if command._repo is not None else None
148-
state = None
149-
repo = None
150-
if owner is not None and not query_config:
151-
# Gix refreshes index/ODB snapshots itself; configuration is loaded at open.
152-
paths = [
153-
path,
154-
os.fspath(owner.common_dir),
155-
os.path.join(owner.common_dir, "config"),
156-
os.path.join(path, "config.worktree"),
157-
]
158-
paths += [
159-
effective.get("GIT_CONFIG_SYSTEM", "/etc/gitconfig"),
160-
effective.get("GIT_CONFIG_GLOBAL", os.path.expanduser("~/.gitconfig")),
161-
os.path.join(effective.get("XDG_CONFIG_HOME", os.path.expanduser("~/.config")), "git", "config"),
162-
os.path.join(owner.common_dir, "objects", "info", "alternates"),
163-
os.path.join(path, "commondir"),
164-
os.path.join(path, "gitdir"),
165-
]
166-
stamps: List[Any] = []
167-
for filename in paths:
168-
try:
169-
info = os.stat(filename)
170-
stamps.append((info.st_dev, info.st_ino, info.st_size, info.st_mtime_ns, info.st_ctime_ns))
171-
except FileNotFoundError:
172-
stamps.append(None)
173-
state = (path, tuple(sorted(effective.items())), tuple(stamps))
174-
repo = owner._gix_repository
175-
if repo is not None and os.path.realpath(repo.git_dir()) != os.path.realpath(path):
176-
repo = None
177-
if repo is not None and state != owner._gix_state:
178-
repo.reload()
179-
if repo is None:
180-
repo = gix.open_opts(path, options)
147+
repo = gix.open_opts(path, options)
181148
snapshot = repo.config_snapshot()
182149
if snapshot.string("extensions.refStorage") == b"reftable":
183150
raise _Unsupported("reftable (GIX-1)")
184151
if snapshot.string("extensions.compatObjectFormat") is not None:
185152
raise _Unsupported("compatibility object format (GIX-19)")
186-
if effective.get("GIT_WORK_TREE") and (owner is None or query_config or state != owner._gix_state):
153+
if effective.get("GIT_WORK_TREE"):
187154
workdir = effective["GIT_WORK_TREE"]
188155
if not os.path.isabs(workdir):
189156
workdir = os.path.join(command.working_dir or os.getcwd(), workdir)
190157
repo.set_workdir(workdir)
191-
if owner is not None and not query_config:
192-
owner._gix_repository = repo
193-
# ponytail: includes can load arbitrary files; reopen until Gix exposes their source paths.
194-
if state != owner._gix_state:
195-
included = re.search(rb"\[include(?:if)?[\s\]]", repo.config_snapshot().plumbing().to_bstring(), re.I)
196-
owner._gix_state = None if included else state
197158
return repo
198159

199160

‎git/cmd.py‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,7 +1166,7 @@ def __init__(self, working_dir: Union[None, PathLike] = None) -> None:
11661166
# Extra environment variables to pass to git commands
11671167
self._environment: Dict[str, Optional[str]] = {}
11681168
self._repository_environment: Dict[str, Optional[str]] = {}
1169-
self._repo: Any = None # Weak reference; the Repo owns native resources.
1169+
self._repo: Any = None # Weak reference to the Repo's native repository accessor.
11701170

11711171
# Cached version slots
11721172
self._version_info = None
@@ -1587,9 +1587,6 @@ def execute(
15871587
raise GitCommandNotFound(redacted_command, err) from err
15881588
else:
15891589
_backend.record("Git.execute", "CLI process")
1590-
owner = self._repo() if self._repo is not None else None
1591-
if owner is not None:
1592-
owner._gix_state = None
15931590
# Replace with a typeguard for Popen[bytes]?
15941591
proc.stdout = cast(BinaryIO, proc.stdout)
15951592
proc.stderr = cast(BinaryIO, proc.stderr)

‎git/repo/base.py‎

Lines changed: 11 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import shlex
1717
import sys
1818
import tempfile
19-
from threading import RLock
2019
import weakref
2120

2221
import gitdb
@@ -125,8 +124,6 @@ class Repo:
125124

126125
# Must exist, or __del__ will fail in case we raise on `__init__()`.
127126
git = cast("Git", None)
128-
_gix_repository: Any = None
129-
_gix_state: Any = None
130127

131128
working_dir: PathLike
132129
"""The working directory of the git command."""
@@ -460,7 +457,6 @@ def __init__(
460457
break
461458

462459
self.working_dir = self._working_tree_dir or self.common_dir
463-
self._gix_lock = RLock()
464460
self.git = self.GitCommandWrapperType(self.working_dir)
465461
self.git._repo = weakref.ref(self)
466462
self.git._environment.update(GIT_DIR=git_dir, **environment)
@@ -471,7 +467,6 @@ def __init__(
471467
if "GIT_WORK_TREE" not in environment:
472468
self.git._repository_environment["GIT_WORK_TREE"] = os.fspath(self._working_tree_dir)
473469
if native is not NotImplemented:
474-
self._gix_repository = native
475470
self._empty_tree_hexsha = str(native.empty_tree().id)
476471
else:
477472
with tempfile.TemporaryFile() as empty:
@@ -494,32 +489,18 @@ def _get_gix_repository(
494489
command: Optional[Git] = None,
495490
env: Optional[Dict[str, Any]] = None,
496491
query_config: bool = False,
497-
recreate: bool = False,
498492
) -> Any:
499-
"""Access native state, reusing the repository unless recreation is requested.
493+
"""Open a fresh native repository for each operation.
500494
501-
This method owns the reuse policy so it can later become configurable.
502-
Configuration queries currently open a separate handle without the execution
503-
restrictions applied to the retained handle. Other operations retain the
504-
existing best-effort refresh of configuration, environment and CLI changes.
495+
Configuration queries omit execution restrictions so synthetic settings
496+
do not appear in configuration results.
505497
"""
506-
with self._gix_lock:
507-
if recreate:
508-
self._gix_repository = self._gix_state = None
509-
return _backend._open_repository(
510-
command if command is not None else self.git, env or {}, query_config=query_config
511-
)
512-
513-
def __getstate__(self) -> Dict[str, Any]:
514-
return {
515-
key: value
516-
for key, value in self.__dict__.items()
517-
if key not in ("_gix_repository", "_gix_state", "_gix_lock")
518-
}
498+
return _backend._open_repository(
499+
command if command is not None else self.git, env or {}, query_config=query_config
500+
)
519501

520502
def __setstate__(self, state: Dict[str, Any]) -> None:
521503
self.__dict__.update(state)
522-
self._gix_lock = RLock()
523504
self.git._repo = weakref.ref(self)
524505

525506
def __enter__(self) -> "Repo":
@@ -535,14 +516,14 @@ def __del__(self) -> None:
535516
pass
536517

537518
def close(self) -> None:
538-
"""Release native repository state and persistent Git object readers.
519+
"""Release persistent Git object readers.
539520
540521
Use ``with Repo(path) as repo:`` to close automatically, including on errors.
541-
Close every repository instance and any separately opened readers before
542-
removing its directory, especially on Windows where open handles prevent
543-
deletion. :func:`git.util.rmtree` also handles Git's read-only files.
522+
Close every repository instance and release any separately opened native
523+
handles or readers before removing its directory, especially on Windows
524+
where open handles prevent deletion. :func:`git.util.rmtree` also handles
525+
Git's read-only files.
544526
"""
545-
self._gix_repository = self._gix_state = None
546527
if self.git:
547528
self.git.clear_cache()
548529
# Tempfiles objects on Windows are holding references to open files until

‎test/performance/README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ instance open per worker, but creates fresh commit/tree/index wrappers per
77
invocation so their cached properties cannot turn the measurement into a no-op.
88
The CLI's persistent `cat-file` processes and the filesystem cache are warmed.
99
Repository creation, imports, preflight and result hashing are outside timing.
10-
Native handle refresh and compatibility checks remain inside timing, exposing
11-
the benefit of retaining a Gix repository across operations. Opening and
10+
Native repository opening and compatibility checks remain inside operation
11+
timing; each native operation opens a fresh Gix handle. Opening and
1212
discovery retain explicit CLI fallbacks tracked in `cli-budget.json`.
1313
`open_repository` and `discover_repository` instead create and close a new
1414
`Repo` in each timed invocation; they do not join the already-open journey.

‎test/test_cleanup.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ def case(self, *resources):
229229
assert (directory / "locked").exists()
230230
assert not (directory / "removable").exists()
231231
for repo in repositories:
232-
assert repo._gix_repository is None
232+
assert repo.git.cat_file_header is repo.git.cat_file_all is None
233233

234234

235235
def test_directory_decorator_preserves_failed_test_artifacts(tmp_path, monkeypatch):

‎test/test_docs.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ def test_repo_context_releases_resources_before_removal():
2424
assert repo.head.commit.tree["file"].data_stream.read() == b"contents\n"
2525
processes = [reader.proc for reader in (repo.git.cat_file_header, repo.git.cat_file_all) if reader]
2626

27-
assert repo._gix_repository is None
2827
assert repo.git.cat_file_header is repo.git.cat_file_all is None
2928
assert all(process.poll() is not None for process in processes)
3029
# [remove-repo-directory]

‎test/test_fixture_health.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ def test_scripted_writable_fixture_copies_and_closes_repository():
3232
(root / "file").write_text("private")
3333
repo.index.add(["file"])
3434
repo.index.commit("Private change", skip_hooks=True)
35-
assert repo._gix_repository is None
3635
assert not root.exists()
3736
assert (baseline / "repo/file").read_text() == "initial"
3837

0 commit comments

Comments
 (0)