Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Refreshing doesn't invalidate cached version_info #1829

Closed
EliahKagan opened this issue Feb 19, 2024 · 1 comment · Fixed by #1838
Closed

Refreshing doesn't invalidate cached version_info #1829

EliahKagan opened this issue Feb 19, 2024 · 1 comment · Fixed by #1838

Comments

@EliahKagan
Copy link
Contributor

EliahKagan commented Feb 19, 2024

The version_info property of Git instances is cached per-instance, while the git.refresh function and Git.refresh class method modify global state. When version_info has been read on a Git instance, refreshing affects the behavior of that instance, but does not invalidate the cache, causing the version_info property and dynamic version method to give inconsistent results even when no changes to environment variables or the filesystem have occurred.

Here's an example produced on a CentOS 7 system where I have both the system git and a newer upstream git installed:

ek in 🌐 Eald in GitPython on  main via 🐍 v3.12.1 via 🅒 GitPython
❯ type -a git
git is /home/ek/bin/git
git is /usr/bin/git

ek in 🌐 Eald in GitPython on  main via 🐍 v3.12.1 via 🅒 GitPython
❯ git --version
git version 2.43.0

ek in 🌐 Eald in GitPython on  main via 🐍 v3.12.1 via 🅒 GitPython
❯ /usr/bin/git --version
git version 1.8.3.1

ek in 🌐 Eald in GitPython on  main via 🐍 v3.12.1 via 🅒 GitPython
❯ python
Python 3.12.1 | packaged by conda-forge | (main, Dec 23 2023, 08:03:24) [GCC 12.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import git
>>> g1 = git.Git()
>>> g2 = git.Git()
>>> g1.version_info
(2, 43, 0)
>>> git.refresh("/usr/bin/git")
>>> g1.version()
'git version 1.8.3.1'
>>> g2.version()
'git version 1.8.3.1'
>>> g1.version_info
(2, 43, 0)
>>> g2.version_info
(1, 8, 3, 1)

Because I had accessed g1.version_info before refreshing, the stale version information is handed back even after the refresh.

This is admittedly consistent with how version_info is documented:

This value is generated on demand and is cached.

But it seems to me that the documentation, as currently written, does not prevent it from being surprising. I think that either this should be made clear in the version_info property docstring, or the behavior should be changed so that refreshing invalidates all Git instances' cached version_info properties. I am not sure which is better, because I don't know why version_info is cached (and cached per instance).

This issue can probably be considered minor. In particular, this does not prevent FetchInfo.refresh (which git.refresh calls after calling Git.refresh) from updating FetchInfo._flag_map correctly, because FetchInfo.refresh accesses version_info on a newly created Git instance:

if Git().version_info[:2] >= (2, 10):

@Byron
Copy link
Member

Byron commented Feb 19, 2024

Thanks for investigating!

Indeed, since version_info is also used within GitPython, it's definitely a mistake to leave it stale. But even if it wasn't, users might well rely on it.

The reason this didn't come up earlier is probably that people rather restart the program than refreshing the Git version after changes.

EliahKagan added a commit to EliahKagan/GitPython that referenced this issue Feb 22, 2024
Most of these don't pass yet, as such invalidation isn't
implemented yet (gitpython-developers#1829).
EliahKagan added a commit to EliahKagan/GitPython that referenced this issue Feb 22, 2024
EliahKagan added a commit to EliahKagan/GitPython that referenced this issue Feb 22, 2024
EliahKagan added a commit to EliahKagan/GitPython that referenced this issue Feb 22, 2024
Most of these don't pass yet, as such invalidation isn't
implemented yet (gitpython-developers#1829).
EliahKagan added a commit to EliahKagan/GitPython that referenced this issue Feb 22, 2024
EliahKagan added a commit to EliahKagan/GitPython that referenced this issue Feb 26, 2024
This further improves the text previously introduced in gitpython-developers#1829 and
improved in gitpython-developers#1844.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants