From 474d65d90d19f3e823eae33e39f20fb79cd6f4ae Mon Sep 17 00:00:00 2001 From: Nick Williams Date: Wed, 1 Jul 2026 12:59:38 -0500 Subject: [PATCH 01/13] Add support for an in-memory APP-level config backend that can be reset `Config` now has two subclasses: `DefaultConfig` and `RepositoryConfig`: - `DefaultConfig` represents the total sum config of program, system, XDG, and global (user) configurations as obtained from `git_config_open_default`. Even if none of those files exist, one can still create an empty `DefaultConfig`. - `RepositoryConfig` represents repository configurations and is the type now returned by `Repository.config` and `Repository.config_snapshot`. - `RepositoryConfig` contains the ability to enter/exit as a context manager, enabling an in-memory config backend with level `GIT_CONFIG_LEVEL_APP`. This allows users to add temporary, in-memory overrides to the repository config that affect repository operations without persisting permanently to the repository's config file. --- pygit2/__init__.py | 8 +- pygit2/_libgit2/ffi.pyi | 147 ++++- pygit2/_run.py | 12 +- pygit2/callbacks.py | 4 +- pygit2/config.py | 1202 +++++++++++++++++++++++++++++++++-- pygit2/decl/config.h | 112 ++++ pygit2/decl/config_bridge.h | 35 + pygit2/errors.py | 13 +- pygit2/repository.py | 33 +- test/test_config.py | 176 ++++- 10 files changed, 1655 insertions(+), 87 deletions(-) create mode 100644 pygit2/decl/config_bridge.h diff --git a/pygit2/__init__.py b/pygit2/__init__.py index 8d4346ff..d1988fc4 100644 --- a/pygit2/__init__.py +++ b/pygit2/__init__.py @@ -304,7 +304,11 @@ git_fetch_options, git_proxy_options, ) -from .config import Config +from .config import ( + Config, + DefaultConfig, + RepositoryConfig, +) from .credentials import * from .errors import Passthrough, check_error from .ffi import C, ffi @@ -897,6 +901,7 @@ def filter_unregister(name: str) -> None: 'get_credentials', 'config', 'Config', + 'DefaultConfig', 'credentials', 'CredentialType', 'Username', @@ -921,6 +926,7 @@ def filter_unregister(name: str) -> None: 'Remote', 'repository', 'Repository', + 'RepositoryConfig', 'branches', 'references', 'settings', diff --git a/pygit2/_libgit2/ffi.pyi b/pygit2/_libgit2/ffi.pyi index 690abf2b..791e4b58 100644 --- a/pygit2/_libgit2/ffi.pyi +++ b/pygit2/_libgit2/ffi.pyi @@ -22,9 +22,21 @@ # along with this program; see the file COPYING. If not, write to # the Free Software Foundation, 51 Franklin Street, Fifth Floor, # Boston, MA 02110-1301, USA. - -from typing import Any, Generic, Literal, NewType, SupportsIndex, TypeVar, overload - +from __future__ import annotations + +from collections.abc import Callable +from typing import ( + Any, + Generic, + Literal, + NewType, + ParamSpec, + SupportsIndex, + TypeVar, + overload, +) + +P = ParamSpec('P') T = TypeVar('T') NULL_TYPE = NewType('NULL_TYPE', object) @@ -45,6 +57,9 @@ class int64_t: class ssize_t: def __getitem__(self, item: Literal[0]) -> int: ... +class uintptr_t: + def __int__(self) -> int: ... + class _Pointer(Generic[T]): def __setitem__(self, item: Literal[0], a: T) -> None: ... @overload @@ -181,15 +196,65 @@ class GitConfigC: pass class GitConfigIteratorC: - # incomplete - pass + backend: 'GitConfigBackendC' + flags: int + next: Callable[['GitConfigBackendEntryC', GitConfigIteratorC], int] + free: Callable[[GitConfigIteratorC], None] class GitConfigEntryC: # incomplete name: char_pointer value: char_pointer + backend_type: char_pointer + origin_path: char_pointer + include_depth: int level: int +class GitConfigBackendEntryC: + entry: GitConfigEntryC + free: Callable[[GitConfigBackendEntryC], None] + +class GitConfigBackendC: + version: int + readonly: int + cfg: GitConfigC + open: Callable[[GitConfigBackendC, int, GitRepositoryC], int] + get: Callable[ + [GitConfigBackendC, char_pointer, _Pointer[GitConfigBackendEntryC]], int + ] + set: Callable[[GitConfigBackendC, char_pointer, char_pointer], int] + set_multivar: Callable[ + [GitConfigBackendC, char_pointer, char_pointer, char_pointer], int + ] + del_: Callable[[GitConfigBackendC, char_pointer], int] + del_multivar: Callable[[GitConfigBackendC, char_pointer, char_pointer], int] + iterator: Callable[[_Pointer[GitConfigIteratorC], GitConfigBackendC], int] + snapshot: Callable[[_Pointer[GitConfigBackendC], GitConfigBackendC], int] + lock: Callable[[GitConfigBackendC], int] + unlock: Callable[[GitConfigBackendC, int], int] + free: Callable[[GitConfigBackendC], None] + +class GitConfigBackendMemoryOptionsC: + version: int + backend_type: char_pointer + origin_path: char_pointer + +class PyGitConfigBackendWrapperC: + parent: GitConfigBackendC + self: Any + +class PyGitConfigBackendEntryC: + parent: GitConfigBackendEntryC + owner: PyGitConfigBackendWrapperC + +class PyGitConfigIteratorWrapperC: + parent: GitConfigIteratorC + self: Any + +class PyGitConfigIteratorEntryC: + parent: GitConfigBackendEntryC + owner: PyGitConfigIteratorWrapperC + class GitDescribeFormatOptionsC: version: int abbreviated_size: int @@ -334,10 +399,38 @@ def new(a: Literal['git_config *']) -> GitConfigC: ... @overload def new(a: Literal['git_config **']) -> _Pointer[GitConfigC]: ... @overload +def new(a: Literal['git_config_iterator *']) -> GitConfigIteratorC: ... +@overload def new(a: Literal['git_config_iterator **']) -> _Pointer[GitConfigIteratorC]: ... @overload def new(a: Literal['git_config_entry **']) -> _Pointer[GitConfigEntryC]: ... @overload +def new(a: Literal['git_config_backend *']) -> GitConfigBackendC: ... +@overload +def new(a: Literal['git_config_backend **']) -> _Pointer[GitConfigBackendC]: ... +@overload +def new(a: Literal['git_config_backend_entry *']) -> GitConfigBackendEntryC: ... +@overload +def new( + a: Literal['_pygit_in_memory_backend_entry *'], +) -> PyGitConfigBackendEntryC: ... +@overload +def new( + a: Literal['_pygit_in_memory_backend_iterator_entry *'], +) -> PyGitConfigIteratorEntryC: ... +@overload +def new( + a: Literal['git_config_backend_memory_options *'], +) -> GitConfigBackendMemoryOptionsC: ... +@overload +def new(a: Literal['git_config_level_t[]'], b: list[int]) -> list[int]: ... +@overload +def new(a: Literal['_pygit_in_memory_backend *']) -> PyGitConfigBackendWrapperC: ... +@overload +def new( + a: Literal['_pygit_in_memory_backend_iterator *'], +) -> PyGitConfigIteratorWrapperC: ... +@overload def new(a: Literal['git_describe_format_options *']) -> GitDescribeFormatOptionsC: ... @overload def new(a: Literal['git_describe_options *']) -> GitDescribeOptionsC: ... @@ -399,7 +492,12 @@ def new( def new( a: Literal['char *[]'], b: list[Any] ) -> ArrayC[char_pointer]: ... # For string arrays +@overload +def addressof(a: object) -> _Pointer[object]: ... +@overload def addressof(a: object, attribute: str) -> _Pointer[object]: ... +def def_extern() -> Callable[[Callable[P, T]], Callable[P, T]]: ... +def from_handle(a: _Pointer[T]) -> T: ... def new_handle(a: T) -> _Pointer[T]: ... class buffer(bytes): @@ -420,3 +518,42 @@ def cast(a: Literal['size_t'], b: object) -> int: ... def cast(a: Literal['ssize_t'], b: object) -> int: ... @overload def cast(a: Literal['char *'], b: object) -> char_pointer: ... +@overload +def cast(a: Literal['uintptr_t'], b: object) -> uintptr_t: ... +@overload +def cast(a: Literal['git_config_level_t'], b: int) -> int: ... +@overload +def cast( + a: Literal['git_config_backend *'], + b: PyGitConfigBackendWrapperC, +) -> GitConfigBackendC: ... +@overload +def cast( + a: Literal['_pygit_in_memory_backend *'], + b: GitConfigBackendC, +) -> PyGitConfigBackendWrapperC: ... +@overload +def cast( + a: Literal['git_config_iterator *'], + b: PyGitConfigIteratorWrapperC, +) -> GitConfigIteratorC: ... +@overload +def cast( + a: Literal['_pygit_in_memory_backend_iterator *'], + b: GitConfigIteratorC, +) -> PyGitConfigIteratorWrapperC: ... +@overload +def cast( + a: Literal['git_config_backend_entry *'], + b: PyGitConfigBackendEntryC | PyGitConfigIteratorEntryC, +) -> GitConfigBackendEntryC: ... +@overload +def cast( + a: Literal['_pygit_in_memory_backend_entry *'], + b: GitConfigBackendEntryC, +) -> PyGitConfigBackendEntryC: ... +@overload +def cast( + a: Literal['_pygit_in_memory_backend_iterator_entry *'], + b: GitConfigBackendEntryC, +) -> PyGitConfigIteratorEntryC: ... diff --git a/pygit2/_run.py b/pygit2/_run.py index 85d31f69..e16f99bc 100644 --- a/pygit2/_run.py +++ b/pygit2/_run.py @@ -24,11 +24,10 @@ # Boston, MA 02110-1301, USA. """ -This is an special module, it provides stuff used by by pygit2 at run-time. +This is a special module, it provides stuff used by pygit2 at run-time. """ # Import from the Standard Library -import codecs import sys from pathlib import Path @@ -69,6 +68,7 @@ 'clone.h', 'common.h', 'config.h', + 'config_bridge.h', 'describe.h', 'errors.h', 'graph.h', @@ -88,17 +88,17 @@ ] h_source = [] for h_file in h_files: - h_file = dir_path / 'decl' / h_file # type: ignore - with codecs.open(h_file, 'r', 'utf-8') as f: - h_source.append(f.read()) + h_path = dir_path / 'decl' / h_file + h_source.append(h_path.read_text(encoding='utf-8')) C_HEADER_SRC = '\n'.join(h_source) C_PREAMBLE = """\ #include +#include #include #include -""" +""" + (dir_path / 'decl' / 'config_bridge.h').read_text(encoding='utf-8') # ffi _, libgit2_kw = get_libgit2_paths() diff --git a/pygit2/callbacks.py b/pygit2/callbacks.py index 1072f8cf..89808cc4 100644 --- a/pygit2/callbacks.py +++ b/pygit2/callbacks.py @@ -557,7 +557,7 @@ def wrapper(*args): data._stored_exception = e return C.GIT_EUSER - return ffi.def_extern()(wrapper) # type: ignore[attr-defined] + return ffi.def_extern()(wrapper) # type: ignore[arg-type] def libgit2_callback_void(f: Callable[P, T]) -> Callable[P, T]: @@ -577,7 +577,7 @@ def wrapper(*args): data._stored_exception = e pass # Function returns void, so we can't do much here. - return ffi.def_extern()(wrapper) # type: ignore[attr-defined] + return ffi.def_extern()(wrapper) # type: ignore[arg-type] @libgit2_callback diff --git a/pygit2/config.py b/pygit2/config.py index e7522ffe..68fc04c5 100644 --- a/pygit2/config.py +++ b/pygit2/config.py @@ -22,10 +22,15 @@ # along with this program; see the file COPYING. If not, write to # the Free Software Foundation, 51 Franklin Street, Fifth Floor, # Boston, MA 02110-1301, USA. +from __future__ import annotations -from collections.abc import Callable, Iterator +import contextlib +import re +import threading +from collections.abc import Callable, Generator, Iterator from os import PathLike -from typing import TYPE_CHECKING, Optional +from types import TracebackType +from typing import TYPE_CHECKING, Literal, Self, cast, overload, override try: from functools import cached_property @@ -33,12 +38,26 @@ from cached_property import cached_property # type: ignore # Import from pygit2 +from .enums import ConfigLevel from .errors import check_error from .ffi import C, ffi from .utils import to_bytes if TYPE_CHECKING: - from ._libgit2.ffi import GitConfigC, GitConfigEntryC + from ._libgit2.ffi import ( + GitConfigBackendC, + GitConfigBackendEntryC, + GitConfigC, + GitConfigEntryC, + GitConfigIteratorC, + GitRepositoryC, + PyGitConfigBackendEntryC, + PyGitConfigBackendWrapperC, + PyGitConfigIteratorEntryC, + PyGitConfigIteratorWrapperC, + _Pointer, + char_pointer, + ) from .repository import BaseRepository @@ -64,9 +83,10 @@ def __next__(self) -> 'ConfigEntry': return self._next_entry() def _next_entry(self) -> 'ConfigEntry': + self._config._stored_exception = None centry = ffi.new('git_config_entry **') err = C.git_config_next(centry, self._iter) - check_error(err) + check_error(err, user_exception=self._config._stored_exception) return ConfigEntry._from_c(centry[0], self) @@ -98,38 +118,72 @@ class Config: might change mid-iteration if you don't use a snapshot. This class can technically be used to manually read and write a repository's - configuration file by pointing the constructor to the repository's + local configuration by pointing the constructor to the repository's ``.git/config`` file, but this is not recommended. The resulting ``Config`` object represents only the configuration directly within ``.git/config``. It does not represent the total effective configuration for that repository - that includes the combined system, global (user), and global (user) XDG. - Instead, use :meth:`BaseRepository.config` for loading a repository's - configuration. + that includes the combined program, system, XDG, global (user), and local + configurations. Instead, use :meth:`BaseRepository.config` or + :meth:`BaseRepository.config_snapshot` for loading a local configuration + and see :class:`RepositoryConfig` for special behaviors supported by the + local configuration. """ - _repo: Optional['BaseRepository'] _config: 'GitConfigC' - def __init__(self, path: PathLike | str | None = None) -> None: - cconfig = ffi.new('git_config **') + @overload + def __init__(self, /): + """Constructs a new, empty ``Config`` object pointing to no file. - if not path: - err = C.git_config_new(cconfig) - else: - path_bytes = to_bytes(path) - err = C.git_config_open_ondisk(cconfig, path_bytes) + To persist changes made to this ``Config`` object, you must use + :meth:`Config.add_file`. + """ + ... - check_error(err, io=True) - self._repo = None - self._config = cconfig[0] + @overload + def __init__(self, path: PathLike | str, /) -> None: + """Constructs a ``Config`` object backed by the specified file. - @classmethod - def from_c(cls, repo: Optional['BaseRepository'], ptr: 'GitConfigC') -> 'Config': - config = cls.__new__(cls) - config._repo = repo - config._config = ptr + The configuration from the specified file is loaded, and subsequent writes + will persist to that file. Additional files can be added to the config, + with different levels, using :meth:`Config.add_file`. + """ + ... + + @overload + def __init__(self, *, c_config: 'GitConfigC', is_snapshot: bool) -> None: + """For internal use only. + + Constructs a ``Config`` object from a config object pointer. + """ + ... + + def __init__( + self, + path: PathLike | str | None = None, + *, + c_config: 'GitConfigC | None' = None, + is_snapshot: bool = False, + ) -> None: + if path is not None and c_config is not None: + raise ValueError('Cannot initialize Config from both path and c_config') + + self._is_snapshot = is_snapshot + self._stored_exception: BaseException | None = None + + if c_config is not None: + self._config = c_config + else: + cconfig = ffi.new('git_config **') + + if not path: + err = C.git_config_new(cconfig) + else: + path_bytes = to_bytes(path) + err = C.git_config_open_ondisk(cconfig, path_bytes) - return config + check_error(err, io=True) + self._config = cconfig[0] def __del__(self) -> None: try: @@ -137,30 +191,42 @@ def __del__(self) -> None: except AttributeError: pass - def _get(self, key: str | bytes) -> tuple[int, 'ConfigEntry']: + def _check_error(self, err: int, io: bool = False): + try: + check_error(err, io=io, user_exception=self._stored_exception) + finally: + self._stored_exception = None + + def _get(self, key: str | bytes) -> tuple[int, 'ConfigEntry | None']: key = str_to_bytes(key, 'key') entry = ffi.new('git_config_entry **') err = C.git_config_get_entry(entry, self._config, key) - return err, ConfigEntry._from_c(entry[0]) + if err >= 0: + return err, ConfigEntry._from_c(entry[0]) + + return err, None def _get_entry(self, key: str | bytes) -> 'ConfigEntry': + self._stored_exception = None err, entry = self._get(key) if err == C.GIT_ENOTFOUND: raise KeyError(key) - check_error(err) + self._check_error(err) + assert entry is not None return entry def __contains__(self, key: str | bytes) -> bool: - err, cstr = self._get(key) + self._stored_exception = None + err, _ = self._get(key) if err == C.GIT_ENOTFOUND: return False - check_error(err) + self._check_error(err) return True @@ -175,9 +241,10 @@ def __getitem__(self, key: str | bytes) -> str | None: return entry.value def __setitem__(self, key: str | bytes, value: bool | int | str | bytes) -> None: + self._stored_exception = None key = str_to_bytes(key, 'key') - err = 0 + err: int if isinstance(value, bool): err = C.git_config_set_bool(self._config, key, value) elif isinstance(value, int): @@ -185,13 +252,14 @@ def __setitem__(self, key: str | bytes, value: bool | int | str | bytes) -> None else: err = C.git_config_set_string(self._config, key, to_bytes(value)) - check_error(err) + self._check_error(err) def __delitem__(self, key: str | bytes) -> None: + self._stored_exception = None key = str_to_bytes(key, 'key') err = C.git_config_delete_entry(self._config, key) - check_error(err) + self._check_error(err) def __iter__(self) -> Iterator['ConfigEntry']: """ @@ -200,9 +268,10 @@ def __iter__(self) -> Iterator['ConfigEntry']: variable. Be aware that this may return multiple versions of each entry if they are set multiple times in the configuration files. """ + self._stored_exception = None citer = ffi.new('git_config_iterator **') err = C.git_config_iterator_new(citer, self._config) - check_error(err) + self._check_error(err) return ConfigIterator(self, citer[0]) @@ -214,12 +283,13 @@ def get_multivar( The optional ''regex'' parameter is expected to be a regular expression to filter the variables we're interested in. """ + self._stored_exception = None name = str_to_bytes(name, 'name') regex_bytes = to_bytes(regex or None) citer = ffi.new('git_config_iterator **') err = C.git_config_multivar_iterator_new(citer, self._config, name, regex_bytes) - check_error(err) + self._check_error(err) return ConfigMultivarIterator(self, citer[0]) @@ -230,23 +300,25 @@ def set_multivar( expression to indicate which values to replace. Changes are persisted to the configuration file(s) backing this ``Config``. """ + self._stored_exception = None name = str_to_bytes(name, 'name') regex = str_to_bytes(regex, 'regex') value = str_to_bytes(value, 'value') err = C.git_config_set_multivar(self._config, name, regex, value) - check_error(err) + self._check_error(err) def delete_multivar(self, name: str | bytes, regex: str | bytes) -> None: """Delete a multivar ''name''. ''regexp'' is a regular expression to indicate which values to delete. Changes are persisted to the configuration file(s) backing this ``Config``. """ + self._stored_exception = None name = str_to_bytes(name, 'name') regex = str_to_bytes(regex, 'regex') err = C.git_config_delete_multivar(self._config, name, regex) - check_error(err) + self._check_error(err) def get_bool(self, key: str | bytes) -> bool: """Look up *key* and parse its value as a boolean as per the git-config @@ -255,11 +327,11 @@ def get_bool(self, key: str | bytes) -> bool: Truthy values are: 'true', 1, 'on' or 'yes'. Falsy values are: 'false', 0, 'off' and 'no' """ - + self._stored_exception = None entry = self._get_entry(key) res = ffi.new('int *') err = C.git_config_parse_bool(res, entry.c_value) - check_error(err) + self._check_error(err) return res[0] != 0 @@ -270,33 +342,53 @@ def get_int(self, key: bytes | str) -> int: A value can have a suffix 'k', 'm' or 'g' which stand for 'kilo', 'mega' and 'giga' respectively. """ - + self._stored_exception = None entry = self._get_entry(key) res = ffi.new('int64_t *') err = C.git_config_parse_int64(res, entry.c_value) - check_error(err) + self._check_error(err) return res[0] - def add_file(self, path: str | PathLike, level: int = 0, force: int = 0) -> None: + def add_file( + self, + path: str | PathLike, + level: ConfigLevel | int | None = None, + force: int = 0, + ) -> None: """Add a config file instance to an existing config.""" + self._stored_exception = None + if level is None: + level = 0 + elif isinstance(level, ConfigLevel): + level = level.value err = C.git_config_add_file_ondisk( self._config, to_bytes(path), level, ffi.NULL, force ) - check_error(err) + self._check_error(err) - def snapshot(self) -> 'Config': - """Create a snapshot from this ``Config`` object. + @property + def is_snapshot(self) -> bool: + """Indicates whether this Config object is a read-only snapshot + of the underlying configuration. + """ + return self._is_snapshot + + def snapshot(self) -> Config: + """Create a read-only snapshot of this ``Config`` object. This means that looking up multiple values will use the same version of the configuration files. """ - ccfg = ffi.new('git_config **') - err = C.git_config_snapshot(ccfg, self._config) - check_error(err) + return Config(c_config=self._c_snapshot(), is_snapshot=True) - return Config.from_c(self._repo, ccfg[0]) + def _c_snapshot(self) -> 'GitConfigC': + self._stored_exception = None + c_config = ffi.new('git_config **') + err = C.git_config_snapshot(c_config, self._config) + self._check_error(err) + return c_config[0] # # Methods to parse a string according to the git-config rules @@ -338,6 +430,8 @@ def get_system_config() -> 'Config': The system configuration file is the one found at ``/etc/gitconfig`` or ``%PROGRAMFILES%\\Git\\etc\\gitconfig``, depending on the operating system. + + Raises ``IOError`` if the configuration file is not found. """ return Config._from_found_config(C.git_config_find_system) @@ -349,6 +443,8 @@ def get_global_config() -> 'Config': location for Git, which is ``$HOME/.gitconfig``. This will not find the file at the XDG-compatible user config file location (for that, see :meth:`Config.get_xdg_config`). + + Raises ``IOError`` if the configuration file is not found. """ return Config._from_found_config(C.git_config_find_global) @@ -359,10 +455,1016 @@ def get_xdg_config() -> 'Config': The XDG-compatible user config file follows the XDG Base Directory Specification. This file is located at ``$HOME/.config/git/config``. This will not find the file at the standard user config location (for that, see :meth:`Config.get_global_config`). + + Raises ``IOError`` if the configuration file is not found. """ return Config._from_found_config(C.git_config_find_xdg) +class DefaultConfig(Config): + """A special-case :class:`Config` extension representing the total default configuration. + + This extension to the base ``Config`` class represents the total default configuration + outside the context of a repository (for that, see :class:`RepositoryConfig`). It also + serves as a semantic indicator of the scope of the configuration. + + The ``DefaultConfig`` includes the program, system, XDG, and global (user) configurations. + This is, in essence, the "effective" configuration that Git uses when performing + operations that are not against a repository. When a read operations occurs, the + configurations are searched in the following order: global (user), XDG, system, + and then program data. + """ + + @overload + def __init__(self, /) -> None: + """Load the total default configuration and construct a ``DefaultConfig`` from it.""" + ... + + @overload + def __init__(self, *, c_snapshot: 'GitConfigC') -> None: + """For internal use only. + + Constructs a ``DefaultConfig`` from a given snapshot config object pointer. + The resulting configuration will be read-only. + """ + ... + + def __init__(self, *, c_snapshot: 'GitConfigC | None' = None): + if c_snapshot is not None: + super().__init__(c_config=c_snapshot, is_snapshot=True) + else: + c_config = ffi.new('git_config **') + err = C.git_config_open_default(c_config) + check_error(err) + super().__init__(c_config=c_config[0], is_snapshot=False) + + @override + def snapshot(self) -> DefaultConfig: + """Create a read-only snapshot of this ``DefaultConfig`` object. + + This means that looking up multiple values will use the same version + of the configuration files. + + Raises ``TypeError`` if this is already a snapshot. + """ + if self._is_snapshot: + raise TypeError('This default config is already a snapshot.') + return DefaultConfig(c_snapshot=self._c_snapshot()) + + +class RepositoryConfig(Config): + """A special-case :class:`Config` extension that handles local (repository) configuration. + + This extension to the base ``Config`` class handles some of the special behaviors + associated with repository configs, as well as serving as a semantic indicator for + the scope of the configuration. You should not construct it directly, but instead + use one of :meth:`BaseRepository.config` or :meth:`BaseRepository.config_snapshot` + to obtain the local configuration. + + The ``RepositoryConfig`` represents not just the configuration present in + ``.git/config``, but the sum total of that plus the program, system, XDG, and global + (user) configurations. This is, in essence, the "effective" configuration that Git uses + when performing operations against this repository. When a read operation occurs, + the local configuration is searched first, then the global (user), XDG, system, and + finally program data configurations, in that order. When a write operation occurs, + only the local configuration is changed. + + The ``RepositoryConfig`` can also be used as a context manager to effect a temporary + in-memory override of the local configuration. When the context manager is entered, + an empty in-memory configuration backend is assigned to the configuration and given + the highest read priority. During this context, write operations change the in-memory + backend and do not affect the local configuration file. Read operations consult the + in-memory backend first before then consulting the usual order. When the context + manager exits, the in-memory backend is erased, undoing any changes made to it and + allowing write operations to resume affecting the local configuration. + + Only writeable ``RepositoryConfig`` objects can be used as a context manager. + Read-only snapshot ``RepositoryConfig`` objects cannot. + """ + + @overload + def __init__( + self, + repo: 'BaseRepository', + c_repo: 'GitRepositoryC', + *, + do_snapshot: bool = False, + ) -> None: + """For internal use only. + + See :meth:`BaseRepository.config` for obtaining a repository configuration + or :meth:`BaseRepository.config_snapshot` for obtaining a snapshot. + + Constructs a new ``RepositoryConfig`` from the given repository. + + If ``do_snapshot`` is ``True`` (defaults to ``False``), a read-only snapshot + configuration will be created from the repository. Otherwise, a writeable + configuration will be created. + """ + ... + + @overload + def __init__( + self, + repo: 'BaseRepository', + c_repo: 'GitRepositoryC', + *, + c_snapshot: 'GitConfigC', + ) -> None: + """For internal use only. + + See :meth:`BaseRepository.config_snapshot` for obtaining a repository + configuration snapshot. + + Constructs a ``RepositoryConfig`` from a given snapshot config object pointer. + The resulting configuration will be read-only. + """ + ... + + def __init__( + self, + repo: 'BaseRepository', + c_repo: 'GitRepositoryC', + *, + do_snapshot: bool = False, + c_snapshot: 'GitConfigC | None' = None, + ) -> None: + self._repo = repo + self._c_repo = c_repo + self._backend_added = False + self._backend = RepositoryConfig._InMemoryBackend(self) + + if c_snapshot is not None: + super().__init__(c_config=c_snapshot, is_snapshot=True) + else: + c_config = ffi.new('git_config **') + if do_snapshot: + err = C.git_repository_config_snapshot(c_config, self._c_repo) + else: + err = C.git_repository_config(c_config, self._c_repo) + check_error(err) + super().__init__(c_config=c_config[0], is_snapshot=do_snapshot) + + @override + def snapshot(self) -> RepositoryConfig: + """Create a read-only snapshot of this ``RepositoryConfig`` object. + + This means that looking up multiple values will use the same version + of the configuration files. The resulting ``RepositoryConfig`` cannot be + used as a context manager, because it is read-only. + + Raises ``TypeError`` if this is already a snapshot. + """ + if self._is_snapshot: + raise TypeError('This repository config is already a snapshot.') + return RepositoryConfig(self._repo, self._c_repo, c_snapshot=self._c_snapshot()) + + def __enter__(self) -> Self: + """Enter a context where all writes occur against an in-memory configuration. + + When entered, all subsequent writes occur against an in-memory configuration + backend and do not get persisted to the repository's underlying config file. + As long as the context endures, repository operations will use the sum total + configuration that includes the in-memory configuration. + + Raises ``TypeError`` if this is a read-only snapshot of the local configuration. + """ + if self._is_snapshot: + raise TypeError( + 'A read-only repository config snapshot cannot be used as a context manager, ' + 'because its backend data cannot be changed.' + ) + if not self._backend_added: + self._backend.add_to_config(self._config, self._c_repo) + self._backend_added = True + self._change_write_priority(ConfigLevel.APP) + return self + + def __exit__( + self, + exc_type: type[BaseException] | None, + exc_val: BaseException | None, + exc_tb: TracebackType | None, + ) -> Literal[False]: + """Exit the context so that writes occur against the repository config again. + + When exited, any in-memory configuration is erased so that it is no longer + effective for repository operations, and subsequent writes again occur against + the repository's config and persist to the repository's config file. + """ + if self._backend_added: + self._change_write_priority(ConfigLevel.LOCAL) + self._backend.clear() + return False + + def _change_write_priority(self, level: ConfigLevel) -> None: + """For internal use only. + + By default, when libgit2 creates a ``git_config`` object for a repository, it sets + the write order to ``{ GIT_CONFIG_LEVEL_LOCAL }``. This means that writes go + only to the local config in ``.git/config`` and nowhere else. We need to change + this to ``{ GIT_CONFIG_LEVEL_APP }` when entering the context and then back to + ``{ GIT_CONFIG_LEVEL_LOCAL }`` when exiting the context. + """ + c_levels = ffi.new( + 'git_config_level_t[]', + [ffi.cast('git_config_level_t', level.value)], + ) + err = C.git_config_set_writeorder(self._config, c_levels, 1) + check_error(err) + + class _InMemoryBackend: + """For internal use only. + + An in-memory ``git_config_backend`` implementing the details of + ``_pygit_in_memory_backend``. libgit2 has a built-in in-memory backend that can + be constructed with (as of 1.9.5) ``git_config_backend_from_string`` or + ``git_config_backend_from_values``, but that backend is read-only and cannot + be mutated. To implement the semantics of temporary app-level configuration + in :class:`RepositoryConfiguration`, we need to implement our own backend. + + We could do so completely in C, but that has some serious downsides, notably + all the memory management and how easy it is to get wrong and either leak or + segfault. It's easier, and safer, to implement the backend primarily in Python, + using C structs and CFFI to bridge the implementation so that libgit2 C code + can call its member functions. + + Because of the way CFFI works, the `member` functions must be standalone + functions and cannot be member functions of this class. See all of the + ``_config_memory_*`` functions below. + """ + + type_string = cast('char_pointer', ffi.new('char[]', b'in-memory')) + origin_path_string = cast('char_pointer', ffi.new('char[]', b'')) + + def __init__(self, config: RepositoryConfig) -> None: + self._config = config + + self._read_data: dict[ + str, list[RepositoryConfig._InMemoryBackend._Entry] + ] = {} + self._write_data: dict[ + str, list[RepositoryConfig._InMemoryBackend._Entry] + ] = self._read_data + + self._locked = False + self._readers_lock = threading.Lock() + self._write_lock = threading.Lock() + self._locked_write_lock = threading.Lock() + self._readers = 0 + + self._c_backend: 'PyGitConfigBackendWrapperC | None' = None + self._c_handle = ffi.new_handle(self) + + self._iterators: dict[int, RepositoryConfig._InMemoryBackend._Iterator] = {} + self._c_entries: dict[int, 'PyGitConfigBackendEntryC'] = {} + + @contextlib.contextmanager + def read_lock(self) -> Generator[None, None, None]: + """For internal use only. + + Yield a lock to protect `_read_data`. The lock will not block other readers + from simultaneously reading `_read_data`. The lock will prevent writers from + mutating `_write_data` unless this backend is "locked" (in the midst of a + transaction). + """ + with self._readers_lock: + self._readers += 1 + if self._readers == 1: + self._write_lock.acquire() # first reader blocks all writers + + try: + yield + finally: + with self._readers_lock: + self._readers -= 1 + if self._readers == 0: + self._write_lock.release() # last reader unblocks all writers + + @contextlib.contextmanager + def write_lock(self) -> Generator[None, None, None]: + """For internal use only. + + If this backend is "locked" (in the midst of a transaction), yield a lock + to protect `_write_data`, which is a separate object from `_read_data`, and + so it won't block readers. If this backend is not "locked," yield a lock to + protect `_write_data`/`_read_data`, which are the same object, and so it + will block readers. + """ + if self._locked: + with self._locked_write_lock: + yield + else: + with self._write_lock: + yield + + def add_to_config( + self, + c_config: 'GitConfigC', + c_repo: 'GitRepositoryC', + ) -> None: + """For internal use only. + + Adds the backend to the repository's `git_config`. Called by + :meth:`RepositoryConfig.__enter__` the first time it enters, but not any + subsequent times. This is because it's not possible to remove a backend + from a config with libgit2's public API, and so we rely on clearing + the backend's contents on `__exit__`. + """ + if self._c_backend is not None: + raise ValueError('add_to_config called twice') + + self._c_backend = ffi.new('_pygit_in_memory_backend *') + assert self._c_backend is not None + self._c_backend.self = self._c_handle + self._c_backend.parent.version = 1 + self._c_backend.parent.readonly = 0 + self._c_backend.parent.open = C._config_memory_backend_open + self._c_backend.parent.get = C._config_memory_backend_get + self._c_backend.parent.set = C._config_memory_backend_set + self._c_backend.parent.set_multivar = C._config_memory_backend_set_multivar + setattr(self._c_backend.parent, 'del', C._config_memory_backend_del) + self._c_backend.parent.del_multivar = C._config_memory_backend_del_multivar + self._c_backend.parent.iterator = C._config_memory_backend_iterator + self._c_backend.parent.snapshot = C._config_memory_backend_snapshot + self._c_backend.parent.lock = C._config_memory_backend_lock + self._c_backend.parent.unlock = C._config_memory_backend_unlock + self._c_backend.parent.free = C._config_memory_backend_free + + err = C.git_config_add_backend( + c_config, + ffi.cast('git_config_backend *', self._c_backend), + ConfigLevel.APP.value, + c_repo, + 1, # force + ) + check_error(err) + + def clear(self) -> None: + """For internal use only. + + Erases all contents of the backend. Called by + :meth:`RepositoryConfig.__exit__` each time it exits. + """ + with self.write_lock(): + self._read_data.clear() + self._write_data.clear() + self._iterators.clear() + self._c_entries.clear() + + def _multivar_generator( + self, + ) -> Generator[ + tuple[str, 'RepositoryConfig._InMemoryBackend._Entry'], + None, + None, + ]: + """For internal use only. + + Creates a generator yielding the contents of this backend for use by a + :class:`RepositoryConfig._InMemoryBackend._Iterator`. + """ + for key in self._read_data.keys(): + for value in self._read_data[key]: + yield key, value + + class _Entry: + """For internal use only. + + The value stored in ``_read_data`` and ``_write_data`` to prolong the life of + C strings until their corresponding values are removed or the backend is + cleared or freed. + """ + + def __init__(self, name: str, value: str) -> None: + self.name = name + self.c_name = cast( + 'char_pointer', + ffi.new('char[]', name.encode('utf-8')), + ) + self.value = value + self.c_value = cast( + 'char_pointer', + ffi.new('char[]', value.encode('utf-8')), + ) + + def __repr__(self): + return ( + f'_Entry("{ffi.string(self.c_name).decode("utf-8")}", ' + f'"{ffi.string(self.c_value).decode("utf-8")}")' + ) + + class _Iterator: + """For internal use only. + + Backs the ``_pygit_in_memory_backend_iterator`` object. + """ + + def __init__( + self, + backend: RepositoryConfig._InMemoryBackend, + c_iterator: 'PyGitConfigIteratorWrapperC', + ) -> None: + self._backend = backend + self._generator = backend._multivar_generator() + self._c_handle = ffi.new_handle(self) + self._c_iterator = c_iterator + self._c_entries: dict[int, 'PyGitConfigIteratorEntryC'] = {} + + def __next__(self) -> tuple[str, RepositoryConfig._InMemoryBackend._Entry]: + return next(self._generator) + + def __enter__(self) -> Self: + self._backend._iterators[id(self)] = self + return self + + def __exit__( + self, + exc_type: type[BaseException] | None, + exc_val: BaseException | None, + exc_tb: TracebackType | None, + ) -> Literal[False]: + self._backend._iterators.pop(id(self), None) + return False + + +def _populate_memory_backend_entry( + entry: 'GitConfigBackendEntryC', + source: RepositoryConfig._InMemoryBackend._Entry, + free: Callable[[GitConfigBackendEntryC], None], +) -> None: + """For internal use only. + + Helper function used by ``_config_memory_backend_get`` and + ``_config_memory_iterator_next`` to populate an entry with the data from the + backend. + """ + entry.free = free + entry.entry.name = source.c_name + entry.entry.value = source.c_value + entry.entry.backend_type = RepositoryConfig._InMemoryBackend.type_string + entry.entry.origin_path = RepositoryConfig._InMemoryBackend.origin_path_string + entry.entry.include_depth = 0 + entry.entry.level = ConfigLevel.APP.value + + +@ffi.def_extern() +def _config_memory_backend_open( + _: 'GitConfigBackendC', + __: int, + ___: 'GitRepositoryC', +) -> int: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend`` struct, invoked by libgit2 + immediately after a backend is constructed. In our case, this doesn't need to do + anything, because Python manages the backend instance and all of its data + structures. + + C signature: + int open( + git_config_backend *backend, + git_config_level_t level, + const git_repository *repo); + """ + return 0 + + +@ffi.def_extern() +def _config_memory_backend_get( + backend: 'GitConfigBackendC', + name: char_pointer, + out: '_Pointer[GitConfigBackendEntryC]', +) -> int: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend`` struct, invoked by libgit2 + on each of a config's backends, in order of level, until a backend returns + something other than ``GIT_ENOTFOUND``. If a match is found for ``name``, constructs + a ``_pygit_in_memory_backend_entry``, stores it in the ``_Backend`` to prevent + destruction, and returns 0. + + Obtains a read lock to prevent writers from mutating the backend while it is + being read. + + C signature: + int get( + git_config_backend *backend, + const char *name, + git_config_backend_entry **entry); + """ + backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) + self = cast( + RepositoryConfig._InMemoryBackend, + ffi.from_handle(backend_wrapper.self), + ) + try: + key = ffi.string(name).decode('utf-8') + if key not in self._read_data or not self._read_data[key]: + return C.GIT_ENOTFOUND + + value = self._read_data[key][0] + entry = ffi.new('_pygit_in_memory_backend_entry *') + ptr = int(ffi.cast('uintptr_t', entry)) + entry.owner = backend_wrapper + _populate_memory_backend_entry( + entry.parent, + value, + C._config_memory_backend_entry_free, + ) + self._c_entries[ptr] = entry + out[0] = ffi.cast('git_config_backend_entry *', entry) + except BaseException as e: + self._config._stored_exception = e + return C.GIT_EUSER + return 0 + + +@ffi.def_extern() +def _config_memory_backend_set( + backend: 'GitConfigBackendC', + name: char_pointer, + value: char_pointer, +) -> int: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend`` struct, invoked by libgit2 + on the config's first non-read-only backend (in the order defined by + ``git_config_set_writeorder``) when other code calls ``git_config_set_*`` on that + config. Replaces all values at the specified ``name`` with the new ``value``. + + Obtains a write lock to prevent readers from reading the backend while it is + being mutated. + + C signature: + int set(git_config_backend *backend, const char *name, const char *value); + """ + backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) + self = cast( + RepositoryConfig._InMemoryBackend, + ffi.from_handle(backend_wrapper.self), + ) + try: + key = ffi.string(name).decode('utf-8') + decoded_value = ffi.string(value).decode('utf-8') + self._write_data[key] = [ + RepositoryConfig._InMemoryBackend._Entry(key, decoded_value), + ] + except BaseException as e: + self._config._stored_exception = e + return C.GIT_EUSER + return 0 + + +@ffi.def_extern() +def _config_memory_backend_set_multivar( + backend: 'GitConfigBackendC', + name: char_pointer, + regexp: char_pointer, + value: char_pointer, +) -> int: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend`` struct, invoked by libgit2 + on the config's first non-read-only backend (in the order defined by + ``git_config_set_writeorder``) when other code calls ``git_config_set_multivar`` on + that config. If no current values with the given ``name`` exist, creates a new + multivar. Appends the ``value`` to the multivar and, if ``regexp`` is not ``NULL``, + removes all other values that match the regular expression case-sensitively. + + Obtains a write lock to prevent readers from reading the backend while it is + being mutated. + + C signature: + int set_multivar( + git_config_backend *, + const char *name, + const char *regexp, + const char *value); + """ + backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) + self = cast( + RepositoryConfig._InMemoryBackend, + ffi.from_handle(backend_wrapper.self), + ) + try: + key = ffi.string(name).decode('utf-8') + with self.write_lock(): + if key in self._write_data and regexp != ffi.NULL: + expression = re.compile(ffi.string(regexp).decode('utf-8')) + self._write_data[key] = [ + v for v in self._write_data[key] if not expression.search(v.value) + ] + elif key not in self._write_data: + self._write_data[key] = [] + self._write_data[key].append( + RepositoryConfig._InMemoryBackend._Entry( + key, + ffi.string(value).decode('utf-8'), + ), + ) + except BaseException as e: + self._config._stored_exception = e + return C.GIT_EUSER + return 0 + + +@ffi.def_extern() +def _config_memory_backend_del( + backend: 'GitConfigBackendC', + name: char_pointer, +) -> int: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend`` struct, invoked by libgit2 + on the config's first non-read-only backend (in the order defined by + ``git_config_set_writeorder``) when other code calls ``git_config_delete_entry`` on + that config. Deletes all values with the specified name. + + Obtains a write lock to prevent readers from reading the backend while it is + being mutated. + + C signature: + int del(git_config_backend *backend, const char *name); + """ + backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) + self = cast( + RepositoryConfig._InMemoryBackend, + ffi.from_handle(backend_wrapper.self), + ) + try: + key = ffi.string(name).decode('utf-8') + with self.write_lock(): + if key not in self._write_data: + return C.GIT_ENOTFOUND + del self._write_data[key] + except BaseException as e: + self._config._stored_exception = e + return C.GIT_EUSER + return 0 + + +@ffi.def_extern() +def _config_memory_backend_del_multivar( + backend: 'GitConfigBackendC', + name: char_pointer, + regexp: char_pointer, +) -> int: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend`` struct, invoked by libgit2 + on all of a config's non-read-only backends when other code calls + ``git_config_delete_multivar`` on that config. If the ``regexp`` is ``NULL``, + deletes all values with the specified ``name``. Otherwise, deletes any values + with the specified ``name`` that match the regular expression case-sensitively. + + Obtains a write lock to prevent readers from reading the backend while it is + being mutated. + + C signature: + int del_multivar(git_config_backend *backend, const char *name, const char *regexp); + """ + backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) + self = cast( + RepositoryConfig._InMemoryBackend, + ffi.from_handle(backend_wrapper.self), + ) + try: + key = ffi.string(name).decode('utf-8') + with self.write_lock(): + if key not in self._write_data: + return C.GIT_ENOTFOUND + if regexp == ffi.NULL: + del self._write_data[key] + else: + expression = re.compile(ffi.string(regexp).decode('utf-8')) + self._write_data[key] = [ + v for v in self._write_data[key] if not expression.search(v.value) + ] + except BaseException as e: + self._config._stored_exception = e + return C.GIT_EUSER + return 0 + + +@ffi.def_extern() +def _config_memory_backend_iterator( + out: '_Pointer[GitConfigIteratorC]', + backend: 'GitConfigBackendC', +) -> int: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend`` struct, invoked by libgit2 + on all of a config's backends when other code calls ``git_config_iterator_new`` or + ``git_config_multivar_iterator_new`` on that config followed by ``git_config_next`` + on the resulting iterator. + + Here's how it works: + - User code (or, in this case, :meth:`Config.__iter__` or :meth:`Config.get_multivar`) + calls ``git_config_iterator_new`` or ``git_config_multivar_iterator_new``. + - libgit2 creates a special config iterator that wraps underlying backend iterators + for all the backends backing the config. + - User code (or, in this case, :meth:`ConfigIterator.__next__`) calls + ``git_config_next`` on that config iterator. + - libgit2 invokes this function on the highest-level backend to create the backend + iterator, then immediately invokes ``next`` on that backend iterator to obtain the + first entry. + - libgit2 continues to invoke ``next`` on the backend iterator each time user code + calls ``git_config_next`` on the config iterator, until the backend iterator returns + ``GIT_ITEROVER``. + - libgit2 then invokes this function on the next-higest-level backend to create the + next backend iterator, and so on. + - The config iterator returns ``GIT_ITEROVER`` to the user code only once all backends + have been iterated. + + This constructs a :class:`RepositoryConfig._InMemoryBackend._Iterator` and a + ``_pygit_in_memory_backend_iterator`` and stores references to each in the other, + then enters the former's context manager to prepare for iteration. + + Does not obtain a lock. The only safe way to iterate a backend is to create a + snapshot of the config. + + C signature: + int iterator(git_config_iterator **out, git_config_backend * backend); + """ + backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) + self = cast( + RepositoryConfig._InMemoryBackend, + ffi.from_handle(backend_wrapper.self), + ) + + try: + iterator = ffi.new('_pygit_in_memory_backend_iterator *') + py_iterator = RepositoryConfig._InMemoryBackend._Iterator(self, iterator) + iterator.self = py_iterator._c_handle + iterator.parent.backend = backend + iterator.parent.flags = 0 + iterator.parent.next = C._config_memory_iterator_next + iterator.parent.free = C._config_memory_iterator_free + out[0] = ffi.cast('git_config_iterator *', iterator) + + py_iterator.__enter__() + except BaseException as e: + self._config._stored_exception = e + return C.GIT_EUSER + + return 0 + + +@ffi.def_extern() +def _config_memory_backend_snapshot( + _: '_Pointer[GitConfigBackendC]', + __: 'GitConfigBackendC', +) -> int: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend`` struct, invoked by libgit2 + on all of a config's backends when other code calls ``git_config_snapshot`` on + that config. + + TODO: Implement this, the easiest way for which will be to use + ``git_config_backend_from_string`` (an immutable in-memory backend) when it becomes + available in 1.9.5. + + C signature: + int snapshot(git_config_backend **out, git_config_backend *backend); + """ + # backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) + # self = cast( + # RepositoryConfig._InMemoryBackend, + # ffi.from_handle(backend_wrapper.self), + # ) + return C.GIT_PASSTHROUGH + + +@ffi.def_extern() +def _config_memory_backend_lock(backend: 'GitConfigBackendC') -> int: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend`` struct, invoked by libgit2 + on all of a config's non-read-only backends when other code calls + ``git_config_lock`` on that config to begin a transaction. In practice, this is + not currently used, as PyGit2 does not (yet?) support config transactions. + + This sets ``_write_data`` to a deep copy of the current value of ``_read_data`` + so that any changes made to ``_write_data`` during the transaction are not + visible to readers unless and until ``unlock(true)`` is called. + + C signature: + int lock(git_config_backend * backend); + """ + backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) + self = cast( + RepositoryConfig._InMemoryBackend, + ffi.from_handle(backend_wrapper.self), + ) + try: + with self.write_lock(): + if self._locked: + return C.GIT_ELOCKED + self._locked = True + # this will return a different lock because _locked changed + with self.write_lock(): + self._write_data = {k: v[:] for k, v in self._read_data.items()} + except BaseException as e: + self._config._stored_exception = e + return C.GIT_EUSER + return 0 + + +@ffi.def_extern() +def _config_memory_backend_unlock(backend: 'GitConfigBackendC', success: int) -> int: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend`` struct, invoked by libgit2 + on all of a config's non-read-only backends when other code calls + ``git_transaction_commit`` or ``git_transaction_free`` on a transaction obtained + from ``git_config_lock``. If ``git_transaction_commit` was called, this function + is invoked with a true ``success`` value. If ``git_transaction_free`` is called + without ``git_transaction_commit`` having first been called, this function is + invoked with a false ``success`` value. In practice, this is not currently used, + as PyGit2 does not (yet?) support config transactions. + + If ``success`` is true (non-zero), this "commits" all changes made during the lock + period by pointing ``_read_data`` to ``_write_data``, thus discarding the contents + of ``_read_data``. If ``success`` is false (zero), this "rolls back" all changes + made during the lock period by pointing ``_write_data`` to ``_read_data``, thus + discarding the contents of ``_write_data``. + + C signature: + int unlock(git_config_backend * backend, int success); + """ + backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) + self = cast( + RepositoryConfig._InMemoryBackend, + ffi.from_handle(backend_wrapper.self), + ) + try: + with self.write_lock(): + if not self._locked: + return C.GIT_EINVALID + self._locked = False + # this will return a different lock because _locked changed + with self.write_lock(): + if success == 0: + self._write_data = self._read_data + else: + self._read_data = self._write_data + except BaseException as e: + self._config._stored_exception = e + return C.GIT_EUSER + return 0 + + +@ffi.def_extern() +def _config_memory_backend_free(backend: 'GitConfigBackendC') -> None: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend`` struct, invoked by libgit2 + when it discards the in-memory backend. This occurs only when the repository + config is freed, which is only when the repository itself is freed. + + C signature: + void free(git_config_backend *backend); + """ + backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) + self = cast( + RepositoryConfig._InMemoryBackend, + ffi.from_handle(backend_wrapper.self), + ) + try: + self.clear() + except BaseException as e: + self._config._stored_exception = e + # nothing we can do here because of the void return type + + +@ffi.def_extern() +def _config_memory_backend_entry_free(entry: 'GitConfigBackendEntryC') -> None: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend_entry`` struct, invoked + by libgit2 when it discards an entry obtained from ``_config_memory_backend_get`` + (and, in this specific case, by :meth:`ConfigEntry.__del__` when it calls + ``git_config_entry_free``). + + Removes the entry previously stored in :class:`RepostiroyConfig._InMemoryBackend`'s + store of ``git_config_backend_entry`` instances. + + C signature: + void free(git_config_backend_entry *entry); + """ + sub_entry = ffi.cast('_pygit_in_memory_backend_entry *', entry) + self = cast( + RepositoryConfig._InMemoryBackend, + ffi.from_handle(sub_entry.owner.self), + ) + try: + ptr = int(ffi.cast('uintptr_t', entry)) + if ptr in self._c_entries: + del self._c_entries[ptr] + except BaseException as e: + self._config._stored_exception = e + # nothing we can do here because of the void return type + + +@ffi.def_extern() +def _config_memory_iterator_next( + out: '_Pointer[GitConfigBackendEntryC]', + iterator: 'GitConfigIteratorC', +) -> int: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend_iterator`` struct, invoked + by libgit2 when it wants the next entry from the iterator (and, in this specific + case, by :meth:`ConfigIterator.__next__` when it calls ``git_config_next``). + + C signature: + int next(git_config_backend_entry **out, git_config_iterator *iterator); + """ + iterator_wrapper = ffi.cast('_pygit_in_memory_backend_iterator *', iterator) + self = cast( + RepositoryConfig._InMemoryBackend._Iterator, + ffi.from_handle(iterator_wrapper.self), + ) + try: + key, value = next(self) + entry = ffi.new('_pygit_in_memory_backend_iterator_entry *') + ptr = int(ffi.cast('uintptr_t', entry)) + entry.owner = iterator_wrapper + _populate_memory_backend_entry( + entry.parent, + value, + C._config_memory_iterator_entry_free, + ) + self._c_entries[ptr] = entry + out[0] = ffi.cast('git_config_backend_entry *', entry) + return 0 + except StopIteration: + return C.GIT_ITEROVER + except BaseException as e: + self._backend._config._stored_exception = e + return C.GIT_EUSER + + +@ffi.def_extern() +def _config_memory_iterator_free(iterator: 'GitConfigIteratorC') -> None: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend_iterator`` struct, invoked + by libgit2 when it discards an iterator (and, in this specific case, by + :meth:`ConfigIterator.__del__` when it calls ``git_config_iterator_free``). + + Exits the :class:`RepositoryConfig._InMemoryBackend._Iterator`'s context manager + so that it releases its reference to the backend, the + ``_pygit_in_memory_backend_iterator`` instance, and the + ``git_config_backend_entry`` instances it stored. + + C signature: + void free(git_config_iterator *iterator); + """ + iterator_wrapper = ffi.cast('_pygit_in_memory_backend_iterator *', iterator) + self = cast( + RepositoryConfig._InMemoryBackend._Iterator, + ffi.from_handle(iterator_wrapper.self), + ) + try: + self.__exit__(None, None, None) + except BaseException as e: + self._backend._config._stored_exception = e + # nothing we can do here because of the void return type + + +@ffi.def_extern() +def _config_memory_iterator_entry_free(entry: 'GitConfigBackendEntryC') -> None: + """For internal use only. + + 'Member' function of the ``_pygit_in_memory_backend_iterator_entry`` struct, invoked + by libgit2 when it discards an entry obtained from an iterator (and, in this + specific case, by :meth:`ConfigEntry.__del__` when it calls + ``git_config_entry_free``). + + Removes the entry previously stored in + :class:`RepostiroyConfig._InMemoryBackend._Iterator`'s store of + ``git_config_backend_entry`` instances. + + C signature: + void free(git_config_backend_entry *entry); + """ + sub_entry = ffi.cast('_pygit_in_memory_backend_iterator_entry *', entry) + self = cast( + RepositoryConfig._InMemoryBackend._Iterator, + ffi.from_handle(sub_entry.owner.self), + ) + try: + ptr = int(ffi.cast('uintptr_t', entry)) + if ptr in self._c_entries: + del self._c_entries[ptr] + except BaseException as e: + self._backend._config._stored_exception = e + # nothing we can do here because of the void return type + + class ConfigEntry: """An entry in a configuration object.""" @@ -396,7 +1498,7 @@ def _from_c( return entry def __del__(self) -> None: - if self.iterator is None: + if self.iterator is None and self._entry != ffi.NULL: C.git_config_entry_free(self._entry) @property diff --git a/pygit2/decl/config.h b/pygit2/decl/config.h index 82003d73..b6c11f4f 100644 --- a/pygit2/decl/config.h +++ b/pygit2/decl/config.h @@ -1,4 +1,7 @@ typedef struct git_config_iterator git_config_iterator; +typedef struct git_config_backend git_config_backend; +typedef struct git_config_backend_entry git_config_backend_entry; +typedef struct git_config_backend_memory_options git_config_backend_memory_options; typedef enum { GIT_CONFIG_LEVEL_PROGRAMDATA = 1, @@ -20,6 +23,42 @@ typedef struct git_config_entry { git_config_level_t level; } git_config_entry; +struct git_config_backend_entry { + struct git_config_entry entry; + void (*free)(struct git_config_backend_entry *); +}; + +struct git_config_backend { + unsigned int version; + int readonly; + struct git_config *cfg; + int (*open)(struct git_config_backend *, git_config_level_t, const git_repository *); + int (*get)(struct git_config_backend *, const char *, git_config_backend_entry **); + int (*set)(struct git_config_backend *, const char *, const char *); + int (*set_multivar)(git_config_backend *, const char *, const char *, const char *); + int (*del)(struct git_config_backend *, const char *); + int (*del_multivar)(struct git_config_backend *, const char *, const char *); + int (*iterator)(git_config_iterator **, struct git_config_backend *); + int (*snapshot)(struct git_config_backend **, struct git_config_backend *); + int (*lock)(struct git_config_backend *); + int (*unlock)(struct git_config_backend *, int); + void (*free)(struct git_config_backend *); +}; + + +struct git_config_backend_memory_options { + unsigned int version; + const char *backend_type; + const char *origin_path; +}; + +struct git_config_iterator { + git_config_backend *backend; + unsigned int flags; + int (*next)(git_config_backend_entry **, git_config_iterator *); + void (*free)(git_config_iterator *); +}; + void git_config_entry_free(git_config_entry *); void git_config_free(git_config *cfg); int git_config_get_entry( @@ -49,6 +88,79 @@ int git_config_delete_multivar(git_config *cfg, const char *name, const char *re int git_config_new(git_config **out); int git_config_snapshot(git_config **out, git_config *config); int git_config_open_ondisk(git_config **out, const char *path); +int git_config_open_default(git_config **out); int git_config_find_system(git_buf *out); int git_config_find_global(git_buf *out); int git_config_find_xdg(git_buf *out); +int git_config_set_writeorder(git_config *config, git_config_level_t *levels, size_t len); + +int git_config_init_backend(git_config_backend *backend, unsigned int version); +int git_config_add_backend( + git_config *config, + git_config_backend *backend, + git_config_level_t level, + const git_repository *repo, + int force); + +// Python functions invocable from C in support of the in-memory APP level backend provided +// by PyGit2. See config.py for more details. +extern "Python" int _config_memory_backend_open( + struct git_config_backend *backend, + git_config_level_t level, + const git_repository *repo); + +extern "Python" int _config_memory_backend_get( + struct git_config_backend * backend, + const char *name, + git_config_backend_entry **out); + +extern "Python" int _config_memory_backend_set( + struct git_config_backend *backend, + const char *name, + const char *value); + +extern "Python" int _config_memory_backend_set_multivar( + git_config_backend *backend, + const char *name, + const char *regexp, + const char *value); + +extern "Python" int _config_memory_backend_del( + struct git_config_backend *backend, + const char *name); + +extern "Python" int _config_memory_backend_del_multivar( + struct git_config_backend *backend, + const char *name, + const char *regexp); + +extern "Python" int _config_memory_backend_iterator( + git_config_iterator **out, + struct git_config_backend *backend); + +extern "Python" int _config_memory_backend_snapshot( + struct git_config_backend **out, + struct git_config_backend *backend); + +extern "Python" int _config_memory_backend_lock( + struct git_config_backend *backend); + +extern "Python" int _config_memory_backend_unlock( + struct git_config_backend *backend, + int success); + +extern "Python" void _config_memory_backend_free( + struct git_config_backend *backend); + +extern "Python" void _config_memory_backend_entry_free( + struct git_config_backend_entry *entry); + +extern "Python" int _config_memory_iterator_next( + git_config_backend_entry **out, + git_config_iterator *iter); + +extern "Python" void _config_memory_iterator_free( + git_config_iterator *iter); + +extern "Python" void _config_memory_iterator_entry_free( + struct git_config_backend_entry *entry); diff --git a/pygit2/decl/config_bridge.h b/pygit2/decl/config_bridge.h new file mode 100644 index 00000000..ce5a447f --- /dev/null +++ b/pygit2/decl/config_bridge.h @@ -0,0 +1,35 @@ +/* + * These C structs need to be constructible and usable in Python code using CFFI like all + * other libgit2 structs. They follow a common pattern found throughout libgit2, where a + * "private" struct "extends" a struct from the public API by placing that "public" struct + * as the first member of the private struct. As long as pointers are used to access the + * private struct, it can safely be cast to and from the public struct because of the way + * the memory layout works. + * + * In support of this, these structs are both included in / compiled into pygit2._libgit2.c + * and loaded into CFFI's runtime definitions. + */ + +typedef struct _pygit_in_memory_backend _pygit_in_memory_backend; +struct _pygit_in_memory_backend { + git_config_backend parent; + void * self; +}; + +typedef struct _pygit_in_memory_backend_entry _pygit_in_memory_backend_entry; +struct _pygit_in_memory_backend_entry { + git_config_backend_entry parent; + _pygit_in_memory_backend * owner; +}; + +typedef struct _pygit_in_memory_backend_iterator _pygit_in_memory_backend_iterator; +struct _pygit_in_memory_backend_iterator { + git_config_iterator parent; + void * self; +}; + +typedef struct _pygit_in_memory_backend_iterator_entry _pygit_in_memory_backend_iterator_entry; +struct _pygit_in_memory_backend_iterator_entry { + git_config_backend_entry parent; + _pygit_in_memory_backend_iterator * owner; +}; diff --git a/pygit2/errors.py b/pygit2/errors.py index 02278ddb..5da3ee61 100644 --- a/pygit2/errors.py +++ b/pygit2/errors.py @@ -27,15 +27,24 @@ from ._pygit2 import GitError from .ffi import C, ffi -__all__ = ['GitError'] +__all__ = ['GitError', 'check_error'] value_errors = set([C.GIT_EEXISTS, C.GIT_EINVALIDSPEC, C.GIT_EAMBIGUOUS]) -def check_error(err: int, io: bool = False) -> None: +def check_error( + err: int, + io: bool = False, + user_exception: BaseException | None = None, +) -> None: if err >= 0: return + if err == C.GIT_EUSER and user_exception: + raise user_exception + # may need this? depends on whether this raise "adds to" or "replaces" the traceback: + # raise user_exception.with_traceback(user_exception.__traceback__) + # These are special error codes, they should never reach here test = err != C.GIT_EUSER and err != C.GIT_PASSTHROUGH assert test, f'Unexpected error code {err}' diff --git a/pygit2/repository.py b/pygit2/repository.py index f3c34e81..1aaeeb16 100644 --- a/pygit2/repository.py +++ b/pygit2/repository.py @@ -56,7 +56,7 @@ git_checkout_options, git_stash_apply_options, ) -from .config import Config +from .config import RepositoryConfig from .enums import ( AttrCheck, BlameFlag, @@ -288,31 +288,28 @@ def __repr__(self) -> str: # Configuration # @property - def config(self) -> Config: - """The configuration file for this repository. + def config(self) -> RepositoryConfig: + """The configuration for this repository. + + This includes any program, system, XDG, and global (user) configuration + present on the system, with the local repository config set to the highest + priority for reads and the only option for writes. If the configuration hasn't been set yet, the default config for - repository will be returned, including global and system configurations - (if they are available). + repository will be created and returned. """ - cconfig = ffi.new('git_config **') - err = C.git_repository_config(cconfig, self._repo) - check_error(err) - - return Config.from_c(self, cconfig[0]) + return RepositoryConfig(self, self._repo) @property - def config_snapshot(self): - """A snapshot for this repositiory's configuration + def config_snapshot(self) -> RepositoryConfig: + """A read-only snapshot of this repository's configuration. This allows reads over multiple values to use the same version - of the configuration files. + of the configuration files. As with :meth:`BaseRepository.config`, the + snapshot includes any program, system, XDG, or global (user) + present on the system. """ - cconfig = ffi.new('git_config **') - err = C.git_repository_config_snapshot(cconfig, self._repo) - check_error(err) - - return Config.from_c(self, cconfig[0]) + return RepositoryConfig(self, self._repo, do_snapshot=True) # # References diff --git a/test/test_config.py b/test/test_config.py index 88c7d0c4..46286f6d 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -28,7 +28,14 @@ import pytest -from pygit2 import Config, Repository, Settings +from pygit2 import ( + Config, + DefaultConfig, + GitError, + Repository, + RepositoryConfig, + Settings, +) from . import utils @@ -39,7 +46,7 @@ def config_path(tmp_path: Path) -> Path: @pytest.fixture -def config(testrepo: Repository) -> Generator[Config, None, None]: +def config(testrepo: Repository) -> Generator[RepositoryConfig, None, None]: yield testrepo.config @@ -62,6 +69,12 @@ def test_system_config() -> None: pytest.skip(f'Unavailable for testing: {e}') +def test_default_config() -> None: + # this shouldn't throw, even if get_global_config and git_system_config don't find configs + config = DefaultConfig() + assert 'pygit2.test.default.config' not in config + + def test_new(config_path: Path) -> None: # Touch file config_path.touch() @@ -220,7 +233,8 @@ def test_parsing() -> None: assert 1024 == Config.parse_int('1k') -def test_repository_config_snapshot(config: Config) -> None: +def test_repository_config_snapshot(config: RepositoryConfig) -> None: + assert not config.is_snapshot assert 'core.bare' in config assert not config.get_bool('core.bare') assert 'core.editor' in config @@ -229,12 +243,19 @@ def test_repository_config_snapshot(config: Config) -> None: assert config.get_int('core.repositoryformatversion') == 0 snapshot = config.snapshot() + assert not config.is_snapshot + assert snapshot.is_snapshot assert 'core.bare' in snapshot assert not snapshot.get_bool('core.bare') assert 'core.editor' in snapshot assert snapshot['core.editor'] == 'ed' assert 'core.repositoryformatversion' in snapshot assert snapshot.get_int('core.repositoryformatversion') == 0 + utils.assertRaisesWithArg( + GitError, + "cannot set 'something.other.changed': the configuration is read-only", + lambda: snapshot.set_multivar('something.other.changed', '^$', 'foo'), + ) assert 'core.snapshot1' not in config assert 'core.snapshot1' not in snapshot @@ -255,16 +276,24 @@ def test_non_repository_config_snapshot(config_path: Path) -> None: new_file.write('[something "other"]\n\there = false') config = Config(config_path) + assert not config.is_snapshot assert 'this.that' in config assert config.get_bool('this.that') assert 'something.other.here' in config assert not config.get_bool('something.other.here') snapshot = config.snapshot() + assert not config.is_snapshot + assert snapshot.is_snapshot assert 'this.that' in snapshot assert snapshot.get_bool('this.that') assert 'something.other.here' in snapshot assert not snapshot.get_bool('something.other.here') + utils.assertRaisesWithArg( + GitError, + "cannot set 'something.other.changed': the configuration is read-only", + lambda: snapshot.set_multivar('something.other.changed', '^$', 'foo'), + ) assert 'this.snapshot1' not in config assert 'this.snapshot1' not in snapshot @@ -277,3 +306,144 @@ def test_non_repository_config_snapshot(config_path: Path) -> None: 'this.snapshot1', lambda: snapshot.get_int('this.snapshot1'), ) + + +def test_default_config_snapshot() -> None: + config = DefaultConfig() + assert not config.is_snapshot + snapshot = config.snapshot() + assert not config.is_snapshot + assert snapshot.is_snapshot + utils.assertRaisesWithArg( + GitError, + "cannot set 'something.other.changed': the configuration is read-only", + lambda: snapshot.set_multivar('something.other.changed', '^$', 'foo'), + ) + + +def test_repository_config_in_memory_overrides(config: RepositoryConfig) -> None: + assert not config.is_snapshot + assert 'core.bare' in config + assert not config.get_bool('core.bare') + assert 'core.editor' in config + assert config['core.editor'] == 'ed' + assert 'core.repositoryformatversion' in config + assert config.get_int('core.repositoryformatversion') == 0 + + assert 'core.override1' not in config + assert 'core.override2' not in config + assert 'core.override3' not in config + assert 'core.override4' not in config + assert 'core.local1' not in config + assert 'core.local2' not in config + assert 'core.local3' not in config + + with config: + # these should be unaffected + assert 'core.bare' in config + assert not config.get_bool('core.bare') + assert 'core.editor' in config + assert config['core.editor'] == 'ed' + assert 'core.repositoryformatversion' in config + assert config.get_int('core.repositoryformatversion') == 0 + + # now we should be able to add these to the local in-memory config + assert 'core.override1' not in config + config['core.override1'] = True + assert 'core.override1' in config + assert config.get_bool('core.override1') + + assert 'core.override2' not in config + config['core.override2'] = 42 + assert 'core.override2' in config + assert config.get_int('core.override2') == 42 + + assert 'core.override3' not in config + config['core.override3'] = 'foo' + assert 'core.override3' in config + assert config['core.override3'] == 'foo' + + assert 'core.override4' not in config + config.set_multivar('core.override4', '^$', 'bar') + assert 'core.override4' in config + assert list(config.get_multivar('core.override4')) == ['bar'] + config.set_multivar('core.override4', '^$', 'baz') + assert list(config.get_multivar('core.override4')) == ['bar', 'baz'] + config.set_multivar('core.override4', '^ba', 'qux') + assert list(config.get_multivar('core.override4')) == ['qux'] + + # these should not have been added yet + assert 'core.local1' not in config + assert 'core.local2' not in config + assert 'core.local3' not in config + + # it all should have been erased + assert 'core.override1' not in config + assert 'core.override2' not in config + assert 'core.override3' not in config + assert 'core.override4' not in config + + # now let's add our local configs to the actual file backend + assert 'core.local1' not in config + config['core.local1'] = False + assert 'core.local1' in config + assert not config.get_bool('core.local1') + assert 'core.local2' not in config + config['core.local2'] = 56 + assert 'core.local2' in config + assert config.get_int('core.local2') == 56 + assert 'core.local3' not in config + config['core.local3'] = 'lorem ipsum' + assert 'core.local3' in config + assert config['core.local3'] == 'lorem ipsum' + + with config: + # these should be unaffected + assert 'core.bare' in config + assert not config.get_bool('core.bare') + assert 'core.editor' in config + assert config['core.editor'] == 'ed' + assert 'core.repositoryformatversion' in config + assert config.get_int('core.repositoryformatversion') == 0 + assert 'core.local1' in config + assert not config.get_bool('core.local1') + assert 'core.local2' in config + assert config.get_int('core.local2') == 56 + assert 'core.local3' in config + assert config['core.local3'] == 'lorem ipsum' + + # let's try some different values now + assert 'core.override1' not in config + config['core.override1'] = False + assert 'core.override1' in config + assert not config.get_bool('core.override1') + + assert 'core.override2' not in config + config['core.override2'] = 81 + assert 'core.override2' in config + assert config.get_int('core.override2') == 81 + + assert 'core.override3' not in config + config['core.override3'] = 'dolor simet' + assert 'core.override3' in config + assert config['core.override3'] == 'dolor simet' + + # it all should have been erased again + assert 'core.override1' not in config + assert 'core.override2' not in config + assert 'core.override3' not in config + assert 'core.override4' not in config + + # but these should not have been erased + assert 'core.bare' in config + assert not config.get_bool('core.bare') + assert 'core.editor' in config + assert config['core.editor'] == 'ed' + assert 'core.repositoryformatversion' in config + assert config.get_int('core.repositoryformatversion') == 0 + assert 'core.local1' in config + assert not config.get_bool('core.local1') + assert 'core.local2' in config + assert config.get_int('core.local2') == 56 + assert 'core.local3' in config + assert config['core.local3'] == 'lorem ipsum' From 1f9d088dd6e01be045591b3df81fe7c0e5d0fcdf Mon Sep 17 00:00:00 2001 From: Nick Williams Date: Wed, 1 Jul 2026 13:35:16 -0500 Subject: [PATCH 02/13] Minor tweak --- pygit2/config.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/pygit2/config.py b/pygit2/config.py index 68fc04c5..a2975482 100644 --- a/pygit2/config.py +++ b/pygit2/config.py @@ -381,6 +381,8 @@ def snapshot(self) -> Config: This means that looking up multiple values will use the same version of the configuration files. """ + if self._is_snapshot: + raise TypeError('This config is already a snapshot.') return Config(c_config=self._c_snapshot(), is_snapshot=True) def _c_snapshot(self) -> 'GitConfigC': @@ -538,6 +540,9 @@ class RepositoryConfig(Config): manager exits, the in-memory backend is erased, undoing any changes made to it and allowing write operations to resume affecting the local configuration. + The context manager can be re-entered and then re-exited repeatedly; it is not a + one-use-only operation. + Only writeable ``RepositoryConfig`` objects can be used as a context manager. Read-only snapshot ``RepositoryConfig`` objects cannot. """ @@ -824,9 +829,10 @@ def _multivar_generator( Creates a generator yielding the contents of this backend for use by a :class:`RepositoryConfig._InMemoryBackend._Iterator`. """ - for key in self._read_data.keys(): - for value in self._read_data[key]: - yield key, value + with self.read_lock(): + for key in self._read_data.keys(): + for value in self._read_data[key]: + yield key, value class _Entry: """For internal use only. @@ -1181,8 +1187,8 @@ def _config_memory_backend_iterator( ``_pygit_in_memory_backend_iterator`` and stores references to each in the other, then enters the former's context manager to prepare for iteration. - Does not obtain a lock. The only safe way to iterate a backend is to create a - snapshot of the config. + Obtains a read lock for the duration of iteration, automatically released when iteration + either completes or breaks, without waiting for the iterator to be freed. C signature: int iterator(git_config_iterator **out, git_config_backend * backend); From b6db60ab50c8e265df429118d8d10d0f3a0e811b Mon Sep 17 00:00:00 2001 From: Nick Williams Date: Wed, 1 Jul 2026 15:03:02 -0500 Subject: [PATCH 03/13] Self-review --- pygit2/_libgit2/ffi.pyi | 20 +++++++++++------ pygit2/config.py | 50 +++++++++++++++++++++++------------------ pygit2/errors.py | 2 -- pygit2/repository.py | 2 +- 4 files changed, 42 insertions(+), 32 deletions(-) diff --git a/pygit2/_libgit2/ffi.pyi b/pygit2/_libgit2/ffi.pyi index 791e4b58..d0dc1f71 100644 --- a/pygit2/_libgit2/ffi.pyi +++ b/pygit2/_libgit2/ffi.pyi @@ -60,6 +60,8 @@ class ssize_t: class uintptr_t: def __int__(self) -> int: ... +class git_config_level_t(int): ... + class _Pointer(Generic[T]): def __setitem__(self, item: Literal[0], a: T) -> None: ... @overload @@ -226,6 +228,7 @@ class GitConfigBackendC: set_multivar: Callable[ [GitConfigBackendC, char_pointer, char_pointer, char_pointer], int ] + # this unfortunate name is actually `del`, but that conflicts with a Python keyword del_: Callable[[GitConfigBackendC, char_pointer], int] del_multivar: Callable[[GitConfigBackendC, char_pointer, char_pointer], int] iterator: Callable[[_Pointer[GitConfigIteratorC], GitConfigBackendC], int] @@ -411,6 +414,15 @@ def new(a: Literal['git_config_backend **']) -> _Pointer[GitConfigBackendC]: ... @overload def new(a: Literal['git_config_backend_entry *']) -> GitConfigBackendEntryC: ... @overload +def new( + a: Literal['git_config_backend_memory_options *'], +) -> GitConfigBackendMemoryOptionsC: ... +@overload +def new( + a: Literal['git_config_level_t[]'], + b: list[git_config_level_t], +) -> list[git_config_level_t]: ... +@overload def new( a: Literal['_pygit_in_memory_backend_entry *'], ) -> PyGitConfigBackendEntryC: ... @@ -419,12 +431,6 @@ def new( a: Literal['_pygit_in_memory_backend_iterator_entry *'], ) -> PyGitConfigIteratorEntryC: ... @overload -def new( - a: Literal['git_config_backend_memory_options *'], -) -> GitConfigBackendMemoryOptionsC: ... -@overload -def new(a: Literal['git_config_level_t[]'], b: list[int]) -> list[int]: ... -@overload def new(a: Literal['_pygit_in_memory_backend *']) -> PyGitConfigBackendWrapperC: ... @overload def new( @@ -521,7 +527,7 @@ def cast(a: Literal['char *'], b: object) -> char_pointer: ... @overload def cast(a: Literal['uintptr_t'], b: object) -> uintptr_t: ... @overload -def cast(a: Literal['git_config_level_t'], b: int) -> int: ... +def cast(a: Literal['git_config_level_t'], b: int) -> git_config_level_t: ... @overload def cast( a: Literal['git_config_backend *'], diff --git a/pygit2/config.py b/pygit2/config.py index a2975482..92f59544 100644 --- a/pygit2/config.py +++ b/pygit2/config.py @@ -135,8 +135,7 @@ class Config: def __init__(self, /): """Constructs a new, empty ``Config`` object pointing to no file. - To persist changes made to this ``Config`` object, you must use - :meth:`Config.add_file`. + To make changes to this ``Config`` object, you must use :meth:`Config.add_file`. """ ... @@ -174,16 +173,16 @@ def __init__( if c_config is not None: self._config = c_config else: - cconfig = ffi.new('git_config **') + c_config_ptr = ffi.new('git_config **') if not path: - err = C.git_config_new(cconfig) + err = C.git_config_new(c_config_ptr) else: path_bytes = to_bytes(path) - err = C.git_config_open_ondisk(cconfig, path_bytes) + err = C.git_config_open_ondisk(c_config_ptr, path_bytes) check_error(err, io=True) - self._config = cconfig[0] + self._config = c_config_ptr[0] def __del__(self) -> None: try: @@ -380,6 +379,8 @@ def snapshot(self) -> Config: This means that looking up multiple values will use the same version of the configuration files. + + Raises ``TypeError`` if this is already a snapshot. """ if self._is_snapshot: raise TypeError('This config is already a snapshot.') @@ -472,7 +473,7 @@ class DefaultConfig(Config): The ``DefaultConfig`` includes the program, system, XDG, and global (user) configurations. This is, in essence, the "effective" configuration that Git uses when performing - operations that are not against a repository. When a read operations occurs, the + operations that are not against a repository. When a read operation occurs, the configurations are searched in the following order: global (user), XDG, system, and then program data. """ @@ -535,10 +536,11 @@ class RepositoryConfig(Config): in-memory override of the local configuration. When the context manager is entered, an empty in-memory configuration backend is assigned to the configuration and given the highest read priority. During this context, write operations change the in-memory - backend and do not affect the local configuration file. Read operations consult the - in-memory backend first before then consulting the usual order. When the context - manager exits, the in-memory backend is erased, undoing any changes made to it and - allowing write operations to resume affecting the local configuration. + backend and do not affect the local configuration file. Read operations—including those + performed by Git itself—consult the in-memory backend first before then consulting the + usual order. When the context manager exits, the in-memory backend is erased, undoing + any changes made to it and allowing write operations to resume affecting the local + configuration. The context manager can be re-entered and then re-exited repeatedly; it is not a one-use-only operation. @@ -699,7 +701,7 @@ class _InMemoryBackend: ``_config_memory_*`` functions below. """ - type_string = cast('char_pointer', ffi.new('char[]', b'in-memory')) + type_string = cast('char_pointer', ffi.new('char[]', b'pygit2-in-memory')) origin_path_string = cast('char_pointer', ffi.new('char[]', b'')) def __init__(self, config: RepositoryConfig) -> None: @@ -728,9 +730,9 @@ def __init__(self, config: RepositoryConfig) -> None: def read_lock(self) -> Generator[None, None, None]: """For internal use only. - Yield a lock to protect `_read_data`. The lock will not block other readers - from simultaneously reading `_read_data`. The lock will prevent writers from - mutating `_write_data` unless this backend is "locked" (in the midst of a + Yield a lock to protect ``_read_data``. The lock will not block other readers + from simultaneously reading ``_read_data`` but will prevent writers from + mutating ``_write_data`` unless this backend is "locked" (in the midst of a transaction). """ with self._readers_lock: @@ -751,10 +753,10 @@ def write_lock(self) -> Generator[None, None, None]: """For internal use only. If this backend is "locked" (in the midst of a transaction), yield a lock - to protect `_write_data`, which is a separate object from `_read_data`, and - so it won't block readers. If this backend is not "locked," yield a lock to - protect `_write_data`/`_read_data`, which are the same object, and so it - will block readers. + to protect ``_write_data``, which is a separate object from ``_read_data`` + (so it won't block readers). If this backend is not "locked," yield a lock + to protect ``_write_data``/``_read_data``, which are the same object (so it + will block readers). """ if self._locked: with self._locked_write_lock: @@ -770,11 +772,11 @@ def add_to_config( ) -> None: """For internal use only. - Adds the backend to the repository's `git_config`. Called by + Adds the backend to the repository's ``git_config``. Called by :meth:`RepositoryConfig.__enter__` the first time it enters, but not any subsequent times. This is because it's not possible to remove a backend from a config with libgit2's public API, and so we rely on clearing - the backend's contents on `__exit__`. + the backend's contents on ``__exit__``. """ if self._c_backend is not None: raise ValueError('add_to_config called twice') @@ -788,6 +790,7 @@ def add_to_config( self._c_backend.parent.get = C._config_memory_backend_get self._c_backend.parent.set = C._config_memory_backend_set self._c_backend.parent.set_multivar = C._config_memory_backend_set_multivar + # this unfortunate name conflicts with a Python keyword, so we must use setattr setattr(self._c_backend.parent, 'del', C._config_memory_backend_del) self._c_backend.parent.del_multivar = C._config_memory_backend_del_multivar self._c_backend.parent.iterator = C._config_memory_backend_iterator @@ -801,7 +804,7 @@ def add_to_config( ffi.cast('git_config_backend *', self._c_backend), ConfigLevel.APP.value, c_repo, - 1, # force + 1, # force=true ) check_error(err) @@ -927,6 +930,9 @@ def _config_memory_backend_open( anything, because Python manages the backend instance and all of its data structures. + The third argument, ``repo``, may be ``NULL`` if this backend was applied to + other than a repository config. + C signature: int open( git_config_backend *backend, diff --git a/pygit2/errors.py b/pygit2/errors.py index 5da3ee61..e2a5c51a 100644 --- a/pygit2/errors.py +++ b/pygit2/errors.py @@ -42,8 +42,6 @@ def check_error( if err == C.GIT_EUSER and user_exception: raise user_exception - # may need this? depends on whether this raise "adds to" or "replaces" the traceback: - # raise user_exception.with_traceback(user_exception.__traceback__) # These are special error codes, they should never reach here test = err != C.GIT_EUSER and err != C.GIT_PASSTHROUGH diff --git a/pygit2/repository.py b/pygit2/repository.py index 1aaeeb16..b285919a 100644 --- a/pygit2/repository.py +++ b/pygit2/repository.py @@ -295,7 +295,7 @@ def config(self) -> RepositoryConfig: present on the system, with the local repository config set to the highest priority for reads and the only option for writes. - If the configuration hasn't been set yet, the default config for + If the configuration hasn't been set yet, the default config for the repository will be created and returned. """ return RepositoryConfig(self, self._repo) From d0051f01244ee7057280746219d608dd928fba27 Mon Sep 17 00:00:00 2001 From: Nick Williams Date: Wed, 1 Jul 2026 15:22:25 -0500 Subject: [PATCH 04/13] Make _InMemoryBackend a top-level class instead of a subclass --- pygit2/config.py | 495 +++++++++++++++++++++-------------------------- 1 file changed, 218 insertions(+), 277 deletions(-) diff --git a/pygit2/config.py b/pygit2/config.py index 92f59544..b06a3bec 100644 --- a/pygit2/config.py +++ b/pygit2/config.py @@ -599,7 +599,7 @@ def __init__( self._repo = repo self._c_repo = c_repo self._backend_added = False - self._backend = RepositoryConfig._InMemoryBackend(self) + self._backend = _InMemoryBackend(self) if c_snapshot is not None: super().__init__(c_config=c_snapshot, is_snapshot=True) @@ -680,226 +680,219 @@ def _change_write_priority(self, level: ConfigLevel) -> None: err = C.git_config_set_writeorder(self._config, c_levels, 1) check_error(err) - class _InMemoryBackend: - """For internal use only. - An in-memory ``git_config_backend`` implementing the details of - ``_pygit_in_memory_backend``. libgit2 has a built-in in-memory backend that can - be constructed with (as of 1.9.5) ``git_config_backend_from_string`` or - ``git_config_backend_from_values``, but that backend is read-only and cannot - be mutated. To implement the semantics of temporary app-level configuration - in :class:`RepositoryConfiguration`, we need to implement our own backend. - - We could do so completely in C, but that has some serious downsides, notably - all the memory management and how easy it is to get wrong and either leak or - segfault. It's easier, and safer, to implement the backend primarily in Python, - using C structs and CFFI to bridge the implementation so that libgit2 C code - can call its member functions. - - Because of the way CFFI works, the `member` functions must be standalone - functions and cannot be member functions of this class. See all of the - ``_config_memory_*`` functions below. - """ +class _InMemoryBackend: + """For internal use only. + + An in-memory ``git_config_backend`` implementing the details of + ``_pygit_in_memory_backend``. libgit2 has a built-in in-memory backend that can + be constructed with (as of 1.9.5) ``git_config_backend_from_string`` or + ``git_config_backend_from_values``, but that backend is read-only and cannot + be mutated. To implement the semantics of temporary app-level configuration + in :class:`RepositoryConfiguration`, we need to implement our own backend. + + We could do so completely in C, but that has some serious downsides, notably + all the memory management and how easy it is to get wrong and either leak or + segfault. It's easier, and safer, to implement the backend primarily in Python, + using C structs and CFFI to bridge the implementation so that libgit2 C code + can call its member functions. + + Because of the way CFFI works, the `member` functions must be standalone + functions and cannot be member functions of this class. See all of the + ``_config_memory_*`` functions below. + """ - type_string = cast('char_pointer', ffi.new('char[]', b'pygit2-in-memory')) - origin_path_string = cast('char_pointer', ffi.new('char[]', b'')) + type_string = cast('char_pointer', ffi.new('char[]', b'pygit2-in-memory')) + origin_path_string = cast('char_pointer', ffi.new('char[]', b'')) - def __init__(self, config: RepositoryConfig) -> None: - self._config = config + def __init__(self, config: RepositoryConfig) -> None: + self._config = config - self._read_data: dict[ - str, list[RepositoryConfig._InMemoryBackend._Entry] - ] = {} - self._write_data: dict[ - str, list[RepositoryConfig._InMemoryBackend._Entry] - ] = self._read_data + self._read_data: dict[str, list[_InMemoryBackend.Entry]] = {} + self._write_data: dict[str, list[_InMemoryBackend.Entry]] = self._read_data - self._locked = False - self._readers_lock = threading.Lock() - self._write_lock = threading.Lock() - self._locked_write_lock = threading.Lock() - self._readers = 0 + self._locked = False + self._readers_lock = threading.Lock() + self._write_lock = threading.Lock() + self._locked_write_lock = threading.Lock() + self._readers = 0 - self._c_backend: 'PyGitConfigBackendWrapperC | None' = None - self._c_handle = ffi.new_handle(self) + self._c_backend: 'PyGitConfigBackendWrapperC | None' = None + self._c_handle = ffi.new_handle(self) - self._iterators: dict[int, RepositoryConfig._InMemoryBackend._Iterator] = {} - self._c_entries: dict[int, 'PyGitConfigBackendEntryC'] = {} + self._iterators: dict[int, _InMemoryBackend.Iterator] = {} + self._c_entries: dict[int, 'PyGitConfigBackendEntryC'] = {} - @contextlib.contextmanager - def read_lock(self) -> Generator[None, None, None]: - """For internal use only. + @contextlib.contextmanager + def read_lock(self) -> Generator[None, None, None]: + """For internal use only. - Yield a lock to protect ``_read_data``. The lock will not block other readers - from simultaneously reading ``_read_data`` but will prevent writers from - mutating ``_write_data`` unless this backend is "locked" (in the midst of a - transaction). - """ + Yield a lock to protect ``_read_data``. The lock will not block other readers + from simultaneously reading ``_read_data`` but will prevent writers from + mutating ``_write_data`` unless this backend is "locked" (in the midst of a + transaction). + """ + with self._readers_lock: + self._readers += 1 + if self._readers == 1: + self._write_lock.acquire() # first reader blocks all writers + + try: + yield + finally: with self._readers_lock: - self._readers += 1 - if self._readers == 1: - self._write_lock.acquire() # first reader blocks all writers + self._readers -= 1 + if self._readers == 0: + self._write_lock.release() # last reader unblocks all writers + + @contextlib.contextmanager + def write_lock(self) -> Generator[None, None, None]: + """For internal use only. - try: + If this backend is "locked" (in the midst of a transaction), yield a lock + to protect ``_write_data``, which is a separate object from ``_read_data`` + (so it won't block readers). If this backend is not "locked," yield a lock + to protect ``_write_data``/``_read_data``, which are the same object (so it + will block readers). + """ + if self._locked: + with self._locked_write_lock: yield - finally: - with self._readers_lock: - self._readers -= 1 - if self._readers == 0: - self._write_lock.release() # last reader unblocks all writers - - @contextlib.contextmanager - def write_lock(self) -> Generator[None, None, None]: - """For internal use only. - - If this backend is "locked" (in the midst of a transaction), yield a lock - to protect ``_write_data``, which is a separate object from ``_read_data`` - (so it won't block readers). If this backend is not "locked," yield a lock - to protect ``_write_data``/``_read_data``, which are the same object (so it - will block readers). - """ - if self._locked: - with self._locked_write_lock: - yield - else: - with self._write_lock: - yield + else: + with self._write_lock: + yield + + def add_to_config( + self, + c_config: 'GitConfigC', + c_repo: 'GitRepositoryC', + ) -> None: + """For internal use only. + + Adds the backend to the repository's ``git_config``. Called by + :meth:`RepositoryConfig.__enter__` the first time it enters, but not any + subsequent times. This is because it's not possible to remove a backend + from a config with libgit2's public API, and so we rely on clearing + the backend's contents on ``__exit__``. + """ + if self._c_backend is not None: + raise ValueError('add_to_config called twice') + + self._c_backend = ffi.new('_pygit_in_memory_backend *') + assert self._c_backend is not None + self._c_backend.self = self._c_handle + self._c_backend.parent.version = 1 + self._c_backend.parent.readonly = 0 + self._c_backend.parent.open = C._config_memory_backend_open + self._c_backend.parent.get = C._config_memory_backend_get + self._c_backend.parent.set = C._config_memory_backend_set + self._c_backend.parent.set_multivar = C._config_memory_backend_set_multivar + # this unfortunate name conflicts with a Python keyword, so we must use setattr + setattr(self._c_backend.parent, 'del', C._config_memory_backend_del) + self._c_backend.parent.del_multivar = C._config_memory_backend_del_multivar + self._c_backend.parent.iterator = C._config_memory_backend_iterator + self._c_backend.parent.snapshot = C._config_memory_backend_snapshot + self._c_backend.parent.lock = C._config_memory_backend_lock + self._c_backend.parent.unlock = C._config_memory_backend_unlock + self._c_backend.parent.free = C._config_memory_backend_free + + err = C.git_config_add_backend( + c_config, + ffi.cast('git_config_backend *', self._c_backend), + ConfigLevel.APP.value, + c_repo, + 1, # force=true + ) + check_error(err) - def add_to_config( + def clear(self) -> None: + """For internal use only. + + Erases all contents of the backend. Called by + :meth:`RepositoryConfig.__exit__` each time it exits. + """ + with self.write_lock(): + self._read_data.clear() + self._write_data.clear() + self._iterators.clear() + self._c_entries.clear() + + def _multivar_generator( + self, + ) -> Generator[tuple[str, '_InMemoryBackend.Entry'], None, None]: + """For internal use only. + + Creates a generator yielding the contents of this backend for use by a + :class:`_InMemoryBackend.Iterator`. + """ + with self.read_lock(): + for key in self._read_data.keys(): + for value in self._read_data[key]: + yield key, value + + class Entry: + """For internal use only. + + The value stored in ``_read_data`` and ``_write_data`` to prolong the life of + C strings until their corresponding values are removed or the backend is + cleared or freed. + """ + + def __init__(self, name: str, value: str) -> None: + self.name = name + self.c_name = cast( + 'char_pointer', + ffi.new('char[]', name.encode('utf-8')), + ) + self.value = value + self.c_value = cast( + 'char_pointer', + ffi.new('char[]', value.encode('utf-8')), + ) + + def __repr__(self): + return ( + f'Entry("{ffi.string(self.c_name).decode("utf-8")}", ' + f'"{ffi.string(self.c_value).decode("utf-8")}")' + ) + + class Iterator: + """For internal use only. + + Backs the ``_pygit_in_memory_backend_iterator`` object. + """ + + def __init__( self, - c_config: 'GitConfigC', - c_repo: 'GitRepositoryC', + backend: _InMemoryBackend, + c_iterator: 'PyGitConfigIteratorWrapperC', ) -> None: - """For internal use only. - - Adds the backend to the repository's ``git_config``. Called by - :meth:`RepositoryConfig.__enter__` the first time it enters, but not any - subsequent times. This is because it's not possible to remove a backend - from a config with libgit2's public API, and so we rely on clearing - the backend's contents on ``__exit__``. - """ - if self._c_backend is not None: - raise ValueError('add_to_config called twice') - - self._c_backend = ffi.new('_pygit_in_memory_backend *') - assert self._c_backend is not None - self._c_backend.self = self._c_handle - self._c_backend.parent.version = 1 - self._c_backend.parent.readonly = 0 - self._c_backend.parent.open = C._config_memory_backend_open - self._c_backend.parent.get = C._config_memory_backend_get - self._c_backend.parent.set = C._config_memory_backend_set - self._c_backend.parent.set_multivar = C._config_memory_backend_set_multivar - # this unfortunate name conflicts with a Python keyword, so we must use setattr - setattr(self._c_backend.parent, 'del', C._config_memory_backend_del) - self._c_backend.parent.del_multivar = C._config_memory_backend_del_multivar - self._c_backend.parent.iterator = C._config_memory_backend_iterator - self._c_backend.parent.snapshot = C._config_memory_backend_snapshot - self._c_backend.parent.lock = C._config_memory_backend_lock - self._c_backend.parent.unlock = C._config_memory_backend_unlock - self._c_backend.parent.free = C._config_memory_backend_free - - err = C.git_config_add_backend( - c_config, - ffi.cast('git_config_backend *', self._c_backend), - ConfigLevel.APP.value, - c_repo, - 1, # force=true - ) - check_error(err) + self._backend = backend + self._generator = backend._multivar_generator() + self._c_handle = ffi.new_handle(self) + self._c_iterator = c_iterator + self._c_entries: dict[int, 'PyGitConfigIteratorEntryC'] = {} - def clear(self) -> None: - """For internal use only. + def __next__(self) -> tuple[str, _InMemoryBackend.Entry]: + return next(self._generator) - Erases all contents of the backend. Called by - :meth:`RepositoryConfig.__exit__` each time it exits. - """ - with self.write_lock(): - self._read_data.clear() - self._write_data.clear() - self._iterators.clear() - self._c_entries.clear() + def __enter__(self) -> Self: + self._backend._iterators[id(self)] = self + return self - def _multivar_generator( + def __exit__( self, - ) -> Generator[ - tuple[str, 'RepositoryConfig._InMemoryBackend._Entry'], - None, - None, - ]: - """For internal use only. - - Creates a generator yielding the contents of this backend for use by a - :class:`RepositoryConfig._InMemoryBackend._Iterator`. - """ - with self.read_lock(): - for key in self._read_data.keys(): - for value in self._read_data[key]: - yield key, value - - class _Entry: - """For internal use only. - - The value stored in ``_read_data`` and ``_write_data`` to prolong the life of - C strings until their corresponding values are removed or the backend is - cleared or freed. - """ - - def __init__(self, name: str, value: str) -> None: - self.name = name - self.c_name = cast( - 'char_pointer', - ffi.new('char[]', name.encode('utf-8')), - ) - self.value = value - self.c_value = cast( - 'char_pointer', - ffi.new('char[]', value.encode('utf-8')), - ) - - def __repr__(self): - return ( - f'_Entry("{ffi.string(self.c_name).decode("utf-8")}", ' - f'"{ffi.string(self.c_value).decode("utf-8")}")' - ) - - class _Iterator: - """For internal use only. - - Backs the ``_pygit_in_memory_backend_iterator`` object. - """ - - def __init__( - self, - backend: RepositoryConfig._InMemoryBackend, - c_iterator: 'PyGitConfigIteratorWrapperC', - ) -> None: - self._backend = backend - self._generator = backend._multivar_generator() - self._c_handle = ffi.new_handle(self) - self._c_iterator = c_iterator - self._c_entries: dict[int, 'PyGitConfigIteratorEntryC'] = {} - - def __next__(self) -> tuple[str, RepositoryConfig._InMemoryBackend._Entry]: - return next(self._generator) - - def __enter__(self) -> Self: - self._backend._iterators[id(self)] = self - return self - - def __exit__( - self, - exc_type: type[BaseException] | None, - exc_val: BaseException | None, - exc_tb: TracebackType | None, - ) -> Literal[False]: - self._backend._iterators.pop(id(self), None) - return False + exc_type: type[BaseException] | None, + exc_val: BaseException | None, + exc_tb: TracebackType | None, + ) -> Literal[False]: + self._backend._iterators.pop(id(self), None) + return False def _populate_memory_backend_entry( entry: 'GitConfigBackendEntryC', - source: RepositoryConfig._InMemoryBackend._Entry, + source: _InMemoryBackend.Entry, free: Callable[[GitConfigBackendEntryC], None], ) -> None: """For internal use only. @@ -911,8 +904,8 @@ def _populate_memory_backend_entry( entry.free = free entry.entry.name = source.c_name entry.entry.value = source.c_value - entry.entry.backend_type = RepositoryConfig._InMemoryBackend.type_string - entry.entry.origin_path = RepositoryConfig._InMemoryBackend.origin_path_string + entry.entry.backend_type = _InMemoryBackend.type_string + entry.entry.origin_path = _InMemoryBackend.origin_path_string entry.entry.include_depth = 0 entry.entry.level = ConfigLevel.APP.value @@ -966,10 +959,7 @@ def _config_memory_backend_get( git_config_backend_entry **entry); """ backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) - self = cast( - RepositoryConfig._InMemoryBackend, - ffi.from_handle(backend_wrapper.self), - ) + self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: key = ffi.string(name).decode('utf-8') if key not in self._read_data or not self._read_data[key]: @@ -1012,16 +1002,11 @@ def _config_memory_backend_set( int set(git_config_backend *backend, const char *name, const char *value); """ backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) - self = cast( - RepositoryConfig._InMemoryBackend, - ffi.from_handle(backend_wrapper.self), - ) + self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: key = ffi.string(name).decode('utf-8') decoded_value = ffi.string(value).decode('utf-8') - self._write_data[key] = [ - RepositoryConfig._InMemoryBackend._Entry(key, decoded_value), - ] + self._write_data[key] = [_InMemoryBackend.Entry(key, decoded_value)] except BaseException as e: self._config._stored_exception = e return C.GIT_EUSER @@ -1055,10 +1040,7 @@ def _config_memory_backend_set_multivar( const char *value); """ backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) - self = cast( - RepositoryConfig._InMemoryBackend, - ffi.from_handle(backend_wrapper.self), - ) + self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: key = ffi.string(name).decode('utf-8') with self.write_lock(): @@ -1070,10 +1052,7 @@ def _config_memory_backend_set_multivar( elif key not in self._write_data: self._write_data[key] = [] self._write_data[key].append( - RepositoryConfig._InMemoryBackend._Entry( - key, - ffi.string(value).decode('utf-8'), - ), + _InMemoryBackend.Entry(key, ffi.string(value).decode('utf-8')), ) except BaseException as e: self._config._stored_exception = e @@ -1100,10 +1079,7 @@ def _config_memory_backend_del( int del(git_config_backend *backend, const char *name); """ backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) - self = cast( - RepositoryConfig._InMemoryBackend, - ffi.from_handle(backend_wrapper.self), - ) + self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: key = ffi.string(name).decode('utf-8') with self.write_lock(): @@ -1137,10 +1113,7 @@ def _config_memory_backend_del_multivar( int del_multivar(git_config_backend *backend, const char *name, const char *regexp); """ backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) - self = cast( - RepositoryConfig._InMemoryBackend, - ffi.from_handle(backend_wrapper.self), - ) + self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: key = ffi.string(name).decode('utf-8') with self.write_lock(): @@ -1189,7 +1162,7 @@ def _config_memory_backend_iterator( - The config iterator returns ``GIT_ITEROVER`` to the user code only once all backends have been iterated. - This constructs a :class:`RepositoryConfig._InMemoryBackend._Iterator` and a + This constructs a :class:`_InMemoryBackend.Iterator` and a ``_pygit_in_memory_backend_iterator`` and stores references to each in the other, then enters the former's context manager to prepare for iteration. @@ -1200,26 +1173,20 @@ def _config_memory_backend_iterator( int iterator(git_config_iterator **out, git_config_backend * backend); """ backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) - self = cast( - RepositoryConfig._InMemoryBackend, - ffi.from_handle(backend_wrapper.self), - ) - + self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: iterator = ffi.new('_pygit_in_memory_backend_iterator *') - py_iterator = RepositoryConfig._InMemoryBackend._Iterator(self, iterator) + py_iterator = _InMemoryBackend.Iterator(self, iterator) iterator.self = py_iterator._c_handle iterator.parent.backend = backend iterator.parent.flags = 0 iterator.parent.next = C._config_memory_iterator_next iterator.parent.free = C._config_memory_iterator_free out[0] = ffi.cast('git_config_iterator *', iterator) - py_iterator.__enter__() except BaseException as e: self._config._stored_exception = e return C.GIT_EUSER - return 0 @@ -1242,10 +1209,7 @@ def _config_memory_backend_snapshot( int snapshot(git_config_backend **out, git_config_backend *backend); """ # backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) - # self = cast( - # RepositoryConfig._InMemoryBackend, - # ffi.from_handle(backend_wrapper.self), - # ) + # self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) return C.GIT_PASSTHROUGH @@ -1266,10 +1230,7 @@ def _config_memory_backend_lock(backend: 'GitConfigBackendC') -> int: int lock(git_config_backend * backend); """ backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) - self = cast( - RepositoryConfig._InMemoryBackend, - ffi.from_handle(backend_wrapper.self), - ) + self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: with self.write_lock(): if self._locked: @@ -1307,10 +1268,7 @@ def _config_memory_backend_unlock(backend: 'GitConfigBackendC', success: int) -> int unlock(git_config_backend * backend, int success); """ backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) - self = cast( - RepositoryConfig._InMemoryBackend, - ffi.from_handle(backend_wrapper.self), - ) + self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: with self.write_lock(): if not self._locked: @@ -1340,10 +1298,7 @@ def _config_memory_backend_free(backend: 'GitConfigBackendC') -> None: void free(git_config_backend *backend); """ backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) - self = cast( - RepositoryConfig._InMemoryBackend, - ffi.from_handle(backend_wrapper.self), - ) + self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: self.clear() except BaseException as e: @@ -1360,17 +1315,14 @@ def _config_memory_backend_entry_free(entry: 'GitConfigBackendEntryC') -> None: (and, in this specific case, by :meth:`ConfigEntry.__del__` when it calls ``git_config_entry_free``). - Removes the entry previously stored in :class:`RepostiroyConfig._InMemoryBackend`'s - store of ``git_config_backend_entry`` instances. + Removes the entry previously stored in :class:`_InMemoryBackend`'s store of + ``git_config_backend_entry`` instances. C signature: void free(git_config_backend_entry *entry); """ sub_entry = ffi.cast('_pygit_in_memory_backend_entry *', entry) - self = cast( - RepositoryConfig._InMemoryBackend, - ffi.from_handle(sub_entry.owner.self), - ) + self = cast(_InMemoryBackend, ffi.from_handle(sub_entry.owner.self)) try: ptr = int(ffi.cast('uintptr_t', entry)) if ptr in self._c_entries: @@ -1395,10 +1347,7 @@ def _config_memory_iterator_next( int next(git_config_backend_entry **out, git_config_iterator *iterator); """ iterator_wrapper = ffi.cast('_pygit_in_memory_backend_iterator *', iterator) - self = cast( - RepositoryConfig._InMemoryBackend._Iterator, - ffi.from_handle(iterator_wrapper.self), - ) + self = cast(_InMemoryBackend.Iterator, ffi.from_handle(iterator_wrapper.self)) try: key, value = next(self) entry = ffi.new('_pygit_in_memory_backend_iterator_entry *') @@ -1427,19 +1376,15 @@ def _config_memory_iterator_free(iterator: 'GitConfigIteratorC') -> None: by libgit2 when it discards an iterator (and, in this specific case, by :meth:`ConfigIterator.__del__` when it calls ``git_config_iterator_free``). - Exits the :class:`RepositoryConfig._InMemoryBackend._Iterator`'s context manager - so that it releases its reference to the backend, the - ``_pygit_in_memory_backend_iterator`` instance, and the - ``git_config_backend_entry`` instances it stored. + Exits the :class:`_InMemoryBackend.Iterator`'s context manager so that it + releases its reference to the backend, the ``_pygit_in_memory_backend_iterator`` + instance, and the ``git_config_backend_entry`` instances it stored. C signature: void free(git_config_iterator *iterator); """ iterator_wrapper = ffi.cast('_pygit_in_memory_backend_iterator *', iterator) - self = cast( - RepositoryConfig._InMemoryBackend._Iterator, - ffi.from_handle(iterator_wrapper.self), - ) + self = cast(_InMemoryBackend.Iterator, ffi.from_handle(iterator_wrapper.self)) try: self.__exit__(None, None, None) except BaseException as e: @@ -1456,18 +1401,14 @@ def _config_memory_iterator_entry_free(entry: 'GitConfigBackendEntryC') -> None: specific case, by :meth:`ConfigEntry.__del__` when it calls ``git_config_entry_free``). - Removes the entry previously stored in - :class:`RepostiroyConfig._InMemoryBackend._Iterator`'s store of - ``git_config_backend_entry`` instances. + Removes the entry previously stored in :class:`_InMemoryBackend.Iterator`'s store + of ``git_config_backend_entry`` instances. C signature: void free(git_config_backend_entry *entry); """ sub_entry = ffi.cast('_pygit_in_memory_backend_iterator_entry *', entry) - self = cast( - RepositoryConfig._InMemoryBackend._Iterator, - ffi.from_handle(sub_entry.owner.self), - ) + self = cast(_InMemoryBackend.Iterator, ffi.from_handle(sub_entry.owner.self)) try: ptr = int(ffi.cast('uintptr_t', entry)) if ptr in self._c_entries: From cb5065753eb1cee0179b4cf371205e627e1d1d5f Mon Sep 17 00:00:00 2001 From: Nick Williams Date: Wed, 1 Jul 2026 16:07:02 -0500 Subject: [PATCH 05/13] Extract in memory backend context manager to support default config --- pygit2/config.py | 192 +++++++++++++++++++++++++++----------------- test/test_config.py | 118 +++++++++++++++++++++++++++ 2 files changed, 238 insertions(+), 72 deletions(-) diff --git a/pygit2/config.py b/pygit2/config.py index b06a3bec..62e25587 100644 --- a/pygit2/config.py +++ b/pygit2/config.py @@ -24,6 +24,7 @@ # Boston, MA 02110-1301, USA. from __future__ import annotations +import abc import contextlib import re import threading @@ -464,7 +465,83 @@ def get_xdg_config() -> 'Config': return Config._from_found_config(C.git_config_find_xdg) -class DefaultConfig(Config): +class _InMemoryAppBackendConfig(Config, abc.ABC): + """For internal use only. + + This base class for :class:`DefaultConfig` and :class:`RepositoryConfig` implements + the in-memory backend context manager semantics documented in those respective + classes. + """ + + def __init__(self, *, c_config: 'GitConfigC', is_snapshot: bool) -> None: + """For internal use only. + + Constructs a ``Config`` object from a config object pointer. + """ + super().__init__(c_config=c_config, is_snapshot=is_snapshot) + self._backend_added = False + self._backend = _InMemoryBackend(self) + + @abc.abstractmethod + def _add_backend_to_config(self, backend: _InMemoryBackend) -> None: + """Adds the backend to the config object.""" + + def __enter__(self) -> Self: + """Enter a context where all writes occur against an in-memory configuration. + + When entered, all subsequent writes occur against an in-memory configuration + backend and do not get persisted to the underlying config file(s). As long as + the context endures, Git operations will use the sum total configuration that + includes the in-memory configuration. + + Raises ``TypeError`` if this is a read-only snapshot of the configuration. + """ + if self._is_snapshot: + raise TypeError( + 'A read-only config snapshot cannot be used as a context manager, ' + 'because its backend data cannot be changed.' + ) + if not self._backend_added: + self._add_backend_to_config(self._backend) + self._backend_added = True + self._change_write_priority(ConfigLevel.APP) + return self + + def __exit__( + self, + exc_type: type[BaseException] | None, + exc_val: BaseException | None, + exc_tb: TracebackType | None, + ) -> Literal[False]: + """Exit the context so that writes occur against the repository config again. + + When exited, any in-memory configuration is erased so that it is no longer + effective for repository operations, and subsequent writes again occur against + the repository's config and persist to the repository's config file. + """ + if self._backend_added: + self._change_write_priority(ConfigLevel.LOCAL) + self._backend.clear() + return False + + def _change_write_priority(self, level: ConfigLevel) -> None: + """For internal use only. + + By default, when libgit2 creates a ``git_config`` object for a repository, it sets + the write order to ``{ GIT_CONFIG_LEVEL_LOCAL }``. This means that writes go + only to the local config in ``.git/config`` and nowhere else. We need to change + this to ``{ GIT_CONFIG_LEVEL_APP }` when entering the context and then back to + ``{ GIT_CONFIG_LEVEL_LOCAL }`` when exiting the context. + """ + c_levels = ffi.new( + 'git_config_level_t[]', + [ffi.cast('git_config_level_t', level.value)], + ) + err = C.git_config_set_writeorder(self._config, c_levels, 1) + check_error(err) + + +class DefaultConfig(_InMemoryAppBackendConfig): """A special-case :class:`Config` extension representing the total default configuration. This extension to the base ``Config`` class represents the total default configuration @@ -476,6 +553,21 @@ class DefaultConfig(Config): operations that are not against a repository. When a read operation occurs, the configurations are searched in the following order: global (user), XDG, system, and then program data. + + The ``DefaultConfig`` can also be used as a context manager to effect a temporary + in-memory override of the default configuration. When the context manager is entered, + an empty in-memory configuration backend is assigned to the configuration and given + the highest read priority. During this context, write operations change the in-memory + backend and do not affect the configuration file(s). Read operations—including those + performed by Git itself—consult the in-memory backend first before then consulting the + usual order. When the context manager exits, the in-memory backend's contents are + erased, undoing any changes made to it. + + The context manager can be re-entered and then re-exited repeatedly; it is not a + one-use-only operation. + + Only writeable ``DefaultConfig`` objects can be used as a context manager. + Read-only snapshot ``DefaultConfig`` objects cannot. """ @overload @@ -514,8 +606,12 @@ def snapshot(self) -> DefaultConfig: raise TypeError('This default config is already a snapshot.') return DefaultConfig(c_snapshot=self._c_snapshot()) + @override + def _add_backend_to_config(self, backend: _InMemoryBackend) -> None: + backend.add_to_config(self._config) + -class RepositoryConfig(Config): +class RepositoryConfig(_InMemoryAppBackendConfig): """A special-case :class:`Config` extension that handles local (repository) configuration. This extension to the base ``Config`` class handles some of the special behaviors @@ -538,9 +634,9 @@ class RepositoryConfig(Config): the highest read priority. During this context, write operations change the in-memory backend and do not affect the local configuration file. Read operations—including those performed by Git itself—consult the in-memory backend first before then consulting the - usual order. When the context manager exits, the in-memory backend is erased, undoing - any changes made to it and allowing write operations to resume affecting the local - configuration. + usual order. When the context manager exits, the in-memory backend's contents are + erased, undoing any changes made to it and allowing write operations to resume + affecting the local configuration. The context manager can be re-entered and then re-exited repeatedly; it is not a one-use-only operation. @@ -598,8 +694,6 @@ def __init__( ) -> None: self._repo = repo self._c_repo = c_repo - self._backend_added = False - self._backend = _InMemoryBackend(self) if c_snapshot is not None: super().__init__(c_config=c_snapshot, is_snapshot=True) @@ -626,59 +720,9 @@ def snapshot(self) -> RepositoryConfig: raise TypeError('This repository config is already a snapshot.') return RepositoryConfig(self._repo, self._c_repo, c_snapshot=self._c_snapshot()) - def __enter__(self) -> Self: - """Enter a context where all writes occur against an in-memory configuration. - - When entered, all subsequent writes occur against an in-memory configuration - backend and do not get persisted to the repository's underlying config file. - As long as the context endures, repository operations will use the sum total - configuration that includes the in-memory configuration. - - Raises ``TypeError`` if this is a read-only snapshot of the local configuration. - """ - if self._is_snapshot: - raise TypeError( - 'A read-only repository config snapshot cannot be used as a context manager, ' - 'because its backend data cannot be changed.' - ) - if not self._backend_added: - self._backend.add_to_config(self._config, self._c_repo) - self._backend_added = True - self._change_write_priority(ConfigLevel.APP) - return self - - def __exit__( - self, - exc_type: type[BaseException] | None, - exc_val: BaseException | None, - exc_tb: TracebackType | None, - ) -> Literal[False]: - """Exit the context so that writes occur against the repository config again. - - When exited, any in-memory configuration is erased so that it is no longer - effective for repository operations, and subsequent writes again occur against - the repository's config and persist to the repository's config file. - """ - if self._backend_added: - self._change_write_priority(ConfigLevel.LOCAL) - self._backend.clear() - return False - - def _change_write_priority(self, level: ConfigLevel) -> None: - """For internal use only. - - By default, when libgit2 creates a ``git_config`` object for a repository, it sets - the write order to ``{ GIT_CONFIG_LEVEL_LOCAL }``. This means that writes go - only to the local config in ``.git/config`` and nowhere else. We need to change - this to ``{ GIT_CONFIG_LEVEL_APP }` when entering the context and then back to - ``{ GIT_CONFIG_LEVEL_LOCAL }`` when exiting the context. - """ - c_levels = ffi.new( - 'git_config_level_t[]', - [ffi.cast('git_config_level_t', level.value)], - ) - err = C.git_config_set_writeorder(self._config, c_levels, 1) - check_error(err) + @override + def _add_backend_to_config(self, backend: _InMemoryBackend) -> None: + backend.add_to_config(self._config, self._c_repo) class _InMemoryBackend: @@ -689,7 +733,8 @@ class _InMemoryBackend: be constructed with (as of 1.9.5) ``git_config_backend_from_string`` or ``git_config_backend_from_values``, but that backend is read-only and cannot be mutated. To implement the semantics of temporary app-level configuration - in :class:`RepositoryConfiguration`, we need to implement our own backend. + in :class:`DefaultConfiguration` and :class:`RepositoryConfiguration`, we need + to implement our own backend. We could do so completely in C, but that has some serious downsides, notably all the memory management and how easy it is to get wrong and either leak or @@ -705,7 +750,7 @@ class _InMemoryBackend: type_string = cast('char_pointer', ffi.new('char[]', b'pygit2-in-memory')) origin_path_string = cast('char_pointer', ffi.new('char[]', b'')) - def __init__(self, config: RepositoryConfig) -> None: + def __init__(self, config: _InMemoryAppBackendConfig) -> None: self._config = config self._read_data: dict[str, list[_InMemoryBackend.Entry]] = {} @@ -765,15 +810,17 @@ def write_lock(self) -> Generator[None, None, None]: def add_to_config( self, c_config: 'GitConfigC', - c_repo: 'GitRepositoryC', + c_repo: 'GitRepositoryC | None' = None, ) -> None: """For internal use only. - Adds the backend to the repository's ``git_config``. Called by - :meth:`RepositoryConfig.__enter__` the first time it enters, but not any - subsequent times. This is because it's not possible to remove a backend + Adds the backend to the ``git_config``. Called by + :meth:`_InMemoryAppBackendConfig.__enter__` the first time it enters, but not + any subsequent times. This is because it's not possible to remove a backend from a config with libgit2's public API, and so we rely on clearing the backend's contents on ``__exit__``. + + The ``c_repo`` is optional and applies only to repository configurations. """ if self._c_backend is not None: raise ValueError('add_to_config called twice') @@ -800,7 +847,7 @@ def add_to_config( c_config, ffi.cast('git_config_backend *', self._c_backend), ConfigLevel.APP.value, - c_repo, + ffi.NULL if c_repo is None else c_repo, 1, # force=true ) check_error(err) @@ -809,7 +856,7 @@ def clear(self) -> None: """For internal use only. Erases all contents of the backend. Called by - :meth:`RepositoryConfig.__exit__` each time it exits. + :meth:`_InMemoryAppBackendConfig.__exit__` each time it exits. """ with self.write_lock(): self._read_data.clear() @@ -817,7 +864,7 @@ def clear(self) -> None: self._iterators.clear() self._c_entries.clear() - def _multivar_generator( + def multivar_generator( self, ) -> Generator[tuple[str, '_InMemoryBackend.Entry'], None, None]: """For internal use only. @@ -868,7 +915,7 @@ def __init__( c_iterator: 'PyGitConfigIteratorWrapperC', ) -> None: self._backend = backend - self._generator = backend._multivar_generator() + self._generator = backend.multivar_generator() self._c_handle = ffi.new_handle(self) self._c_iterator = c_iterator self._c_entries: dict[int, 'PyGitConfigIteratorEntryC'] = {} @@ -1291,8 +1338,9 @@ def _config_memory_backend_free(backend: 'GitConfigBackendC') -> None: """For internal use only. 'Member' function of the ``_pygit_in_memory_backend`` struct, invoked by libgit2 - when it discards the in-memory backend. This occurs only when the repository - config is freed, which is only when the repository itself is freed. + when it discards the in-memory backend. This occurs only when the + config is freed. For a repository config, this is only when the repository itself + is freed. For the default config, this is only when the application is unloaded. C signature: void free(git_config_backend *backend); diff --git a/test/test_config.py b/test/test_config.py index 46286f6d..94982131 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -334,6 +334,7 @@ def test_repository_config_in_memory_overrides(config: RepositoryConfig) -> None assert 'core.override2' not in config assert 'core.override3' not in config assert 'core.override4' not in config + assert 'core.override5' not in config assert 'core.local1' not in config assert 'core.local2' not in config assert 'core.local3' not in config @@ -372,6 +373,30 @@ def test_repository_config_in_memory_overrides(config: RepositoryConfig) -> None config.set_multivar('core.override4', '^ba', 'qux') assert list(config.get_multivar('core.override4')) == ['qux'] + # try deleting some stuff + assert 'core.override5' not in config + config['core.override5'] = 'to be deleted' + assert 'core.override5' in config + assert config['core.override5'] == 'to be deleted' + del config['core.override5'] + assert 'core.override5' not in config + + config.set_multivar('core.override5', '^$', 'lorem') + config.set_multivar('core.override5', '^$', 'ipsum') + config.set_multivar('core.override5', '^$', 'dolor') + config.set_multivar('core.override5', '^$', 'simet') + assert 'core.override5' in config + assert list( + config.get_multivar('core.override5') + ) == ['lorem', 'ipsum', 'dolor', 'simet'] + config.delete_multivar('core.override5', r'.*or.*') + assert 'core.override5' in config + assert list( + config.get_multivar('core.override5') + ) == ['ipsum', 'simet'] + config.delete_multivar('core.override5', r'.*') + assert 'core.override5' not in config + # these should not have been added yet assert 'core.local1' not in config assert 'core.local2' not in config @@ -382,6 +407,7 @@ def test_repository_config_in_memory_overrides(config: RepositoryConfig) -> None assert 'core.override2' not in config assert 'core.override3' not in config assert 'core.override4' not in config + assert 'core.override5' not in config # now let's add our local configs to the actual file backend assert 'core.local1' not in config @@ -447,3 +473,95 @@ def test_repository_config_in_memory_overrides(config: RepositoryConfig) -> None assert config.get_int('core.local2') == 56 assert 'core.local3' in config assert config['core.local3'] == 'lorem ipsum' + + +def test_default_config_in_memory_overrides() -> None: + config = DefaultConfig() + assert not config.is_snapshot + + assert 'core.override1' not in config + assert 'core.override2' not in config + assert 'core.override3' not in config + assert 'core.override4' not in config + assert 'core.override5' not in config + + with config: + # now we should be able to add these to the local in-memory config + assert 'core.override1' not in config + config['core.override1'] = True + assert 'core.override1' in config + assert config.get_bool('core.override1') + + assert 'core.override2' not in config + config['core.override2'] = 42 + assert 'core.override2' in config + assert config.get_int('core.override2') == 42 + + assert 'core.override3' not in config + config['core.override3'] = 'foo' + assert 'core.override3' in config + assert config['core.override3'] == 'foo' + + assert 'core.override4' not in config + config.set_multivar('core.override4', '^$', 'bar') + assert 'core.override4' in config + assert list(config.get_multivar('core.override4')) == ['bar'] + config.set_multivar('core.override4', '^$', 'baz') + assert list(config.get_multivar('core.override4')) == ['bar', 'baz'] + config.set_multivar('core.override4', '^ba', 'qux') + assert list(config.get_multivar('core.override4')) == ['qux'] + + # try deleting some stuff + assert 'core.override5' not in config + config['core.override5'] = 'to be deleted' + assert 'core.override5' in config + assert config['core.override5'] == 'to be deleted' + del config['core.override5'] + assert 'core.override5' not in config + + config.set_multivar('core.override5', '^$', 'lorem') + config.set_multivar('core.override5', '^$', 'ipsum') + config.set_multivar('core.override5', '^$', 'dolor') + config.set_multivar('core.override5', '^$', 'simet') + assert 'core.override5' in config + assert list( + config.get_multivar('core.override5') + ) == ['lorem', 'ipsum', 'dolor', 'simet'] + config.delete_multivar('core.override5', r'.*or.*') + assert 'core.override5' in config + assert list( + config.get_multivar('core.override5') + ) == ['ipsum', 'simet'] + config.delete_multivar('core.override5', r'.*') + assert 'core.override5' not in config + + # it all should have been erased + assert 'core.override1' not in config + assert 'core.override2' not in config + assert 'core.override3' not in config + assert 'core.override4' not in config + assert 'core.override5' not in config + + with config: + # let's try some different values now + assert 'core.override1' not in config + config['core.override1'] = False + assert 'core.override1' in config + assert not config.get_bool('core.override1') + + assert 'core.override2' not in config + config['core.override2'] = 81 + assert 'core.override2' in config + assert config.get_int('core.override2') == 81 + + assert 'core.override3' not in config + config['core.override3'] = 'dolor simet' + assert 'core.override3' in config + assert config['core.override3'] == 'dolor simet' + + # it all should have been erased again + assert 'core.override1' not in config + assert 'core.override2' not in config + assert 'core.override3' not in config + assert 'core.override4' not in config + assert 'core.override5' not in config From 04ca57341fbff09a382acd564e5ccd805bbf87da Mon Sep 17 00:00:00 2001 From: Nick Williams Date: Thu, 2 Jul 2026 11:52:41 -0500 Subject: [PATCH 06/13] Lots of documentation improvements --- docs/config.rst | 19 +- pygit2/config.py | 538 ++++++++++++++++++++++++++++++++++---------- test/test_config.py | 26 ++- 3 files changed, 442 insertions(+), 141 deletions(-) diff --git a/docs/config.rst b/docs/config.rst index 19258aad..638b9aac 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -2,18 +2,27 @@ Configuration files ********************************************************************** -.. autoclass:: pygit2.Repository - :members: config - :noindex: +.. autoclass:: pygit2.repository.BaseRepository + :members: config, config_snapshot -The Config type +The Config types ================ .. autoclass:: pygit2.Config :members: :undoc-members: - :special-members: __contains__, __delitem__, __getitem__, __iter__, __setitem__ + :special-members: __contains__, __delitem__, __getitem__, __init__, __iter__, __setitem__ + +.. autoclass:: pygit2.DefaultConfig + :members: __enter__, __exit__ + :undoc-members: + :special-members: __init__ + +.. autoclass:: pygit2.RepositoryConfig + :members: __enter__, __exit__ + :undoc-members: + :special-members: __init__ The ConfigEntry type diff --git a/pygit2/config.py b/pygit2/config.py index 62e25587..60c01ef4 100644 --- a/pygit2/config.py +++ b/pygit2/config.py @@ -31,7 +31,19 @@ from collections.abc import Callable, Generator, Iterator from os import PathLike from types import TracebackType -from typing import TYPE_CHECKING, Literal, Self, cast, overload, override + +# Suppressing UP035 because ruff complains about using `Type` instead of `type` but +# Sphinx complains about using `type`, and there's no workaround for the Sphinx issue. +from typing import ( # noqa: UP035 + TYPE_CHECKING, + Literal, + NoReturn, + Self, + Type, + cast, + overload, + override, +) try: from functools import cached_property @@ -63,13 +75,15 @@ def str_to_bytes(value: str | bytes, name: str) -> bytes: - if not isinstance(value, str): - raise TypeError(f'{name} must be a string') + if isinstance(value, bytes): + return value + if isinstance(value, str): + return to_bytes(value) - return to_bytes(value) + raise TypeError(f'{name} must be a string or bytes') -class ConfigIterator: +class _BaseIterator: def __init__(self, config, ptr) -> None: self._iter = ptr self._config = config @@ -77,12 +91,9 @@ def __init__(self, config, ptr) -> None: def __del__(self) -> None: C.git_config_iterator_free(self._iter) - def __iter__(self) -> 'ConfigIterator': + def __iter__(self) -> Self: return self - def __next__(self) -> 'ConfigEntry': - return self._next_entry() - def _next_entry(self) -> 'ConfigEntry': self._config._stored_exception = None centry = ffi.new('git_config_entry **') @@ -92,8 +103,13 @@ def _next_entry(self) -> 'ConfigEntry': return ConfigEntry._from_c(centry[0], self) -class ConfigMultivarIterator(ConfigIterator): - def __next__(self) -> str | None: # type: ignore[override] +class ConfigIterator(_BaseIterator): + def __next__(self) -> 'ConfigEntry': + return self._next_entry() + + +class ConfigMultivarIterator(_BaseIterator): + def __next__(self) -> str | None: entry = self._next_entry() return entry.value @@ -106,9 +122,9 @@ class Config: the constructor or by using one of the static methods :meth:`Config.get_system_config`, :meth:`Config.get_global_config`, or :meth:`Config.get_xdg_config`. Additional files can be loaded into the - `Config` object using :meth:`Config.add_file`. + ``Config`` object using :meth:`Config.add_file`. - Changes made to the configuration with :meth:`Config.set_multivar` are + Changes made to the configuration with one of the mutator methods are immediately persisted to disk. Reads performed with accessor methods like :meth:`Config.get_multivar` or :meth:`Config.__getitem__` may result in reading from different versions of the configuration file if this or @@ -118,16 +134,27 @@ class Config: is especially important when iterating, as the contents of the config might change mid-iteration if you don't use a snapshot. + Some ``Config`` objects may be backed by multiple files/backends, such as + if you call :meth:`Config.add_file` with different levels or construct + one of :class:`DefaultConfig` or :class:`RepositoryConfig`. In this case, + read operations begin at the backend with the highest + :class:`pygit2.enums.ConfigLevel` and proceed down one level at a time, + and write operations affect only the highest-level non-read-only backend. + This class can technically be used to manually read and write a repository's local configuration by pointing the constructor to the repository's ``.git/config`` file, but this is not recommended. The resulting ``Config`` object represents only the configuration directly within ``.git/config``. It does not represent the total effective configuration for that repository that includes the combined program, system, XDG, global (user), and local - configurations. Instead, use :meth:`BaseRepository.config` or - :meth:`BaseRepository.config_snapshot` for loading a local configuration - and see :class:`RepositoryConfig` for special behaviors supported by the - local configuration. + configurations. Instead, use :meth:`pygit2.repository.BaseRepository.config` + or :meth:`pygit2.repository.BaseRepository.config_snapshot` for loading a + local configuration and see :class:`RepositoryConfig` for special behaviors + supported by the local configuration. + + For the non-repository global configuration used by Git during operations + not involving an existing repository (such as cloning), see + :class:`DefaultConfig`. """ _config: 'GitConfigC' @@ -145,8 +172,13 @@ def __init__(self, path: PathLike | str, /) -> None: """Constructs a ``Config`` object backed by the specified file. The configuration from the specified file is loaded, and subsequent writes - will persist to that file. Additional files can be added to the config, - with different levels, using :meth:`Config.add_file`. + will persist to that file. The file backend is assigned + :attr:`pygit2.enums.ConfigLevel.LOCAL`. Additional files can be added to the + config, with different levels, using :meth:`Config.add_file`. + + :param path: The path to the configuration file to load as a string or path-like + value. + :raises IOError: If the path specified does not exist or could not be opened. """ ... @@ -165,6 +197,29 @@ def __init__( c_config: 'GitConfigC | None' = None, is_snapshot: bool = False, ) -> None: + """Constructs a ``Config`` object. + + **Overload 1: __init__()** + + Constructs a new, empty ``Config`` object pointing to no file. To make changes + to this ``Config`` object, you must use :meth:`Config.add_file`. + + **Overload 2: __init__(path)** + + The configuration from the specified file is loaded, and subsequent writes + will persist to that file. The file backend is assigned + :attr:`pygit2.enums.ConfigLevel.LOCAL`. Additional files can be added to the + config, with different levels, using :meth:`Config.add_file`. + + :param path: The path to the configuration file to load as a string or path-like + value. + :raises IOError: If the path specified does not exist or could not be opened. + + **Overload 3: __init__(c_config, is_snapshot)** + + For internal use only. + """ + # ^^ see https://github.com/sphinx-doc/sphinx/issues/7787 if path is not None and c_config is not None: raise ValueError('Cannot initialize Config from both path and c_config') @@ -197,31 +252,37 @@ def _check_error(self, err: int, io: bool = False): finally: self._stored_exception = None - def _get(self, key: str | bytes) -> tuple[int, 'ConfigEntry | None']: - key = str_to_bytes(key, 'key') + def _get(self, name: str | bytes) -> tuple[int, 'ConfigEntry | None']: + name = str_to_bytes(name, 'name') entry = ffi.new('git_config_entry **') - err = C.git_config_get_entry(entry, self._config, key) + err = C.git_config_get_entry(entry, self._config, name) if err >= 0: return err, ConfigEntry._from_c(entry[0]) return err, None - def _get_entry(self, key: str | bytes) -> 'ConfigEntry': + def _get_entry(self, name: str | bytes) -> 'ConfigEntry': self._stored_exception = None - err, entry = self._get(key) + err, entry = self._get(name) if err == C.GIT_ENOTFOUND: - raise KeyError(key) + raise KeyError(name) self._check_error(err) assert entry is not None return entry - def __contains__(self, key: str | bytes) -> bool: + def __contains__(self, name: str | bytes) -> bool: + """Indicates whether the configuration contains ``name``. + + :param name: The name of the config var to look for. + :returns: ``True`` if any file/backend in this ``Config`` object contains any + config var with the name ``name``, ``False`` otherwise. + """ self._stored_exception = None - err, _ = self._get(key) + err, _ = self._get(name) if err == C.GIT_ENOTFOUND: return False @@ -230,43 +291,96 @@ def __contains__(self, key: str | bytes) -> bool: return True - def __getitem__(self, key: str | bytes) -> str | None: - """ - When using the mapping interface, the value is returned as a string. In - order to apply the git-config parsing rules, you can use - :meth:`Config.get_bool` or :meth:`Config.get_int`. + def __getitem__(self, name: str | bytes) -> str | None: + """Obtain the first value found for a config var with name ``name``. + + Looks for a config var with name ``name`` and returns its value as a string + or ``None`` if the config var exists but the value is empty. If you want to + apply the Git config parsing rules, use :meth:`Config.get_bool` or + :meth:`Config.get_int` instead of ``[name]``. + + If the config var found is a multivar, only the first value is returned. To + obtain all values, use :meth:`Config.get_multivar` instead of ``[name]``. + + If this ``Config`` is backed by multiple files/backends, the one with the highest + :class:`pygit2.enums.ConfigLevel` is searched first, and then the next-highest + level, and so on. + + :param name: The name of the config var to look for. + + :returns: The string value found for the config var with name ``name`` or, + in the case of a valueless flag, ``None``. + + :raises KeyError: If no config var with name ``name`` is found in any + of this ``Config``'s files/backends. """ - entry = self._get_entry(key) + entry = self._get_entry(name) return entry.value - def __setitem__(self, key: str | bytes, value: bool | int | str | bytes) -> None: + def __setitem__(self, name: str | bytes, value: bool | int | str | bytes) -> None: + """Set the config var with name ``name`` to the specified ``value``. + + If this ``Config`` is backed by multiple files/backends, only the first + non-read-only backend with the highest :class:`pygit2.enums.ConfigLevel` is + changed. If that backend is a file, changes are persisted to disk immediately. + + If the config var exists already, it is replaced. If it is already a multivar, + all values are removed in favor of this new value. If you want to add values + to a multivar instead of replacing them, use :meth:`Config.set_multivar` instead + of ``[name] = value``. + + :param name: The name of the config var to set. + :param value: The new value to set. + """ self._stored_exception = None - key = str_to_bytes(key, 'key') + name = str_to_bytes(name, 'name') err: int if isinstance(value, bool): - err = C.git_config_set_bool(self._config, key, value) + err = C.git_config_set_bool(self._config, name, value) elif isinstance(value, int): - err = C.git_config_set_int64(self._config, key, value) + err = C.git_config_set_int64(self._config, name, value) else: - err = C.git_config_set_string(self._config, key, to_bytes(value)) + err = C.git_config_set_string(self._config, name, to_bytes(value)) self._check_error(err) - def __delitem__(self, key: str | bytes) -> None: + def __delitem__(self, name: str | bytes) -> None: + """Deletes the config var with name ``name``. + + If this ``Config`` is backed by multiple files/backends, only the first + non-read-only backend with the highest :class:`pygit2.enums.ConfigLevel` is + changed. If that backend is a file, changes are persisted to disk immediately. + + If the config var is a multivar, all values are removed. If you want to remove + only some values, use :meth:`Config.delete_multivar` instead of ``del [name]``. + + :param name: Name of the config var to delete. + :raises KeyError: If no config var with name ``name`` is found in the first + non-read-only backend with the highest ``ConfigLevel``. + """ self._stored_exception = None - key = str_to_bytes(key, 'key') + name = str_to_bytes(name, 'name') - err = C.git_config_delete_entry(self._config, key) + err = C.git_config_delete_entry(self._config, name) self._check_error(err) def __iter__(self) -> Iterator['ConfigEntry']: - """ - Iterate over configuration entries, returning a ``ConfigEntry`` - objects. These contain the name, level, and value of each configuration + """Iterate over configuration entries in the form of + :class:`pygit2.config.ConfigEntry` objects. + + Creates and returns an iterator over all of the entries in this ``Config`` + object. Entries contain the name, level, and value of each configuration variable. Be aware that this may return multiple versions of each entry if they are set multiple times in the configuration files. + + If this ``Config`` is backed by multiple files/backends, iteration begins with + all config vars from the backend with the highest + :class:`pygit2.enums.ConfigLevel` first, and then proceeds to the one with the + next-highest level, and so on. + + :returns: An iterator over :class:`pygit2.config.ConfigEntry` objects. """ self._stored_exception = None citer = ffi.new('git_config_iterator **') @@ -276,12 +390,28 @@ def __iter__(self) -> Iterator['ConfigEntry']: return ConfigIterator(self, citer[0]) def get_multivar( - self, name: str | bytes, regex: str | None = None - ) -> ConfigMultivarIterator: - """Get each value of a multivar ''name'' as a list of strings. - - The optional ''regex'' parameter is expected to be a regular expression - to filter the variables we're interested in. + self, + name: str | bytes, + regex: str | None = None, + ) -> Iterator[str | None]: + """Get each value of a multivar ``name`` as an iterator of strings. + + If this ``Config`` is backed by multiple files/backends, iteration begins with + all matching multivars from the backend with the highest + :class:`pygit2.enums.ConfigLevel` first, and then proceeds to the one with the + next-highest level, and so on. + + :param name: The name of the multivar to iterate. + :param regex: If specified, only multivar values matching this regular expression + will be iterated. The regular expression is matched case-sensitively. + To iterate over all values, do not specify this argument, or use + the regular expression ``r'.*'``. + + :returns: An iterator of config var values, each of which will be either a + string or, in the case of a valueless flag, ``None``. + + :raises KeyError: If no config var with name ``name`` is found in any of the + files/backends for this config. """ self._stored_exception = None name = str_to_bytes(name, 'name') @@ -294,11 +424,24 @@ def get_multivar( return ConfigMultivarIterator(self, citer[0]) def set_multivar( - self, name: str | bytes, regex: str | bytes, value: str | bytes + self, + name: str | bytes, + regex: str | bytes, + value: str | bytes, ) -> None: - """Set a multivar ''name'' to ''value''. ''regexp'' is a regular - expression to indicate which values to replace. Changes are persisted - to the configuration file(s) backing this ``Config``. + """Add a ``value`` to multivar ``name``, optionally replacing other values + matching ``regex``. + + If this ``Config`` is backed by multiple files/backends, only the first + non-read-only backend with the highest :class:`pygit2.enums.ConfigLevel` is + changed. If that backend is a file, changes are persisted to disk immediately. + + :param name: The name of the multivar to set. + :param regex: A (required) regular expression to indicate which values to + replace. It is matched case-sensitively. Values that match are + removed, values that do not match are kept. To merely add + without removing any values, use the regular expression ``r'^$'``. + :param value: The value to add to the multivar. """ self._stored_exception = None name = str_to_bytes(name, 'name') @@ -309,9 +452,20 @@ def set_multivar( self._check_error(err) def delete_multivar(self, name: str | bytes, regex: str | bytes) -> None: - """Delete a multivar ''name''. ''regexp'' is a regular expression to - indicate which values to delete. Changes are persisted to the - configuration file(s) backing this ``Config``. + """Delete values from a multivar with name ``name``. + + If this ``Config`` is backed by multiple files/backends, only the first + non-read-only backend with the highest :class:`pygit2.enums.ConfigLevel` is + changed. If that backend is a file, changes are persisted to disk immediately. + + :param name: The name of the multivar to delete. + :param regex: A (required) regular expression to indicate which values to + delete. It is matched case-sensitively. To delete all values of + the multivar, use the regular expression ``r'.*'`` or, + alternatively, ``del config[name]``. + + :raises KeyError: If no config var with name ``name`` is found in the first + non-read-only backend with the highest ``ConfigLevel``. """ self._stored_exception = None name = str_to_bytes(name, 'name') @@ -320,51 +474,107 @@ def delete_multivar(self, name: str | bytes, regex: str | bytes) -> None: err = C.git_config_delete_multivar(self._config, name, regex) self._check_error(err) - def get_bool(self, key: str | bytes) -> bool: - """Look up *key* and parse its value as a boolean as per the git-config - rules. Return a boolean value (True or False). + def get_bool(self, name: str | bytes) -> bool: + """Obtain the first value found for a config var with name ``name`` as a boolean. + + Looks for a config var with name ``name`` and returns its value as a ``bool``. + If the config var found is a multivar, only the first value is returned. If you + need all the values of a multivar, use :meth:`Config.parse_bool` on each + value returned by :meth:`Config.get_multivar`, instead. + + If this ``Config`` is backed by multiple files/backends, the one with the highest + :class:`pygit2.enums.ConfigLevel` is searched first, and then the next-highest + level, and so on. + + Truthy values are: 'true', '1', 'on', 'yes', or an empty value / ``NULL``. + Falsy values are: 'false', '0', 'off', or 'no'. + + :param name: The name of the config var to look for. + + :returns: The boolean value found for the config var with name ``name`` as + parsed by ``git_config_parse_bool`` rules. - Truthy values are: 'true', 1, 'on' or 'yes'. Falsy values are: 'false', - 0, 'off' and 'no' + :raises KeyError: If no config var with name ``name`` is found in any + of this ``Config``'s files/backends. + :raises ValueError: If the value does not match one of the expected truthy + or falsy values. """ self._stored_exception = None - entry = self._get_entry(key) - res = ffi.new('int *') - err = C.git_config_parse_bool(res, entry.c_value) - self._check_error(err) + entry = self._get_entry(name) + return self.parse_bool(entry.value) - return res[0] != 0 + def get_int(self, name: bytes | str) -> int: + """Obtain the first value found for a config var with name ``name`` as an integer. + + Looks for a config var with name ``name`` and returns its value as an ``int``. + If the config var found is a multivar, only the first value is returned. If you + need all the values of a multivar, use :meth:`Config.parse_int` on each + value returned by :meth:`Config.get_multivar`, instead. + + If this ``Config`` is backed by multiple files/backends, the one with the highest + :class:`pygit2.enums.ConfigLevel` is searched first, and then the next-highest + level, and so on. + + A value can have a suffix 'k', 'm', or 'g' which stand for 'kilo', + 'mega', and 'giga', respectively. + + :param name: The name of the config var to look for. - def get_int(self, key: bytes | str) -> int: - """Look up *key* and parse its value as an integer as per the git-config - rules. Return an integer. + :returns: The integer value found for the config var with name ``name`` as + parsed by ``git_config_parse_int64`` rules. - A value can have a suffix 'k', 'm' or 'g' which stand for 'kilo', - 'mega' and 'giga' respectively. + :raises KeyError: If no config var with name ``name`` is found in any + of this ``Config``'s files/backends. + :raises ValueError: If the value is ``None`` or empty or otherwise does not + match the ``git_config_parse_int64`` parsing rules. """ self._stored_exception = None - entry = self._get_entry(key) - res = ffi.new('int64_t *') - err = C.git_config_parse_int64(res, entry.c_value) - self._check_error(err) - - return res[0] + entry = self._get_entry(name) + return self.parse_int(entry.value) def add_file( self, path: str | PathLike, level: ConfigLevel | int | None = None, - force: int = 0, + force: bool | int = False, ) -> None: - """Add a config file instance to an existing config.""" + """Add a config file backend to this ``Config`` object. + + In a multi-backend ``Config`` object, write operations occur only against + the first non-read-only backend with the highest + :class:`pygit2.enums.ConfigLevel`, and read operations start with the backend + with the highest :class:`pygit2.enums.ConfigLevel` and continue down through + lower backends. Keep this in mind when choosing the value for ``level``. + + :param path: The path of the config file to add. + :param level: The config level for the new file backend. If not specified, the + special value "0" is used. This makes this new file the + lowest-priority backend of all, superseded by all other backends + in this config. + :param force: If another backend (of any kind) with the same level already + exists, that backend will be replaced if ``force`` is ``True`` or + 1, otherwise a ``ValueError`` will be raised. + + :raises ValueError: If another backend with the same level already exists and + ``force`` was not specified or was ``False`` or 0. + """ self._stored_exception = None if level is None: level = 0 elif isinstance(level, ConfigLevel): level = level.value + if force is True: + force = 1 + elif force is False: + force = 0 + err = C.git_config_add_file_ondisk( - self._config, to_bytes(path), level, ffi.NULL, force + self._config, + to_bytes(path), + level, + ffi.NULL, + force, ) self._check_error(err) @@ -372,6 +582,8 @@ def add_file( def is_snapshot(self) -> bool: """Indicates whether this Config object is a read-only snapshot of the underlying configuration. + + :returns: ``True`` if it's a read-only snapshot, ``False`` otherwise. """ return self._is_snapshot @@ -381,7 +593,7 @@ def snapshot(self) -> Config: This means that looking up multiple values will use the same version of the configuration files. - Raises ``TypeError`` if this is already a snapshot. + :raises TypeError: If this is already a snapshot. """ if self._is_snapshot: raise TypeError('This config is already a snapshot.') @@ -399,7 +611,21 @@ def _c_snapshot(self) -> 'GitConfigC': # @staticmethod - def parse_bool(text: str) -> bool: + def parse_bool(text: str | None) -> bool: + """Parses the provided string ``text`` as a boolean value according to + Git config parsing rules. + + Truthy values are: 'true', '1', 'on', 'yes', or an empty value / ``NULL``. + Falsy values are: 'false', '0', 'off', or 'no'. + + :param text: The string to convert. + + :returns: The boolean value of the ``text`` as parsed by + ``git_config_parse_bool`` rules. + + :raises ValueError: If the value does not match one of the expected truthy + or falsy values. + """ res = ffi.new('int *') err = C.git_config_parse_bool(res, to_bytes(text)) check_error(err) @@ -407,7 +633,24 @@ def parse_bool(text: str) -> bool: return res[0] != 0 @staticmethod - def parse_int(text: str) -> int: + def parse_int(text: str | None) -> int: + """Parses the provided string ``text`` as an integer value according to + Git config parsing rules. + + A value can have a suffix 'k', 'm', or 'g' which stand for 'kilo', + 'mega', and 'giga', respectively. + + :param text: The string to convert. + + :returns: The integer value of the ``text`` as parsed by + ``git_config_parse_int64`` rules. + + :raises ValueError: If the value is ``None`` or empty or otherwise does not + match the ``git_config_parse_int64`` parsing rules. + """ + if not text: + raise ValueError(f'Text value {text!r} is not parseable as an integer.') + res = ffi.new('int64_t *') err = C.git_config_parse_int64(res, to_bytes(text)) check_error(err) @@ -435,7 +678,7 @@ def get_system_config() -> 'Config': The system configuration file is the one found at ``/etc/gitconfig`` or ``%PROGRAMFILES%\\Git\\etc\\gitconfig``, depending on the operating system. - Raises ``IOError`` if the configuration file is not found. + :raises IOError: If the configuration file is not found. """ return Config._from_found_config(C.git_config_find_system) @@ -448,7 +691,7 @@ def get_global_config() -> 'Config': at the XDG-compatible user config file location (for that, see :meth:`Config.get_xdg_config`). - Raises ``IOError`` if the configuration file is not found. + :raises IOError: If the configuration file is not found. """ return Config._from_found_config(C.git_config_find_global) @@ -460,7 +703,7 @@ def get_xdg_config() -> 'Config': This file is located at ``$HOME/.config/git/config``. This will not find the file at the standard user config location (for that, see :meth:`Config.get_global_config`). - Raises ``IOError`` if the configuration file is not found. + :raises IOError: If the configuration file is not found. """ return Config._from_found_config(C.git_config_find_xdg) @@ -490,11 +733,13 @@ def __enter__(self) -> Self: """Enter a context where all writes occur against an in-memory configuration. When entered, all subsequent writes occur against an in-memory configuration - backend and do not get persisted to the underlying config file(s). As long as + backend and do not get persisted to the underlying config file. As long as the context endures, Git operations will use the sum total configuration that includes the in-memory configuration. - Raises ``TypeError`` if this is a read-only snapshot of the configuration. + :returns: ``self`` + + :raises TypeError: If this is a read-only snapshot of the configuration. """ if self._is_snapshot: raise TypeError( @@ -509,15 +754,17 @@ def __enter__(self) -> Self: def __exit__( self, - exc_type: type[BaseException] | None, + exc_type: Type[BaseException] | None, exc_val: BaseException | None, exc_tb: TracebackType | None, ) -> Literal[False]: - """Exit the context so that writes occur against the repository config again. + """Exit the context so that writes occur against the config file again. When exited, any in-memory configuration is erased so that it is no longer - effective for repository operations, and subsequent writes again occur against - the repository's config and persist to the repository's config file. + effective for Git operations, and subsequent writes again occur against + the underlying config file and persist to this file. + + :returns: ``False`` """ if self._backend_added: self._change_write_priority(ConfigLevel.LOCAL) @@ -542,16 +789,17 @@ def _change_write_priority(self, level: ConfigLevel) -> None: class DefaultConfig(_InMemoryAppBackendConfig): - """A special-case :class:`Config` extension representing the total default configuration. + """A special-case :class:`Config` extension representing the total default + configuration. This extension to the base ``Config`` class represents the total default configuration outside the context of a repository (for that, see :class:`RepositoryConfig`). It also serves as a semantic indicator of the scope of the configuration. - The ``DefaultConfig`` includes the program, system, XDG, and global (user) configurations. - This is, in essence, the "effective" configuration that Git uses when performing - operations that are not against a repository. When a read operation occurs, the - configurations are searched in the following order: global (user), XDG, system, + The ``DefaultConfig`` includes the program, system, XDG, and global (user) + configurations. This is, in essence, the "effective" configuration that Git uses when + performing operations that are not against a repository. When a read operation occurs, + the configurations are searched in the following order: global (user), XDG, system, and then program data. The ``DefaultConfig`` can also be used as a context manager to effect a temporary @@ -572,7 +820,9 @@ class DefaultConfig(_InMemoryAppBackendConfig): @overload def __init__(self, /) -> None: - """Load the total default configuration and construct a ``DefaultConfig`` from it.""" + """Load the total default configuration and construct a ``DefaultConfig`` + from it. + """ ... @overload @@ -585,6 +835,16 @@ def __init__(self, *, c_snapshot: 'GitConfigC') -> None: ... def __init__(self, *, c_snapshot: 'GitConfigC | None' = None): + """Constructs a ``DefaultConfig`` object. + + **Overload 1: __init__()** + + Loads the total default configuration. + + **Overload 2: __init__(c_snapshot)** + + For internal use only. + """ if c_snapshot is not None: super().__init__(c_config=c_snapshot, is_snapshot=True) else: @@ -600,7 +860,7 @@ def snapshot(self) -> DefaultConfig: This means that looking up multiple values will use the same version of the configuration files. - Raises ``TypeError`` if this is already a snapshot. + :raises TypeError: If this is already a snapshot. """ if self._is_snapshot: raise TypeError('This default config is already a snapshot.') @@ -610,33 +870,47 @@ def snapshot(self) -> DefaultConfig: def _add_backend_to_config(self, backend: _InMemoryBackend) -> None: backend.add_to_config(self._config) + @override + def add_file( + self, + path: str | PathLike, + level: ConfigLevel | int | None = None, + force: bool | int = False, + ) -> NoReturn: + """ + :raises TypeError: The default configuration does not support adding files. + """ + raise TypeError('The default configuration does not support adding files.') + class RepositoryConfig(_InMemoryAppBackendConfig): - """A special-case :class:`Config` extension that handles local (repository) configuration. + """A special-case :class:`Config` extension that handles local (repository) + configuration. This extension to the base ``Config`` class handles some of the special behaviors associated with repository configs, as well as serving as a semantic indicator for the scope of the configuration. You should not construct it directly, but instead - use one of :meth:`BaseRepository.config` or :meth:`BaseRepository.config_snapshot` - to obtain the local configuration. + use one of :meth:`pygit2.repository.BaseRepository.config` or + :meth:`pygit2.repository.BaseRepository.config_snapshot` to obtain the local + configuration. The ``RepositoryConfig`` represents not just the configuration present in ``.git/config``, but the sum total of that plus the program, system, XDG, and global - (user) configurations. This is, in essence, the "effective" configuration that Git uses - when performing operations against this repository. When a read operation occurs, - the local configuration is searched first, then the global (user), XDG, system, and - finally program data configurations, in that order. When a write operation occurs, - only the local configuration is changed. + (user) configurations. This is, in essence, the "effective" configuration that Git + uses when performing operations against this repository. When a read operation + occurs, the local configuration is searched first, then the global (user), XDG, + system, and finally program data configurations, in that order. When a write + operation occurs, only the local configuration is changed. The ``RepositoryConfig`` can also be used as a context manager to effect a temporary in-memory override of the local configuration. When the context manager is entered, an empty in-memory configuration backend is assigned to the configuration and given - the highest read priority. During this context, write operations change the in-memory - backend and do not affect the local configuration file. Read operations—including those - performed by Git itself—consult the in-memory backend first before then consulting the - usual order. When the context manager exits, the in-memory backend's contents are - erased, undoing any changes made to it and allowing write operations to resume - affecting the local configuration. + the highest read priority. During this context, write operations change the + in-memory backend and do not affect the local configuration file. Read + operations—including those performed by Git itself—consult the in-memory backend + first before then consulting the usual order. When the context manager exits, the + in-memory backend's contents are erased, undoing any changes made to it and + allowing write operations to resume affecting the local configuration. The context manager can be re-entered and then re-exited repeatedly; it is not a one-use-only operation. @@ -655,8 +929,9 @@ def __init__( ) -> None: """For internal use only. - See :meth:`BaseRepository.config` for obtaining a repository configuration - or :meth:`BaseRepository.config_snapshot` for obtaining a snapshot. + See :meth:`pygit2.repository.BaseRepository.config` for obtaining a repository + configuration or :meth:`pygit2.repository.BaseRepository.config_snapshot` for + obtaining a snapshot. Constructs a new ``RepositoryConfig`` from the given repository. @@ -676,8 +951,8 @@ def __init__( ) -> None: """For internal use only. - See :meth:`BaseRepository.config_snapshot` for obtaining a repository - configuration snapshot. + See :meth:`pygit2.repository.BaseRepository.config_snapshot` for obtaining a + repository configuration snapshot. Constructs a ``RepositoryConfig`` from a given snapshot config object pointer. The resulting configuration will be read-only. @@ -692,6 +967,7 @@ def __init__( do_snapshot: bool = False, c_snapshot: 'GitConfigC | None' = None, ) -> None: + """Constructs a ``RepositoryConfig`` object. For internal use only.""" self._repo = repo self._c_repo = c_repo @@ -714,7 +990,7 @@ def snapshot(self) -> RepositoryConfig: of the configuration files. The resulting ``RepositoryConfig`` cannot be used as a context manager, because it is read-only. - Raises ``TypeError`` if this is already a snapshot. + :raises TypeError: If this is already a snapshot. """ if self._is_snapshot: raise TypeError('This repository config is already a snapshot.') @@ -724,6 +1000,20 @@ def snapshot(self) -> RepositoryConfig: def _add_backend_to_config(self, backend: _InMemoryBackend) -> None: backend.add_to_config(self._config, self._c_repo) + @override + def add_file( + self, + path: str | PathLike, + level: ConfigLevel | int | None = None, + force: bool | int = False, + ) -> NoReturn: + """ + :raises TypeError: The local repository configuration does not support adding files. + """ + raise TypeError( + 'The local repository configuration does not support adding files.', + ) + class _InMemoryBackend: """For internal use only. @@ -733,8 +1023,8 @@ class _InMemoryBackend: be constructed with (as of 1.9.5) ``git_config_backend_from_string`` or ``git_config_backend_from_values``, but that backend is read-only and cannot be mutated. To implement the semantics of temporary app-level configuration - in :class:`DefaultConfiguration` and :class:`RepositoryConfiguration`, we need - to implement our own backend. + in :class:`DefaultConfig` and :class:`RepositoryConfig`, we need to implement + our own backend. We could do so completely in C, but that has some serious downsides, notably all the memory management and how easy it is to get wrong and either leak or @@ -929,7 +1219,7 @@ def __enter__(self) -> Self: def __exit__( self, - exc_type: type[BaseException] | None, + exc_type: Type[BaseException] | None, exc_val: BaseException | None, exc_tb: TracebackType | None, ) -> Literal[False]: @@ -1470,11 +1760,11 @@ class ConfigEntry: """An entry in a configuration object.""" _entry: 'GitConfigEntryC' - iterator: ConfigIterator | None + iterator: _BaseIterator | None @classmethod def _from_c( - cls, ptr: 'GitConfigEntryC', iterator: ConfigIterator | None = None + cls, ptr: 'GitConfigEntryC', iterator: _BaseIterator | None = None ) -> 'ConfigEntry': """Builds the entry from a ``git_config_entry`` pointer. diff --git a/test/test_config.py b/test/test_config.py index 94982131..df05f216 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -386,14 +386,15 @@ def test_repository_config_in_memory_overrides(config: RepositoryConfig) -> None config.set_multivar('core.override5', '^$', 'dolor') config.set_multivar('core.override5', '^$', 'simet') assert 'core.override5' in config - assert list( - config.get_multivar('core.override5') - ) == ['lorem', 'ipsum', 'dolor', 'simet'] + assert list(config.get_multivar('core.override5')) == [ + 'lorem', + 'ipsum', + 'dolor', + 'simet', + ] config.delete_multivar('core.override5', r'.*or.*') assert 'core.override5' in config - assert list( - config.get_multivar('core.override5') - ) == ['ipsum', 'simet'] + assert list(config.get_multivar('core.override5')) == ['ipsum', 'simet'] config.delete_multivar('core.override5', r'.*') assert 'core.override5' not in config @@ -524,14 +525,15 @@ def test_default_config_in_memory_overrides() -> None: config.set_multivar('core.override5', '^$', 'dolor') config.set_multivar('core.override5', '^$', 'simet') assert 'core.override5' in config - assert list( - config.get_multivar('core.override5') - ) == ['lorem', 'ipsum', 'dolor', 'simet'] + assert list(config.get_multivar('core.override5')) == [ + 'lorem', + 'ipsum', + 'dolor', + 'simet', + ] config.delete_multivar('core.override5', r'.*or.*') assert 'core.override5' in config - assert list( - config.get_multivar('core.override5') - ) == ['ipsum', 'simet'] + assert list(config.get_multivar('core.override5')) == ['ipsum', 'simet'] config.delete_multivar('core.override5', r'.*') assert 'core.override5' not in config From c055ece7051e624b4e95f016cc7e34ca91f4f2c5 Mon Sep 17 00:00:00 2001 From: Nick Williams Date: Thu, 2 Jul 2026 11:59:36 -0500 Subject: [PATCH 07/13] Update AUTHORS and add assertions to test --- AUTHORS.md | 1 + test/test_config.py | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/AUTHORS.md b/AUTHORS.md index 0868ce4a..a6d600f0 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -209,6 +209,7 @@ Authors: Michał Górny Mukunda Rao Katta Na'aman Hirschfeld + Nicholas Williams Nicolas Rybowski Nicolás Sanguinetti Nikita Kartashov diff --git a/test/test_config.py b/test/test_config.py index df05f216..bde5faf9 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -256,6 +256,12 @@ def test_repository_config_snapshot(config: RepositoryConfig) -> None: "cannot set 'something.other.changed': the configuration is read-only", lambda: snapshot.set_multivar('something.other.changed', '^$', 'foo'), ) + utils.assertRaisesWithArg( + TypeError, + 'A read-only config snapshot cannot be used as a context manager, ' + 'because its backend data cannot be changed.', + snapshot.__enter__, + ) assert 'core.snapshot1' not in config assert 'core.snapshot1' not in snapshot @@ -319,6 +325,12 @@ def test_default_config_snapshot() -> None: "cannot set 'something.other.changed': the configuration is read-only", lambda: snapshot.set_multivar('something.other.changed', '^$', 'foo'), ) + utils.assertRaisesWithArg( + TypeError, + 'A read-only config snapshot cannot be used as a context manager, ' + 'because its backend data cannot be changed.', + snapshot.__enter__, + ) def test_repository_config_in_memory_overrides(config: RepositoryConfig) -> None: From da438ebf50b7df537521398a636f18ba094762d2 Mon Sep 17 00:00:00 2001 From: Nick Williams Date: Thu, 2 Jul 2026 12:35:14 -0500 Subject: [PATCH 08/13] Case insensitivity --- pygit2/config.py | 26 +++++++++++++++----------- test/test_config.py | 12 ++++++++---- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/pygit2/config.py b/pygit2/config.py index 60c01ef4..1fd4b380 100644 --- a/pygit2/config.py +++ b/pygit2/config.py @@ -1298,7 +1298,7 @@ def _config_memory_backend_get( backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: - key = ffi.string(name).decode('utf-8') + key = ffi.string(name).decode('utf-8').lower() if key not in self._read_data or not self._read_data[key]: return C.GIT_ENOTFOUND @@ -1341,7 +1341,7 @@ def _config_memory_backend_set( backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: - key = ffi.string(name).decode('utf-8') + key = ffi.string(name).decode('utf-8').lower() decoded_value = ffi.string(value).decode('utf-8') self._write_data[key] = [_InMemoryBackend.Entry(key, decoded_value)] except BaseException as e: @@ -1379,7 +1379,7 @@ def _config_memory_backend_set_multivar( backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: - key = ffi.string(name).decode('utf-8') + key = ffi.string(name).decode('utf-8').lower() with self.write_lock(): if key in self._write_data and regexp != ffi.NULL: expression = re.compile(ffi.string(regexp).decode('utf-8')) @@ -1418,7 +1418,7 @@ def _config_memory_backend_del( backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: - key = ffi.string(name).decode('utf-8') + key = ffi.string(name).decode('utf-8').lower() with self.write_lock(): if key not in self._write_data: return C.GIT_ENOTFOUND @@ -1452,7 +1452,7 @@ def _config_memory_backend_del_multivar( backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: - key = ffi.string(name).decode('utf-8') + key = ffi.string(name).decode('utf-8').lower() with self.write_lock(): if key not in self._write_data: return C.GIT_ENOTFOUND @@ -1635,13 +1635,17 @@ def _config_memory_backend_free(backend: 'GitConfigBackendC') -> None: C signature: void free(git_config_backend *backend); """ - backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) - self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: - self.clear() - except BaseException as e: - self._config._stored_exception = e - # nothing we can do here because of the void return type + backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) + self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) + try: + self.clear() + except BaseException as e: + self._config._stored_exception = e + # nothing we can do here because of the void return type + except (AttributeError, TypeError, RuntimeError, ReferenceError): + # The Python interpreter is exiting when this is called. Nothing we can do. + pass @ffi.def_extern() diff --git a/test/test_config.py b/test/test_config.py index bde5faf9..71db305e 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -451,11 +451,13 @@ def test_repository_config_in_memory_overrides(config: RepositoryConfig) -> None assert 'core.local3' in config assert config['core.local3'] == 'lorem ipsum' - # let's try some different values now + # let's try some different values now (and case insensitivity) assert 'core.override1' not in config - config['core.override1'] = False + config['core.oVeRrIdE1'] = False assert 'core.override1' in config + assert 'core.oVeRrIdE1' in config assert not config.get_bool('core.override1') + assert not config.get_bool('core.oVeRrIdE1') assert 'core.override2' not in config config['core.override2'] = 81 @@ -557,11 +559,13 @@ def test_default_config_in_memory_overrides() -> None: assert 'core.override5' not in config with config: - # let's try some different values now + # let's try some different values now (and case insensitivity) assert 'core.override1' not in config - config['core.override1'] = False + config['core.oVeRrIdE1'] = False assert 'core.override1' in config + assert 'core.oVeRrIdE1' in config assert not config.get_bool('core.override1') + assert not config.get_bool('core.oVeRrIdE1') assert 'core.override2' not in config config['core.override2'] = 81 From 6b17ab75a448e77eaa7ed66ec8a64fdebc0658dd Mon Sep 17 00:00:00 2001 From: Nick Williams Date: Thu, 2 Jul 2026 19:54:56 -0500 Subject: [PATCH 09/13] Address memory management issues with ConfigEntry --- docs/config.rst | 6 +- pygit2/config.py | 190 +++++++++++++++++++++++++++++------------------ pygit2/enums.py | 4 + 3 files changed, 124 insertions(+), 76 deletions(-) diff --git a/docs/config.rst b/docs/config.rst index 638b9aac..f33070c2 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -15,12 +15,12 @@ The Config types :special-members: __contains__, __delitem__, __getitem__, __init__, __iter__, __setitem__ .. autoclass:: pygit2.DefaultConfig - :members: __enter__, __exit__ + :members: __enter__, __exit__, add_file, snapshot :undoc-members: :special-members: __init__ .. autoclass:: pygit2.RepositoryConfig - :members: __enter__, __exit__ + :members: __enter__, __exit__, add_file, snapshot :undoc-members: :special-members: __init__ @@ -29,4 +29,4 @@ The ConfigEntry type ==================== .. autoclass:: pygit2.config.ConfigEntry - :members: name, value, level + :members: level, name, raw_level, raw_name, raw_value, value diff --git a/pygit2/config.py b/pygit2/config.py index 1fd4b380..654be1ed 100644 --- a/pygit2/config.py +++ b/pygit2/config.py @@ -84,23 +84,45 @@ def str_to_bytes(value: str | bytes, name: str) -> bytes: class _BaseIterator: - def __init__(self, config, ptr) -> None: - self._iter = ptr + def __init__(self, config: 'Config', c_iter: 'GitConfigIteratorC') -> None: self._config = config + self._iter = c_iter + + self._freed = False + self._c_entry_ptr = ffi.new('git_config_entry **') + + def _free(self): + if not self._freed: + self._freed = True + self._c_entry_ptr = None # do not free; see comment in _next_entry + C.git_config_iterator_free(self._iter) def __del__(self) -> None: - C.git_config_iterator_free(self._iter) + self._free() # fallback in case we never hit StopIteration below def __iter__(self) -> Self: return self def _next_entry(self) -> 'ConfigEntry': - self._config._stored_exception = None - centry = ffi.new('git_config_entry **') - err = C.git_config_next(centry, self._iter) - check_error(err, user_exception=self._config._stored_exception) - - return ConfigEntry._from_c(centry[0], self) + try: + # git_config_next (or, rather, the file backend specifically) re-uses a + # git_config_entry buffer from one call to the next. And more so than just + # this, it re-uses a buffer even from one iterator to the next. It's a + # matter of discussion whether this is correct behavior, but what it means + # is that we should never call git_config_entry_free on an entry allocated + # by git_config_next. And, in fact, we must completely copy the data out of + # one entry before the next git_config_next invocation, or else it might + # get corrupted. See: + # - https://github.com/libgit2/libgit2/issues/7307 + # - https://github.com/libgit2/pygit2/issues/970 + # - https://libgit2.org/docs/reference/v1.9.4/config/git_config_next.html + self._config._stored_exception = None + err = C.git_config_next(self._c_entry_ptr, self._iter) + check_error(err, user_exception=self._config._stored_exception) + return ConfigEntry(self._c_entry_ptr[0]) + except StopIteration: + self._free() # release the memory as soon as we can + raise class ConfigIterator(_BaseIterator): @@ -255,11 +277,14 @@ def _check_error(self, err: int, io: bool = False): def _get(self, name: str | bytes) -> tuple[int, 'ConfigEntry | None']: name = str_to_bytes(name, 'name') - entry = ffi.new('git_config_entry **') - err = C.git_config_get_entry(entry, self._config, name) + c_entry_ptr = ffi.new('git_config_entry **') + err = C.git_config_get_entry(c_entry_ptr, self._config, name) if err >= 0: - return err, ConfigEntry._from_c(entry[0]) + try: + return err, ConfigEntry(c_entry_ptr[0]) + finally: + C.git_config_entry_free(c_entry_ptr[0]) return err, None @@ -383,11 +408,11 @@ def __iter__(self) -> Iterator['ConfigEntry']: :returns: An iterator over :class:`pygit2.config.ConfigEntry` objects. """ self._stored_exception = None - citer = ffi.new('git_config_iterator **') - err = C.git_config_iterator_new(citer, self._config) + c_iter_ptr = ffi.new('git_config_iterator **') + err = C.git_config_iterator_new(c_iter_ptr, self._config) self._check_error(err) - return ConfigIterator(self, citer[0]) + return ConfigIterator(self, c_iter_ptr[0]) def get_multivar( self, @@ -417,11 +442,16 @@ def get_multivar( name = str_to_bytes(name, 'name') regex_bytes = to_bytes(regex or None) - citer = ffi.new('git_config_iterator **') - err = C.git_config_multivar_iterator_new(citer, self._config, name, regex_bytes) + c_iter_ptr = ffi.new('git_config_iterator **') + err = C.git_config_multivar_iterator_new( + c_iter_ptr, + self._config, + name, + regex_bytes, + ) self._check_error(err) - return ConfigMultivarIterator(self, citer[0]) + return ConfigMultivarIterator(self, c_iter_ptr[0]) def set_multivar( self, @@ -601,10 +631,10 @@ def snapshot(self) -> Config: def _c_snapshot(self) -> 'GitConfigC': self._stored_exception = None - c_config = ffi.new('git_config **') - err = C.git_config_snapshot(c_config, self._config) + c_config_ptr = ffi.new('git_config **') + err = C.git_config_snapshot(c_config_ptr, self._config) self._check_error(err) - return c_config[0] + return c_config_ptr[0] # # Methods to parse a string according to the git-config rules @@ -848,10 +878,10 @@ def __init__(self, *, c_snapshot: 'GitConfigC | None' = None): if c_snapshot is not None: super().__init__(c_config=c_snapshot, is_snapshot=True) else: - c_config = ffi.new('git_config **') - err = C.git_config_open_default(c_config) + c_config_ptr = ffi.new('git_config **') + err = C.git_config_open_default(c_config_ptr) check_error(err) - super().__init__(c_config=c_config[0], is_snapshot=False) + super().__init__(c_config=c_config_ptr[0], is_snapshot=False) @override def snapshot(self) -> DefaultConfig: @@ -974,13 +1004,13 @@ def __init__( if c_snapshot is not None: super().__init__(c_config=c_snapshot, is_snapshot=True) else: - c_config = ffi.new('git_config **') + c_config_ptr = ffi.new('git_config **') if do_snapshot: - err = C.git_repository_config_snapshot(c_config, self._c_repo) + err = C.git_repository_config_snapshot(c_config_ptr, self._c_repo) else: - err = C.git_repository_config(c_config, self._c_repo) + err = C.git_repository_config(c_config_ptr, self._c_repo) check_error(err) - super().__init__(c_config=c_config[0], is_snapshot=do_snapshot) + super().__init__(c_config=c_config_ptr[0], is_snapshot=do_snapshot) @override def snapshot(self) -> RepositoryConfig: @@ -1763,63 +1793,77 @@ def _config_memory_iterator_entry_free(entry: 'GitConfigBackendEntryC') -> None: class ConfigEntry: """An entry in a configuration object.""" - _entry: 'GitConfigEntryC' - iterator: _BaseIterator | None + def __init__(self, c_entry: 'GitConfigEntryC') -> None: + """For internal use only.""" + # The c_entry will not be valid for the entire life of this ConfigEntry, so we + # must copy *all* of the data out of it at construction and *not* store the + # c_entry. + self._raw_name = ffi.string(c_entry.name) + self._name = self._raw_name.decode('utf-8') + + self._raw_value: bytes | None = None + if c_entry.value is not None and c_entry.value != ffi.NULL: + self._raw_value = ffi.string(c_entry.value) - @classmethod - def _from_c( - cls, ptr: 'GitConfigEntryC', iterator: _BaseIterator | None = None - ) -> 'ConfigEntry': - """Builds the entry from a ``git_config_entry`` pointer. + self._raw_level = c_entry.level + self._level = ConfigLevel(self._raw_level) - ``iterator`` must be a ``ConfigIterator`` instance if the entry was - created during ``git_config_iterator`` actions. + @property + def raw_name(self) -> bytes: + """The entry's name as encoded bytes. + + :returns: The raw name obtained from the entry without decoding it. """ - entry = cls.__new__(cls) - entry._entry = ptr - entry.iterator = iterator - - # It should be enough to keep a reference to iterator, so we only call - # git_config_iterator_free when we've deleted all ConfigEntry objects. - # But it's not, to reproduce the error comment the lines below and run - # the script in https://github.com/libgit2/pygit2/issues/970 - # So instead we load the Python object immediately. Ideally we should - # investigate libgit2 source code. - if iterator is not None: - entry.raw_name = entry.raw_name - entry.raw_value = entry.raw_value - entry.level = entry.level + return self._raw_name - return entry + @property + def raw_value(self) -> bytes | None: + """The entry's value as encoded bytes. - def __del__(self) -> None: - if self.iterator is None and self._entry != ffi.NULL: - C.git_config_entry_free(self._entry) + The value may be ``None`` if the config entry this represents was a + valueless flag. + + :returns: The raw value obtained from the entry without decoding it. + """ + return self._raw_value @property - def c_value(self) -> 'ffi.char_pointer': - """The raw ``cData`` entry value.""" - return self._entry.value + def raw_level(self) -> int: + """The entry's level as an integer. - @cached_property - def raw_name(self) -> bytes: - return ffi.string(self._entry.name) + :returns: The raw ``git_config_level_t`` value from the entry. + """ + return self._raw_level - @cached_property - def raw_value(self) -> bytes | None: - return ffi.string(self.c_value) if self.c_value != ffi.NULL else None + @property + def level(self) -> ConfigLevel: + """The entry's level as a :class:`pygit2.enums.ConfigLevel`. - @cached_property - def level(self) -> int: - """The entry's ``git_config_level_t`` value.""" - return self._entry.level + :returns: The ``ConfigLevel`` equivalent of the entry's ``git_config_level_t``. + """ + return self._level @property def name(self) -> str: - """The entry's name.""" - return self.raw_name.decode('utf-8') + """The entry's name. - @property + :returns: The name. + """ + return self._name + + @cached_property def value(self) -> str | None: - """The entry's value as a string.""" - return self.raw_value.decode('utf-8') if self.raw_value is not None else None + """The entry's value as a string. + + The value may be ``None`` if the config entry this represents was a + valueless flag. + + The value is UTF-8 decoded from the :meth:`ConfigEntry.raw_value` the first time + this property is accessed. If you know or suspect that the value cannot be UTF-8 + decoded, access :meth:`ConfigEntry.raw_value`, instead, and decode it by other + means. + + :returns: The UTF-8 decoded string value from this entry. + :raises UnicodeDecodeError: If the value cannot be UTF-8 decoded. + """ + return self._raw_value.decode('utf-8') if self._raw_value is not None else None diff --git a/pygit2/enums.py b/pygit2/enums.py index 5534b85b..7b9e4d30 100644 --- a/pygit2/enums.py +++ b/pygit2/enums.py @@ -236,6 +236,10 @@ class ConfigLevel(IntEnum): (from higher to lower) when searching for config entries in git.git. """ + UNSPECIFIED = 0 + """The lowest priority level supported by libgit2. Not part of the ``git_config_level_t`` + enumeration because C enums allow non-member values while Python enums do not.""" + PROGRAMDATA = _pygit2.GIT_CONFIG_LEVEL_PROGRAMDATA 'System-wide on Windows, for compatibility with portable git' From 9897aea0ddc6e3e2ecdcad8c66b26a5b63ee096b Mon Sep 17 00:00:00 2001 From: Nick Williams Date: Mon, 6 Jul 2026 11:35:18 -0500 Subject: [PATCH 10/13] Improve backend to support snapshots --- pygit2/_build.py | 8 + pygit2/config.py | 386 +++++++++++++++++++++++++++++-------------- pygit2/decl/errors.h | 6 +- test/test_config.py | 52 ++++++ 4 files changed, 323 insertions(+), 129 deletions(-) diff --git a/pygit2/_build.py b/pygit2/_build.py index 3a5908b8..539fa3ce 100644 --- a/pygit2/_build.py +++ b/pygit2/_build.py @@ -70,5 +70,13 @@ def get_libgit2_paths() -> tuple[Path, dict[str, list[str]]]: 'libraries': ['git2'], 'include_dirs': [str(x) for x in include_dirs], 'library_dirs': [str(x) for x in library_dirs], + # For debugging memory issues (segfaults) during development, uncomment the + # following lines and rebuild everything. YMMV + # 'extra_compile_args': [ + # '-fsanitize=address', + # '-fsanitize-address-use-after-scope', + # '-fsanitize-address-use-odr-indicator', + # ], + # 'extra_link_args': ['-fsanitize=address'], }, ) diff --git a/pygit2/config.py b/pygit2/config.py index 654be1ed..4198c862 100644 --- a/pygit2/config.py +++ b/pygit2/config.py @@ -27,7 +27,9 @@ import abc import contextlib import re +import sys import threading +import weakref from collections.abc import Callable, Generator, Iterator from os import PathLike from types import TracebackType @@ -86,7 +88,7 @@ def str_to_bytes(value: str | bytes, name: str) -> bytes: class _BaseIterator: def __init__(self, config: 'Config', c_iter: 'GitConfigIteratorC') -> None: self._config = config - self._iter = c_iter + self._c_iter = c_iter self._freed = False self._c_entry_ptr = ffi.new('git_config_entry **') @@ -95,7 +97,7 @@ def _free(self): if not self._freed: self._freed = True self._c_entry_ptr = None # do not free; see comment in _next_entry - C.git_config_iterator_free(self._iter) + C.git_config_iterator_free(self._c_iter) def __del__(self) -> None: self._free() # fallback in case we never hit StopIteration below @@ -117,7 +119,7 @@ def _next_entry(self) -> 'ConfigEntry': # - https://github.com/libgit2/pygit2/issues/970 # - https://libgit2.org/docs/reference/v1.9.4/config/git_config_next.html self._config._stored_exception = None - err = C.git_config_next(self._c_entry_ptr, self._iter) + err = C.git_config_next(self._c_entry_ptr, self._c_iter) check_error(err, user_exception=self._config._stored_exception) return ConfigEntry(self._c_entry_ptr[0]) except StopIteration: @@ -249,7 +251,7 @@ def __init__( self._stored_exception: BaseException | None = None if c_config is not None: - self._config = c_config + self._c_config = c_config else: c_config_ptr = ffi.new('git_config **') @@ -260,11 +262,11 @@ def __init__( err = C.git_config_open_ondisk(c_config_ptr, path_bytes) check_error(err, io=True) - self._config = c_config_ptr[0] + self._c_config = c_config_ptr[0] def __del__(self) -> None: try: - C.git_config_free(self._config) + C.git_config_free(self._c_config) except AttributeError: pass @@ -278,7 +280,7 @@ def _get(self, name: str | bytes) -> tuple[int, 'ConfigEntry | None']: name = str_to_bytes(name, 'name') c_entry_ptr = ffi.new('git_config_entry **') - err = C.git_config_get_entry(c_entry_ptr, self._config, name) + err = C.git_config_get_entry(c_entry_ptr, self._c_config, name) if err >= 0: try: @@ -363,11 +365,11 @@ def __setitem__(self, name: str | bytes, value: bool | int | str | bytes) -> Non err: int if isinstance(value, bool): - err = C.git_config_set_bool(self._config, name, value) + err = C.git_config_set_bool(self._c_config, name, value) elif isinstance(value, int): - err = C.git_config_set_int64(self._config, name, value) + err = C.git_config_set_int64(self._c_config, name, value) else: - err = C.git_config_set_string(self._config, name, to_bytes(value)) + err = C.git_config_set_string(self._c_config, name, to_bytes(value)) self._check_error(err) @@ -388,7 +390,7 @@ def __delitem__(self, name: str | bytes) -> None: self._stored_exception = None name = str_to_bytes(name, 'name') - err = C.git_config_delete_entry(self._config, name) + err = C.git_config_delete_entry(self._c_config, name) self._check_error(err) def __iter__(self) -> Iterator['ConfigEntry']: @@ -409,7 +411,7 @@ def __iter__(self) -> Iterator['ConfigEntry']: """ self._stored_exception = None c_iter_ptr = ffi.new('git_config_iterator **') - err = C.git_config_iterator_new(c_iter_ptr, self._config) + err = C.git_config_iterator_new(c_iter_ptr, self._c_config) self._check_error(err) return ConfigIterator(self, c_iter_ptr[0]) @@ -445,7 +447,7 @@ def get_multivar( c_iter_ptr = ffi.new('git_config_iterator **') err = C.git_config_multivar_iterator_new( c_iter_ptr, - self._config, + self._c_config, name, regex_bytes, ) @@ -478,7 +480,7 @@ def set_multivar( regex = str_to_bytes(regex, 'regex') value = str_to_bytes(value, 'value') - err = C.git_config_set_multivar(self._config, name, regex, value) + err = C.git_config_set_multivar(self._c_config, name, regex, value) self._check_error(err) def delete_multivar(self, name: str | bytes, regex: str | bytes) -> None: @@ -501,7 +503,7 @@ def delete_multivar(self, name: str | bytes, regex: str | bytes) -> None: name = str_to_bytes(name, 'name') regex = str_to_bytes(regex, 'regex') - err = C.git_config_delete_multivar(self._config, name, regex) + err = C.git_config_delete_multivar(self._c_config, name, regex) self._check_error(err) def get_bool(self, name: str | bytes) -> bool: @@ -600,7 +602,7 @@ def add_file( force = 0 err = C.git_config_add_file_ondisk( - self._config, + self._c_config, to_bytes(path), level, ffi.NULL, @@ -632,7 +634,7 @@ def snapshot(self) -> Config: def _c_snapshot(self) -> 'GitConfigC': self._stored_exception = None c_config_ptr = ffi.new('git_config **') - err = C.git_config_snapshot(c_config_ptr, self._config) + err = C.git_config_snapshot(c_config_ptr, self._c_config) self._check_error(err) return c_config_ptr[0] @@ -746,14 +748,58 @@ class _InMemoryAppBackendConfig(Config, abc.ABC): classes. """ - def __init__(self, *, c_config: 'GitConfigC', is_snapshot: bool) -> None: + @overload + def __init__(self, *, c_config: 'GitConfigC', is_snapshot: bool) -> None: ... + + @overload + def __init__( + self, + *, + c_config: 'GitConfigC', + snapshot_of: _InMemoryAppBackendConfig, + ) -> None: ... + + def __init__( + self, + *, + c_config: 'GitConfigC', + is_snapshot: bool = False, + snapshot_of: _InMemoryAppBackendConfig | None = None, + ) -> None: """For internal use only. Constructs a ``Config`` object from a config object pointer. + + The c_config can be a non-snapshot from initial creation (is_snapshot=False, + snapshot_of=None); a snapshot from initial creation (is_snapshot=True, + snapshot_of=None); or, a snapshot derived from a previously-created c_config + that may or may not already have the in-memory backend attached to it + (snapshot_of=not None). + + In the latter case, we check whether snapshot_of._on_snapshot_completion is + set; if it is, that means the in-memory backend was attached when the + snapshot was created, and libgit2 invoked its snapshot callback. This config + object doesn't yet know about the backend snapshot, and the backend snapshot + doesn't yet know about this config object. We invoke _on_snapshot_completion + to inform the backend snapshot about this config object and obtain the + backend snapshot instance. """ - super().__init__(c_config=c_config, is_snapshot=is_snapshot) - self._backend_added = False - self._backend = _InMemoryBackend(self) + super().__init__( + c_config=c_config, + is_snapshot=is_snapshot or snapshot_of is not None, + ) + + self._on_snapshot_completion: ( + Callable[[_InMemoryAppBackendConfig], _InMemoryBackend] | None + ) = None + + if snapshot_of is not None and snapshot_of._on_snapshot_completion is not None: + self._backend_added = True + self._backend = snapshot_of._on_snapshot_completion(self) + snapshot_of._on_snapshot_completion = None + else: + self._backend_added = False + self._backend = _InMemoryBackend(config=self) @abc.abstractmethod def _add_backend_to_config(self, backend: _InMemoryBackend) -> None: @@ -814,7 +860,7 @@ def _change_write_priority(self, level: ConfigLevel) -> None: 'git_config_level_t[]', [ffi.cast('git_config_level_t', level.value)], ) - err = C.git_config_set_writeorder(self._config, c_levels, 1) + err = C.git_config_set_writeorder(self._c_config, c_levels, 1) check_error(err) @@ -856,7 +902,7 @@ def __init__(self, /) -> None: ... @overload - def __init__(self, *, c_snapshot: 'GitConfigC') -> None: + def __init__(self, *, snapshot_of: DefaultConfig) -> None: """For internal use only. Constructs a ``DefaultConfig`` from a given snapshot config object pointer. @@ -864,7 +910,7 @@ def __init__(self, *, c_snapshot: 'GitConfigC') -> None: """ ... - def __init__(self, *, c_snapshot: 'GitConfigC | None' = None): + def __init__(self, *, snapshot_of: DefaultConfig | None = None): """Constructs a ``DefaultConfig`` object. **Overload 1: __init__()** @@ -875,8 +921,11 @@ def __init__(self, *, c_snapshot: 'GitConfigC | None' = None): For internal use only. """ - if c_snapshot is not None: - super().__init__(c_config=c_snapshot, is_snapshot=True) + if snapshot_of is not None: + super().__init__( + c_config=snapshot_of._c_snapshot(), + snapshot_of=snapshot_of, + ) else: c_config_ptr = ffi.new('git_config **') err = C.git_config_open_default(c_config_ptr) @@ -894,11 +943,12 @@ def snapshot(self) -> DefaultConfig: """ if self._is_snapshot: raise TypeError('This default config is already a snapshot.') - return DefaultConfig(c_snapshot=self._c_snapshot()) + self._on_snapshot_completion = None + return DefaultConfig(snapshot_of=self) @override def _add_backend_to_config(self, backend: _InMemoryBackend) -> None: - backend.add_to_config(self._config) + backend.add_to_config(self._c_config) @override def add_file( @@ -977,7 +1027,7 @@ def __init__( repo: 'BaseRepository', c_repo: 'GitRepositoryC', *, - c_snapshot: 'GitConfigC', + snapshot_of: RepositoryConfig, ) -> None: """For internal use only. @@ -995,14 +1045,16 @@ def __init__( c_repo: 'GitRepositoryC', *, do_snapshot: bool = False, - c_snapshot: 'GitConfigC | None' = None, + snapshot_of: RepositoryConfig | None = None, ) -> None: """Constructs a ``RepositoryConfig`` object. For internal use only.""" self._repo = repo self._c_repo = c_repo - - if c_snapshot is not None: - super().__init__(c_config=c_snapshot, is_snapshot=True) + if snapshot_of is not None: + super().__init__( + c_config=snapshot_of._c_snapshot(), + snapshot_of=snapshot_of, + ) else: c_config_ptr = ffi.new('git_config **') if do_snapshot: @@ -1024,11 +1076,12 @@ def snapshot(self) -> RepositoryConfig: """ if self._is_snapshot: raise TypeError('This repository config is already a snapshot.') - return RepositoryConfig(self._repo, self._c_repo, c_snapshot=self._c_snapshot()) + self._on_snapshot_completion = None + return RepositoryConfig(self._repo, self._c_repo, snapshot_of=self) @override def _add_backend_to_config(self, backend: _InMemoryBackend) -> None: - backend.add_to_config(self._config, self._c_repo) + backend.add_to_config(self._c_config, self._c_repo) @override def add_file( @@ -1070,10 +1123,43 @@ class _InMemoryBackend: type_string = cast('char_pointer', ffi.new('char[]', b'pygit2-in-memory')) origin_path_string = cast('char_pointer', ffi.new('char[]', b'')) - def __init__(self, config: _InMemoryAppBackendConfig) -> None: - self._config = config + _instances: dict[int, _InMemoryBackend] = {} + + @overload + def __init__(self, *, config: _InMemoryAppBackendConfig) -> None: ... + + @overload + def __init__(self, *, snapshot_of: _InMemoryBackend) -> None: ... + + def __init__( + self, + *, + config: _InMemoryAppBackendConfig | None = None, + snapshot_of: _InMemoryBackend | None = None, + ) -> None: + self._weak_config: weakref.ReferenceType[_InMemoryAppBackendConfig] + self._read_data: dict[str, list[_InMemoryBackend.Entry]] + + if snapshot_of is not None: + if config is not None: + raise ValueError('Cannot specify both `config` and `snapshot_of`.') + config = snapshot_of._weak_config() + if config is None: + raise ValueError( + 'Cannot create snapshot of backend with null weak reference to ' + 'parent config.' + ) + config._on_snapshot_completion = self._do_on_snapshot_completion + self._weak_config = weakref.ref(config) + self._is_snapshot = True + self._read_data = {k: v[:] for k, v in snapshot_of._read_data.items()} + elif config is not None: + self._weak_config = weakref.ref(config) + self._is_snapshot = False + self._read_data = {} + else: + raise ValueError('Either `config` or `snapshot_of` must be specified.') - self._read_data: dict[str, list[_InMemoryBackend.Entry]] = {} self._write_data: dict[str, list[_InMemoryBackend.Entry]] = self._read_data self._locked = False @@ -1088,15 +1174,25 @@ def __init__(self, config: _InMemoryAppBackendConfig) -> None: self._iterators: dict[int, _InMemoryBackend.Iterator] = {} self._c_entries: dict[int, 'PyGitConfigBackendEntryC'] = {} + def _do_on_snapshot_completion( + self, + actual_config: _InMemoryAppBackendConfig, + ) -> Self: + self._weak_config = weakref.ref(actual_config) + return self + @contextlib.contextmanager def read_lock(self) -> Generator[None, None, None]: - """For internal use only. - - Yield a lock to protect ``_read_data``. The lock will not block other readers + """Yield a lock to protect ``_read_data``. The lock will not block other readers from simultaneously reading ``_read_data`` but will prevent writers from mutating ``_write_data`` unless this backend is "locked" (in the midst of a transaction). """ + if self._is_snapshot: + # If it's a snapshot, it's read-only. + yield + return + with self._readers_lock: self._readers += 1 if self._readers == 1: @@ -1112,14 +1208,18 @@ def read_lock(self) -> Generator[None, None, None]: @contextlib.contextmanager def write_lock(self) -> Generator[None, None, None]: - """For internal use only. - - If this backend is "locked" (in the midst of a transaction), yield a lock + """If this backend is "locked" (in the midst of a transaction), yield a lock to protect ``_write_data``, which is a separate object from ``_read_data`` (so it won't block readers). If this backend is not "locked," yield a lock to protect ``_write_data``/``_read_data``, which are the same object (so it will block readers). """ + if self._is_snapshot: + raise RuntimeError( + 'You have found a bug in PyGit2. write_lock() should never be called ' + 'for snapshot backends.', + ) + if self._locked: with self._locked_write_lock: yield @@ -1127,21 +1227,8 @@ def write_lock(self) -> Generator[None, None, None]: with self._write_lock: yield - def add_to_config( - self, - c_config: 'GitConfigC', - c_repo: 'GitRepositoryC | None' = None, - ) -> None: - """For internal use only. - - Adds the backend to the ``git_config``. Called by - :meth:`_InMemoryAppBackendConfig.__enter__` the first time it enters, but not - any subsequent times. This is because it's not possible to remove a backend - from a config with libgit2's public API, and so we rely on clearing - the backend's contents on ``__exit__``. - - The ``c_repo`` is optional and applies only to repository configurations. - """ + def initialize_backend(self) -> PyGitConfigBackendWrapperC: + """Initializes the C backend object.""" if self._c_backend is not None: raise ValueError('add_to_config called twice') @@ -1149,7 +1236,7 @@ def add_to_config( assert self._c_backend is not None self._c_backend.self = self._c_handle self._c_backend.parent.version = 1 - self._c_backend.parent.readonly = 0 + self._c_backend.parent.readonly = 1 if self._is_snapshot else 0 self._c_backend.parent.open = C._config_memory_backend_open self._c_backend.parent.get = C._config_memory_backend_get self._c_backend.parent.set = C._config_memory_backend_set @@ -1163,9 +1250,25 @@ def add_to_config( self._c_backend.parent.unlock = C._config_memory_backend_unlock self._c_backend.parent.free = C._config_memory_backend_free + return self._c_backend + + def add_to_config( + self, + c_config: 'GitConfigC', + c_repo: 'GitRepositoryC | None' = None, + ) -> None: + """Adds the backend to the ``git_config``. Called by + :meth:`_InMemoryAppBackendConfig.__enter__` the first time it enters, but not + any subsequent times. This is because it's not possible to remove a backend + from a config with libgit2's public API, and so we rely on clearing + the backend's contents on ``__exit__``. + + The ``c_repo`` is optional and applies only to repository configurations. + """ + c_backend = self.initialize_backend() err = C.git_config_add_backend( c_config, - ffi.cast('git_config_backend *', self._c_backend), + ffi.cast('git_config_backend *', c_backend), ConfigLevel.APP.value, ffi.NULL if c_repo is None else c_repo, 1, # force=true @@ -1173,23 +1276,20 @@ def add_to_config( check_error(err) def clear(self) -> None: - """For internal use only. - - Erases all contents of the backend. Called by + """Erases all contents of the backend. Called by :meth:`_InMemoryAppBackendConfig.__exit__` each time it exits. """ - with self.write_lock(): - self._read_data.clear() - self._write_data.clear() - self._iterators.clear() - self._c_entries.clear() + if not self._is_snapshot: + with self.write_lock(): + self._read_data.clear() + self._write_data.clear() + self._iterators.clear() + self._c_entries.clear() def multivar_generator( self, ) -> Generator[tuple[str, '_InMemoryBackend.Entry'], None, None]: - """For internal use only. - - Creates a generator yielding the contents of this backend for use by a + """Creates a generator yielding the contents of this backend for use by a :class:`_InMemoryBackend.Iterator`. """ with self.read_lock(): @@ -1197,6 +1297,18 @@ def multivar_generator( for value in self._read_data[key]: yield key, value + def store_exception(self, e: BaseException) -> None: + config = self._weak_config() + if config: + config._stored_exception = e + else: + sys.stderr.write( + f'Failed to store exception in C handler for later use in Python ' + f'because backend config weakref has already been garbage ' + f'collected: {e}\n', + ) + sys.stderr.flush() + class Entry: """For internal use only. @@ -1257,31 +1369,11 @@ def __exit__( return False -def _populate_memory_backend_entry( - entry: 'GitConfigBackendEntryC', - source: _InMemoryBackend.Entry, - free: Callable[[GitConfigBackendEntryC], None], -) -> None: - """For internal use only. - - Helper function used by ``_config_memory_backend_get`` and - ``_config_memory_iterator_next`` to populate an entry with the data from the - backend. - """ - entry.free = free - entry.entry.name = source.c_name - entry.entry.value = source.c_value - entry.entry.backend_type = _InMemoryBackend.type_string - entry.entry.origin_path = _InMemoryBackend.origin_path_string - entry.entry.include_depth = 0 - entry.entry.level = ConfigLevel.APP.value - - @ffi.def_extern() def _config_memory_backend_open( - _: 'GitConfigBackendC', - __: int, - ___: 'GitRepositoryC', + backend: 'GitConfigBackendC', + level: int, + _: 'GitRepositoryC', ) -> int: """For internal use only. @@ -1299,6 +1391,24 @@ def _config_memory_backend_open( git_config_level_t level, const git_repository *repo); """ + backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) + self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) + try: + if level != ConfigLevel.APP.value: # sanity check + self.store_exception( + ValueError( + f'Unsupported config level {level} in _config_memory_backend_open', + ), + ) + return C.GIT_EUSER + # Backend instances can be long-lived, even after *our* normal Python + # references are gone. We have to store a reference to the Python object + # "owning" the C backend to prevent from_handle from crashing. We'll + # later remove this in `_config_memory_backend_free`. + _InMemoryBackend._instances[id(self)] = self + except BaseException as e: + self.store_exception(e) + return C.GIT_EUSER return 0 @@ -1336,15 +1446,17 @@ def _config_memory_backend_get( entry = ffi.new('_pygit_in_memory_backend_entry *') ptr = int(ffi.cast('uintptr_t', entry)) entry.owner = backend_wrapper - _populate_memory_backend_entry( - entry.parent, - value, - C._config_memory_backend_entry_free, - ) + entry.parent.free = C._config_memory_backend_entry_free + entry.parent.entry.name = value.c_name + entry.parent.entry.value = value.c_value + entry.parent.entry.backend_type = _InMemoryBackend.type_string + entry.parent.entry.origin_path = _InMemoryBackend.origin_path_string + entry.parent.entry.include_depth = 0 + entry.parent.entry.level = ConfigLevel.APP.value self._c_entries[ptr] = entry out[0] = ffi.cast('git_config_backend_entry *', entry) except BaseException as e: - self._config._stored_exception = e + self.store_exception(e) return C.GIT_EUSER return 0 @@ -1371,11 +1483,13 @@ def _config_memory_backend_set( backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: + if self._is_snapshot: + return C.GIT_EREADONLY key = ffi.string(name).decode('utf-8').lower() decoded_value = ffi.string(value).decode('utf-8') self._write_data[key] = [_InMemoryBackend.Entry(key, decoded_value)] except BaseException as e: - self._config._stored_exception = e + self.store_exception(e) return C.GIT_EUSER return 0 @@ -1409,6 +1523,8 @@ def _config_memory_backend_set_multivar( backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: + if self._is_snapshot: + return C.GIT_EREADONLY key = ffi.string(name).decode('utf-8').lower() with self.write_lock(): if key in self._write_data and regexp != ffi.NULL: @@ -1422,7 +1538,7 @@ def _config_memory_backend_set_multivar( _InMemoryBackend.Entry(key, ffi.string(value).decode('utf-8')), ) except BaseException as e: - self._config._stored_exception = e + self.store_exception(e) return C.GIT_EUSER return 0 @@ -1448,13 +1564,15 @@ def _config_memory_backend_del( backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: + if self._is_snapshot: + return C.GIT_EREADONLY key = ffi.string(name).decode('utf-8').lower() with self.write_lock(): if key not in self._write_data: return C.GIT_ENOTFOUND del self._write_data[key] except BaseException as e: - self._config._stored_exception = e + self.store_exception(e) return C.GIT_EUSER return 0 @@ -1482,6 +1600,8 @@ def _config_memory_backend_del_multivar( backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: + if self._is_snapshot: + return C.GIT_EREADONLY key = ffi.string(name).decode('utf-8').lower() with self.write_lock(): if key not in self._write_data: @@ -1494,7 +1614,7 @@ def _config_memory_backend_del_multivar( v for v in self._write_data[key] if not expression.search(v.value) ] except BaseException as e: - self._config._stored_exception = e + self.store_exception(e) return C.GIT_EUSER return 0 @@ -1552,15 +1672,15 @@ def _config_memory_backend_iterator( out[0] = ffi.cast('git_config_iterator *', iterator) py_iterator.__enter__() except BaseException as e: - self._config._stored_exception = e + self.store_exception(e) return C.GIT_EUSER return 0 @ffi.def_extern() def _config_memory_backend_snapshot( - _: '_Pointer[GitConfigBackendC]', - __: 'GitConfigBackendC', + out: '_Pointer[GitConfigBackendC]', + backend: 'GitConfigBackendC', ) -> int: """For internal use only. @@ -1568,16 +1688,19 @@ def _config_memory_backend_snapshot( on all of a config's backends when other code calls ``git_config_snapshot`` on that config. - TODO: Implement this, the easiest way for which will be to use - ``git_config_backend_from_string`` (an immutable in-memory backend) when it becomes - available in 1.9.5. - C signature: int snapshot(git_config_backend **out, git_config_backend *backend); """ - # backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) - # self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) - return C.GIT_PASSTHROUGH + backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) + self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) + try: + snapshot = _InMemoryBackend(snapshot_of=self) + c_backend = snapshot.initialize_backend() + out[0] = ffi.cast('git_config_backend *', c_backend) + except BaseException as e: + self.store_exception(e) + return C.GIT_EUSER + return 0 @ffi.def_extern() @@ -1599,6 +1722,8 @@ def _config_memory_backend_lock(backend: 'GitConfigBackendC') -> int: backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: + if self._is_snapshot: + return C.GIT_EREADONLY with self.write_lock(): if self._locked: return C.GIT_ELOCKED @@ -1607,7 +1732,7 @@ def _config_memory_backend_lock(backend: 'GitConfigBackendC') -> int: with self.write_lock(): self._write_data = {k: v[:] for k, v in self._read_data.items()} except BaseException as e: - self._config._stored_exception = e + self.store_exception(e) return C.GIT_EUSER return 0 @@ -1637,6 +1762,8 @@ def _config_memory_backend_unlock(backend: 'GitConfigBackendC', success: int) -> backend_wrapper = ffi.cast('_pygit_in_memory_backend *', backend) self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: + if self._is_snapshot: + return C.GIT_EREADONLY with self.write_lock(): if not self._locked: return C.GIT_EINVALID @@ -1648,7 +1775,7 @@ def _config_memory_backend_unlock(backend: 'GitConfigBackendC', success: int) -> else: self._read_data = self._write_data except BaseException as e: - self._config._stored_exception = e + self.store_exception(e) return C.GIT_EUSER return 0 @@ -1670,8 +1797,11 @@ def _config_memory_backend_free(backend: 'GitConfigBackendC') -> None: self = cast(_InMemoryBackend, ffi.from_handle(backend_wrapper.self)) try: self.clear() + instance_key = id(self) + if instance_key in _InMemoryBackend._instances: + del _InMemoryBackend._instances[instance_key] except BaseException as e: - self._config._stored_exception = e + self.store_exception(e) # nothing we can do here because of the void return type except (AttributeError, TypeError, RuntimeError, ReferenceError): # The Python interpreter is exiting when this is called. Nothing we can do. @@ -1700,7 +1830,7 @@ def _config_memory_backend_entry_free(entry: 'GitConfigBackendEntryC') -> None: if ptr in self._c_entries: del self._c_entries[ptr] except BaseException as e: - self._config._stored_exception = e + self.store_exception(e) # nothing we can do here because of the void return type @@ -1725,18 +1855,20 @@ def _config_memory_iterator_next( entry = ffi.new('_pygit_in_memory_backend_iterator_entry *') ptr = int(ffi.cast('uintptr_t', entry)) entry.owner = iterator_wrapper - _populate_memory_backend_entry( - entry.parent, - value, - C._config_memory_iterator_entry_free, - ) + entry.parent.free = C._config_memory_iterator_entry_free + entry.parent.entry.name = value.c_name + entry.parent.entry.value = value.c_value + entry.parent.entry.backend_type = _InMemoryBackend.type_string + entry.parent.entry.origin_path = _InMemoryBackend.origin_path_string + entry.parent.entry.include_depth = 0 + entry.parent.entry.level = ConfigLevel.APP.value self._c_entries[ptr] = entry out[0] = ffi.cast('git_config_backend_entry *', entry) return 0 except StopIteration: return C.GIT_ITEROVER except BaseException as e: - self._backend._config._stored_exception = e + self._backend.store_exception(e) return C.GIT_EUSER @@ -1760,7 +1892,7 @@ def _config_memory_iterator_free(iterator: 'GitConfigIteratorC') -> None: try: self.__exit__(None, None, None) except BaseException as e: - self._backend._config._stored_exception = e + self._backend.store_exception(e) # nothing we can do here because of the void return type @@ -1786,7 +1918,7 @@ def _config_memory_iterator_entry_free(entry: 'GitConfigBackendEntryC') -> None: if ptr in self._c_entries: del self._c_entries[ptr] except BaseException as e: - self._backend._config._stored_exception = e + self._backend.store_exception(e) # nothing we can do here because of the void return type diff --git a/pygit2/decl/errors.h b/pygit2/decl/errors.h index 93783568..27474faa 100644 --- a/pygit2/decl/errors.h +++ b/pygit2/decl/errors.h @@ -39,7 +39,10 @@ typedef enum { GIT_EINDEXDIRTY = -34, /**< Unsaved changes in the index would be overwritten */ GIT_EAPPLYFAIL = -35, /**< Patch application failed */ GIT_EOWNER = -36, /**< The object is not owned by the current user */ - GIT_TIMEOUT = -37 /**< The operation timed out */ + GIT_TIMEOUT = -37, /**< The operation timed out */ + GIT_EUNCHANGED = -38, /**< There were no changes */ + GIT_ENOTSUPPORTED = -39, /**< An option is not supported */ + GIT_EREADONLY = -40 /**< The subject is read-only */ } git_error_code; typedef struct { @@ -47,5 +50,4 @@ typedef struct { int klass; } git_error; - const git_error * git_error_last(void); diff --git a/test/test_config.py b/test/test_config.py index 71db305e..7f18b817 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -469,6 +469,37 @@ def test_repository_config_in_memory_overrides(config: RepositoryConfig) -> None assert 'core.override3' in config assert config['core.override3'] == 'dolor simet' + # let's make sure snapshots work, too + snapshot = config.snapshot() + utils.assertRaisesWithArg( + TypeError, + 'A read-only config snapshot cannot be used as a context manager, ' + 'because its backend data cannot be changed.', + snapshot.__enter__, + ) + utils.assertRaisesWithArg( + GitError, + "cannot set 'core.override4': the configuration is read-only", + lambda: snapshot.set_multivar('core.override4', '^$', 'foo'), + ) + assert 'core.editor' in snapshot + assert snapshot['core.editor'] == 'ed' + assert 'core.repositoryformatversion' in snapshot + assert snapshot.get_int('core.repositoryformatversion') == 0 + assert 'core.local1' in snapshot + assert not snapshot.get_bool('core.local1') + assert 'core.local2' in snapshot + assert snapshot.get_int('core.local2') == 56 + assert 'core.local3' in snapshot + assert snapshot['core.local3'] == 'lorem ipsum' + assert 'core.oVeRrIdE1' in snapshot + assert not snapshot.get_bool('core.override1') + assert not snapshot.get_bool('core.oVeRrIdE1') + assert 'core.override2' in snapshot + assert snapshot.get_int('core.override2') == 81 + assert 'core.override3' in snapshot + assert snapshot['core.override3'] == 'dolor simet' + # it all should have been erased again assert 'core.override1' not in config assert 'core.override2' not in config @@ -577,6 +608,27 @@ def test_default_config_in_memory_overrides() -> None: assert 'core.override3' in config assert config['core.override3'] == 'dolor simet' + # let's make sure snapshots work, too + snapshot = config.snapshot() + utils.assertRaisesWithArg( + TypeError, + 'A read-only config snapshot cannot be used as a context manager, ' + 'because its backend data cannot be changed.', + snapshot.__enter__, + ) + utils.assertRaisesWithArg( + GitError, + "cannot set 'core.override4': the configuration is read-only", + lambda: snapshot.set_multivar('core.override4', '^$', 'foo'), + ) + assert 'core.oVeRrIdE1' in snapshot + assert not snapshot.get_bool('core.override1') + assert not snapshot.get_bool('core.oVeRrIdE1') + assert 'core.override2' in snapshot + assert snapshot.get_int('core.override2') == 81 + assert 'core.override3' in snapshot + assert snapshot['core.override3'] == 'dolor simet' + # it all should have been erased again assert 'core.override1' not in config assert 'core.override2' not in config From b489bbbeada081b278b8918c7a93e9d8cec89bc4 Mon Sep 17 00:00:00 2001 From: Nick Williams Date: Mon, 6 Jul 2026 14:32:31 -0500 Subject: [PATCH 11/13] Fix default write order for default config --- pygit2/config.py | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/pygit2/config.py b/pygit2/config.py index 4198c862..6f08c3ff 100644 --- a/pygit2/config.py +++ b/pygit2/config.py @@ -30,7 +30,7 @@ import sys import threading import weakref -from collections.abc import Callable, Generator, Iterator +from collections.abc import Callable, Generator, Iterator, Sequence from os import PathLike from types import TracebackType @@ -805,6 +805,10 @@ def __init__( def _add_backend_to_config(self, backend: _InMemoryBackend) -> None: """Adds the backend to the config object.""" + @abc.abstractmethod + def _default_write_order(self) -> Sequence[ConfigLevel]: + """Get the default write order to restore on context exit.""" + def __enter__(self) -> Self: """Enter a context where all writes occur against an in-memory configuration. @@ -825,7 +829,7 @@ def __enter__(self) -> Self: if not self._backend_added: self._add_backend_to_config(self._backend) self._backend_added = True - self._change_write_priority(ConfigLevel.APP) + self._set_write_order((ConfigLevel.APP,)) return self def __exit__( @@ -843,24 +847,26 @@ def __exit__( :returns: ``False`` """ if self._backend_added: - self._change_write_priority(ConfigLevel.LOCAL) + self._set_write_order(self._default_write_order()) self._backend.clear() return False - def _change_write_priority(self, level: ConfigLevel) -> None: + def _set_write_order(self, levels: Sequence[ConfigLevel]) -> None: """For internal use only. By default, when libgit2 creates a ``git_config`` object for a repository, it sets the write order to ``{ GIT_CONFIG_LEVEL_LOCAL }``. This means that writes go only to the local config in ``.git/config`` and nowhere else. We need to change this to ``{ GIT_CONFIG_LEVEL_APP }` when entering the context and then back to - ``{ GIT_CONFIG_LEVEL_LOCAL }`` when exiting the context. + ``{ GIT_CONFIG_LEVEL_LOCAL }`` when exiting the context. For the non-repo "default" + config, the default we want to restore is ``{ GIT_CONFIG_LEVEL_GLOBAL, + GIT_CONFIG_LEVEL_XDG, GIT_CONFIG_LEVEL_SYSTEM, GIT_CONFIG_LEVEL_PROGRAMDATA }``. """ c_levels = ffi.new( 'git_config_level_t[]', - [ffi.cast('git_config_level_t', level.value)], + [ffi.cast('git_config_level_t', level.value) for level in levels], ) - err = C.git_config_set_writeorder(self._c_config, c_levels, 1) + err = C.git_config_set_writeorder(self._c_config, c_levels, len(levels)) check_error(err) @@ -950,6 +956,15 @@ def snapshot(self) -> DefaultConfig: def _add_backend_to_config(self, backend: _InMemoryBackend) -> None: backend.add_to_config(self._c_config) + @override + def _default_write_order(self) -> Sequence[ConfigLevel]: + return ( + ConfigLevel.GLOBAL, + ConfigLevel.XDG, + ConfigLevel.SYSTEM, + ConfigLevel.PROGRAMDATA, + ) + @override def add_file( self, @@ -1083,6 +1098,10 @@ def snapshot(self) -> RepositoryConfig: def _add_backend_to_config(self, backend: _InMemoryBackend) -> None: backend.add_to_config(self._c_config, self._c_repo) + @override + def _default_write_order(self) -> Sequence[ConfigLevel]: + return (ConfigLevel.LOCAL,) + @override def add_file( self, @@ -1230,7 +1249,7 @@ def write_lock(self) -> Generator[None, None, None]: def initialize_backend(self) -> PyGitConfigBackendWrapperC: """Initializes the C backend object.""" if self._c_backend is not None: - raise ValueError('add_to_config called twice') + raise ValueError('initialize_backend called twice') self._c_backend = ffi.new('_pygit_in_memory_backend *') assert self._c_backend is not None From 082d13f285f962b447648bd33585fbf096e26e9f Mon Sep 17 00:00:00 2001 From: Nick Williams Date: Mon, 6 Jul 2026 14:43:31 -0500 Subject: [PATCH 12/13] Address Codespell complaints --- pygit2/config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pygit2/config.py b/pygit2/config.py index 6f08c3ff..496a61a6 100644 --- a/pygit2/config.py +++ b/pygit2/config.py @@ -107,9 +107,9 @@ def __iter__(self) -> Self: def _next_entry(self) -> 'ConfigEntry': try: - # git_config_next (or, rather, the file backend specifically) re-uses a + # git_config_next (or, rather, the file backend specifically) reuses a # git_config_entry buffer from one call to the next. And more so than just - # this, it re-uses a buffer even from one iterator to the next. It's a + # this, it reuses a buffer even from one iterator to the next. It's a # matter of discussion whether this is correct behavior, but what it means # is that we should never call git_config_entry_free on an entry allocated # by git_config_next. And, in fact, we must completely copy the data out of From f45b57b1944e697471b746827a9f4a09454f6e7a Mon Sep 17 00:00:00 2001 From: Nick Williams Date: Mon, 6 Jul 2026 14:48:52 -0500 Subject: [PATCH 13/13] Remove use of @override not yet supported in Python 3.11 --- pygit2/config.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/pygit2/config.py b/pygit2/config.py index 496a61a6..cf35f563 100644 --- a/pygit2/config.py +++ b/pygit2/config.py @@ -44,7 +44,6 @@ Type, cast, overload, - override, ) try: @@ -938,7 +937,6 @@ def __init__(self, *, snapshot_of: DefaultConfig | None = None): check_error(err) super().__init__(c_config=c_config_ptr[0], is_snapshot=False) - @override def snapshot(self) -> DefaultConfig: """Create a read-only snapshot of this ``DefaultConfig`` object. @@ -952,11 +950,9 @@ def snapshot(self) -> DefaultConfig: self._on_snapshot_completion = None return DefaultConfig(snapshot_of=self) - @override def _add_backend_to_config(self, backend: _InMemoryBackend) -> None: backend.add_to_config(self._c_config) - @override def _default_write_order(self) -> Sequence[ConfigLevel]: return ( ConfigLevel.GLOBAL, @@ -965,7 +961,6 @@ def _default_write_order(self) -> Sequence[ConfigLevel]: ConfigLevel.PROGRAMDATA, ) - @override def add_file( self, path: str | PathLike, @@ -1079,7 +1074,6 @@ def __init__( check_error(err) super().__init__(c_config=c_config_ptr[0], is_snapshot=do_snapshot) - @override def snapshot(self) -> RepositoryConfig: """Create a read-only snapshot of this ``RepositoryConfig`` object. @@ -1094,15 +1088,12 @@ def snapshot(self) -> RepositoryConfig: self._on_snapshot_completion = None return RepositoryConfig(self._repo, self._c_repo, snapshot_of=self) - @override def _add_backend_to_config(self, backend: _InMemoryBackend) -> None: backend.add_to_config(self._c_config, self._c_repo) - @override def _default_write_order(self) -> Sequence[ConfigLevel]: return (ConfigLevel.LOCAL,) - @override def add_file( self, path: str | PathLike,