Skip to content

Fix version_info cache invalidation, typing, parsing, and serialization #1838

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
Feb 23, 2024
Merged
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix Git.version_info pickling
For #1836.

This uses a property with the same logic as before for version_info
caching, except that the _version_info backing attribute holds the
value None, rather than being unset, for the uncomputed state. This
rectifies the inconistency between the property's behavior and the
way the associated states are represented by its __setstate__ and
__getstate__ methods for pickling.

Because the Git class only used LazyMixin as an implementation
detail of the version_info attribute, it is removed as a subclass.
  • Loading branch information
EliahKagan committed Feb 22, 2024
commit f699a387e19764e591dbca392035a5f9e457b06d
32 changes: 14 additions & 18 deletions git/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
UnsafeProtocolError,
)
from git.util import (
LazyMixin,
cygpath,
expand_path,
is_cygwin_git,
Expand Down Expand Up @@ -287,7 +286,7 @@ def dict_to_slots_and__excluded_are_none(self: object, d: Mapping[str, Any], exc
## -- End Utilities -- @}


class Git(LazyMixin):
class Git:
"""The Git class manages communication with the Git binary.

It provides a convenient interface to calling the Git binary, such as in::
Expand Down Expand Up @@ -784,6 +783,7 @@ def __init__(self, working_dir: Union[None, PathLike] = None):
self._environment: Dict[str, str] = {}

# Cached command slots
self._version_info: Union[Tuple[int, int, int, int], None] = None
self.cat_file_header: Union[None, TBD] = None
self.cat_file_all: Union[None, TBD] = None

Expand All @@ -795,8 +795,8 @@ def __getattr__(self, name: str) -> Any:
Callable object that will execute call :meth:`_call_process` with
your arguments.
"""
if name[0] == "_":
return LazyMixin.__getattr__(self, name)
if name.startswith("_"):
return super().__getattribute__(name)
return lambda *args, **kwargs: self._call_process(name, *args, **kwargs)

def set_persistent_git_options(self, **kwargs: Any) -> None:
Expand All @@ -811,20 +811,6 @@ def set_persistent_git_options(self, **kwargs: Any) -> None:

self._persistent_git_options = self.transform_kwargs(split_single_char_options=True, **kwargs)

def _set_cache_(self, attr: str) -> None:
if attr == "_version_info":
# We only use the first 4 numbers, as everything else could be strings in fact (on Windows).
process_version = self._call_process("version") # Should be as default *args and **kwargs used.
version_numbers = process_version.split(" ")[2]

self._version_info = cast(
Tuple[int, int, int, int],
tuple(int(n) for n in version_numbers.split(".")[:4] if n.isdigit()),
)
else:
super()._set_cache_(attr)
# END handle version info

@property
def working_dir(self) -> Union[None, PathLike]:
""":return: Git directory we are working on"""
Expand All @@ -838,6 +824,16 @@ def version_info(self) -> Tuple[int, int, int, int]:

This value is generated on demand and is cached.
"""
if self._version_info is None:
# We only use the first 4 numbers, as everything else could be strings in fact (on Windows).
process_version = self._call_process("version") # Should be as default *args and **kwargs used.
version_numbers = process_version.split(" ")[2]

self._version_info = cast(
Tuple[int, int, int, int],
tuple(int(n) for n in version_numbers.split(".")[:4] if n.isdigit()),
)

return self._version_info

@overload
Expand Down