Repository navigation
Fix for TypeError in Repository.ident (plus typo fixes) - #1461
Merged
jdavid merged 3 commits intoMay 3, 2026
Merged
jdavid merged 3 commits into
jdavid merged 3 commits into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
Since its introduction in commit 81520c9 (Update to libgit2 v0.23), the reflog identity getter (Repository.ident) has been passing the (char **) variables directly to ffi.string() as C-strings, when they really are pointers to C-strings. Fix by dereferencing the pointer to a C-string first. Furthermore, Repository.ident assumes that said variables hold valid C-strings, when the reflog identity is initialized to NULL, and can be explicitly set to such with `set_ident (None, None)`. So map None to ffi.NULL (which maybe_string() happens to already do, so I simply delegated to that...). I also added a few regression tests to ensure neither issue shows up again. Signed-off-by: Ethan Meng <ethan@rapidcow.org>
rapidcow
marked this pull request as ready for review
April 30, 2026 23:28
Since set_ident() already has type annotations, it seems fitting that we adorn ident() with those too. :) Also, to_bytes() is perfectly capable of mapping None to ffi.NULL, so the parameters on set_ident() should be `str | None` (aka Optional[str]). Signed-off-by: Ethan Meng <ethan@rapidcow.org>
Repository.default_signature is a read-only descriptor, not a getter method. Signed-off-by: Ethan Meng <ethan@rapidcow.org>
rapidcow
marked this pull request as draft
April 30, 2026 23:32
Copy link
Copy Markdown
Contributor
Author
|
The commit wording was a bit bad, sorry... # git range-diff @{u}~3..@{u} @~3..@
1: ea5b51b7 ! 1: d32573ec fix(repository): dereference reflog identity correctly
@@ Commit message
as C-strings, when they really are pointers to C-strings.
Fix by dereferencing the pointer to a C-string first.
- Furthermore, Repository.ident assumes that libgit2 holds
- a valid C-string, when the reflog identity is initialized
+ Furthermore, Repository.ident assumes that said variables
+ hold valid C-strings, when the reflog identity is initialized
to NULL, and can be explicitly set to such with `set_ident
(None, None)`. So map None to ffi.NULL (which maybe_string()
happens to already do, so I simply delegated to that...).
2: 9b65c2d6 = 2: 379b4d82 fix(repository): amend annotations on reflog identity
3: 1deb18ba = 3: e7bd6d26 docs(repository): fix default_signature typos
|
Sorry, something went wrong.
rapidcow
marked this pull request as ready for review
April 30, 2026 23:36
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As of pygit2 1.19.2 (on my Python 3.14.4, Linux 6.19.14-artix1-1 x86_64), the Repository.ident property triggers TypeError:
Python 3.14.4 (main, Apr 15 2026, 07:34:47) [GCC 15.2.1 20260209] on linux Type "help", "copyright", "credits" or "license" for more information. >>> import pygit2; pygit2.__version__ '1.19.2' >>> repo = pygit2.init_repository('/tmp/foo') >>> repo.ident Traceback (most recent call last): File "<python-input-2>", line 1, in <module> repo.ident File "/usr/lib/python3.14/site-packages/pygit2/repository.py", line 1588, in ident return (ffi.string(cname).decode('utf-8'), ffi.string(cemail).decode('utf-8')) ~~~~~~~~~~^^^^^^^ TypeError: string(): unexpected cdata 'char * *' argumentcname and cemail are char ** variables, which are pointers to C-strings, not the C-strings themselves. So they should be passed as cname[0] and cemail[0].
In addition, cname[0] and cemail[0] appear to be NULL by default... so clearly we have to handle that separately.
Either way, I wrote up a patch (plus a typo I happened to catch when trying to skim the docs for other mentions of identity). Not sure if this is the right way to contribute here... any feedback* is appreciated! :)
*Specifically, (1) I'm not certain if utils.maybe_string() is the right tool for this, since the original UTF-8 decode mode was "strict", but is now "surrogateescape" (but I don't want to introduce a separate function just for this, so...) Also (2) I used PEP 585 syntax for the tuple annotation, while keeping Optional since it has already been imported (pygit2 itself seems to use a mix of pre- and post-PEP 585 syntax, and ruff doesn't appear to complain either way, so I picked the ones I liked the best without importing more names...)