From 41174c4017eab80215fb4dfc5afb9b2e3853aa9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=2E=20David=20Ib=C3=A1=C3=B1ez?= Date: Wed, 12 Aug 2026 17:16:38 +0200 Subject: [PATCH] Fix status_file (and index/tree path handling) for non-ASCII paths Encode repository-internal paths as UTF-8 instead of filesystem encoding, and normalize to NFC on macOS to match Git's core.precomposeunicode default. status_file now also accepts raw bytes. Add regression tests covering non-ASCII, non-breaking space, NFC/NFD, and raw bytes paths. Fixes #687 Assisted-by: Kimi Code --- pygit2/_pygit2.pyi | 6 +-- pygit2/index.py | 26 ++++++++----- pygit2/utils.py | 27 +++++++++++++ src/repository.c | 4 +- src/treebuilder.c | 4 +- src/utils.c | 95 +++++++++++++++++++++++++++++++++++++++++++++ src/utils.h | 1 + test/test_status.py | 10 +++++ 8 files changed, 156 insertions(+), 17 deletions(-) diff --git a/pygit2/_pygit2.pyi b/pygit2/_pygit2.pyi index f8ca06bd..d5488da8 100644 --- a/pygit2/_pygit2.pyi +++ b/pygit2/_pygit2.pyi @@ -760,7 +760,7 @@ class Repository: def status( self, untracked_files: str = 'all', ignored: bool = False ) -> dict[str, int]: ... - def status_file(self, path: str, /) -> int: ... + def status_file(self, path: str | bytes, /) -> int: ... def walk( self, oid: _OidArg | None, sort_mode: SortMode = SortMode.NONE ) -> Walker: ... @@ -850,9 +850,9 @@ class Tree(Object): @disjoint_base class TreeBuilder: def clear(self) -> None: ... - def get(self, name: str, /) -> Object: ... + def get(self, name: str | bytes, /) -> Object: ... def insert(self, name: str, oid: _OidArg, attr: int) -> None: ... - def remove(self, name: str, /) -> None: ... + def remove(self, name: str | bytes, /) -> None: ... def write(self) -> Oid: ... def __len__(self) -> int: ... diff --git a/pygit2/index.py b/pygit2/index.py index 611940d1..ba36c742 100644 --- a/pygit2/index.py +++ b/pygit2/index.py @@ -33,7 +33,13 @@ from .enums import DiffOption, FileMode from .errors import check_error from .ffi import C, ffi -from .utils import GenericIterator, StrArray, decode_fs_path, encode_fs_path +from .utils import ( + GenericIterator, + StrArray, + decode_fs_path, + encode_fs_path, + encode_git_path, +) if typing.TYPE_CHECKING: from .repository import Repository @@ -79,7 +85,7 @@ def __len__(self) -> int: return C.git_index_entrycount(self._index) def __contains__(self, path) -> bool: - err = C.git_index_find(ffi.NULL, self._index, encode_fs_path(path)) + err = C.git_index_find(ffi.NULL, self._index, encode_git_path(path)) if err == C.GIT_ENOTFOUND: return False @@ -89,7 +95,7 @@ def __contains__(self, path) -> bool: def __getitem__(self, key: str | int | PathLike[str]) -> 'IndexEntry': centry = ffi.NULL if isinstance(key, str) or hasattr(key, '__fspath__'): - centry = C.git_index_get_bypath(self._index, encode_fs_path(key), 0) + centry = C.git_index_get_bypath(self._index, encode_git_path(key), 0) elif isinstance(key, int): if key >= 0: centry = C.git_index_get_byindex(self._index, key) @@ -180,12 +186,12 @@ def write_tree(self, repo: 'Repository | None' = None) -> Oid: def remove(self, path: PathLike[str] | str, level: int = 0) -> None: """Remove an entry from the Index.""" - err = C.git_index_remove(self._index, encode_fs_path(path), level) + err = C.git_index_remove(self._index, encode_git_path(path), level) check_error(err, io=True) def remove_directory(self, path: PathLike[str] | str, level: int = 0) -> None: """Remove a directory from the Index.""" - err = C.git_index_remove_directory(self._index, encode_fs_path(path), level) + err = C.git_index_remove_directory(self._index, encode_git_path(path), level) check_error(err, io=True) def remove_all(self, pathspecs: typing.Sequence[str | PathLike[str]]) -> None: @@ -221,7 +227,7 @@ def add(self, path_or_entry: 'IndexEntry | str | PathLike[str]') -> None: err = C.git_index_add(self._index, centry) elif isinstance(path_or_entry, str) or hasattr(path_or_entry, '__fspath__'): path = path_or_entry - err = C.git_index_add_bypath(self._index, encode_fs_path(path)) + err = C.git_index_add_bypath(self._index, encode_git_path(path)) else: raise TypeError('argument must be string, Path or IndexEntry') @@ -475,7 +481,7 @@ def _to_c(self) -> tuple['ffi.GitIndexEntryC', 'ffi.ArrayC[ffi.char]']: # basically memcpy() ffi.buffer(ffi.addressof(centry, 'id'))[:] = self.id.raw[:] centry.mode = int(self.mode) - path = ffi.new('char[]', encode_fs_path(self.path)) + path = ffi.new('char[]', encode_git_path(self.path)) centry.path = path return centry, path @@ -503,7 +509,7 @@ def __getitem__(self, path): ctheirs = ffi.new('git_index_entry **') err = C.git_index_conflict_get( - cancestor, cours, ctheirs, self._index._index, encode_fs_path(path) + cancestor, cours, ctheirs, self._index._index, encode_git_path(path) ) check_error(err) @@ -514,7 +520,7 @@ def __getitem__(self, path): return ancestor, ours, theirs def __delitem__(self, path): - err = C.git_index_conflict_remove(self._index._index, encode_fs_path(path)) + err = C.git_index_conflict_remove(self._index._index, encode_git_path(path)) check_error(err) def __iter__(self): @@ -526,7 +532,7 @@ def __contains__(self, path): ctheirs = ffi.new('git_index_entry **') err = C.git_index_conflict_get( - cancestor, cours, ctheirs, self._index._index, encode_fs_path(path) + cancestor, cours, ctheirs, self._index._index, encode_git_path(path) ) if err == C.GIT_ENOTFOUND: return False diff --git a/pygit2/utils.py b/pygit2/utils.py index 95e382c3..e33130ec 100644 --- a/pygit2/utils.py +++ b/pygit2/utils.py @@ -25,6 +25,8 @@ import contextlib import os +import sys +import unicodedata from collections.abc import Generator, Iterator, Sequence from types import TracebackType from typing import ( @@ -84,6 +86,31 @@ def encode_fs_path( return os.fsencode(s) # type: ignore[arg-type] +@overload +def encode_git_path(s: PathStrOrBytes) -> bytes: ... +@overload +def encode_git_path(s: 'ffi.NULL_TYPE | None') -> 'ffi.NULL_TYPE': ... +def encode_git_path( + s: 'PathStrOrBytes | ffi.NULL_TYPE | None', +) -> 'bytes | ffi.NULL_TYPE': + """Encode a path that lives inside a Git repository. + + str and PathLike values are encoded as UTF-8 (with surrogateescape for + round-trip of non-UTF-8 bytes). On macOS they are also normalized to NFC, + matching Git's core.precomposeunicode default behaviour. + """ + if s is None or s == ffi.NULL: + return ffi.NULL + + if isinstance(s, bytes): + return s + + text = os.fspath(s) # type: ignore[arg-type] + if sys.platform == 'darwin': + text = unicodedata.normalize('NFC', text) + return text.encode('utf-8', 'surrogateescape') + + # TODO decode_string uses errors='surrogateescape', but encode_string defaults # to errors='strict', so a value read from libgit2 with bad bytes cannot be # written back without raising. Decide whether encode_string should default to diff --git a/src/repository.c b/src/repository.c index d3e50fce..92c280d7 100644 --- a/src/repository.c +++ b/src/repository.c @@ -1824,7 +1824,7 @@ Repository_status(Repository *self, PyObject *args, PyObject *kw) PyDoc_STRVAR(Repository_status_file__doc__, - "status_file(path: str) -> enums.FileStatus\n" + "status_file(path: str | bytes) -> enums.FileStatus\n" "\n" "Returns the status of the given file path."); @@ -1832,7 +1832,7 @@ PyObject * Repository_status_file(Repository *self, PyObject *value) { PyObject *tvalue; - char *path = pgit_borrow_fsdefault(value, &tvalue); + char *path = pgit_borrow_gitpath(value, &tvalue); if (!path) return NULL; diff --git a/src/treebuilder.c b/src/treebuilder.c index ddcb6757..acbbbc5a 100644 --- a/src/treebuilder.c +++ b/src/treebuilder.c @@ -107,7 +107,7 @@ PyObject * TreeBuilder_get(TreeBuilder *self, PyObject *py_filename) { PyObject *tvalue; - char *filename = pgit_borrow_fsdefault(py_filename, &tvalue); + char *filename = pgit_borrow_gitpath(py_filename, &tvalue); if (filename == NULL) return NULL; @@ -135,7 +135,7 @@ PyObject * TreeBuilder_remove(TreeBuilder *self, PyObject *py_filename) { PyObject *tvalue; - char *filename = pgit_borrow_fsdefault(py_filename, &tvalue); + char *filename = pgit_borrow_gitpath(py_filename, &tvalue); if (filename == NULL) return NULL; diff --git a/src/utils.c b/src/utils.c index f013183a..05e216f2 100644 --- a/src/utils.c +++ b/src/utils.c @@ -80,6 +80,101 @@ pgit_borrow_fsdefault(PyObject *value, PyObject **tvalue) return PyBytes_AS_STRING(bytes); } +/** + * Return a borrowed C string for a path inside a Git repository. + * + * The input may be a str, bytes, or os.PathLike. str/PathLike values are + * encoded as UTF-8 (with surrogateescape for round-trip of non-UTF-8 bytes). + * On macOS they are also normalized to NFC, matching Git's + * core.precomposeunicode default behaviour. bytes values are returned + * unchanged as raw path bytes. + */ +static PyObject *unicode_normalize = NULL; + +static int +ensure_unicode_normalize(void) +{ + if (unicode_normalize != NULL) { + return 0; + } + + PyObject *mod = PyImport_ImportModule("unicodedata"); + if (mod == NULL) { + return -1; + } + + unicode_normalize = PyObject_GetAttrString(mod, "normalize"); + Py_DECREF(mod); + if (unicode_normalize == NULL) { + return -1; + } + + return 0; +} + +char* +pgit_borrow_gitpath(PyObject *value, PyObject **tvalue) +{ + PyObject *py_path = NULL; + + if (PyUnicode_Check(value)) { + py_path = value; + Py_INCREF(py_path); + } else if (PyBytes_Check(value)) { + Py_INCREF(value); + *tvalue = value; + return PyBytes_AsString(value); + } else { + py_path = PyOS_FSPath(value); + if (py_path == NULL) { + return NULL; + } + } + + if (PyBytes_Check(py_path)) { + *tvalue = py_path; + return PyBytes_AsString(py_path); + } + +#ifdef __APPLE__ + if (ensure_unicode_normalize() < 0) { + Py_DECREF(py_path); + return NULL; + } + + PyObject *form = PyUnicode_FromString("NFC"); + if (form == NULL) { + Py_DECREF(py_path); + return NULL; + } + + PyObject *normalized = PyObject_CallFunctionObjArgs( + unicode_normalize, form, py_path, NULL); + Py_DECREF(form); + Py_DECREF(py_path); + if (normalized == NULL) { + return NULL; + } + + PyObject *bytes = PyUnicode_AsEncodedString( + normalized, "utf-8", "surrogateescape"); + Py_DECREF(normalized); + if (bytes == NULL) { + return NULL; + } +#else + PyObject *bytes = PyUnicode_AsEncodedString( + py_path, "utf-8", "surrogateescape"); + Py_DECREF(py_path); + if (bytes == NULL) { + return NULL; + } +#endif + + *tvalue = bytes; + return PyBytes_AsString(bytes); +} + /** * Return a pointer to the underlying C string in 'value'. The pointer is * guaranteed by 'tvalue', decrease its refcount when done with the string. diff --git a/src/utils.h b/src/utils.h index 560a8467..b489c3d3 100644 --- a/src/utils.h +++ b/src/utils.h @@ -90,6 +90,7 @@ to_unicode_n(const char *value, size_t len, const char *encoding, const char* pgit_borrow(PyObject *value); const char* pgit_borrow_encoding(PyObject *value, const char *encoding, const char *errors, PyObject **tvalue); char* pgit_borrow_fsdefault(PyObject *value, PyObject **tvalue); +char* pgit_borrow_gitpath(PyObject *value, PyObject **tvalue); char* pgit_strdup(PyObject *value); diff --git a/test/test_status.py b/test/test_status.py index 5ebb4f89..d2880ced 100644 --- a/test/test_status.py +++ b/test/test_status.py @@ -124,3 +124,13 @@ def test_status_file_unicode_normalization(tmp_path: Path, path: str) -> None: repo.index.add(path) repo.index.write() assert repo.status_file(path) == FileStatus.INDEX_NEW + + +def test_status_file_bytes_path(tmp_path: Path) -> None: + """status_file must accept raw UTF-8 bytes for a path.""" + repo = pygit2.init_repository(str(tmp_path / 'repo')) + path = 'täst_é.txt' + (Path(repo.workdir) / path).write_text('hello') + repo.index.add(path) + repo.index.write() + assert repo.status_file(path.encode('utf-8')) == FileStatus.INDEX_NEW